Learning emails 4534 #5196
Learning emails 4534 #5196
Conversation
…/casa into learning-emails-4534
rae-stanton
requested review from
compwron,
FireLemons and
littleforest
as code owners
|
CodeClimate seems to be complaining about duplicated code in some mailers - I guess that's unavoidable here. |
app/views/learning_hours_mailer/learning_hours_report_email.html.erb
Outdated
Show resolved
Hide resolved
app/views/learning_hours_mailer/learning_hours_report_email.html.erb
Outdated
Show resolved
Hide resolved
app/views/learning_hours_mailer/learning_hours_report_email.html.erb
Outdated
Show resolved
Hide resolved
Removed some extra cruft from the mailer view, updated mailer, and updated rake task
|
@littleforest I updated the commits here - thanks for reviewing it. Let me know if anything else needs adjusting. |
|
@rae-stanton can you merge in main and push back out? Let's see if those flaky tests will pass. |
Yeah! Just making sure - you want me to go ahead and run the suite (at least, the things I touched in the mailer) while in main and make sure they're passing? Then pull back out and see what changes need to be made? Thanks! @littleforest I forgot to tag you in this. |
|
@rae-stanton no, I meant can you merge the |
|
@littleforest I gotcha! I went ahead and upstreamed |
What GitHub issue is this PR for, if any?
Resolves #4534
What changed, and why?
Added new learning hour report functionality for supervisors and admins to receive on a monthly basis. This is available for selection for admin edit and supervisor edit pages.
How will this affect user permissions?
How is this tested? (please write tests!) 💖💪
Mailer spec test added for core mailer, happy to add extra test coverage, but I wasn't sure if tests on the small model changes for admins/supervisors were needed to reflect toggling receiving learning hours reports monthly. I don't have access to Heroku for seeing if the cron jobs are working as expected, so that may need to be tested out by someone with Heroku access.
Screenshots please :)