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

Change content type back to application/json for H2 files #585

Merged
merged 6 commits into from
Jun 15, 2023

Conversation

peetucket
Copy link
Member

@peetucket peetucket commented Jun 7, 2023

Why was this change made? 🤔

Part of sul-dlss/happy-heron#3075

Needs to go with sul-dlss/happy-heron#3083 and sul-dlss/sdr-client#284

Todo:

  • update openapi.yml file to indicate that application/json files can be set to content type application/x-stanford-json if you do not want validation to occur

How was this change tested? 🤨

On QA with H2 and added new specs here

Verified

This commit was signed with the committer’s verified signature.
m4tx Mateusz Maćkowski
…tent_type, reset back to application/json
@peetucket peetucket force-pushed the relax_put_disk_request_check branch from 8ce9bfc to c6b4757 Compare June 8, 2023 23:36
@peetucket peetucket changed the title [WIP] Override active storage validation of file contents [WIP] Change content type back to application/json for H2 files Jun 8, 2023
@peetucket peetucket changed the title [WIP] Change content type back to application/json for H2 files Change content type back to application/json for H2 files Jun 9, 2023
@peetucket peetucket marked this pull request as ready for review June 9, 2023 19:37

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link
Member

@jmartin-sul jmartin-sul left a comment

Choose a reason for hiding this comment

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

one nitpicky comment suggestion, and two nitpicky questions re: tests, but the application logic change LGTM.

peetucket and others added 3 commits June 15, 2023 08:28
Co-authored-by: Johnathan Martin <jmartin-sul@users.noreply.github.com>
Copy link
Member

@jmartin-sul jmartin-sul left a comment

Choose a reason for hiding this comment

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

thanks for the openapi.yml edit, and for rolling in the tests @edsu experimented with for reproducing the undesired 400 in #586! had a note to myself to add the former and to try doing the latter 🎉

requesting changes for a small thing: i think the openapi comment needs to be moved to the /v1/disk route. no good deed, and all that 😬

@peetucket peetucket force-pushed the relax_put_disk_request_check branch from 5d9941d to 930a199 Compare June 15, 2023 17:09
@peetucket peetucket requested a review from jmartin-sul June 15, 2023 17:24
Copy link
Member

@jmartin-sul jmartin-sul left a comment

Choose a reason for hiding this comment

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

🌮

@jmartin-sul jmartin-sul merged commit 02e3a4f into main Jun 15, 2023
@jmartin-sul jmartin-sul deleted the relax_put_disk_request_check branch June 15, 2023 18:28
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.

None yet

2 participants