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

Add generalized de-dupe for ISBN and LCCN for edit edition form #8280

Merged
merged 3 commits into from
Sep 18, 2023

Conversation

sbwhitt
Copy link
Collaborator

@sbwhitt sbwhitt commented Sep 11, 2023

Closes #8281

Implements the new de-dupe function for the edit edition page that accepts any identifier type.

Technical

Testing

  • Add a valid identifier of any type
  • Attempt to add the same identifier for each type
  • Duplicate identifiers will be rejected with an informative error message
  • Duplicate identifier values of different Id types will be able to be added
    • Eg. 1234567890 for ISBN 10 and 1234567890 for LCCN is allowed

Screenshot

LCCN
image
image

ISBN 10
image
image

Stakeholders

@scottbarnes

@sbwhitt sbwhitt changed the title Add generalized de-dupe for isbn and lccn Add generalized de-dupe for ISBN and LCCN for edit edition form Sep 11, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2023

Codecov Report

Merging #8280 (72539d2) into master (60026bf) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #8280      +/-   ##
==========================================
+ Coverage   17.08%   17.10%   +0.02%     
==========================================
  Files          75       75              
  Lines        3946     3947       +1     
  Branches      675      675              
==========================================
+ Hits          674      675       +1     
  Misses       2842     2842              
  Partials      430      430              
Files Changed Coverage Δ
openlibrary/plugins/openlibrary/js/edit.js 39.06% <100.00%> (-0.63%) ⬇️
openlibrary/plugins/openlibrary/js/idValidation.js 100.00% <100.00%> (ø)

@sbwhitt sbwhitt force-pushed the feat/id-dupe branch 2 times, most recently from 62cc49c to 72539d2 Compare September 12, 2023 13:54
Copy link
Collaborator

@scottbarnes scottbarnes 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 this! One suggestion to consider.

openlibrary/plugins/openlibrary/js/edit.js Outdated Show resolved Hide resolved
openlibrary/plugins/openlibrary/js/edit.js Outdated Show resolved Hide resolved
openlibrary/plugins/openlibrary/js/edit.js Outdated Show resolved Hide resolved
openlibrary/plugins/openlibrary/js/edit.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@scottbarnes scottbarnes left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for this!

@scottbarnes scottbarnes merged commit a797a05 into internetarchive:master Sep 18, 2023
@sbwhitt sbwhitt deleted the feat/id-dupe branch September 18, 2023 15:20
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.

Add generalized de-dupe function for ISBN and LCCN for edit edition form
3 participants