-
Notifications
You must be signed in to change notification settings - Fork 50
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
Implement Backend Functionality for Email Sending #89
Conversation
…ion: - Enhanced the subscription logic to check for existing subscribers and manage reactivation. - Implemented the unsubscription logic to handle user requests using a unique token. - Integrated JavaMailSender for sending subscription confirmation emails: - Set up email configuration to enable sending emails via Gmail. - Added functionality to send an email notification to users upon successful subscription.
@ajaynegi45 @Guhapriya01 Despite the application not running on my local machine right now, I made the necessary changes related to the newsletter subscription and unsubscription functionality when my application was running properly. While testing, the application failed to start, but I believe the changes I implemented are correct. Therefore, I have raised the PR for review. Please let me know if any modifications are needed. I've tried to keep the code as less redundant as possible, and as you suggested, I configured the email functionality using Gmail. In the unsubscribe link, I used 'localhost:8080', which we will need to update to our actual domain name once it is finalized. If the changes are okay for you then please assign the PR to me add labels and merge it. 😊 |
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.
Hi @shreya5653,
Thanks for your work on the newsletter features! I have a few suggestions:
-
HTTP Status Codes: The API should return appropriate status codes (e.g., 400 for bad requests) to indicate the outcome of operations.
-
API URL Convention: For consistency with other endpoints, please change the URL from
/newsletter
to/api/newsletter
. -
Redundant Email Service: The
EmailService1
class overlaps with the existingEmailService
. Consider using the existing one and making changes to the structure if needed. -
Unnecessary Configuration: The
MailConfig
class is redundant since email settings are already defined inapplication.properties
.
Let me know if you have any questions regarding these.
@Guhapriya01 |
Thanks for making the changes! They look great. I wanted to discuss the current structure of the To enhance the design, I suggest we keep the Additionally, as @rishabhrawat05 suggested in issue #87, we could add two new notification types: Looking forward to hearing your thoughts on this approach! |
Thanks for sharing your thoughts! I was planning to keep the logic for notification emails, newsletter emails, and subscription emails separate to avoid complexity and enhance understanding. Since I’m not entirely sure how we’ll send notifications yet, I found it more convenient to focus on implementing the core functionalities first before merging any logic. I appreciate your suggestion about creating separate services, but I’m concerned that it might increase complexity and take additional time to implement all the necessary functionality for sending notifications and emails. So far, I’ve successfully added the backend functionality for sending subscription and unsubscription emails and handling the corresponding errors. I think we can revisit this design later once we have a clearer picture. Let me know! |
Hey @shreya5653, Since your approach for the core functionality is solid, would you be open to making another PR later for those design changes we discussed and for adding authorization with |
Hello @Guhapriya01 I really appreciate you moving ahead with merging this one. Looking forward to revisiting the improvements when the time is right! Thanks again! |
Hello @shreya5653, Thank you for your contribution! 😊 I’ve merged your PR, and I appreciate you being open to updating the design and adding the authorization later. Great work! |
Thank you so much! @Guhapriya01 😊 |
#87
Modified existing codes for newsletter subscription and unsubscription:
Integrated JavaMailSender for sending subscription confirmation emails: