Email templates support about tc-notifications HOT 17 CLOSED

i forgot to attach it. here it is

email-connect (1).zip

from tc-notifications.

OK, build time inlining is fine then.
Mustache vs handlebars - it's up to you, both will serve the purpose here

from tc-notifications.

from tc-notifications.

Thanks @architectt1 for the updated PR.

from tc-notifications.

Note: we can use notifications.action.email.connect.project.specificationModified for all emails at the moment (this event has the above sendgrid template).

from tc-notifications.

@gondzo @vikasrohit About this:

UI proto project has the css inliner support configured and it needs to be integrated into tc-notifications (see src/emails/one/one.template.html vs dist/one.template.html)

I'm not sure what repo is that, would you mind providing a link to it? Thanks.

from tc-notifications.

Should we use a single email template with logic in it to be able to display the aforementioned data for each event type?

from tc-notifications.

@gondzo @vikasrohit I wanted to confirm with you the approach I was thinking of using:

• As versioned source we will have html and scss files.
• On project build, we will compile the scss to css, and inline the html with that css.
• On runtime, the compiled html will be used as template to inject the different fields.

Do you think this is a good approach?

from tc-notifications.

Is it easier to add the html for all notifications first and then inline all the css - at build time we don't know all the contents of the email?

from tc-notifications.

Maybe we could use handlebars templates for each event type - they should be easy to maintain in the future (just a suggestion)?

from tc-notifications.

Is it easier to add the html for all notifications first and then inline all the css

If you mean on runtime, I think think would present a bit of a performance issue. I understand the style doesn't change, so we should be able to inline that on build time. The template would only contain the part we need to insert in the {{contents}} as said in

In tc notifications, we need to build only the content inside "body table.container", inline the css and send that as the contents placeholder value

Regarding:

at build time we don't know all the contents of the email?

No, at build time we build the template with all the inline styling. The result would be the template with inline css that when variables get replaced on runtime, that will be used as {{contents}}.

Does that make sense/do you see a disadvantage in this approach?

from tc-notifications.

Maybe we could use handlebars templates for each event type - they should be easy to maintain in the future (just a suggestion)?

I'm a bit more used to mustache, although I'm open to suggestions. Do you see an advantage of handlebars over using mustache?

from tc-notifications.

I have some questions:

Bundle notifications should contain the same data as individual notifications, but they should be grouped by projectId and grouped by notification type

Please confirm my understanding of this is correct: on a period basis (current logic of bundled notifications in this project), we should send a single email with a section for each project, and in each of those sections, a section for each group of notification type as defined in here.

So for example, if we have scheduled notifications of types

• 'notifications.connect.project.topic.created'
• 'notifications.connect.project.topic.deleted'
• 'notifications.connect.project.created'

for a given user and project, then we would need to send an email with two sections for that project, one with the first two notifications (and a New posts and replies section title), and another one for the third one (with a Project status changes title).

Regarding this:

Also, multiple notifications of the same type should render just one notification

This makes sense, although the grouping might need further insight into what data is associated to the specific notifications types, depending on what is needed to be displayed. For example, a post notification could have an associated message, and we might want to include that into the email. But if we have two posts for the same project, and we want to group them, we would loose the ability to display this information. That is an observation, and the overall question I'd like an answer for is, given multiple notifications of the same type, should we just include the description of the event (in your example, Project notification was updated), and the list of users that triggered that event?

from tc-notifications.

We can keep the last requirement out of scope for now and implement it
later on (single notification for same event types)

I think we already handling this by showing the count of such events against that event here.

from tc-notifications.

@gondzo @vikasrohit I closed my previous PR and created a new one with just the solicited changes. That should make reviewing easier.

from tc-notifications.

@gondzo we can close this one now, right?

from tc-notifications.

yes

from tc-notifications.