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

Theses not publishable with duplicate filenames #1236

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

JPrevost
Copy link
Member

@JPrevost JPrevost commented Oct 4, 2023

Why are these changes being introduced:

  • we cannot send theses to preservation with duplicate filenames
  • preservation is a step after publication, so doing this check before publication will ensure we don't publish things we can't preserve

Relevant ticket(s):

How does this address that need:

  • Moves the baggable? logic to it's own module
  • Inlcudes that module in both SIP and Thesis models
  • Uses the unique_filenames? check as part of the thesis publication checks
  • Adds a visual display to the processor checklist to indicate if filenames are unique

Document any side effects to this change:

  • There is no direct mechanism in the application to allow a processor to resolve duplicate filename issues

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?

YES

@mitlib mitlib temporarily deployed to thesis-submit-pr-1236 October 4, 2023 14:27 Inactive
assert_equal true, thesis.departments_have_dspace_name?
assert_equal true, thesis.degrees_have_types?
assert_equal true, thesis.accession_number.present?
assert thesis.valid?
Copy link
Member Author

Choose a reason for hiding this comment

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

These changes are just to use the preferred syntax for asserting truthy/falsey:

https://minitest.rubystyle.guide/#assert-truthy

assert thesis.departments_have_dspace_name?
assert thesis.degrees_have_types?
assert thesis.accession_number.present?
assert thesis.unique_filenames?(thesis)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only new assertion in this section.

@coveralls
Copy link

coveralls commented Oct 4, 2023

Coverage Status

coverage: 98.367% (+0.006%) from 98.361% when pulling 2f3faf3 on etd648-baggable-thesis-for-publication into 1a92aa6 on main.

@jazairi jazairi self-assigned this Oct 4, 2023
Copy link
Contributor

@jazairi jazairi left a comment

Choose a reason for hiding this comment

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

I don't have time for a thorough review of this today (i.e., I have not verified the changes locally), but from what I can tell it looks like it solves the problem. If you'd like, I can give it a closer look on Tuesday either pre- or post-merge.

I still think we need a processor workflow to rename files and probably one or more new file purposes, but I think we agree that should be follow-on work and this should merge to avoid any additional issues in prod.

Speaking of which, we cannot deploy to prod until this ticket closes.

Why are these changes being introduced:

* we cannot send theses to preservation with duplicate filenames
* preservation is a step after publication, so doing this check before
  publication will ensure we don't publish things we can't preserve

Relevant ticket(s):

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

How does this address that need:

* Moves the baggable? logic to it's own module
* Inlcudes that module in both SIP and Thesis models
* Uses the unique_filenames? check as part of the thesis publication
  checks
* Adds a visual display to the processor checklist to indicate if
  filenames are unique

Document any side effects to this change:

* There is no direct mechanism in the application to allow a processor
  to resolve duplicate filename issues
@JPrevost JPrevost force-pushed the etd648-baggable-thesis-for-publication branch from 91aa546 to 2f3faf3 Compare October 4, 2023 16:14
@JPrevost JPrevost temporarily deployed to thesis-submit-pr-1236 October 4, 2023 16:15 Inactive
@JPrevost JPrevost merged commit 2fad84a into main Oct 4, 2023
1 check passed
@JPrevost JPrevost deleted the etd648-baggable-thesis-for-publication branch October 4, 2023 17:03
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