phac-nml / irida-next

IRIDA Next
https://phac-nml.github.io/irida-next/
Apache License 2.0
8 stars 3 forks source link

Email notifications for pipelines #519

Closed ksierks closed 5 months ago

ksierks commented 6 months ago

What does this PR do and why?

Implemented emails for notifying the submitter when a pipeline completes or errors. Email template style copied from #464 for consistency. Addresses half of #476.

Screenshots or screen recordings

image

image

How to set up and validate locally

  1. Install local mock email service, Mailhog.
  2. After setup, open a terminal and run ~/go/bin/MailHog.
  3. Run irida-next.
  4. Open a rails console.
  5. Select a workflow execution. w = WorkflowExecution.first
  6. Make sure email notifications are enabled. w.email_notification = true
  7. Change state to completed or error. w.state = "completed"
  8. Save the workflow execution. w.save
  9. Check mock email service for sent email. Open a browser and navigate to http://0.0.0.0:8025/.
  10. Repeat steps for state error.

Note: Email previews are located at http://localhost:3000/rails/mailers/pipeline_mailer.

PR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

ksierks commented 5 months ago

Hey Katherine, this is working as described. I looked at the emails in actual email clients and unfortunately some of the styling is lost: Work outlook: image

Browser outlook: image

I talked to Deep about this and if it's possible to fix this with roadie, that'd be great. If you find it's not feasible, then an alternative may be needed.

I think the problem might be the custom css in application.css. Importing a separate mailer.tailwind.css file might work. For simplicity, I removed roadie-rails and stuck with the inline plain css and tried to use lots of tables as that's what Outlook likes. I still need to see what it looks like in other mail clients.

ericenns commented 5 months ago

I think the problem might be the custom css in application.css. Importing a separate mailer.tailwind.css file might work. For simplicity, I removed roadie-rails and stuck with the inline plain css and tried to use lots of tables as that's what Outlook likes. I still need to see what it looks like in other mail clients.

The problem was that outlook has very minimal CSS support, especially ignoring any styling inside of <div> tags. https://www.emailonacid.com/blog/article/email-development/how-to-code-emails-for-outlook-2016/

Though I did see some error logging from roadie-rails due to tailwindcss

ksierks commented 5 months ago

I think the problem might be the custom css in application.css. Importing a separate mailer.tailwind.css file might work. For simplicity, I removed roadie-rails and stuck with the inline plain css and tried to use lots of tables as that's what Outlook likes. I still need to see what it looks like in other mail clients.

The problem was that outlook has very minimal CSS support, especially ignoring any styling inside of <div> tags. https://www.emailonacid.com/blog/article/email-development/how-to-code-emails-for-outlook-2016/

Though I did see some error logging from roadie-rails due to tailwindcss

What I meant by the problem, was I was trying to fix the differences b/w the export outlook client and pipeline outlook client. There was some extra table bordering. I was only using Roadie to help emails render tailwindcss, nothing more. Assuming the error is the one in the CI logs, I forgot to commit the gem.lock file. I've removed Roadie completely. From my PR description, the intention was to copy the style from the export email notification PR as it was ready for testing first. I spent a significant amount of time on my commit from yesterday trying to get the email to look good within the outlook email client as that is our main workplace email client. This is a screenshot with how it looks from an Outlook client with a zoom out of 60% to be able to see everything: email If this doesn't address the concern, I'll match what is done from the export email notification PR, thank you.

github-actions[bot] commented 5 months ago

Simplecov Report

Covered Threshold
92% 90%