FOLIO: Support delivery fulfillment preference for requests

[maccabeelevine]

FOLIO supports delivery as a fulfillment preference, instead of just picking up items at the hold shelf. https://docs.folio.org/docs/access/requests/requests/#processing-delivery-requests

TODO

• When merging, document new permissions required
• Add to the ILS drivers wiki page documenting the new optional return value locationsLabel in getRequestGroups.

[maccabeelevine]

This is not ready for detailed review (much left to be done), but it's worth discussing the UX and implementation strategy. Originally I was going to introduce a fulfillment preference concept to VuFind (pick up, delivery, maybe others like electronic delivery). But I took @EreMaijala 's good suggestion from Slack to use the existing request group capability.

There's already support for such field combination in the form of request group + pickup location. If used, the user has to choose a request group first. Then the pickup location list is populated based on that.

I think this works because there is otherwise not exactly a concept of a request group in FOLIO that matches what the wiki defines ("This optional method returns information used to group together collections of pickup locations."). There is the closely related concept of the list of pick up locations associated with this item's physical location, but it's not technically a group of locations, and in any case I handled that scenario already in #3876.

One problem is that the Pickup Location field now does double-duty as a delivery location field, so I will have to find a way to dynamically change the field name if the delivery request group is selected. But that seems doable.

I also don't love how all the delivery logic is forced into functions about getPickupLocations etc., but I guess that's pretty minor.

Does this work conceptually?

[demiankatz]

Thanks, @maccabeelevine! At first glance, I don't see anything obviously wrong with this approach. See below for a handful of minor review comments. I'll also add a TODO comment to document the newly-required permissions.

[maccabeelevine]

Thanks, @maccabeelevine! At first glance, I don't see anything obviously wrong with this approach. See below for a handful of minor review comments. I'll also add a TODO comment to document the newly-required permissions.

Great, thanks. I'll finish the implementation then and address those comments before I mark ready for a full review.

[maccabeelevine]

Actually tested now with FOLIO and it works.

Still have to figure out this:

One problem is that the Pickup Location field now does double-duty as a delivery location field, so I will have to find a way to dynamically change the field name if the delivery request group is selected. But that seems doable.

[maccabeelevine]

One problem is that the Pickup Location field now does double-duty as a delivery location field, so I will have to find a way to dynamically change the field name if the delivery request group is selected. But that seems doable.

Ok I've taken a first crack at this, just for the purposes of the initial hold form. There is no logic (yet) to change the label on any other form that displays requests, or if you never choose a request group in the first place. These are probably low-priority enhancements if they are necessary at all, but I think it's worth considering the effect that change would have around various templates as a (perhaps) necessary side effect if we take this approach of using request group for fulfillment method.

[EreMaijala]

While this probably works fine in most practical cases, I'd prefer a slightly different approach where Folio driver's getConfig would return the translation keys for the group id's. What do you think?

[maccabeelevine]

I'm fine with defining the translation keys in Folio.ini (based on group id) instead of hard-coding in this way. But I'm not clear where you mean to call getConfig()? Here in the template with something like $this->ils()->getConfig() etc.? Or do you mean something internal to the driver's getRequestGroups(), where each group returned would include its translation key?

[EreMaijala]

What I meant:
In HoldsTrait $checkHolds gets the Holds configuration from the driver by calling $catalog->checkFunction() that in turn calls the ILS driver's getConfig() method. So if you return the translation keys alongside other hold configuration, you can access them in HoldsTrait and pass them on to the template. I'd consider something like $checkHolds['requestGroupPickUpLocationLabels'][$id]. If there are only the hard-coded values, I'd leave them out of Folio.ini and just append them to the config in the getConfig() method.

But now that you mentioned it, extending getRequestGroups so that it would return the labels as well would be a much simpler approach. Plus it would probably work better with the "ajaxy" implementation.

[demiankatz]

The code looks fine to me (though I confess I haven't been able to actually test it hands-on); just one thought about scheduling the merge...

[demiankatz]

Since translation is already underway for release 10.1, we have a freeze on adding new translation strings. Since these are essentially just examples, if you really want to get this included in the release, perhaps we could remove the example configurations and language strings from this PR and introduce them in a separate PR targeted for 11.0.

[maccabeelevine]

Ok, I've taken that advice. I did need to leave in one new config, allowDelivery = false, to effectively disable to code, because it breaks without some of the config (which itself breaks without translation). The second PR is #3930.

[demiankatz]

Thanks, @maccabeelevine -- one last question.

Also, since it seems very likely this will get merged in time for 10.1, do you want to go ahead and update the wiki page about permissions to reflect the new requirements?

[maccabeelevine]

Also, since it seems very likely this will get merged in time for 10.1, do you want to go ahead and update the wiki page about permissions to reflect the new requirements?

Yes, done!

[demiankatz]

Thanks, @maccabeelevine!