Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add report emails to preservation workflow #1248

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

jazairi
Copy link
Contributor

@jazairi jazairi commented Nov 9, 2023

Why these changes are being introduced:

We found a bug in the preservation workflow last month that we might have identified sooner if the preservation submission job triggered a report email.

Relevant ticket(s):

https://mitlibraries.atlassian.net/browse/ETD-651

How this addresses that need:

  • Removes the Preservation Submission Prep Job
  • Updates the Preservation Submission Job to accept an array of theses
  • Adds a preservation results email to the Report Mailer that is enqueued on completion of the Preservation Submission Job
  • Updates the preservation rake task to convert the input thesis into an array, so it will continue to work

Side effects of this change:

We not sure why we implemented the 'prep' job pattern: passing in an array of theses so the actual job takes a single thesis as a parameter. There is some risk in removing this job, since we don't remember why it exists in the first place.

Developer

  • All new ENV is documented in README
  • All new ENV has been added to Heroku Pipeline, Staging and Prod
  • ANDI or Wave has been run in accordance to
    our guide and
    all issues introduced by these changes have been resolved or opened as new
    issues (link to those issues in the Pull Request details above)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

Requires database migrations?

NO

Includes new or updated dependencies?

NO

@mitlib mitlib temporarily deployed to thesis-submit-pr-1248 November 9, 2023 18:42 Inactive
@coveralls
Copy link

coveralls commented Nov 9, 2023

Coverage Status

coverage: 98.315% (+0.004%) from 98.311%
when pulling 94b3eab on etd-646-preservation-reporting-eval
into 337eb35 on main.

@jazairi jazairi force-pushed the etd-646-preservation-reporting-eval branch from 3c99cf7 to 02d1fc1 Compare November 9, 2023 18:50
@jazairi jazairi temporarily deployed to thesis-submit-pr-1248 November 9, 2023 18:50 Inactive
Why these changes are being introduced:

We found a bug in the preservation workflow last month that we
might have identified sooner if the preservation submission job
triggered a report email.

Relevant ticket(s):

https://mitlibraries.atlassian.net/browse/ETD-651

How this addresses that need:

* Removes the Preservation Submission Prep Job
* Updates the Preservation Submission Job to accept an array of theses
* Adds a preservation results email to the Report Mailer that is
enqueued on completion of the Preservation Submission Job
* Updates the preservation rake task to convert the input thesis into
an array, so it will continue to work

Side effects of this change:

We not sure why we implemented the 'prep' job pattern: passing in
an array of theses so the actual job takes a single thesis as a
parameter. There is some risk in removing this job, since we don't
remember why it exists in the first place.
@jazairi jazairi force-pushed the etd-646-preservation-reporting-eval branch from 02d1fc1 to 94b3eab Compare November 9, 2023 18:53
@jazairi jazairi temporarily deployed to thesis-submit-pr-1248 November 9, 2023 18:53 Inactive
@@ -19,4 +19,14 @@ def publication_results_email(results)
cc: ENV['MAINTAINER_EMAIL'],
subject: 'DSpace publication results summary')
end

def preservation_results_email(results)
return unless ENV.fetch('DISABLE_ALL_EMAIL', 'true') == 'false' # allows PR builds to disable emails
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still breaks my brain every time I see it. I still can't parse what it does, but only know it works because it works.

@JPrevost JPrevost self-assigned this Nov 9, 2023
@jazairi jazairi merged commit 7c1ad59 into main Nov 15, 2023
2 of 3 checks passed
@jazairi jazairi deleted the etd-646-preservation-reporting-eval branch November 15, 2023 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants