From a51460189f8177ed51649da1f80fd35f6b7e117d Mon Sep 17 00:00:00 2001
From: Ere Maijala <ere.maijala@helsinki.fi>
Date: Fri, 16 Jun 2017 18:29:53 +0300
Subject: [PATCH] Added support for returning status texts from ILS request
check methods (#989)
* Added support for returning status codes from checkRequestIsValid / checkStorageRetrievalRequestIsValid / checkILLRequestIsValid instead of just a boolean, with the old boolean style still supported.
- Made Demo driver always return the new-style array and occasionally return different messages for request validation failures.
- Removed the now-unused blocked* actions.
---
.../src/VuFind/Controller/AjaxController.php | 28 ++++++++---
.../src/VuFind/Controller/HoldsTrait.php | 19 +++-----
.../VuFind/Controller/ILLRequestsTrait.php | 19 +++-----
.../StorageRetrievalRequestsTrait.php | 21 +++------
module/VuFind/src/VuFind/ILS/Driver/Demo.php | 47 +++++++++++++++----
.../src/VuFind/ILS/Driver/MultiBackend.php | 12 +++--
.../src/VuFind/ILS/Logic/TitleHolds.php | 6 ++-
.../src/VuFind/Route/RouteGenerator.php | 6 +--
8 files changed, 95 insertions(+), 63 deletions(-)
diff --git a/module/VuFind/src/VuFind/Controller/AjaxController.php b/module/VuFind/src/VuFind/Controller/AjaxController.php
index cc175cb17d9..de24481c668 100644
--- a/module/VuFind/src/VuFind/Controller/AjaxController.php
+++ b/module/VuFind/src/VuFind/Controller/AjaxController.php
@@ -1028,20 +1028,36 @@ class AjaxController extends AbstractBase
switch ($requestType) {
case 'ILLRequest':
$results = $catalog->checkILLRequestIsValid($id, $data, $patron);
- $msg = $results
- ? 'ill_request_place_text' : 'ill_request_error_blocked';
+ if (is_array($results)) {
+ $msg = $results['status'];
+ $results = $results['valid'];
+ } else {
+ $msg = $results
+ ? 'ill_request_place_text' : 'ill_request_error_blocked';
+ }
break;
case 'StorageRetrievalRequest':
$results = $catalog->checkStorageRetrievalRequestIsValid(
$id, $data, $patron
);
- $msg = $results ? 'storage_retrieval_request_place_text'
- : 'storage_retrieval_request_error_blocked';
+ if (is_array($results)) {
+ $msg = $results['status'];
+ $results = $results['valid'];
+ } else {
+ $msg = $results ? 'storage_retrieval_request_place_text'
+ : 'storage_retrieval_request_error_blocked';
+ }
break;
default:
$results = $catalog->checkRequestIsValid($id, $data, $patron);
- $msg = $results ? 'request_place_text' : 'hold_error_blocked';
- break;
+ if (is_array($results)) {
+ $msg = $results['status'];
+ $results = $results['valid'];
+ } else {
+ $msg = $results ? 'request_place_text'
+ : 'hold_error_blocked';
+ break;
+ }
}
return $this->output(
['status' => $results, 'msg' => $this->translate($msg)],
diff --git a/module/VuFind/src/VuFind/Controller/HoldsTrait.php b/module/VuFind/src/VuFind/Controller/HoldsTrait.php
index 3c1376fc863..0028faea2a0 100644
--- a/module/VuFind/src/VuFind/Controller/HoldsTrait.php
+++ b/module/VuFind/src/VuFind/Controller/HoldsTrait.php
@@ -38,17 +38,6 @@ namespace VuFind\Controller;
*/
trait HoldsTrait
{
- /**
- * Action for dealing with blocked holds.
- *
- * @return mixed
- */
- public function blockedholdAction()
- {
- $this->flashMessenger()->addMessage('hold_error_blocked', 'error');
- return $this->redirectToRecord('#top');
- }
-
/**
* Action for dealing with holds.
*
@@ -87,8 +76,12 @@ trait HoldsTrait
$validRequest = $catalog->checkRequestIsValid(
$driver->getUniqueID(), $gatheredDetails, $patron
);
- if (!$validRequest) {
- return $this->blockedholdAction();
+ if ((is_array($validRequest) && !$validRequest['valid']) || !$validRequest) {
+ $this->flashMessenger()->addErrorMessage(
+ is_array($validRequest)
+ ? $validRequest['status'] : 'hold_error_blocked'
+ );
+ return $this->redirectToRecord('#top');
}
// Send various values to the view so we can build the form:
diff --git a/module/VuFind/src/VuFind/Controller/ILLRequestsTrait.php b/module/VuFind/src/VuFind/Controller/ILLRequestsTrait.php
index d5c7b1bad14..24d3ba082ca 100644
--- a/module/VuFind/src/VuFind/Controller/ILLRequestsTrait.php
+++ b/module/VuFind/src/VuFind/Controller/ILLRequestsTrait.php
@@ -38,17 +38,6 @@ namespace VuFind\Controller;
*/
trait ILLRequestsTrait
{
- /**
- * Action for dealing with blocked ILL requests.
- *
- * @return mixed
- */
- public function blockedILLRequestAction()
- {
- $this->flashMessenger()->addMessage('ill_request_error_blocked', 'error');
- return $this->redirectToRecord('#top');
- }
-
/**
* Action for dealing with ILL requests.
*
@@ -89,8 +78,12 @@ trait ILLRequestsTrait
$validRequest = $catalog->checkILLRequestIsValid(
$driver->getUniqueID(), $gatheredDetails, $patron
);
- if (!$validRequest) {
- return $this->blockedILLRequestAction();
+ if ((is_array($validRequest) && !$validRequest['valid']) || !$validRequest) {
+ $this->flashMessenger()->addErrorMessage(
+ is_array($validRequest)
+ ? $validRequest['status'] : 'ill_request_error_blocked'
+ );
+ return $this->redirectToRecord('#top');
}
// Send various values to the view so we can build the form:
diff --git a/module/VuFind/src/VuFind/Controller/StorageRetrievalRequestsTrait.php b/module/VuFind/src/VuFind/Controller/StorageRetrievalRequestsTrait.php
index cf4f5866eeb..272ff2af2ee 100644
--- a/module/VuFind/src/VuFind/Controller/StorageRetrievalRequestsTrait.php
+++ b/module/VuFind/src/VuFind/Controller/StorageRetrievalRequestsTrait.php
@@ -38,18 +38,6 @@ namespace VuFind\Controller;
*/
trait StorageRetrievalRequestsTrait
{
- /**
- * Action for dealing with blocked storage retrieval requests.
- *
- * @return mixed
- */
- public function blockedStorageRetrievalRequestAction()
- {
- $this->flashMessenger()
- ->addMessage('storage_retrieval_request_error_blocked', 'error');
- return $this->redirectToRecord('#top');
- }
-
/**
* Action for dealing with storage retrieval requests.
*
@@ -90,8 +78,13 @@ trait StorageRetrievalRequestsTrait
$validRequest = $catalog->checkStorageRetrievalRequestIsValid(
$driver->getUniqueID(), $gatheredDetails, $patron
);
- if (!$validRequest) {
- return $this->blockedStorageRetrievalRequestAction();
+ if ((is_array($validRequest) && !$validRequest['valid']) || !$validRequest) {
+ $this->flashMessenger()->addErrorMessage(
+ is_array($validRequest)
+ ? $validRequest['status']
+ : 'storage_retrieval_request_error_blocked'
+ );
+ return $this->redirectToRecord('#top');
}
// Send various values to the view so we can build the form:
diff --git a/module/VuFind/src/VuFind/ILS/Driver/Demo.php b/module/VuFind/src/VuFind/ILS/Driver/Demo.php
index f2a82c33115..ad8eb819fbd 100644
--- a/module/VuFind/src/VuFind/ILS/Driver/Demo.php
+++ b/module/VuFind/src/VuFind/ILS/Driver/Demo.php
@@ -1456,14 +1456,26 @@ class Demo extends AbstractBase
* @param array $data An Array of item data
* @param patron $patron An array of patron data
*
- * @return bool True if request is valid, false if not
+ * @return mixed An array of data on the request including
+ * whether or not it is valid and a status message. Alternatively a boolean
+ * true if request is valid, false if not.
*
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function checkRequestIsValid($id, $data, $patron)
{
$this->checkIntermittentFailure();
- return !$this->isFailing(__METHOD__, 10);
+ if ($this->isFailing(__METHOD__, 10)) {
+ return [
+ 'valid' => false,
+ 'status' => rand() % 3 != 0
+ ? 'hold_error_blocked' : 'Demonstrating a custom failure'
+ ];
+ }
+ return [
+ 'valid' => true,
+ 'status' => 'request_place_text'
+ ];
}
/**
@@ -1559,7 +1571,9 @@ class Demo extends AbstractBase
* @param array $data An Array of item data
* @param patron $patron An array of patron data
*
- * @return bool True if request is valid, false if not
+ * @return mixed An array of data on the request including
+ * whether or not it is valid and a status message. Alternatively a boolean
+ * true if request is valid, false if not.
*
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
@@ -1567,9 +1581,17 @@ class Demo extends AbstractBase
{
$this->checkIntermittentFailure();
if (!$this->storageRetrievalRequests || $this->isFailing(__METHOD__, 10)) {
- return false;
+ return [
+ 'valid' => false,
+ 'status' => rand() % 3 != 0
+ ? 'storage_retrieval_request_error_blocked'
+ : 'Demonstrating a custom failure'
+ ];
}
- return true;
+ return [
+ 'valid' => true,
+ 'status' => 'storage_retrieval_request_place_text'
+ ];
}
/**
@@ -1664,7 +1686,9 @@ class Demo extends AbstractBase
* @param array $data An Array of item data
* @param patron $patron An array of patron data
*
- * @return bool True if request is valid, false if not
+ * @return mixed An array of data on the request including
+ * whether or not it is valid and a status message. Alternatively a boolean
+ * true if request is valid, false if not.
*
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
@@ -1672,9 +1696,16 @@ class Demo extends AbstractBase
{
$this->checkIntermittentFailure();
if (!$this->ILLRequests || $this->isFailing(__METHOD__, 10)) {
- return false;
+ return [
+ 'valid' => false,
+ 'status' => rand() % 3 != 0
+ ? 'ill_request_error_blocked' : 'Demonstrating a custom failure'
+ ];
}
- return true;
+ return [
+ 'valid' => true,
+ 'status' => 'ill_request_place_text'
+ ];
}
/**
diff --git a/module/VuFind/src/VuFind/ILS/Driver/MultiBackend.php b/module/VuFind/src/VuFind/ILS/Driver/MultiBackend.php
index 0b9d7c02a9b..70c96e49a26 100644
--- a/module/VuFind/src/VuFind/ILS/Driver/MultiBackend.php
+++ b/module/VuFind/src/VuFind/ILS/Driver/MultiBackend.php
@@ -584,7 +584,9 @@ class MultiBackend extends AbstractBase implements \Zend\Log\LoggerAwareInterfac
* @param array $data An Array of item data
* @param patron $patron An array of patron data
*
- * @return bool True if request is valid, false if not
+ * @return mixed An array of data on the request including
+ * whether or not it is valid and a status message. Alternatively a boolean
+ * true if request is valid, false if not.
*/
public function checkRequestIsValid($id, $data, $patron)
{
@@ -615,7 +617,9 @@ class MultiBackend extends AbstractBase implements \Zend\Log\LoggerAwareInterfac
* @param array $data An Array of item data
* @param patron $patron An array of patron data
*
- * @return bool True if request is valid, false if not
+ * @return mixed An array of data on the request including
+ * whether or not it is valid and a status message. Alternatively a boolean
+ * true if request is valid, false if not.
*/
public function checkStorageRetrievalRequestIsValid($id, $data, $patron)
{
@@ -960,7 +964,9 @@ class MultiBackend extends AbstractBase implements \Zend\Log\LoggerAwareInterfac
* @param array $data An Array of item data
* @param patron $patron An array of patron data
*
- * @return bool True if request is valid, false if not
+ * @return mixed An array of data on the request including
+ * whether or not it is valid and a status message. Alternatively a boolean
+ * true if request is valid, false if not.
*/
public function checkILLRequestIsValid($id, $data, $patron)
{
diff --git a/module/VuFind/src/VuFind/ILS/Logic/TitleHolds.php b/module/VuFind/src/VuFind/ILS/Logic/TitleHolds.php
index aa9a33fb685..55782b30bdd 100644
--- a/module/VuFind/src/VuFind/ILS/Logic/TitleHolds.php
+++ b/module/VuFind/src/VuFind/ILS/Logic/TitleHolds.php
@@ -211,8 +211,10 @@ class TitleHolds
];
if ($checkHolds != false) {
- $valid = $this->catalog->checkRequestIsValid($id, $data, $patron);
- if ($valid) {
+ $result = $this->catalog->checkRequestIsValid($id, $data, $patron);
+ if ((is_array($result) && $result['valid'])
+ || (is_bool($result) && $result)
+ ) {
return $this->getHoldDetails($data, $checkHolds['HMACKeys']);
}
}
diff --git a/module/VuFind/src/VuFind/Route/RouteGenerator.php b/module/VuFind/src/VuFind/Route/RouteGenerator.php
index 9d857a378af..8e1b229786b 100644
--- a/module/VuFind/src/VuFind/Route/RouteGenerator.php
+++ b/module/VuFind/src/VuFind/Route/RouteGenerator.php
@@ -59,10 +59,8 @@ class RouteGenerator
if (null === $nonTabRecordActions) {
$this->nonTabRecordActions = [
'AddComment', 'DeleteComment', 'AddTag', 'DeleteTag', 'Save',
- 'Email', 'SMS', 'Cite', 'Export', 'RDF', 'Hold', 'BlockedHold',
- 'Home', 'StorageRetrievalRequest', 'AjaxTab',
- 'BlockedStorageRetrievalRequest', 'ILLRequest', 'BlockedILLRequest',
- 'PDF',
+ 'Email', 'SMS', 'Cite', 'Export', 'RDF', 'Hold', 'Home',
+ 'StorageRetrievalRequest', 'AjaxTab', 'ILLRequest', 'PDF',
];
} else {
$this->nonTabRecordActions = $nonTabRecordActions;
--
GitLab