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

FOLIO: Support delivery fulfillment preference for requests #3930

Merged
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions config/vufind/Folio.ini
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,18 @@ extraHoldFields = requiredByDate:pickUpLocation
; requested item's effective location. Only applicable to item-level requests.
;limitPickupLocations = itemEffectiveLocation

; When the extra hold field requestGroup is used, the request group Delivery is offered
; whenever the FOLIO user's Request Preferences have delivery enabled. This setting
; overrides every user's FOLIO setting if it's necessary to globally disable delivery
; for backwards compatibility.
;allowDelivery = false

; When the extra hold fields pickupLocation and requestGroup are used, and requestGroup
; is Delivery, the request form's list of "pickup locations" is actually a list of
; address types matching the addresses defined for the current user. In that context,
; this setting optionally further limits which FOLIO address types are offered.
;limitDeliveryAddressTypes[] = "Campus"

; By default, a "Hold" type request is placed when an item is unavailable and a Page
; when an item is available. This setting overrides the default behavior for
; unavailable items and for all title-level requests. Legal values: "Page", "Hold" or
Expand Down
4 changes: 4 additions & 0 deletions languages/en.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,8 @@ Photo = "Photo"
Physical Description = "Physical Description"
Physical Object = "Physical Object"
pick_up_location = "Pickup Location"
pick_up_location_label_Delivery = "Delivery Address"
pick_up_location_label_Hold Shelf = "Pickup Location"
Place a Hold = "Place a Hold"
Place of birth = "Place of birth"
Place of death = "Place of death"
Expand Down Expand Up @@ -1224,6 +1226,8 @@ renew_success = "Renewal Successful"
renew_success_summary = "Successfully renewed {count, plural, =1 {1 item} other {# items}}."
Renewed = "Renewed"
Request full text = "Request full text"
request_group_fulfillment_method_delivery = "Delivery"
request_group_fulfillment_method_hold_shelf = "Pick up"
request_in_transit = "In Transit to Pickup Location"
request_place_text = "Place a Request"
request_submit_text = "Submit Request"
Expand Down
127 changes: 125 additions & 2 deletions module/VuFind/src/VuFind/ILS/Driver/Folio.php
Original file line number Diff line number Diff line change
Expand Up @@ -1277,6 +1277,10 @@ public function patronLogin($username, $password)
'firstname' => $profile->personal->firstName ?? null,
'lastname' => $profile->personal->lastName ?? null,
'email' => $profile->personal->email ?? null,
'addressTypeIds' => array_map(
fn ($address) => $address->addressTypeId,
$profile->personal->addresses ?? []
),
];
}

Expand Down Expand Up @@ -1508,6 +1512,22 @@ public function renewMyItems($renewDetails)
*/
public function getPickupLocations($patron, $holdInfo = null)
{
if ('Delivery' == ($holdInfo['requestGroupId'] ?? null)) {
$addressTypes = $this->getAddressTypes();
$limitDeliveryAddressTypes = $this->config['Holds']['limitDeliveryAddressTypes'] ?? [];
$deliveryPickupLocations = [];
foreach ($patron['addressTypeIds'] as $addressTypeId) {
$addressType = $addressTypes[$addressTypeId];
if (empty($limitDeliveryAddressTypes) || in_array($addressType, $limitDeliveryAddressTypes)) {
$deliveryPickupLocations[] = [
'locationID' => $addressTypeId,
'locationDisplay' => $addressType,
];
}
}
return $deliveryPickupLocations;
}

$limitedServicePoints = null;
if (
str_contains($this->config['Holds']['limitPickupLocations'] ?? '', 'itemEffectiveLocation')
Expand Down Expand Up @@ -1541,6 +1561,104 @@ public function getPickupLocations($patron, $holdInfo = null)
return $locations;
}

/**
* Get Default Pick Up Location
*
* Returns the default pick up location set in HorizonXMLAPI.ini
*
* @param array $patron Patron information returned by the patronLogin
* method.
* @param array $holdDetails Optional array, only passed in when getting a list
* in the context of placing a hold; contains most of the same values passed to
* placeHold, minus the patron data. May be used to limit the pickup options
* or may be ignored.
*
* @return false|string The default pickup location for the patron or false
* if the user has to choose.
*
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function getDefaultPickUpLocation($patron = false, $holdDetails = null)
{
if ('Delivery' == ($holdDetails['requestGroupId'] ?? null)) {
$deliveryPickupLocations = $this->getPickupLocations($patron, $holdDetails);
if (count($deliveryPickupLocations) == 1) {
return $deliveryPickupLocations[0]['locationDisplay'];
}
}
return false;
}

/**
* Get request groups
*
* @param int $bibId BIB ID
* @param array $patron Patron information returned by the patronLogin
* method.
* @param array $holdDetails Optional array, only passed in when getting a list
* in the context of placing a hold; contains most of the same values passed to
* placeHold, minus the patron data. May be used to limit the request group
* options or may be ignored.
*
* @return array False if request groups not in use or an array of
* associative arrays with id and name keys
*
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function getRequestGroups(
$bibId = null,
$patron = null,
$holdDetails = null
) {
// circulation-storage.request-preferences.collection.get
$response = $this->makeRequest(
'GET',
'/request-preference-storage/request-preference?query=userId==' . $patron['id']
);
$requestPreferencesResponse = json_decode($response->getBody());
$requestPreferences = $requestPreferencesResponse->requestPreferences[0];
$allowHoldShelf = $requestPreferences->holdShelf;
$allowDelivery = $requestPreferences->delivery && ($this->config['Holds']['allowDelivery'] ?? true);
demiankatz marked this conversation as resolved.
Show resolved Hide resolved
if ($allowHoldShelf && $allowDelivery) {
return [
[
'id' => 'Hold Shelf',
'name' => 'fulfillment_method_hold_shelf',
],
[
'id' => 'Delivery',
'name' => 'fulfillment_method_delivery',
],
];
}
return false;
}

/**
* Get list of address types from FOLIO. Cache as needed.
*
* @return array An array mapping an address type id to its name.
*/
protected function getAddressTypes()
{
$cacheKey = 'addressTypes';
$addressTypes = $this->getCachedData($cacheKey);
if (null == $addressTypes) {
$addressTypes = [];
// addresstypes.collection.get
foreach (
$this->getPagedResults(
'addressTypes',
'/addresstypes'
) as $addressType
) {
$addressTypes[$addressType->id] = $addressType->addressType;
}
$this->putCachedData($cacheKey, $addressTypes);
}
return $addressTypes;
}

/**
* This method queries the ILS for a patron's current holds
*
Expand Down Expand Up @@ -1789,12 +1907,17 @@ public function placeHold($holdDetails)
// Account for an API spelling change introduced in mod-circulation v24:
$fulfillmentKey = $this->getModuleMajorVersion('mod-circulation') >= 24
? 'fulfillmentPreference' : 'fulfilmentPreference';
$fulfillmentValue = $holdDetails['requestGroupId'] ?? 'Hold Shelf';
$fulfillmentLocationKey = match ($fulfillmentValue) {
'Hold Shelf' => 'pickupServicePointId',
'Delivery' => 'deliveryAddressTypeId',
};
$requestBody = $baseParams + [
'requesterId' => $holdDetails['patron']['id'],
'requestDate' => date('c'),
$fulfillmentKey => 'Hold Shelf',
$fulfillmentKey => $fulfillmentValue,
'requestExpirationDate' => $requiredBy,
'pickupServicePointId' => $holdDetails['pickUpLocation'],
$fulfillmentLocationKey => $holdDetails['pickUpLocation'],
];
if (!empty($holdDetails['proxiedUser'])) {
$requestBody['requesterId'] = $holdDetails['proxiedUser'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@
"expectedParams": {
"query": "username == foo"
},
"body": "{ \"users\": [ { \"id\": \"fake-id\", \"personal\": { \"firstName\": \"first\", \"lastName\": \"last\", \"email\": \"[email protected]\" } } ] }"
"body": "{ \"users\": [ { \"id\": \"fake-id\", \"personal\": { \"firstName\": \"first\", \"lastName\": \"last\", \"email\": \"[email protected]\", \"addresses\": [] } } ] }"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@
"expectedParams": {
"query": "username == foo"
},
"body": "{ \"users\": [ { \"id\": \"fake-id\", \"personal\": { \"firstName\": \"first\", \"lastName\": \"last\", \"email\": \"[email protected]\" } } ] }"
"body": "{ \"users\": [ { \"id\": \"fake-id\", \"personal\": { \"firstName\": \"first\", \"lastName\": \"last\", \"email\": \"[email protected]\", \"addresses\": [] } } ] }"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ public function testSuccessfulPatronLoginWithOkapi(): void
'firstname' => 'first',
'lastname' => 'last',
'email' => '[email protected]',
'addressTypeIds' => [],
];
$this->assertEquals($expected, $result);
}
Expand Down Expand Up @@ -361,6 +362,7 @@ public function testSuccessfulPatronLoginWithOkapiLegacyAuth(): void
'firstname' => 'first',
'lastname' => 'last',
'email' => '[email protected]',
'addressTypeIds' => [],
];
$this->assertEquals($expected, $result);
}
Expand Down
1 change: 1 addition & 0 deletions themes/bootstrap3/js/hold.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ function setUpHoldRequestForm(recordId) {
$emptyOption.removeAttr('hidden');
}
$select.show();
$('#pickUpLocationLabel').text($self.find(':selected').data('locations-label'));
} else {
$select.hide();
$noResults.show();
Expand Down
6 changes: 5 additions & 1 deletion themes/bootstrap3/templates/record/hold.phtml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@
</option>
<?php endif; ?>
<?php foreach ($this->requestGroups as $group): ?>
<option value="<?=$this->escapeHtmlAttr($group['id'])?>"<?=($selected == $group['id']) ? ' selected="selected"' : ''?>>
<option
value="<?=$this->escapeHtmlAttr($group['id'])?>"
data-locations-label="<?=$this->transEsc('pick_up_location_label_' . $group['id'], null, $this->transEsc('pick_up_location'))?>"
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

<?=($selected == $group['id']) ? ' selected="selected"' : ''?>
>
<?=$this->transEsc('request_group_' . $group['name'], null, $group['name'])?>
</option>
<?php endforeach; ?>
Expand Down
1 change: 1 addition & 0 deletions themes/bootstrap5/js/hold.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ function setUpHoldRequestForm(recordId) {
$emptyOption.removeAttr('hidden');
}
$select.show();
$('#pickUpLocationLabel').text($self.find(':selected').data('locations-label'));
} else {
$select.hide();
$noResults.show();
Expand Down
6 changes: 5 additions & 1 deletion themes/bootstrap5/templates/record/hold.phtml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@
</option>
<?php endif; ?>
<?php foreach ($this->requestGroups as $group): ?>
<option value="<?=$this->escapeHtmlAttr($group['id'])?>"<?=($selected == $group['id']) ? ' selected="selected"' : ''?>>
<option
value="<?=$this->escapeHtmlAttr($group['id'])?>"
data-locations-label="<?=$this->transEsc('pick_up_location_label_' . $group['id'], null, $this->transEsc('pick_up_location'))?>"
<?=($selected == $group['id']) ? ' selected="selected"' : ''?>
>
<?=$this->transEsc('request_group_' . $group['name'], null, $group['name'])?>
</option>
<?php endforeach; ?>
Expand Down
Loading