Support for all email events

[gondzo]

Right now, email notifications are enabled only for post events (topic created/updated, post created/updated). The scope here is to add support for the remaining events. Full list of supported events is available at https://github.com/topcoder-platform/tc-notifications/blob/48d9a203992f1cb3cc20f587c71e44e8bb27d63e/connect/events-config.js#L48

There are two modes of sending email notifications - immediate (as it happens, one by one) or bundled. Both are handled in https://github.com/topcoder-platform/tc-notifications/blob/dev/connect/notificationServices/email.js and the logic for the other events should be added there as well.

Individual notifications should always send projectId, project name, relevant user id, file name and url (only for File uploads), link name and url (only for New project links). Bundle notifications should contain the same data as individual notifications, but they should be grouped by projectId and grouped by notification type - the grouping can be reused from the frontend app - https://github.com/appirio-tech/connect-app/blob/e77f7d0a7daf4efbc1301f943fd1c6a8394f0211/src/routes/settings/routes/notifications/components/NotificationSettingsForm.jsx#L21 Our email server does not support loops in email templates, so bundle emails will send the full html to the email service.

[ngoctay]

Individual notifications should always send projectId, project name, relevant user id, file name and url (only for File uploads), link name and url (only for New project links).

1. Do we need "relevant user id"? I see in the current code, we have "handle" and "authorHandle" already.

2. The project service only sends fileName, I checked the tc-project-service. I will add fileUrl to the code, but please double check with tc-project-service.

3. For "link name and url", tc-project-service only sends "projectId", "projectName", "userId", "initiatorUserId". I will add linkName and linkUrl to the code, but please double check with tc-project-service.

[ngoctay]

Full list of supported events is available at

I don't see "notifications.connect.project.updated" anywhere. Do we need to add this to event-config?

[gondzo]

No, there is no such event, we're using 'notifications.connect.project.specificationModified' instead

[gondzo]

@vikasrohit can you create these topics in Kafka for the new email types (and also, email templates in sendgrid)

CREATED: 'notifications.action.email.connect.project.created', SUBMITTED_FOR_REVIEW: 'notifications.action.email.connect.project.submittedForReview', APPROVED: 'notifications.action.email.connect.project.approved', ACTIVE: 'notifications.action.email.connect.project.active', PAUSED: 'notifications.action.email.connect.project.paused', COMPLETED: 'notifications.action.email.connect.project.completed', CANCELED: 'notifications.action.email.connect.project.canceled', MEMBER_JOINED: 'notifications.action.email.connect.project.member.joined', MEMBER_LEFT: 'notifications.action.email.connect.project.member.left', MEMBER_REMOVED: 'notifications.action.email.connect.project.member.removed', MEMBER_ASSIGNED_AS_OWNER: 'notifications.action.email.connect.project.member.assignedAsOwner', MEMBER_COPILOT_JOINED: 'notifications.action.email.connect.project.member.copilotJoined', MEMBER_MANAGER_JOINED: 'notifications.action.email.connect.project.member.managerJoined',

  LINK_CREATED: 'notifications.action.email.connect.project.linkCreated',
  FILE_UPLOADED: 'notifications.action.email.connect.project.fileUploaded',
  SPECIFICATION_MODIFIED: 'notifications.action.email.connect.project.specificationModified',
  TOPIC_CREATED: 'notifications.action.email.connect.project.topic.created',
  TOPIC_DELETED: 'notifications.action.email.connect.project.topic.deleted',
  POST_CREATED: 'notifications.action.email.connect.project.post.created',
  POST_EDITED: 'notifications.action.email.connect.project.post.edited',
  POST_DELETED: 'notifications.action.email.connect.project.post.deleted',
  POST_MENTION: 'notifications.action.email.connect.project.post.mention',

[vikasrohit]

@gondzo Kafka topics are created. I am creating sendgrid email templates. Would update you once done.

[vikasrohit]

@gondzo long journey of mundane task of email templates is completed. 😬 I still need to update the email service with these templates. However, I have noticed that we are creating too many templates now and sendgrid does not seem to be good at manager transaction templates at least and we are kind of getting confused between production and dev email templates. Do you think there is better way to organize email templates in sendgrid (we add dev in template name right now)? Also, I think we can eliminate some of the templates by grouping them in single email template e.g. all team activity related events can go in single email template, however, we might needed some additional parameters to use that same template e.g. action which would be joined in case a member joined and we need role of the member as well OR we might need to prepare the HTML in tc-notifications instead of letting email service replacing the placeholders because there might be situations where we need programming logic in templates. What do you think about that?

[vikasrohit]

@rmjmccormick @vic-topcoder I would like to have product views as well here as these are new emails which has potential of improving the customer experience but it can annoy them as well if don't use correct email templates or send too many emails which are difficult to organize.

[gondzo]

I think keeping email templates in sendgrid makes sense if multiple services will send same type of email (ie use same template). Otherwise, keeping main email styling in sendgrid template and populating it with raw contents (both html and plain) would be more productive. We're designing new email templates anyway https://github.com/appirio-tech/connect-app/issues/2048 so it would be a great opportunity to go down this road.

For example, we can keep the main styles in sendgrid template (layout, title styles, fonts, links, buttons etc) and the clients can generate raw html with those css classes - without paying attention to exact styling). We can even use handlebars to provide templates for common elements (box, title, image, etc). This would also enable easy redesign of templates in the future without changing the clients or any individual templates.

[vikasrohit]

Thanks @gondzo. Yes, it makes sense to have templates in sendgrid when it is being used by multiple services, however, I think we need templates even if single service is going to send emails using that templates because we don't want to build and deploy to production just to update a template which would be the case if we don't keep templates in a repository like sendgrid. And I think that would be a problem with handlebars as well because with that we have to keep the templates somewhere in code, right (correct me if I am wrong)? Also, the need of visually rich email templates further requires separate email templates for almost every event because when we are making emails visually rich, I can expect every part of the content to be rich and that might require some logic (e.g. hiding/showing based on certain condition or showing different colors for cases). And if we keep a common template e.g. for all project updates, the final template would be very complicated and difficult to maintain.

PS: There is very minimal or no support for logical expressions in sendgrid transaction templates.

[gondzo]

@vikasrohit since sendgrid templates don't support logical expressions, we still need to build the html content in our services (as we're doing right now for bundled emails https://github.com/topcoder-platform/tc-notifications/blob/48a2e230cc634611eb8b28180cd43f8ba77fe30d/connect/notificationServices/email.js#L47 ), so we definitely need to update code if we want different data in the email.

Another thing we have to do is inline all css into html and sendgrid won't do that for us, we have to do that manually in the email service. That makes sendgrid templates quite useless IMO. Also, having templates in Sendgrid makes it hard to verify dev and prod are in sync?

[vikasrohit]

Agreed. However, having templates in sendgrid at least provides some common header/footer e.g. we can put topcoder signature and unsubscribe link via templates. Having said that, I think we should try to keep the number of templates minimum for the time being so that we have less to maintain and we can do that by creating a single template for all project updates (including member join/left events) where we are preparing the actual content of the email in tc-notificaitons (we should not touch tc-email-service in any case).

fyi @rmjmccormick, we are proceeding with very simple email templates for all new events that we are going to support for sending email notifications. There would be only one basic template in the sendgrid for all project updates and we would prepare the actual content of the email in our backend services and send it to sendgrid instead of having a rich template in sendgrid and just send the data to replace it in the template.

[gondzo]

Closing this one and continuing discussion at #59