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

Assign selected files to identified Thesis #708

Merged
merged 2 commits into from
May 19, 2021

Conversation

matt-bernhardt
Copy link
Member

@matt-bernhardt matt-bernhardt commented May 17, 2021

This makes the Transfer processing form actually do work, attaching selected files from a Transfer to the Thesis record identified by the user.

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

@matt-bernhardt matt-bernhardt marked this pull request as draft May 17, 2021 21:17
@mitlib mitlib temporarily deployed to thesis-submit-pr-708 May 17, 2021 21:17 Inactive
@coveralls
Copy link

coveralls commented May 17, 2021

Coverage Status

Coverage increased (+0.03%) to 94.633% when pulling 2616877 on etd-8-transfer-processing-4 into ffbde8d on main.

@matt-bernhardt matt-bernhardt force-pushed the etd-8-transfer-processing-4 branch from a360044 to e3704cb Compare May 17, 2021 21:46
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-708 May 17, 2021 21:47 Inactive
@matt-bernhardt matt-bernhardt force-pushed the etd-8-transfer-processing-4 branch from e3704cb to 5ddda5f Compare May 17, 2021 21:56
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-708 May 17, 2021 21:56 Inactive
@matt-bernhardt matt-bernhardt force-pushed the etd-8-transfer-processing-4 branch from 5ddda5f to 7c867f5 Compare May 18, 2021 15:16
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-708 May 18, 2021 15:16 Inactive
@matt-bernhardt matt-bernhardt marked this pull request as ready for review May 18, 2021 15:31
@matt-bernhardt matt-bernhardt requested review from JPrevost and jazairi and removed request for JPrevost and jazairi May 18, 2021 15:31
** Why are these changes being introduced:

* Submitting the transfer processing form, with files and a thesis
  selected, needs to cause those files to actually be assigned to the
  chosen thesis. The original affiliation between those files and their
  transfer should be retained.

** Relevant ticket(s):

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

** How does this address that need:

* This expands the files method on the Transfer controller to attach the
  blob for each file to the selected Thesis record. The flash message
  sent back to the user is updated to reflect this work (replacing the
  test message used previously).
* In order to provide a meaningful summary to the processor on the
  Transfer queue page, a new "unassigned_files" method is added to the
  Transfer model. This returns the count of files which are only
  attached to _one_ record (i.e the Transfer itself). This count is now
  displayed on the Transfer queue alongside the total number of files,
  i.e. "4 / 10" reflecting four of the original ten files still need
  transferred.
* The display of the Transfer processing form itself still lists all
  received files, but those already assigned to a Thesis are now crossed
  out, with the title of the Thesis displayed.
* The unassigned_files method is now tested at the model and controller
  level.

** Document any side effects to this change:

* The logic is a little messier, with the Transfer controller calling
  Thesis.files.attach to actually do the work - but I don't see a way
  to avoid that at some level. For now, I've tried to focus this work
  on the Transfer side of things, but I'm open to other approaches.
@matt-bernhardt matt-bernhardt force-pushed the etd-8-transfer-processing-4 branch from 7c867f5 to 175a21b Compare May 18, 2021 15:53
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-708 May 18, 2021 15:53 Inactive
@JPrevost JPrevost self-assigned this May 19, 2021
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-708 May 19, 2021 15:56 Inactive
@matt-bernhardt matt-bernhardt merged commit 4d19875 into main May 19, 2021
@matt-bernhardt matt-bernhardt deleted the etd-8-transfer-processing-4 branch May 19, 2021 21: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