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

Prototype for removing files from a thesis #725

Merged
merged 1 commit into from
Jun 29, 2021

Conversation

matt-bernhardt
Copy link
Member

@matt-bernhardt matt-bernhardt commented Jun 23, 2021

This implements the ability to remove a file from a thesis, using the existing link_to_remove_association feature provided by SImpleForm. This allows the user to use an existing UI affordance that is already present in other fields to remove a file, in such a way that it remains in the S3 bucket and attached to its original transfer record.

The user will see a flash message with a link back to that Transfer, allowing them to click the link and get back to it, putting the file in its proper location.

Because the file blob is still attached to its Transfer record, it does not get deleted through this process - and it can still be placed into its correct destination from the transfer processing form.

Ticket

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

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 June 23, 2021 21:50
@mitlib mitlib temporarily deployed to thesis-submit-pr-725 June 23, 2021 21:51 Inactive
@coveralls
Copy link

coveralls commented Jun 23, 2021

Coverage Status

Coverage increased (+0.09%) to 94.749% when pulling ec24803 on etd-307-unlink-files into d172c1f on main.

@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-725 June 24, 2021 18:59 Inactive
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-725 June 24, 2021 21:52 Inactive
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-725 June 25, 2021 13:57 Inactive
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-725 June 25, 2021 18:48 Inactive
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-725 June 25, 2021 19:15 Inactive
@matt-bernhardt matt-bernhardt marked this pull request as ready for review June 25, 2021 19:22
@JPrevost JPrevost self-assigned this Jun 25, 2021
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.

I've made a few comments / suggestions. I'm happy to discuss them to determine which ones are worth addressing now and which we can just say "yup" and ignore for now.

test/controllers/thesis_controller_test.rb Outdated Show resolved Hide resolved
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-725 June 28, 2021 14:19 Inactive
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-725 June 28, 2021 17:44 Inactive
@matt-bernhardt matt-bernhardt requested a review from JPrevost June 28, 2021 17:54
** Why are these changes being introduced:

* There are scenarios wherein a file will be attached to a thesis, and
  will need to be removed (either through user error, or the submission
  of updated copies).

** Relevant ticket(s):

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

** How does this address that need:

* This uses the existing SimpleForm feature for removing linked records
  to enable a "remove" link on each file in the thesis processing form.
  Testing has shown that this will not remove the file from S3, as it
  will still be attached to the transfer which originally contained it.
* Tests of this scenario have been added, to ensure that the resulting
  flash message has a link back to the transfer.
* An additional test to make sure that untargeted files are not affected
  has also been added.

** Document any side effects to this change:

* There is a new partial in the thesis folder, having taken the file
  table fields that used to be in the form template and made them into
  their own partial. This is the pattern used for other add-another
  fields.
* There is now a setup method in the thesis controller tests, similar
  to that used on the transfer controller.
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-725 June 29, 2021 14:03 Inactive
@matt-bernhardt matt-bernhardt merged commit 10deeb1 into main Jun 29, 2021
@matt-bernhardt matt-bernhardt deleted the etd-307-unlink-files branch June 29, 2021 14:17
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