circulation: add control on circulation operation

Closes #1470
If a circulation policy disallow checkout operation, the error message
returned was not user friendly. This commit add a check for checkout
operation on circulation policy; this check return a better error
message.

Closes #1383
If a patron is blocked for any reasons, this user shouldn't be able to
extend any active loan.

Closes #1507
Fixes 'status' facet problem on items result list. Despite if user
select a status facet to filter the results, the facet value didn't
apply.

Co-Authored-by: Renaud Michotte <[email protected]>

rero_ils/config.py

@@ -1608,6 +1608,7 @@ def _(x):
             _('library'): and_term_filter('library.pid'),
             _('item_type'): and_term_filter('item_type.pid'),
             _('vendor'): and_term_filter('vendor.pid'),
+            _('status'): and_term_filter('status'),
             _('issue_status'): and_term_filter('issue.status'),
             # to allow multiple filters support, in this case to filter by
             # "late or claimed"
@@ -2304,10 +2305,12 @@ def _(x):
     ],
     ItemCirculationAction.EXTEND: [
         Item.can_extend,
+        Patron.can_extend,
         PatronType.allow_extend
     ],
     ItemCirculationAction.CHECKOUT: [
         Patron.can_checkout,
+        CircPolicy.allow_checkout,
         PatronType.allow_checkout
     ]
 }

rero_ils/modules/circ_policies/api.py

@@ -29,6 +29,7 @@
 from ..libraries.api import Library
 from ..minters import id_minter
 from ..providers import Provider
+from ..utils import get_patron_from_arguments
 
 # cipo provider
 CircPolicyProvider = type(
@@ -252,15 +253,11 @@ def allow_request(cls, item, **kwargs):
                         'patron' argument.
         :return a tuple with True|False and reasons to disallow if False.
         """
-        from ..patrons.api import Patron
-        required_arguments = ['patron', 'patron_barcode', 'patron_pid']
-        if not any(k in required_arguments for k in kwargs):
-            # 'patron' argument are present into kwargs. This check can't
+        patron = get_patron_from_arguments(**kwargs)
+        if not patron:
+            # none patron get be load from kwargs argument. This check can't
             # be relevant --> return True by default
             return True, []
-        patron = kwargs.get('patron') \
-            or Patron.get_patron_by_barcode(kwargs.get('patron_barcode')) \
-            or Patron.get_record_by_pid(kwargs.get('patron_pid'))
         if 'patron' not in patron.get('roles', []):
             # without 'patron' role, we can't find any patron_type and so we
             # can't find any corresponding cipo --> return False
@@ -276,6 +273,35 @@ def allow_request(cls, item, **kwargs):
             return False, ["Circulation policy disallows the operation."]
         return True, []
 
+    @classmethod
+    def allow_checkout(cls, item, **kwargs):
+        """Check if the cipo corresponding to item/patron allow request.
+
+        :param item : the item to check
+        :param kwargs : To be relevant, additional arguments should contains
+                        'patron' argument.
+        :return a tuple with True|False and reasons to disallow if False.
+        """
+        patron = get_patron_from_arguments(**kwargs)
+        if not patron:
+            # none patron get be load from kwargs argument. This check can't
+            # be relevant --> return True by default
+            return True, []
+        if 'patron' not in patron.get('roles', []):
+            # without 'patron' role, we can't find any patron_type and so we
+            # can't find any corresponding cipo --> return False
+            return False, ["Patron doesn't have the correct role"]
+        library_pid = kwargs['library'].pid if kwargs.get('library') \
+            else item.library_pid
+        cipo = cls.provide_circ_policy(
+            library_pid,
+            patron.patron_type_pid,
+            item.item_type_pid
+        )
+        if not cipo.get('allow_checkout', False):
+            return False, ["Circulation policy disallows the operation."]
+        return True, []
+
 
 class CircPoliciesIndexer(IlsRecordsIndexer):
     """Indexing documents in Elasticsearch."""

rero_ils/modules/patron_types/api.py

@@ -34,8 +34,8 @@
 from ..minters import id_minter
 from ..patron_transactions.api import PatronTransaction
 from ..patrons.api import Patron, PatronsSearch
-from ..patrons.utils import get_patron_from_arguments
 from ..providers import Provider
+from ..utils import get_patron_from_arguments
 
 # provider
 PatronTypeProvider = type(

rero_ils/modules/patrons/api.py

@@ -34,7 +34,6 @@
 from werkzeug.local import LocalProxy
 
 from .models import PatronIdentifier, PatronMetadata
-from .utils import get_patron_from_arguments
 from ..api import IlsRecord, IlsRecordsIndexer, IlsRecordsSearch
 from ..errors import RecordValidationError
 from ..fetchers import id_fetcher
@@ -43,8 +42,8 @@
 from ..organisations.api import Organisation
 from ..patron_transactions.api import PatronTransaction
 from ..providers import Provider
-from ..utils import extracted_data_from_ref, get_ref_for_pid, \
-    trim_barcode_for_record
+from ..utils import extracted_data_from_ref, get_patron_from_arguments, \
+    get_ref_for_pid, trim_barcode_for_record
 
 _datastore = LocalProxy(lambda: current_app.extensions['security'].datastore)
 
@@ -439,7 +438,6 @@ def can_request(cls, item, **kwargs):
             # 'patron' argument are present into kwargs. This check can't
             # be relevant --> return True by default
             return True, []
-
         # a blocked patron can't request any item
         if patron.is_blocked:
             return False, [patron.get_blocked_message()]
@@ -448,24 +446,15 @@ def can_request(cls, item, **kwargs):
 
     @classmethod
     def can_checkout(cls, item, **kwargs):
-        """Check if a patron can checkout an item.
+        """Check if a patron can checkout an item."""
+        # Same logic than can_request :: a blocked patron can't do a checkout
+        return cls.can_request(item, **kwargs)
 
-        :param item: the item to check
-        :param kwargs: To be relevant, additional arguments should contains
-                       'patron' argument.
-        :return a tuple with True|False and reasons to disallow if False.
-        """
-        patron = get_patron_from_arguments(**kwargs)
-        if not patron:
-            # 'patron' argument are present into kwargs. This check can't
-            # be relevant --> return True by default
-            return True, []
-
-        # a blocked patron can't request any item
-        if patron.is_blocked:
-            return False, [patron.get_blocked_message()]
-
-        return True, []
+    @classmethod
+    def can_extend(cls, item, **kwargs):
+        """Check if a patron can extend a loan."""
+        # Same logic than can_request :: a blocked patron can't extend a loan
+        return cls.can_request(item, **kwargs)
 
     @classmethod
     def patrons_with_obsolete_subscription_pids(cls, end_date=None):

rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json

@@ -371,7 +371,7 @@
         },
         "blocked": {
           "title": "Blocking",
-          "description": "A patron with a blocked account cannot request and borrow items, but can still renew and check in items.",
+          "description": "A patron with a blocked account cannot extend, request and borrow items.",
           "type": "boolean"
         },
         "blocked_note": {

rero_ils/modules/patrons/utils.py

@@ -28,15 +28,3 @@ def user_has_patron(user=current_user):
     if patron and 'patron' in patron.get('roles'):
         return True
     return False
-
-
-def get_patron_from_arguments(**kwargs):
-    """Try to load a patron from potential arguments."""
-    from .api import Patron
-    required_arguments = ['patron', 'patron_barcode', 'patron_pid', 'loan']
-    if not any(k in required_arguments for k in kwargs):
-        return None
-    return kwargs.get('patron') \
-        or Patron.get_patron_by_barcode(kwargs.get('patron_barcode')) \
-        or Patron.get_record_by_pid(kwargs.get('patron_pid')) \
-        or Patron.get_record_by_pid(kwargs.get('loan').get('patron_pid'))

rero_ils/modules/utils.py

@@ -407,3 +407,15 @@ def get_record_class_from_schema_or_pid_type(schema=None, pid_type=None):
         .get('RECORDS_REST_ENDPOINTS')
         .get(pid_type, {}).get('record_class'))
     return record_class
+
+
+def get_patron_from_arguments(**kwargs):
+    """Try to load a patron from potential arguments."""
+    from .patrons.api import Patron
+    required_arguments = ['patron', 'patron_barcode', 'patron_pid', 'loan']
+    if not any(k in required_arguments for k in kwargs):
+        return None
+    return kwargs.get('patron') \
+        or Patron.get_patron_by_barcode(kwargs.get('patron_barcode')) \
+        or Patron.get_record_by_pid(kwargs.get('patron_pid')) \
+        or Patron.get_record_by_pid(kwargs.get('loan').get('patron_pid'))

tests/api/circulation/test_actions_views_extend_loan_request.py

@@ -22,6 +22,7 @@
 from utils import postdata
 
 from rero_ils.modules.items.models import ItemStatus
+from rero_ils.modules.patrons.api import Patron
 
 
 def test_extend_loan_missing_parameters(
@@ -77,11 +78,32 @@ def test_extend_loan(
         loc_public_martigny,
         circulation_policies,
         item_on_loan_martigny_patron_and_loan_on_loan):
-    """Test a successful frontend checkout action."""
+    """Test frontend extend action."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     item, patron, loan = item_on_loan_martigny_patron_and_loan_on_loan
     assert item.status == ItemStatus.ON_LOAN
 
+    # Test extend for a blocked patron
+    patron['patron']['blocked'] = True
+    patron['patron']['blocked_note'] = 'Dummy reason'
+    patron.update(patron, dbcommit=True, reindex=True)
+    patron = Patron.get_record_by_pid(patron.pid)
+
+    res, _ = postdata(
+        client,
+        'api_item.extend_loan',
+        dict(
+            item_pid=item.pid,
+            transaction_user_pid=librarian_martigny_no_email.pid,
+            transaction_location_pid=loc_public_martigny.pid
+        )
+    )
+    assert res.status_code == 403
+
+    del patron['patron']['blocked']
+    del patron['patron']['blocked_note']
+    patron.update(patron, dbcommit=True, reindex=True)
+
     # With only needed parameters
     res, _ = postdata(
         client,