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

Set current location from request service point for 'awaiting pickup' items #1020

Merged
merged 3 commits into from
Jul 27, 2023

Conversation

corylown
Copy link
Contributor

@corylown corylown commented Jul 26, 2023

Closes #974

@corylown corylown force-pushed the 974-awaiting-pickup branch 3 times, most recently from eb92274 to 0906b8a Compare July 27, 2023 13:21
@corylown corylown marked this pull request as ready for review July 27, 2023 13:39
Copy link
Contributor

@jcoyne jcoyne left a comment

Choose a reason for hiding this comment

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

Can you confirm you've run this with the folio_postgres_reader_spec.rb and the time isn't adversely affected?

end

# ARS-LOAN, RUM-LOAN, and SPE-LOAN each receives a
# special label in SearchWorks. Other codes are generically
Copy link
Contributor

Choose a reason for hiding this comment

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

Where will these labels come from in Searchworks? Aren't we moving toward using Folio location labels?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The equivalent in FOLIO for these locations would be service points, which I don't think we're doing anything with in SearchWorks yet. We might want to. But this gets us to parity with Symphony by relying on the existing Constants:LOCS mappings.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the intention is to remove Constants::LOCS after migrating to Folio, so we need to handle these in some other way. (see sul-dlss/SearchWorks#3310)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cbeer I'm confused about what the plan is for Constants::LOCS vs FOLIO display names. My guess is we're still going to have location codes that don't map to anything in FOLIO and we'll need to maintain a fallback list in SearchWorks (maybe I'm wrong about this). This PR is a refinement to previous work to map folio statuses to symphony locations so they'll display right in searchworks. Is this no longer what we want to do?

See:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried to capture post-folio work related to this in #1026

Copy link
Contributor

@jcoyne jcoyne left a comment

Choose a reason for hiding this comment

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

Aren't we going to be moving away from item_display and towards using item_display_struct? Do we need this change there as well?

@corylown
Copy link
Contributor Author

Aren't we going to be moving away from item_display and towards using item_display_struct? Do we need this change there as well?

No additional change is needed. This is modifying the current_location value used to construct sirsi_holdings for FOLIO records and sirsi_holdings is what's used to construct the newitem_display_struct field (which is then used to create the legacy item_display field).

@corylown
Copy link
Contributor Author

Can you confirm you've run this with the folio_postgres_reader_spec.rb and the time isn't adversely affected?

It's still within the expected range of 30 to 90 seconds:

Traject::FolioPostgresReader
  creates FolioRecords

Top 1 slowest examples (51.85 seconds, 100.0% of total time):
  Traject::FolioPostgresReader creates FolioRecords
    51.85 seconds ./spec/lib/traject/readers/folio_postgres_reader_spec.rb:19

Finished in 51.86 seconds (files took 1.57 seconds to load)
1 example, 0 failures

@cbeer cbeer merged commit 92700b7 into main Jul 27, 2023
2 checks passed
@cbeer cbeer deleted the 974-awaiting-pickup branch July 27, 2023 22:08
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.

Handling awaiting + in-transit statuses with more specific information
3 participants