FOLIO: Limit request pickupLocation options by item location #3876
Conversation
There was a problem hiding this comment.
Thanks, @maccabeelevine, a few initial thoughts based on an admittedly-cursory quick review. :-)
|
Thanks @maccabeelevine. We've done some testing on this, but with the caveat that it was in v9.1. In our system, we have several service points available for pickup, and title-level holds enabled, so were able to test different scenarios pretty thoroughly. Everything worked as described - item requests were restricted to pickup from the item's effective location, and title-level requests allowed any pickup location. However, we can't imagine a scenario where a library would want to use this functionality in combination with title-level requests. We also have slightly more complicated needs within our library, and would want to be able to apply this to specific service points (most locations in our system allow their items to be picked up anywhere, but one branch with specialised material does not). |
There was a problem hiding this comment.
Thanks, @dybarber, for the testing! Sounds like this could be approved and merged as-is as a first step, or we could wait for @maccabeelevine to add some more nuance, depending on what he thinks is best/feasible.
In the meantime, one small refactoring suggestion based on a deeper review of the code.
| // so limiting by itemEffectiveLocation does not apply | ||
| && $holdInfo['item_id'] ?? false | ||
| ) { | ||
| $response = $this->makeRequest( |
There was a problem hiding this comment.
I think it would make sense to create a protected getItemById() method to abstract this lookup, since we're now doing it in two different places.
| * | ||
| * @param string $itemId UUID | ||
| * | ||
| * @return stdClass The item |
There was a problem hiding this comment.
stdClass doesn't have a use statement in this class, so you should either add one or put a backslash here.
|
Thanks @dybarber ! Yes that's exactly what we hoped it would do -- and like you, I can't see how this might be relevant for title-level requests. I think this is ready for merge. |
There was a problem hiding this comment.
Thanks, @maccabeelevine! If there's sufficient interest in only applying the itemEffectiveLocation logic to specific locations, I see no reason why we couldn't address that as a follow-up (perhaps by adding more syntax to the limitPickupLocations setting), so I'm fine with merging this as a first step now.
FOLIO defines a relationship between each service point and the specific item locations that it is meant to service. When a patron places a hold for an item, we would like them to have to pick it up at the nearest service location -- not ship it to another library.