-
-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Email is sent on successful reservation #121
Changes from 1 commit
087bf5e
9fe8451
d6e8dcc
959b2fd
964ef7b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
require("dotenv").config(); | ||
const nodemailer = require("nodemailer"); | ||
|
||
// Create a Nodemailer transporter using SMTP | ||
const transporter = nodemailer.createTransport({ | ||
service: "gmail", // or your preferred email service | ||
auth: { | ||
user: process.env.EMAIL_USER, | ||
pass: process.env.EMAIL_PASS, | ||
}, | ||
}); | ||
|
||
// Function to send reservation confirmation via email | ||
exports.sendReservationConfirmation = async (email, reservationDetails) => { | ||
const { reservationDate, guests, time } = reservationDetails; | ||
|
||
// Construct the email content | ||
const emailText = ` | ||
Dear Customer, | ||
|
||
We are pleased to confirm your reservation. Here are the details of your reservation: | ||
|
||
Reservation Date: ${reservationDate} | ||
Number of Guests: ${guests} | ||
Reservation Time: ${time} | ||
|
||
Thank you for choosing our service. We look forward to hosting you. | ||
|
||
Best regards, | ||
PlayCafe | ||
`; | ||
|
||
try { | ||
await transporter.sendMail({ | ||
from: process.env.EMAIL_USER, | ||
to: email, | ||
subject: "Reservation Confirmation", | ||
text: emailText, | ||
}); | ||
console.log("Reservation confirmation sent successfully via email"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Implement a proper logging mechanism for production. While console.log is useful for development, it's not suitable for production environments. Consider implementing a proper logging mechanism (e.g., Winston or Bunyan) that can handle different log levels and potentially integrate with monitoring services. Example implementation: const logger = require('./logger'); // Assume this is your logging setup
// ... in the try block
logger.info('Reservation confirmation sent successfully via email', { email }); |
||
} catch (error) { | ||
console.error("Error sending reservation confirmation email:", error); | ||
throw new Error("Failed to send reservation confirmation email"); | ||
} | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve error handling and logging. The current error handling can be enhanced in several ways:
Here's an example of improved error handling: const logger = require('./logger'); // Assume this is your logging setup
try {
// ... (existing code)
} catch (error) {
logger.error('Failed to send reservation confirmation email', { error, email });
if (error.code === 'ECONNREFUSED') {
throw new Error('Failed to connect to email server. Please try again later.');
} else {
throw new Error(`Failed to send reservation confirmation email: ${error.message}`);
}
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,12 +1,14 @@ | ||||||||||||||||||||||||||||||||||||||||||||
const { z } = require("zod"); | ||||||||||||||||||||||||||||||||||||||||||||
const Reservation = require("../models/reservation.model"); | ||||||||||||||||||||||||||||||||||||||||||||
const logger = require("../config/logger"); // Import your logger | ||||||||||||||||||||||||||||||||||||||||||||
const logger = require("../config/logger"); | ||||||||||||||||||||||||||||||||||||||||||||
const { sendReservationConfirmation } = require("../config/nodemailer"); // Import your email function | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Define the Zod schema for reservation validation | ||||||||||||||||||||||||||||||||||||||||||||
const reservationSchema = z.object({ | ||||||||||||||||||||||||||||||||||||||||||||
guests: z.string(), | ||||||||||||||||||||||||||||||||||||||||||||
date: z.string(), | ||||||||||||||||||||||||||||||||||||||||||||
time: z.string(), | ||||||||||||||||||||||||||||||||||||||||||||
email: z.string().email(), // Include email validation in the schema | ||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
async function createReservation(req, res) { | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -25,11 +27,26 @@ async function createReservation(req, res) { | |||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Create the reservation in the database | ||||||||||||||||||||||||||||||||||||||||||||
const reservation = await Reservation.create(validationResult.data); | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+30
to
31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential security risk: Prevent extra unwanted fields in request data The current schema may allow extra fields in Apply this diff to enforce strict schema validation: -const reservationSchema = z.object({
+const reservationSchema = z.object({
guests: z.string(),
date: z.string(),
time: z.string(),
email: z.string().email(),
+}).strict(); // Disallow unknown keys
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Send a confirmation email | ||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||
const { email, date, guests, time } = validationResult.data; | ||||||||||||||||||||||||||||||||||||||||||||
await sendReservationConfirmation(email, { reservationDate: date, guests, time }); | ||||||||||||||||||||||||||||||||||||||||||||
logger.info(`Reservation confirmation email sent to ${email}`); | ||||||||||||||||||||||||||||||||||||||||||||
} catch (emailError) { | ||||||||||||||||||||||||||||||||||||||||||||
logger.error("Error sending reservation confirmation email:", { | ||||||||||||||||||||||||||||||||||||||||||||
message: emailError.message, | ||||||||||||||||||||||||||||||||||||||||||||
stack: emailError.stack, | ||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||
// Email error should not block the main reservation process, so no need to return a failure response | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure sensitive information is not exposed in logs While logging errors is important for debugging, including the full stack trace may inadvertently expose sensitive information. Consider sanitizing the error logs or limiting the details to avoid potential security issues. You might adjust the logging as follows: logger.error("Error sending reservation confirmation email:", {
message: emailError.message,
- stack: emailError.stack,
}); Or use a logging level that masks sensitive details in production environments. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Send the success response | ||||||||||||||||||||||||||||||||||||||||||||
res.status(201).json({ | ||||||||||||||||||||||||||||||||||||||||||||
success: true, | ||||||||||||||||||||||||||||||||||||||||||||
message: "Reservation created successfully", | ||||||||||||||||||||||||||||||||||||||||||||
message: "Reservation created successfully, confirmation email sent", | ||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Success message may be misleading if email fails to send The success response always states "Reservation created successfully, confirmation email sent" even if the email sending fails. This could confuse users if they do not receive a confirmation email. Consider updating the response to accurately reflect the email sending status. Option 1: Modify the success message to be neutral: res.status(201).json({
success: true,
- message: "Reservation created successfully, confirmation email sent",
+ message: "Reservation created successfully",
data: reservation,
}); Option 2: Include an let emailSent = true;
try {
// ... email sending logic ...
} catch (emailError) {
// ... error handling ...
+ emailSent = false;
}
// Send the success response
res.status(201).json({
success: true,
message: "Reservation created successfully",
data: reservation,
+ emailSent: emailSent,
}); This way, the client can handle the response accordingly.
|
||||||||||||||||||||||||||||||||||||||||||||
data: reservation, | ||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||
} catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -48,4 +65,4 @@ async function createReservation(req, res) { | |||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
module.exports = { | ||||||||||||||||||||||||||||||||||||||||||||
createReservation, | ||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import { createContext, useState, useContext } from "react"; | ||
|
||
const AuthContext = createContext(); | ||
|
||
export const AuthProvider = ({ children }) => { | ||
const [email, setEmail] = useState(null); // Store user's email | ||
|
||
return ( | ||
<AuthContext.Provider value={{ email, setEmail }}> | ||
{children} | ||
</AuthContext.Provider> | ||
); | ||
}; | ||
|
||
export const useAuth = () => useContext(AuthContext); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,13 +3,16 @@ import Logo from "../../assets/Logo/playcafe.png"; | |
import { Link, useLocation } from "react-router-dom"; | ||
import { useKindeAuth } from "@kinde-oss/kinde-auth-react"; | ||
import { message } from "antd"; | ||
import { useAuth } from "./AuthContext"; | ||
|
||
const Navbar = () => { | ||
const { login, logout, isAuthenticated } = useKindeAuth(); | ||
const { login, logout, isAuthenticated, getUser } = useKindeAuth(); | ||
const [isScrolled, setIsScrolled] = useState(false); | ||
const [isMenuOpen, setIsMenuOpen] = useState(false); | ||
const location = useLocation(); | ||
const wasAuthenticated = useRef(null); | ||
const { setEmail } = useAuth(); | ||
|
||
const menuItems = [ | ||
{ name: "Home", path: "/" }, | ||
{ name: "Events", path: "/events" }, | ||
|
@@ -41,10 +44,24 @@ const Navbar = () => { | |
} | ||
if (!wasAuthenticated.current && isAuthenticated) { | ||
message.success("Login successful!"); | ||
// Fetch user details only when authentication is successful | ||
fetchUserDetails(); | ||
} | ||
wasAuthenticated.current = isAuthenticated; | ||
}, [isAuthenticated]); | ||
|
||
const fetchUserDetails = async () => { | ||
try { | ||
const user = getUser(); | ||
if (user) { | ||
setEmail(user.email); // Store email in the context | ||
console.log("User email:", user.email); | ||
} | ||
} catch (error) { | ||
console.error("Failed to get user details:", error); | ||
} | ||
}; | ||
|
||
const toggleMenu = () => { | ||
setIsMenuOpen(!isMenuOpen); | ||
}; | ||
|
@@ -67,26 +84,25 @@ const Navbar = () => { | |
const handleLogin = async () => { | ||
try { | ||
await login(); | ||
|
||
} catch (error) { | ||
message.error("Login failed. Please try again."); | ||
} | ||
}; | ||
|
||
|
||
const handleLogout = async () => { | ||
try { | ||
await logout(); | ||
|
||
setEmail(null); | ||
} catch (error) { | ||
message.error("Logout failed. Please try again."); | ||
} | ||
}; | ||
|
||
return ( | ||
<nav | ||
className={`w-full fixed top-0 z-50 transition duration-300 ${isScrolled ? "bg-[#E0F0B1]" : "bg-transparent"} | ||
${isScrolled ? "text-gray-800" : "text-black"} ${isScrolled ? "shadow-lg" : ""}`} | ||
className={`w-full fixed top-0 z-50 transition duration-300 ${ | ||
isScrolled ? "bg-[#E0F0B1]" : "bg-transparent" | ||
} ${isScrolled ? "text-gray-800" : "text-black"} ${isScrolled ? "shadow-lg" : ""}`} | ||
Comment on lines
+103
to
+105
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Simplify conditional className concatenation The current className concatenation with multiple ternary operators can be hard to read and maintain. Consider using the First, install the npm install classnames Import + import classNames from 'classnames'; Refactor the - className={`w-full fixed top-0 z-50 transition duration-300 ${
- isScrolled ? "bg-[#E0F0B1]" : "bg-transparent"
- } ${isScrolled ? "text-gray-800" : "text-black"} ${isScrolled ? "shadow-lg" : ""}`}
+ className={classNames(
+ 'w-full fixed top-0 z-50 transition duration-300',
+ {
+ 'bg-[#E0F0B1] text-gray-800 shadow-lg': isScrolled,
+ 'bg-transparent text-black': !isScrolled,
+ }
+ )} |
||
> | ||
<div className="max-w-7xl mx-auto px-4"> | ||
<div className="flex justify-between items-center h-16"> | ||
|
@@ -133,14 +149,27 @@ const Navbar = () => { | |
{/* Mobile Menu Button */} | ||
<div className="flex md:hidden"> | ||
<button onClick={toggleMenu} className={`${buttonTextClass} focus:outline-none`}> | ||
{isMenuOpen ? | ||
<svg className="h-6 w-6" fill="none" stroke="black" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"> | ||
{isMenuOpen ? ( | ||
<svg | ||
className="h-6 w-6" | ||
fill="none" | ||
stroke="black" | ||
viewBox="0 0 24 24" | ||
xmlns="http://www.w3.org/2000/svg" | ||
> | ||
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth="2" d="M6 18L18 6M6 6l12 12" /> | ||
</svg> | ||
: <svg className="h-6 w-6" fill="none" stroke="black" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"> | ||
) : ( | ||
<svg | ||
className="h-6 w-6" | ||
fill="none" | ||
stroke="black" | ||
viewBox="0 0 24 24" | ||
xmlns="http://www.w3.org/2000/svg" | ||
> | ||
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth="2" d="M4 6h16M4 12h16M4 18h16" /> | ||
</svg> | ||
} | ||
)} | ||
Comment on lines
+152
to
+172
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Extract SVG icons into separate components The inline SVG code for menu icons makes the JSX lengthy and may reduce readability. Consider extracting the SVGs into separate components or files for cleaner code and reusability. Example: Create separate components for the icons: // MenuOpenIcon.jsx
const MenuOpenIcon = () => (
<svg
className="h-6 w-6"
fill="none"
stroke="black"
viewBox="0 0 24 24"
xmlns="http://www.w3.org/2000/svg"
>
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth="2" d="M6 18L18 6M6 6l12 12" />
</svg>
);
export default MenuOpenIcon; // MenuCloseIcon.jsx
const MenuCloseIcon = () => (
<svg
className="h-6 w-6"
fill="none"
stroke="black"
viewBox="0 0 24 24"
xmlns="http://www.w3.org/2000/svg"
>
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth="2" d="M4 6h16M4 12h16M4 18h16" />
</svg>
);
export default MenuCloseIcon; Then update the Navbar component: + import MenuOpenIcon from './MenuOpenIcon';
+ import MenuCloseIcon from './MenuCloseIcon';
...
{isMenuOpen ? (
- <svg>...</svg>
+ <MenuOpenIcon />
) : (
- <svg>...</svg>
+ <MenuCloseIcon />
)} |
||
</button> | ||
</div> | ||
</div> | ||
|
@@ -155,7 +184,7 @@ const Navbar = () => { | |
key={item.name} | ||
to={item.path} | ||
className={`block px-4 py-3 rounded-md text-base font-semibold transition duration-300 | ||
${mobileMenuBaseTextColorClass} hover:bg-amber-300 hover:text-black`} | ||
${mobileMenuBaseTextColorClass} hover:bg-amber-300 hover:text-black`} | ||
> | ||
{item.name} | ||
</Link> | ||
|
@@ -164,15 +193,15 @@ const Navbar = () => { | |
<button | ||
onClick={handleLogout} | ||
className={`block w-full text-left px-4 py-3 rounded-md text-base font-semibold transition duration-300 | ||
${mobileMenuBaseTextColorClass} hover:bg-amber-300 hover:text-black`} | ||
${mobileMenuBaseTextColorClass} hover:bg-amber-300 hover:text-black`} | ||
> | ||
Log Out | ||
</button> | ||
) : ( | ||
<button | ||
onClick={handleLogin} | ||
className={`block w-full text-left px-4 py-3 rounded-md text-base font-semibold transition duration-300 | ||
${mobileMenuBaseTextColorClass} hover:bg-amber-300 hover:text-black`} | ||
${mobileMenuBaseTextColorClass} hover:bg-amber-300 hover:text-black`} | ||
> | ||
Log In | ||
</button> | ||
|
@@ -184,4 +213,4 @@ const Navbar = () => { | |
); | ||
}; | ||
|
||
export default Navbar; | ||
export default Navbar; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider internationalizing the email content.
The email content is currently hardcoded in English. For a potentially global user base, consider implementing internationalization (i18n) for the email content. This would allow for sending emails in the user's preferred language.
Example implementation: