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

Carlos anniversary celebrated #1128

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

cgomezhub
Copy link
Contributor

Description

This part of a new Report Page: Create Company Summary Dashboard for Admins requested by Jae.
Implements # (WBS)
Anniversary Celebrared Component

Related PRS (if any):

This Backend PR is related to FrontendPR#2722.

To test this backend PR you need to checkout the #2722 frontend PR.

Main changes explained:

  • Visualization of volunteer anniversaries within the current week.
  • Comparison of volunteer anniversaries between the current and previous week.
  • Clicking on the volunteer email icon allows you to send a personalized congratulatory email using previously configured -
  • Gmail OAuth settings. (For more information on Gmail setup, please watch the video below.)

How to test:

  1. Check out the current branch.
  2. Do npm install and ... to run this PR locally.
  3. Clear site data/cache.
  4. Log in as admin user.
  5. Navigate to Dashboard > Reports > Total Org Summary > Volunteer Engagement Trends.
  6. Set up the Gmail Api in your account http://console.cloud.google.com/ and https://developers.google.com/ (please watch the Gmail Api setup video belowuntil minut 7).
  7. Set up the environment variables in the .env file of your backend project (see .env image).
  8. Chech the anniversary list.
  9. Select and click on the email icon.
  10. Send a congratulatory email.
  11. Check sent folder of your gmail acount

Screenshots or videos needed:

App features video:
https://www.loom.com/share/d4f75956df7b4ef6946dd17aa8bfa806?sid=fc6227af-88b0-41ec-b0bc-4613367dc07f

@cgomezhub cgomezhub added the High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible label Oct 16, 2024
Copy link

@Ankuriboh Ankuriboh left a comment

Choose a reason for hiding this comment

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

Code works but most of the email sending changes are unnecessary. Only returned data fields related changes are for the visualizing organization summary feature.

reject,
});
} else {
resolve('Email sending is disabled');

Choose a reason for hiding this comment

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

We probably better to just disable the whole queue system instead of resolving each promise in the message queue. IMO, this change noticing email sending is disabled is probably irrelevant to the current scope through.

Comment on lines +17 to +25
.then(result => {
console.log('Email sent successfully:', result);
res.status(200).send(`Email sent successfully to ${to}`);
})
.catch(error => {
console.error('Error sending email:', error);
res.status(500).send('Error sending email');
});

Copy link

@Ankuriboh Ankuriboh Oct 16, 2024

Choose a reason for hiding this comment

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

Not sure why these lines are needed since await emailSender(to, subject, html) is already captured by the try-catch block, and it is already awaited.

@@ -74,6 +74,9 @@ const overviewReportHelper = function () {
_id: 1,
firstName: 1,
lastName: 1,
email: 1,
profilePic: 1,
createdDate:1,
Copy link

@Ankuriboh Ankuriboh Oct 16, 2024

Choose a reason for hiding this comment

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

I don't see the createdDate field is being used in frontend PR, please kindly correctly if I am wrong.

@@ -14,8 +13,16 @@ const sendEmail = async (req, res) => {

console.log('to', to);

Choose a reason for hiding this comment

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

console.log() should be removed or commented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants