-
-
Notifications
You must be signed in to change notification settings - Fork 101
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
Add feedback mail #390
Add feedback mail #390
Conversation
@haseebzaki-07 is attempting to deploy a commit to the bunty's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request primarily enhance the feedback submission process by integrating a thank-you email notification feature. The Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. In the meantime, please ensure that your changes align with our CONTRIBUTING.md. If there are any specific instructions or feedback regarding your PR, we'll provide them here. Thanks again for your contribution! 😊 |
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.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
backend/controller/feedback.controller.js (1)
15-21
: Consider using a dedicated email service provider instead of Gmail.Gmail's SMTP service has limitations for production use:
- Daily sending limits
- Potential blocking of automated emails
- Requires "Less secure app access" which is not recommended
Consider using dedicated email services like SendGrid, Amazon SES, or Mailgun which offer:
- Higher sending limits
- Better deliverability
- Proper API authentication
- Production-grade reliability
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (2)
- backend/controller/feedback.controller.js (1 hunks)
- frontend/src/components/Pages/Event.jsx (2 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/components/Pages/Event.jsx
🧰 Additional context used
🔇 Additional comments (1)
backend/controller/feedback.controller.js (1)
15-21
: Add explicit TLS configuration for email security.The current email transport configuration lacks explicit TLS settings.
[security]
Add secure transport configuration:const transporter = nodemailer.createTransport({ service: "gmail", auth: { user: process.env.EMAIL_USER, pass: process.env.EMAIL_PASS, }, + secure: true, + tls: { + rejectUnauthorized: true + } });
await sendThankYouEmail(feedback); | ||
|
||
res.status(201).json({ | ||
success: true, | ||
message: "Feedback created successfully", | ||
message: "Feedback created successfully and email sent", |
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.
Improve error handling for the email sending process.
The feedback is considered successful even if the email fails to send. This could lead to inconsistent user experience.
Consider this improved implementation:
- await sendThankYouEmail(feedback);
+ try {
+ await sendThankYouEmail(feedback);
+ } catch (error) {
+ logger.error("Failed to send thank you email:", {
+ feedbackId: feedback._id,
+ error: error.message
+ });
+ return res.status(201).json({
+ success: true,
+ message: "Feedback created successfully but failed to send email",
+ data: feedback,
+ });
+ }
📝 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.
await sendThankYouEmail(feedback); | |
res.status(201).json({ | |
success: true, | |
message: "Feedback created successfully", | |
message: "Feedback created successfully and email sent", | |
try { | |
await sendThankYouEmail(feedback); | |
} catch (error) { | |
logger.error("Failed to send thank you email:", { | |
feedbackId: feedback._id, | |
error: error.message | |
}); | |
return res.status(201).json({ | |
success: true, | |
message: "Feedback created successfully but failed to send email", | |
data: feedback, | |
}); | |
} | |
res.status(201).json({ | |
success: true, | |
message: "Feedback created successfully and email sent", |
async function sendThankYouEmail(feedback) { | ||
const mailOptions = { | ||
from: process.env.EMAIL_USER, // Sender email | ||
to: feedback.email, // Receiver email | ||
subject: "Thank you for your feedback!", | ||
text: `Hi ${feedback.name}, | ||
|
||
Thank you for your valuable feedback. Here are the details: | ||
|
||
- Feedback: ${feedback.feedback} | ||
- Rating: ${feedback.rating}/5 | ||
|
||
We appreciate you taking the time to share your thoughts with us! | ||
|
||
Best regards, | ||
Play Cafe`, | ||
}; | ||
|
||
try { | ||
await transporter.sendMail(mailOptions); | ||
logger.info(`Thank-you email sent to ${feedback.email}`); | ||
} catch (error) { | ||
logger.error("Error sending email:", error); | ||
} | ||
} |
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
Enhance email template management and format.
The current implementation has several areas for improvement:
- Hardcoded email template
- Missing HTML version
- Silent error handling
Consider these improvements:
async function sendThankYouEmail(feedback) {
+ const htmlContent = `
+ <h2>Thank you for your feedback!</h2>
+ <p>Hi ${feedback.name},</p>
+ <p>Thank you for your valuable feedback. Here are the details:</p>
+ <ul>
+ <li>Feedback: ${feedback.feedback}</li>
+ <li>Rating: ${feedback.rating}/5</li>
+ </ul>
+ <p>We appreciate you taking the time to share your thoughts with us!</p>
+ <p>Best regards,<br>Play Cafe</p>
+ `;
+
const mailOptions = {
from: process.env.EMAIL_USER,
to: feedback.email,
subject: "Thank you for your feedback!",
text: `Hi ${feedback.name},\n\nThank you...`, // Keep plain text version
+ html: htmlContent,
};
try {
await transporter.sendMail(mailOptions);
logger.info(`Thank-you email sent to ${feedback.email}`);
} catch (error) {
- logger.error("Error sending email:", error);
+ logger.error("Failed to send thank you email:", {
+ feedbackId: feedback._id,
+ email: feedback.email,
+ error: error.message
+ });
+ throw error; // Propagate error to handle it in createFeedback
}
}
Additionally, consider moving the email template to a separate file or using a template engine for better maintainability.
Committable suggestion was skipped due to low confidence.
user: process.env.EMAIL_USER, | ||
pass: process.env.EMAIL_PASS, |
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.
💡 Codebase verification
Environment variables need documentation and example configuration
The verification reveals that while EMAIL_USER and EMAIL_PASS are consistently used across the codebase (in feedback.controller.js and nodemailer.js), there is no documentation or example configuration file (.env.example, .env.template, etc.) to guide developers on setting up these required environment variables.
Action items needed:
- Create a
.env.example
file documenting these required environment variables - Add setup instructions in the README.md for configuring email credentials
- Consider documenting the expected format/requirements for these credentials (e.g., if using specific email providers)
🔗 Analysis chain
Verify environment variable configuration.
Ensure that the required environment variables are properly documented and configured:
- EMAIL_USER
- EMAIL_PASS
Let me try a different approach to verify the environment variables configuration by searching through the entire codebase for any documentation or template files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if environment variables are documented
echo "Checking for environment variables documentation..."
rg -l "EMAIL_USER|EMAIL_PASS" "README*" ".env*"
Length of output: 256
Script:
#!/bin/bash
# Search for environment variable documentation in common config and docs locations
echo "Searching for environment variables documentation..."
fd -t f ".(env|example|sample|template|md|txt)$" -X rg -l "EMAIL_USER|EMAIL_PASS"
# Search for actual usage of these environment variables
echo -e "\nSearching for environment variable usage..."
rg "process\.env\.(EMAIL_USER|EMAIL_PASS)"
Length of output: 913
abe91c3
into
RamakrushnaBiswal:main
fixes #362
This PR adds mail service for the users submitting feedback to the website.
Summary by CodeRabbit
New Features
Bug Fixes
These updates enhance user experience by confirming feedback submission and maintaining a polished interface.