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

Notify processors to create new Archivematica accessions #1235

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

jazairi
Copy link
Contributor

@jazairi jazairi commented Oct 3, 2023

Why these changes are being introduced:

If a processor adds an Archivematica Accession for a thesis that is otherwise ready for publication, they will need to update that thesis before its publication status switches over to 'Publication review'. This could become substantially time consuming when working with large batches of theses.

Relevant ticket(s):

How this addresses that need:

This creates new Degree Period records during the Registrar Data Import job, using a similar pattern to how we create new Degree and Department records. It also adds new Degree Period records to the mailer, so that processors can proactively create Archivematica Accession records for them.

Side effects of this change:

Rubocop identified and fixed a couple of issues unrelated to this change.

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-1235 October 3, 2023 18:43 Inactive
@coveralls
Copy link

coveralls commented Oct 3, 2023

Coverage Status

coverage: 98.361% (+0.008%) from 98.353% when pulling 5b529d5 on etd-640-accession-reporting into 031f501 on main.

@jazairi
Copy link
Contributor Author

jazairi commented Oct 3, 2023

The codeclimate criticism echoes my own concern when I started looking at the registrar import job. I do think it's fixable, but I'm not sure that abstracting all of that code into tiny methods will make it any more readable. I'd welcome reviewer feedback on this.

@JPrevost JPrevost self-assigned this Oct 3, 2023
Copy link
Member

@JPrevost JPrevost left a comment

Choose a reason for hiding this comment

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

For better or worse this solution feels very much like it fits in this code base :)

Solid plan for this, my comments are not blocking just thoughts/questions.

Why these changes are being introduced:

If a processor adds an Archivematica Accession for a thesis that
is otherwise ready for publication, they will need to update that
thesis before its publication status switches over to 'Publication
review'. This could become substantially time consuming when
working with large batches of theses.

Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/ETD-596

How this addresses that need:

This creates new Degree Period records during the Registrar Data
Import job, using a similar pattern to how we create new Degree
and Department records. It also adds new Degree Period records
to the mailer, so that processors can proactively create
Archivematica Accession records for them.

Side effects of this change:

* One of the registrar data fixtures has been updated so that more than
one degree date will be created in the relevant test.
* Rubocop identified and fixed a couple of issues unrelated to this
change.
@jazairi jazairi force-pushed the etd-640-accession-reporting branch from 32af509 to 5b529d5 Compare October 4, 2023 14:45
@jazairi jazairi merged commit 1a92aa6 into main Oct 4, 2023
1 check passed
@jazairi jazairi deleted the etd-640-accession-reporting branch October 4, 2023 15:00
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