Skip to content
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

Merged
merged 5 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions backend/config/nodemailer.js
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
`;
Comment on lines +15 to +32
Copy link
Contributor

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:

const i18next = require('i18next');
// ... (i18next setup)

const emailText = i18next.t('reservationConfirmation', {
  reservationDate,
  guests,
  time
});


try {
await transporter.sendMail({
from: process.env.EMAIL_USER,
to: email,
subject: "Reservation Confirmation",
text: emailText,
});
console.log("Reservation confirmation sent successfully via email");
Copy link
Contributor

Choose a reason for hiding this comment

The 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");
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The 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:

  1. Use a proper logging mechanism instead of console.error.
  2. Include more details in the thrown error message.
  3. Consider adding specific error types for different failure scenarios.

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}`);
  }
}

23 changes: 20 additions & 3 deletions backend/controller/reservation.controller.js
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) {
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential security risk: Prevent extra unwanted fields in request data

The current schema may allow extra fields in req.body that are not defined in the schema, which could lead to overposting vulnerabilities. To mitigate this risk, consider using Zod's .strict() method to ensure only the defined fields are accepted.

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

Committable suggestion was skipped due to low confidence.


// 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
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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
}
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,
});
// Email error should not block the main reservation process, so no need to return a failure response
}


// Send the success response
res.status(201).json({
success: true,
message: "Reservation created successfully",
message: "Reservation created successfully, confirmation email sent",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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 emailSent flag in the response:

 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.

Committable suggestion was skipped due to low confidence.

data: reservation,
});
} catch (error) {
Expand All @@ -48,4 +65,4 @@ async function createReservation(req, res) {

module.exports = {
createReservation,
};
};
1 change: 1 addition & 0 deletions backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"dotenv": "^16.4.5",
"express": "^4.21.0",
"mongoose": "^8.7.0",
"nodemailer": "^6.9.15",
"validator": "^13.12.0",
"winston": "^3.14.2",
"zod": "^3.23.8"
Expand Down
7 changes: 4 additions & 3 deletions frontend/src/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import './App.css';
import Navbar from '../src/components/Shared/Navbar';
import Footer from "../src/components/Shared/Footer"
import { Outlet } from 'react-router-dom';

import { AuthProvider } from './components/Shared/AuthContext';
import {KindeProvider} from "@kinde-oss/kinde-auth-react";

function App() {
return (
<AuthProvider>
<KindeProvider
clientId={import.meta.env.VITE_KINDE_CLIENT_ID}
domain={import.meta.env.VITE_KINDE_DOMAIN}
Expand All @@ -19,9 +20,9 @@ function App() {
<Outlet />
<Footer />
</KindeProvider>

</AuthProvider>

);
}

export default App;
export default App;
6 changes: 5 additions & 1 deletion frontend/src/components/Pages/Register.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useState } from "react";
import { useAuth } from "../Shared/AuthContext";
import pic from "../../assets/img/abt1.jpg";
import pic2 from "../../assets/img/abt1.png";
import pic3 from "../../assets/img/abt2.png";
Expand All @@ -9,6 +10,7 @@ export default function Register() {
const [date, setDate] = useState("");
const [time, setTime] = useState("");
const [guests, setGuests] = useState();
const { email } = useAuth();

const handleSubmit = (e) => {
console.log(guests);
Expand All @@ -22,11 +24,13 @@ export default function Register() {
"Content-Type": "application/json",
},
body: JSON.stringify({
email,
guests,
date,
time,
}),
})

.then((res) => res.json())
.then((data) => console.log(data))
.catch((error) => console.log(error));
Expand Down Expand Up @@ -231,4 +235,4 @@ export default function Register() {
</div>
</>
);
}
}
15 changes: 15 additions & 0 deletions frontend/src/components/Shared/AuthContext.jsx
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);
57 changes: 43 additions & 14 deletions frontend/src/components/Shared/Navbar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
Expand Down Expand Up @@ -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);
};
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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 classnames library to improve readability and manage class names more effectively.

First, install the classnames library if it's not already included:

npm install classnames

Import classnames at the top of the file:

+ import classNames from 'classnames';

Refactor the className prop using classnames:

-       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">
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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>
Expand All @@ -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>
Expand All @@ -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>
Expand All @@ -184,4 +213,4 @@ const Navbar = () => {
);
};

export default Navbar;
export default Navbar;
Loading