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

Make Metric System as Default for Physical Values of the Book. #4134

Conversation

zaheerabbas21
Copy link
Contributor

Closes #2242

Make the Metric System as Default for Physical Values of the Book while editing. Most comments in the issue favored the metric system so went with the metric.

Technical

Not that much technical, just changed the default to centimeters from inches and to kilos from grams.

Testing

docker-compose exec web make test returned no errors.

Screenshot

Screenshot from 2020-11-21 03-52-50

Stakeholders

@seabelis @cdrini

@bicolino34
Copy link
Collaborator

@cdrini please review this PR

@RayBB RayBB added the Needs: Review This issue/PR needs to be reviewed in order to be closed or merged (see comments). [managed] label Oct 26, 2023
@cdrini cdrini assigned RayBB and unassigned cdrini Oct 26, 2023
@cdrini
Copy link
Collaborator

cdrini commented Oct 26, 2023

Apologies for the silence here! We brought this up long ago during a community call and got caught in a quagmire about trying to make this change depending on locale. But this PR looks good as is!

Assigning to @RayBB who graciously agreed to give it a test and move it forward!

@RayBB
Copy link
Collaborator

RayBB commented Jan 27, 2024

Steps to test (from Drini):

  • Defaults test: Adding weight/size without changing anything. Afterwards, go to the books .json view and confirm correct
  • Non defaults test: Modify the default units, add data, save. Confirm JSON correct
  • Read test: edit the previous two records. Are the units selected in the UI correct, and the same as what you entered before?

@RayBB
Copy link
Collaborator

RayBB commented Jan 27, 2024

@cdrini I just completed the tests and all is working!

Spoiler
Make.Metric.System.as.Default.for.Physical.Values.of.the.Book.by.nk4456542.Pull.Request.4134.internetarchive_openlibrary.-.27.January.2024.mp4

@RayBB RayBB self-requested a review January 27, 2024 15:32
Copy link
Collaborator

@RayBB RayBB left a comment

Choose a reason for hiding this comment

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

tested and working as expected

@RayBB RayBB requested a review from cdrini January 27, 2024 15:32
@cdrini cdrini removed the Needs: Review This issue/PR needs to be reviewed in order to be closed or merged (see comments). [managed] label Feb 27, 2024
@cdrini cdrini merged commit d059890 into internetarchive:master Feb 27, 2024
2 checks passed
@cdrini
Copy link
Collaborator

cdrini commented Feb 27, 2024

Thanks @nk4456542 and @RayBB for testing and reviewing!

Achorn pushed a commit to Achorn/openlibrary that referenced this pull request Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Edit form Physical Object section inputs should default to either metric or imperial/American, but not both.
4 participants