diff --git a/rero_ils/config.py b/rero_ils/config.py index f57ac10679..332b6d211e 100644 --- a/rero_ils/config.py +++ b/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 ] } diff --git a/rero_ils/modules/circ_policies/api.py b/rero_ils/modules/circ_policies/api.py index 2480e12933..205288e6c4 100644 --- a/rero_ils/modules/circ_policies/api.py +++ b/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.""" diff --git a/rero_ils/modules/patron_types/api.py b/rero_ils/modules/patron_types/api.py index 614979eb5f..aa1e22b56f 100644 --- a/rero_ils/modules/patron_types/api.py +++ b/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( diff --git a/rero_ils/modules/patrons/api.py b/rero_ils/modules/patrons/api.py index 5f8035ad88..45d4f6367b 100644 --- a/rero_ils/modules/patrons/api.py +++ b/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): diff --git a/rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json b/rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json index 12d2abe310..3c46488c0c 100644 --- a/rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json +++ b/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": { diff --git a/rero_ils/modules/patrons/utils.py b/rero_ils/modules/patrons/utils.py index 14d932ef0f..558b9629c1 100644 --- a/rero_ils/modules/patrons/utils.py +++ b/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')) diff --git a/rero_ils/modules/utils.py b/rero_ils/modules/utils.py index f908ddc438..b2060a3b0f 100644 --- a/rero_ils/modules/utils.py +++ b/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')) diff --git a/tests/api/circulation/test_actions_views_extend_loan_request.py b/tests/api/circulation/test_actions_views_extend_loan_request.py index 2c57cf2cc7..b9359f0339 100644 --- a/tests/api/circulation/test_actions_views_extend_loan_request.py +++ b/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,