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
Alma: make item status mapping to uncertain work with getStatus call. #2923
Alma: make item status mapping to uncertain work with getStatus call. #2923
Conversation
Previously item status mapping to uncertain based on location type only worked with getHolding. This also fixes the getHolding test to return proper electronic holdings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @EreMaijala -- a couple minor questions before I approve this...
@@ -145,18 +145,20 @@ | |||
{ | |||
"id": "1111", | |||
"source": "Solr", | |||
"callnumber": "HB Ab", | |||
"callnumber": "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a strong reason to remove the call number from the test fixture? Seems like adding the subfield back to minimize diffs and exercise more code might be worthwhile... but if this was done for a reason, I obviously don't object to the change. :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not relevant for electronic availability. The test returned physical item availability previously because the fixture contained wrong field (AVA instead of AVE).
$availStr = strtolower($marc->getSubfield($field, 'e')); | ||
$available = 'available' === $availStr; | ||
// No status message available, so set it based on availability: | ||
$statusText = $available ? 'Item in place' : 'Item not in place'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "Item in place" Alma terminology? If not, any particular reason this language was chosen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, and those are what Alma uses in the item statuses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarifications, @EreMaijala. Merging now! :-)
Previously item status mapping to uncertain based on location type only worked with getHolding.
This also fixes the getHolding test to return proper electronic holdings. The previous mocked response had wrong MARC fields.
To make this work the getItemLocationType had to be moved out from the getItemStatusFromLocationTypeMap method because getStatus only had MARC fields to manage with. This makes for the bulk of the changes.