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

Remove ProQuest submission field from thesis forms #1396

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

jazairi
Copy link
Contributor

@jazairi jazairi commented Dec 3, 2024

Why these changes are being introduced:

The Libraries are suspending the pilot with ProQuest, and the thesis forms should be updated.

Relevant ticket(s):

How this addresses that need:

This removes the ProQuest fieldset from the new and edit forms. It also removes the author_fields partial, which includes only this fieldset.

Note that this does not remove any additional ProQuest export functionality, as specifically requested by stakeholders.

Side effects of this change:

Because no functionality outside of the thesis form has been changed, thesis processors can still run ProQuest exports. There is no apparent risk to this, and we can fully remove the feature if stakeholders decide to do so.

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-1396 December 3, 2024 21:46 Inactive
@coveralls
Copy link

coveralls commented Dec 3, 2024

Coverage Status

coverage: 98.313%. remained the same
when pulling 9f627a4 on etd-658-remove-pq-form
into d789473 on main.

@jazairi jazairi temporarily deployed to thesis-submit-pr-1396 December 3, 2024 21:56 Inactive
@JPrevost JPrevost self-assigned this Dec 3, 2024
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.

This looks good. I forgot how complex this feature is under the hood and it took me a while to confirm the Export would work as I expected with the null values we'll have (the tests had coverage for it which was great, but I wanted to see it working locally (and it worked differently than I expected but is doing the correct thing so I'm glad I checked). After further reflection, it makes sense it works the way it does as Users aren't actually required to login and update their metadata so null has always been a supported status for this.

Please tag Mikki in the ETD channel and Jira ticket to review once this merges but before it is moved to production. Production launch for this will be whenever the ETD says to launch it (not sure if they want to update some supporting instructions first or if they'll want this live once they confirm it).

@JPrevost
Copy link
Member

JPrevost commented Dec 4, 2024

Also, I noticed we don't update the strong params (https://github.com/MITLibraries/thing/blob/main/app/controllers/thesis_controller.rb#L193) as part of this but I think because the feature is remaining on the admin-side it makes sense to leave it as-is. I'm saying that here in case you look at it and disagree and think it can come out now. We seem to use the same params for multiple form updates which seems maybe not the best idea but is outside the scope of this changeset.

Why these changes are being introduced:

The Libraries are suspending the pilot with ProQuest, and the
thesis forms should be updated.

Relevant ticket(s):

* [ETD-658](https://mitlibraries.atlassian.net/browse/ETD-658)

How this addresses that need:

This removes the ProQuest fieldset from the new and edit forms.
It also removes the author_fields partial, which includes only this
fieldset.

Note that this does not remove any additional ProQuest export
functionality, as specifically requested by stakeholders.

Side effects of this change:

Because no functionality outside of the thesis form has been
changed, thesis processors can still run ProQuest exports. There
is no apparent risk to this, and we can fully remove the feature
if stakeholders decide to do so.
@jazairi jazairi force-pushed the etd-658-remove-pq-form branch from fa0a4c5 to 9f627a4 Compare December 4, 2024 14:01
@jazairi jazairi temporarily deployed to thesis-submit-pr-1396 December 4, 2024 14:01 Inactive
@jazairi jazairi merged commit 694d3e5 into main Dec 4, 2024
3 checks passed
@jazairi jazairi deleted the etd-658-remove-pq-form branch December 4, 2024 14: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