search

freeCodeCamp / chapter

A self-hosted event management tool for nonprofits
BSD 3-Clause "New" or "Revised" License
1.92k stars 359 forks source link

Consolidate email templates #2071

Open ojeytonwilliams opened 1 year ago

ojeytonwilliams commented 1 year ago

Currently the text of the emails that Chapter sends is embedded directly in the controllers. If we move them into their own file this both removes clutter from the controllers and makes it easier to audit the text and ensure that it has a consistent voice.

server/src/email-templates.ts seems like a good enough place for these helper functions for now.

UrielOfir commented 1 year ago

Hi :) I would like to assign myself for this issue. I didn't understand what did you mean that "emails sending is embedded directly in the controllers". Did you mean to the code in this file- https://github.com/freeCodeCamp/chapter/blob/08cb5b009187e7ce7530350365b881c37daa8c3e/server/src/controllers/Users/resolver.ts line 60?

ojeytonwilliams commented 1 year ago

Hi @UrielOfir. What I was trying to say was that we should not put the email content directly in the resolvers that handle the user's request. For example

https://github.com/freeCodeCamp/chapter/blob/08cb5b009187e7ce7530350365b881c37daa8c3e/server/src/controllers/Users/resolver.ts#L60-L67

bloats the resolver logic. If it read like this

    emailTemplates.sendInstanceRoleChanged({ name: user.name, newRole })

it would be a lot cleaner.

Chirag2193 commented 1 year ago

This issue seems to have been merged under above PR #2319 We ok to close this?

allella commented 1 year ago

It looks like the constants were moved out of the revolvers in PR #2319, but @ojeytonwilliams 's example above seemed to go further by moving the mailerService.sendEmail out of the resolver.

Oliver, is there more to go done here?