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

HOTFIX: Resolve delivery location fetch issue for off-site items with legacy barcode identifiers #460

Merged
merged 8 commits into from
Jan 24, 2025

Conversation

dgcohen
Copy link
Contributor

@dgcohen dgcohen commented Jan 23, 2025

This PR addresses an issue that was identified after the launch of the Hold pages, where certain off-site partner records were showing an error on the hold request page.

The issue occurred because the delivery location fetch was failing. This was due to missing functionality in DFE, where barcodes are extracted from the identifier field for bibs with legacy formatting.

This PR adds the necessary functionality to resolve the issue. I've also opened a follow-up ticket to refactor the solution and add unit tests.

Copy link

vercel bot commented Jan 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
research-catalog ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 24, 2025 2:53pm

@EdwinGuzman EdwinGuzman requested a review from nonword January 23, 2025 22:21
@dgcohen dgcohen changed the title HOTFIX - Fix failing delivery location fetch for off-site items with legacy barcode identifier HOTFIX: Resolve delivery location fetch issue for off-site items with legacy barcode identifiers Jan 24, 2025
Copy link
Member

@EdwinGuzman EdwinGuzman 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 ok to me but should have another person who knows this better approve it. Were there any tests in DFE for these functions?

pages/hold/confirmation/[id].tsx Outdated Show resolved Hide resolved
@dgcohen
Copy link
Contributor Author

dgcohen commented Jan 24, 2025

This looks ok to me but should have another person who knows this better approve it. Were there any tests in DFE for these functions?

Yeah, here's a link to the test: https://github.com/NYPL/discovery-front-end/blob/d9a636027c28548f7bd222bf213f05e9a84d618e/test/unit/LibraryItem.test.js#L41

@dgcohen dgcohen changed the base branch from production to qa January 24, 2025 15:39
@dgcohen dgcohen merged commit b5638a4 into qa Jan 24, 2025
3 checks passed
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.

3 participants