TEC email reminders feature #536

patriciaahuang · 2023-11-04T20:04:18Z

Summary

Implemented a new feature on the TEC dashboard that can either remind individual people or everyone who doesn't have enough TECs that they do not have enough TECs completed yet this semester.

Notion Link

https://www.notion.so/TEC-Add-Support-for-Email-Reminders-07b6c3d821af4eff8cde00d3c9048f37?pvs=4

Test Plan

I checked that the POST requests to /sendMail were successful for both individual reminders and the remind all feature.

To ensure an email actually sends to the members, I tested sending a reminder email to myself, which worked.

dti-github-bot · 2023-11-04T20:04:39Z

[diff-counting] Significant lines: 232.

andrew032011

Can we also list out the available upcoming TEC events in bulleted form in the email?

andrew032011 · 2023-11-04T23:27:18Z

backend/src/API/memberAPI.ts

@@ -74,6 +75,25 @@ export const deleteMember = async (email: string, user: IdolMember): Promise<voi
  );
 };

+export const notifyMember = async (


This seems to be TeamEvent specific. Consider putting this in teamEventAPI instead?

andrew032011 · 2023-11-04T23:27:37Z

backend/src/utils/permissionsManager.ts

@@ -5,6 +5,10 @@ export default class PermissionsManager {
    return this.isLeadOrAdmin(mem);
  }

+  static async canNotifyMembers(mem: IdolMember): Promise<boolean> {


andrew032011 · 2023-11-04T23:29:02Z

backend/src/api.ts

@@ -219,6 +220,9 @@ loginCheckedDelete('/member/:email', async (req, user) => {
 loginCheckedPut('/member', async (req, user) => ({
  member: await updateMember(req, req.body, user)
 }));
+loginCheckedPost('/notifyMember', async (req, user) => ({


/team-event-reminder?

andrew032011 · 2023-11-05T05:09:47Z

Let's also specify the requirement along with how many they have already accumulated. That will give some of them a sense of urgency if it's low.

andrew032011 · 2023-11-05T05:11:34Z

backend/src/API/mailAPI.ts

+export const sendTECReminder = async (req: Request, member: IdolMember): Promise<AxiosResponse> => {
+  const subject = 'TEC Reminder';
+  const text =
+    'Hey! You currently do not have enough team events credits this semester. This is a reminder to get at least 3 team events credits by the end of the semester.';


Can we make the credit amount custom based on the role type? Leads need 6 credits, all other members need only 3.

henry-li-06 · 2023-11-05T13:07:57Z

Don't send emails for non-prod environments :) (e.g. dev and staging)

Don't wanna alarm and upset anyone xD

andrew032011 · 2023-11-08T15:01:51Z

frontend/src/components/Modals/NotifyMemberModal.tsx

+    }
+
+    if (all && members) {
+      members.map((member) => {


You could just do a forEach here.

make sure to also await the MembersAPI.notifyMember(member);

andrew032011 · 2023-11-08T15:11:17Z

backend/src/API/mailAPI.ts

+  const memberEvents = getTeamEventAttendanceByUser(member);
+  let approvedCount = 0;
+  let pendingCount = 0;
+  (await memberEvents).map((event) => {


You could do a forEach here, or use a reduce to be a bit more idiomatic (but for the reduce you will have to traverse the list twice, but that is ok)

andrew032011 · 2023-11-08T15:12:00Z

backend/src/API/mailAPI.ts

+    if (event.status === 'approved') {
+      approvedCount += 1;
+    }
+    if (event.pending) {


event.status === 'pending', we're in the process of a DB schema change driven by this: #521

andrew032011

Sorry for the last nitpicks! We should be good after this.

andrew032011 · 2023-11-08T16:42:14Z

backend/src/API/teamEventsAPI.ts

+ */
+export const notifyMember = async (
+  req: Request,
+  body: IdolMember,


nit: How about we change body to member?

andrew032011 · 2023-11-08T16:42:25Z

backend/src/API/teamEventsAPI.ts

+/**
+ * Reminds a member about completing enough TECs this semester.
+ * @param req - the post request being made by the user
+ * @param user - the user trying to notify member(s)


nit: Add member to the docstring

andrew032011 · 2023-11-08T16:44:44Z

backend/src/API/mailAPI.ts

+    `Hey! You currently have ${approvedCount} team event credits approved and ${pendingCount} team event credits pending this semester. ` +
+    `This is a reminder to get at least ${
+      member.role === 'lead' ? '6' : '3'
+    } team events credits by the end of the semester.\n` +


nit: *team event

andrew032011 · 2023-11-08T17:01:48Z

backend/src/API/mailAPI.ts

 export const sendMemberUpdateNotifications = async (req: Request): Promise<Promise<void>[]> => {
  const subject = 'IDOL Member Profile Change';
  const text =
    'Hey! A DTI member has updated their profile on IDOL. Please visit https://idol.cornelldti.org/admin/member-review to review the changes.';
  return emailAdmins(req, subject, text);
 };
+
+export const sendTECReminder = async (req: Request, member: IdolMember): Promise<AxiosResponse> => {


Since you are creating variables with promises, I'd await them first instead of inlining the await, otherwise you might as well just inline the value and delete the variable entirely.

So instead of

const allEvents = getAllTeamEvents(req.body); const futureEvents = (await allEvents).filter((event) => {

try

const allEvents = await getAllTeamEvents(req.body); const futureEvents = allEvents.filter((event) => {

Same with memberEvents, although I'd personally rename this to memberEventAttendance

andrew032011 · 2023-11-08T17:04:53Z

backend/src/API/mailAPI.ts

+  const memberEvents = getTeamEventAttendanceByUser(member);
+  let approvedCount = 0;
+  let pendingCount = 0;
+  (await memberEvents).forEach((event) => {


I think it's probably more clear to tell a user how many credits they have pending than the number of requests they still have pending.

How about something like this?

memberEvents.forEach((eventAttendance) => { const eventCredit = Number(allEvents.find((event) => event.uuid === eventAttendance.eventUuid)?.numCredits ?? 0); if (eventAttendance.status === 'approved') { approvedCount += eventCredit; } if (eventAttendance.status === 'pending') { pendingCount += eventCredit; } });

andrew032011 · 2023-11-08T17:07:06Z

backend/src/API/mailAPI.ts

+    } team events credits by the end of the semester.\n` +
+    `\n${
+      futureEvents.length === 0
+        ? 'There are currently no upcoming team events, but check IDOL soon for updates.'


check IDOL soon for updates

Thoughts on changing this to There are currently no upcoming team events listed on IDOL, but check the #team-events channel for upcoming team events.

andrew032011 · 2023-11-08T17:09:02Z

backend/src/API/mailAPI.ts

+            Number(event.numCredits) !== 1 ? 'credits' : 'credit'
+          })\n`
+      )
+      .join('')}`;


How about at the end of the message we add:

"To submit your TEC, please visit https://idol.cornelldti.org/forms/teamEventCredits."

andrew032011 · 2023-11-09T18:18:19Z

Don't send emails for non-prod environments :) (e.g. dev and staging)

Don't wanna alarm and upset anyone xD

@patriciaahuang I'll add this change in another PR: #541.

andrew032011

Excellent!! Justin's going to love this.

new tec email reminders feature

d03d25a

patriciaahuang requested a review from a team as a code owner November 4, 2023 20:04

run prettier

418fc93

andrew032011 reviewed Nov 4, 2023

View reviewed changes

andrew032011 requested changes Nov 5, 2023

View reviewed changes

add more detail to email + other minor fixes

98a957e

andrew032011 requested changes Nov 8, 2023

View reviewed changes

account for db schema change + other minor fixes

8a51092

andrew032011 requested changes Nov 8, 2023

View reviewed changes

fix nitpicks

f663cc9

andrew032011 approved these changes Nov 10, 2023

View reviewed changes

Merge branch 'main' into tec-email-reminders

958db63

patriciaahuang merged commit f20b3e3 into main Nov 12, 2023
10 checks passed

patriciaahuang deleted the tec-email-reminders branch November 12, 2023 06:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TEC email reminders feature #536

TEC email reminders feature #536

patriciaahuang commented Nov 4, 2023

dti-github-bot commented Nov 4, 2023 •

edited

Loading

andrew032011 left a comment

andrew032011 Nov 4, 2023

andrew032011 Nov 4, 2023

andrew032011 Nov 4, 2023

andrew032011 commented Nov 5, 2023

andrew032011 Nov 5, 2023

henry-li-06 commented Nov 5, 2023

andrew032011 Nov 8, 2023

andrew032011 Nov 8, 2023

andrew032011 Nov 8, 2023

andrew032011 Nov 8, 2023

andrew032011 left a comment

andrew032011 Nov 8, 2023

andrew032011 Nov 8, 2023

andrew032011 Nov 8, 2023

andrew032011 Nov 8, 2023

andrew032011 Nov 8, 2023

andrew032011 Nov 8, 2023 •

edited

Loading

andrew032011 Nov 8, 2023

andrew032011 commented Nov 9, 2023

andrew032011 left a comment

TEC email reminders feature #536

TEC email reminders feature #536

Conversation

patriciaahuang commented Nov 4, 2023

Summary

Notion Link

Test Plan

dti-github-bot commented Nov 4, 2023 • edited Loading

andrew032011 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrew032011 commented Nov 5, 2023

Choose a reason for hiding this comment

henry-li-06 commented Nov 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrew032011 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrew032011 Nov 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrew032011 commented Nov 9, 2023

andrew032011 left a comment

Choose a reason for hiding this comment

dti-github-bot commented Nov 4, 2023 •

edited

Loading

andrew032011 Nov 8, 2023 •

edited

Loading