diff --git a/data/locations.json b/data/locations.json index 85e8459f0f..2304fbac66 100644 --- a/data/locations.json +++ b/data/locations.json @@ -8,7 +8,8 @@ "$ref": "https://ils.rero.ch/api/libraries/1" }, "is_pickup": true, - "pickup_name": "AOSTE CANT1: Espaces publics" + "pickup_name": "AOSTE CANT1: Espaces publics", + "allow_request": true }, { "pid": "2", @@ -17,7 +18,8 @@ "name": "Bibliographie vald\u00f4taine", "library": { "$ref": "https://ils.rero.ch/api/libraries/1" - } + }, + "allow_request": true }, { "pid": "3", @@ -26,7 +28,8 @@ "name": "Magasin A", "library": { "$ref": "https://ils.rero.ch/api/libraries/1" - } + }, + "allow_request": true }, { "pid": "4", @@ -37,7 +40,8 @@ "library": { "$ref": "https://ils.rero.ch/api/libraries/1" }, - "is_online": true + "is_online": true, + "allow_request": true }, { "pid": "5", @@ -48,7 +52,8 @@ "$ref": "https://ils.rero.ch/api/libraries/2" }, "is_pickup": true, - "pickup_name": "AOSTE CANT2: Espaces publics" + "pickup_name": "AOSTE CANT2: Espaces publics", + "allow_request": true }, { "pid": "7", @@ -57,7 +62,8 @@ "name": "Section fiction", "library": { "$ref": "https://ils.rero.ch/api/libraries/3" - } + }, + "allow_request": true }, { "pid": "8", @@ -66,7 +72,8 @@ "name": "Section documentaires", "library": { "$ref": "https://ils.rero.ch/api/libraries/3" - } + }, + "allow_request": true }, { "pid": "9", @@ -75,7 +82,8 @@ "name": "Section enfants", "library": { "$ref": "https://ils.rero.ch/api/libraries/3" - } + }, + "allow_request": true }, { "pid": "10", @@ -84,7 +92,8 @@ "name": "Section multim\u00e9dia", "library": { "$ref": "https://ils.rero.ch/api/libraries/3" - } + }, + "allow_request": true }, { "pid": "11", @@ -95,7 +104,8 @@ "$ref": "https://ils.rero.ch/api/libraries/3" }, "is_pickup": true, - "pickup_name": "AOSTE-AVISE-PUB: Espaces publics" + "pickup_name": "AOSTE-AVISE-PUB: Espaces publics", + "allow_request": true }, { "pid": "12", @@ -106,7 +116,8 @@ "library": { "$ref": "https://ils.rero.ch/api/libraries/3" }, - "is_online": true + "is_online": true, + "allow_request": true }, { "pid": "13", @@ -117,7 +128,8 @@ "$ref": "https://ils.rero.ch/api/libraries/4" }, "is_pickup": true, - "pickup_name": "AOSTE-LYCEE: Espaces publics" + "pickup_name": "AOSTE-LYCEE: Espaces publics", + "allow_request": true }, { "pid": "14", @@ -126,7 +138,8 @@ "name": "Travaux de maturit\u00e9", "library": { "$ref": "https://ils.rero.ch/api/libraries/4" - } + }, + "allow_request": true }, { "pid": "16", @@ -135,7 +148,8 @@ "name": "Restricted Section", "library": { "$ref": "https://ils.rero.ch/api/libraries/5" - } + }, + "allow_request": true }, { "pid": "17", @@ -146,7 +160,8 @@ "pickup_name": "Public Section", "library": { "$ref": "https://ils.rero.ch/api/libraries/5" - } + }, + "allow_request": true }, { "pid": "18", @@ -157,7 +172,8 @@ "library": { "$ref": "https://ils.rero.ch/api/libraries/5" }, - "is_online": true + "is_online": true, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -169,7 +185,8 @@ }, "is_pickup": true, "pickup_name": "La Citadelle", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -181,7 +198,8 @@ }, "is_pickup": true, "pickup_name": "Beast's Library", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -193,7 +211,8 @@ }, "is_pickup": true, "pickup_name": "Dream's Library", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -205,7 +224,8 @@ }, "is_pickup": true, "pickup_name": "Lux Foundation Library", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -217,7 +237,8 @@ }, "is_pickup": true, "pickup_name": "Hogwarts Library", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -229,7 +250,8 @@ }, "is_pickup": true, "pickup_name": "Biblioth\u00e8que de Babel", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -241,7 +263,8 @@ }, "is_pickup": true, "pickup_name": "Biblioth\u00e8que de l\u2019abbaye", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -253,7 +276,8 @@ }, "is_pickup": true, "pickup_name": "The University's Library", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -265,7 +289,8 @@ }, "is_pickup": true, "pickup_name": "Cemetery of Forgotten Books", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -277,7 +302,8 @@ }, "is_pickup": true, "pickup_name": "Ministry of Truth", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -289,7 +315,8 @@ }, "is_pickup": true, "pickup_name": "Rivendell Library", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -301,7 +328,8 @@ }, "is_pickup": true, "pickup_name": "Matilda's Library", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -313,7 +341,8 @@ }, "is_pickup": true, "pickup_name": "Unseen University Library", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -325,7 +354,8 @@ }, "is_pickup": true, "pickup_name": "Wan Shi Tong's Library", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -337,7 +367,8 @@ }, "is_pickup": true, "pickup_name": "Sunnydale High School library", - "is_online": false + "is_online": false, + "allow_request": true }, { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -349,6 +380,7 @@ }, "is_pickup": true, "pickup_name": "Jedi Archives", - "is_online": false + "is_online": false, + "allow_request": true } -] \ No newline at end of file +] diff --git a/rero_ils/config.py b/rero_ils/config.py index 9cb3e1c1ff..88b5cd3c9b 100644 --- a/rero_ils/config.py +++ b/rero_ils/config.py @@ -55,6 +55,7 @@ from .modules.holdings.api import Holding from .modules.item_types.api import ItemType from .modules.items.api import Item +from .modules.items.models import ItemCirculationAction from .modules.items.permissions import can_create_item_factory, \ can_update_delete_item_factory from .modules.libraries.api import Library @@ -1706,6 +1707,18 @@ def _(x): ) ) +CIRCULATION_ACTIONS_VALIDATION = { + ItemCirculationAction.REQUEST: [ + Location.allow_request, + Item.can_request, + CircPolicy.allow_request, + Patron.can_request + ], + ItemCirculationAction.EXTEND: [ + Item.can_extend + ] +} + WEBPACKEXT_PROJECT = 'rero_ils.webpack:project' # WIKI diff --git a/rero_ils/modules/circ_policies/api.py b/rero_ils/modules/circ_policies/api.py index 0ad0164a39..e868bcb7ab 100644 --- a/rero_ils/modules/circ_policies/api.py +++ b/rero_ils/modules/circ_policies/api.py @@ -243,6 +243,39 @@ def reasons_not_to_delete(self): cannot_delete['links'] = links return cannot_delete + @classmethod + def allow_request(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. + """ + 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 + # 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 + 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_requests', False): + return False, ["Circulation policy disallows the operation."] + return True, [] + class CircPoliciesIndexer(IlsRecordsIndexer): """Indexing documents in Elasticsearch.""" diff --git a/rero_ils/modules/documents/views.py b/rero_ils/modules/documents/views.py index 2e07c84bfc..cfbcd731b9 100644 --- a/rero_ils/modules/documents/views.py +++ b/rero_ils/modules/documents/views.py @@ -40,10 +40,8 @@ title_variant_format_text from ..holdings.api import Holding from ..items.api import Item -from ..items.models import ItemStatus +from ..items.models import ItemCirculationAction, ItemStatus from ..libraries.api import Library -from ..loans.api import Loan -from ..loans.utils import can_be_requested from ..locations.api import Location from ..organisations.api import Organisation from ..patrons.api import Patron @@ -203,27 +201,12 @@ def can_request(item): """Check if the current user can request a given item.""" if current_user.is_authenticated: patron = Patron.get_patron_by_user(current_user) - if patron: - if 'patron' in patron.get('roles') and \ - patron.get_organisation()['pid'] == \ - item.get_library().replace_refs()['organisation']['pid']: - # Complete metadata before Loan creation - loan_metadata = dict(item) - if 'item_pid' not in loan_metadata: - loan_metadata['item_pid'] = item.pid - if 'patron_pid' not in loan_metadata: - loan_metadata['patron_pid'] = patron.pid - # Create "virtual" Loan (not registered) - loan = Loan(loan_metadata) - if not can_be_requested(loan): - return False - patron_barcode = patron.get('barcode') - item_status = item.get('status') - if item_status != 'missing': - loaned_to_patron = item.is_loaned_to_patron(patron_barcode) - requested = item.is_requested_by_patron(patron_barcode) - if not (requested or loaned_to_patron): - return True + can, _ = item.can( + ItemCirculationAction.REQUEST, + patron=patron, + library=Library.get_record_by_pid(patron.library_pid) + ) + return can return False @@ -343,17 +326,26 @@ def item_library_pickup_locations(item): """Get the pickup locations of the library of the given item.""" location_pid = item.replace_refs()['location']['pid'] location = Location.get_record_by_pid(location_pid) - library_pid = location.replace_refs()['library']['pid'] - library = Library.get_record_by_pid(library_pid).replace_refs() - organisation = Organisation.get_record_by_pid( - library['organisation']['pid']) - locations = [] - for library in organisation.get_libraries(): - location = Location.get_record_by_pid( - library.get_pickup_location_pid()) - if location: - locations.append(location) - return locations + # Either the location defines some 'restrict_pickup_to' either not. + # * If 'restrict_pickup_to' is defined, then only these locations are + # eligible as possible pickup_locations + # * Otherwise, get all organisation pickup locations of the item belongs to + if 'restrict_pickup_to' in location: + # Get all pickup locations as Location objects and append it to the + # location item (removing possible None values) + pickup_locations = list(filter(None, [ + Location.get_record_by_pid(loc_pid) + for loc_pid in location.restrict_pickup_to + ])) + return pickup_locations + else: + org = Organisation.get_record_by_pid(location.organisation_pid) + # Get the pickup location from each library of the item organisation + # (removing possible None value) + return list(filter(None, [ + Location.get_record_by_pid(library.get_pickup_location_pid()) + for library in org.get_libraries() + ])) @blueprint.app_template_filter() diff --git a/rero_ils/modules/items/api/circulation.py b/rero_ils/modules/items/api/circulation.py index 6e274a830c..68cc7c5067 100644 --- a/rero_ils/modules/items/api/circulation.py +++ b/rero_ils/modules/items/api/circulation.py @@ -28,9 +28,10 @@ from invenio_circulation.proxies import current_circulation from invenio_circulation.search.api import search_by_pid from invenio_i18n.ext import current_i18n +from invenio_records_rest.utils import obj_or_import_string from invenio_search import current_search -from ..models import ItemStatus +from ..models import ItemCirculationAction, ItemStatus from ...api import IlsRecord from ...circ_policies.api import CircPolicy from ...documents.api import Document @@ -419,30 +420,77 @@ def last_location_pid(self): return loan_location_pid return self.location_pid - def can_extend(self, loan): - """Checks if the patron has the rights to renew this item.""" - from ...loans.utils import extend_loan_data_is_valid - can_extend = True - patron_pid = loan.get('patron_pid') - patron_type_pid = Patron.get_record_by_pid( - patron_pid).patron_type_pid - circ_policy = CircPolicy.provide_circ_policy( - self.library_pid, - patron_type_pid, - self.item_type_pid + # CIRCULATION METHODS ===================================================== + def can(self, action, **kwargs): + """Check if a specific action is allowed on this item. + + :param action : the action to check as ItemCirculationAction part + :param kwargs : all others named arguments useful to check + :return a tuple with True|False to know if the action is possible and + a list of reasons to disallow if False. + """ + can, reasons = True, [] + actions = current_app.config.get('CIRCULATION_ACTIONS_VALIDATION', {}) + for func_name in actions.get(action, []): + func_callback = obj_or_import_string(func_name) + func_can, func_reasons = func_callback(self, **kwargs) + reasons += func_reasons + can = can and func_can + return can, reasons + + @classmethod + def can_request(cls, item, **kwargs): + """Check if an item can be requested regarding the item status. + + :param item : the item to check. + :param kwargs : other arguments. + :return a tuple with True|False and reasons to disallow if False. + """ + reasons = [] + if item.status in [ItemStatus.MISSING, ItemStatus.EXCLUDED]: + reasons.append("Item status disallows the operation.") + if 'patron' in kwargs: + patron = kwargs['patron'] + if patron.organisation_pid != item.organisation_pid: + reasons.append("Item and patron are not in the same " + "organisation.") + if patron.get('barcode') and \ + item.is_loaned_to_patron(patron.get('barcode')): + reasons.append("Item is already checked-out or requested by " + "patron.") + return len(reasons) == 0, reasons + + @classmethod + def can_extend(cls, item, **kwargs): + """Checks if a patron has the rights to renew an item. + + :param item : the item to check. + :param kwargs : additional arguments. To be relevant this method + require 'loan' argument. + :return a tuple with True|False and reasons to disallow if False. + """ + from rero_ils.modules.loans.utils import extend_loan_data_is_valid + if 'loan' not in kwargs: # this method is not relevant + return True, [] + loan = kwargs['loan'] + patron = Patron.get_record_by_pid(loan.get('patron_pid')) + cipo = CircPolicy.provide_circ_policy( + item.library_pid, + patron.patron_type_pid, + item.item_type_pid ) extension_count = loan.get('extension_count', 0) - if not ( - circ_policy.get('number_renewals') > 0 and - extension_count < circ_policy.get('number_renewals') and + if not (cipo.get('number_renewals') > 0 and + extension_count < cipo.get('number_renewals') and extend_loan_data_is_valid( loan.get('end_date'), - circ_policy.get('renewal_duration'), - self.library_pid - ) - ) or self.number_of_requests(): - can_extend = False - return can_extend + cipo.get('renewal_duration'), + item.library_pid + )): + return False, ['Circulation policies disallows the operation.'] + if item.number_of_requests(): + return False, ['A pending request exists on this item.'] + return True, [] def action_filter(self, action, loan): """Filter actions.""" @@ -459,7 +507,8 @@ def action_filter(self, action, loan): 'new_action': None } if action == 'extend': - if not self.can_extend(loan): + can, reasons = self.can(ItemCirculationAction.EXTEND, loan=loan) + if not can: data['action_validated'] = False if action == 'checkout': if not circ_policy.get('allow_checkout'): diff --git a/rero_ils/modules/items/api_views.py b/rero_ils/modules/items/api_views.py index 52e3fdee2e..3b4bec7882 100644 --- a/rero_ils/modules/items/api_views.py +++ b/rero_ils/modules/items/api_views.py @@ -29,11 +29,11 @@ from werkzeug.exceptions import NotFound from .api import Item -from .models import ItemStatus +from .models import ItemCirculationAction from ..circ_policies.api import CircPolicy +from ..documents.views import item_library_pickup_locations from ..libraries.api import Library from ..loans.api import Loan -from ..loans.utils import can_be_requested from ..patrons.api import Patron from ...permissions import librarian_permission @@ -354,56 +354,62 @@ def item_availability(item_pid): }) -@api_blueprint.route( - '//can_request//', methods=['GET']) +@api_blueprint.route('//can_request', methods=['GET']) @check_authentication @jsonify_error -def can_request(item_pid, library_pid, patron_barcode): - """HTTP request to check if a librarian can request an item for a patron. - - required_parameters: item_pid, library_pid, patron_barcode - """ - return is_librarian_can_request_item_for_patron( - item_pid, library_pid, patron_barcode) +def can_request(item_pid): + """HTTP request to check if an item can be requested. + Depending of query string argument, either only check if configuration + allows the request of this item ; either if a librarian can request an + item for a patron. -def jsonify_response(response=False, reason=None): - """Jsonify api response.""" - return jsonify({ - 'can_request': response, - 'reason': reason - }) + `api/item//can_request` : + --> only check config + `api/item//can_request?library_pid=&patron_barcode=`: + --> check if the patron can request this item (check the cipo) + """ + kwargs = {} + item = Item.get_record_by_pid(item_pid) + if not item: + abort(404, 'Item not found') + patron_barcode = flask_request.args.get('patron_barcode') + if patron_barcode: + kwargs['patron'] = Patron.get_patron_by_barcode(patron_barcode) + if not kwargs['patron']: + abort(404, 'Patron not found') + library_pid = flask_request.args.get('library_pid') + if library_pid: + kwargs['library'] = Library.get_record_by_pid(library_pid) + if not kwargs['library']: + abort(404, 'Library not found') + + # as to item if the request is possible with these data. + can, reasons = item.can(ItemCirculationAction.REQUEST, **kwargs) + + # check the `reasons_not_request` array. If it's empty, the request is + # allowed ; if not the request is disallow and we need to return the + # reasons why + response = {'can': can} + if reasons: + response['reasons'] = { + 'others': {reason: True for reason in reasons} + } + return jsonify(response) -def is_librarian_can_request_item_for_patron( - item_pid, library_pid, patron_barcode): - """Check if a librarian can request an item for a patron. +@api_blueprint.route('//pickup_locations', methods=['GET']) +@check_authentication +@jsonify_error +def get_pickup_locations(item_pid): + """HTTP request to return the available pickup locations for an item. - required_parameters: item_pid, library_pid, patron_barcode + :param item_pid: the item pid """ item = Item.get_record_by_pid(item_pid) if not item: - return jsonify_response(reason='Item not found.') - patron = Patron.get_patron_by_barcode(patron_barcode) - if not patron: - return jsonify_response(reason='Patron not found.') - library = Library.get_record_by_pid(library_pid) - if not library: - return jsonify_response(reason='Library not found.') - # Create a loan - loan = Loan({ - 'patron_pid': patron.pid, 'item_pid': item.pid, - 'library_pid': library_pid}) - if not can_be_requested(loan): - return jsonify_response( - reason='Request not allowed by the circulation policy.') - if item.status != ItemStatus.MISSING: - loaned_to_patron = item.is_loaned_to_patron(patron_barcode) - if loaned_to_patron: - return jsonify_response( - reason='Item is already checked-out or requested by patron.') - return jsonify_response( - response=True, reason='Request is possible.') - else: - return jsonify_response( - reason='Item status does not allow requests.') + abort(404, 'Item not found') + locations = item_library_pickup_locations(item) + return jsonify({ + 'locations': locations + }) diff --git a/rero_ils/modules/items/models.py b/rero_ils/modules/items/models.py index 9813ac5eea..86f1a27d20 100644 --- a/rero_ils/modules/items/models.py +++ b/rero_ils/modules/items/models.py @@ -19,6 +19,8 @@ from __future__ import absolute_import +from enum import Enum + from invenio_db import db from invenio_pidstore.models import RecordIdentifier from invenio_records.models import RecordMetadataBase @@ -52,3 +54,12 @@ class ItemStatus(object): IN_TRANSIT = 'in_transit' EXCLUDED = 'excluded' MISSING = 'missing' + + +class ItemCirculationAction(Enum): + """Enum class to list all possible action about an item.""" + + CHECKOUT = 'checkout' + CHECKIN = 'checkin' + REQUEST = 'request' + EXTEND = 'extend' diff --git a/rero_ils/modules/loans/api.py b/rero_ils/modules/loans/api.py index 23559de17e..334d5d1397 100644 --- a/rero_ils/modules/loans/api.py +++ b/rero_ils/modules/loans/api.py @@ -32,6 +32,7 @@ from ..api import IlsRecord, IlsRecordError, IlsRecordsIndexer, \ IlsRecordsSearch from ..documents.api import Document +from ..items.models import ItemCirculationAction from ..libraries.api import Library from ..locations.api import Location from ..notifications.api import Notification, NotificationsSearch, \ @@ -172,18 +173,20 @@ def organisation_pid(self): @property def library_pid(self): - """Get library PID regarding transaction location PID or location.""" - from ..items.api import Item + """Get library PID regarding loan location.""" + return Location.get_record_by_pid(self.location_pid).library_pid + @property + def location_pid(self): + """Get loan transaction_location PID or item owning location.""" + from ..items.api import Item location_pid = self.get('transaction_location_pid') item_pid = self.get('item_pid') if not location_pid and item_pid: - item = Item.get_record_by_pid(item_pid) - return item.holding_library_pid + return Item.get_record_by_pid(item_pid).holding_location_pid elif location_pid: - loc = Location.get_record_by_pid(location_pid) - return loc.library_pid + return location_pid return IlsRecordError.PidDoesNotExist( self.provider.pid_type, 'library_pid' @@ -302,7 +305,11 @@ def patron_profile_loans(patron_pid): loan['library_name'] = Library.get_record_by_pid( item.holding_library_pid).get('name') if loan['state'] == 'ITEM_ON_LOAN': - loan['can_renew'] = item.can_extend(loan) + can, reasons = item.can( + ItemCirculationAction.EXTEND, + loan=loan + ) + loan['can_renew'] = can checkouts.append(loan) elif loan['state'] in [ 'PENDING', diff --git a/rero_ils/modules/loans/cli.py b/rero_ils/modules/loans/cli.py index c19c7cb166..8df022e3e8 100644 --- a/rero_ils/modules/loans/cli.py +++ b/rero_ils/modules/loans/cli.py @@ -271,7 +271,7 @@ def create_loan(barcode, transaction_type, loanable_items, verbose=False, transaction_user_pid=user_pid, transaction_date=transaction_date, pickup_location_pid=get_random_pickup_location( - requested_patron.pid), + requested_patron.pid, item), document_pid=item.replace_refs()['document']['pid'], ) loan.create_notification(notification_type='recall') @@ -315,7 +315,7 @@ def create_request(barcode, transaction_type, loanable_items, verbose=False, transaction_user_pid=user_pid, transaction_date=transaction_date, pickup_location_pid=get_random_pickup_location( - rank_1_patron.pid), + rank_1_patron.pid, item), document_pid=item.replace_refs()['document']['pid'], ) transaction_date = datetime.now(timezone.utc).isoformat() @@ -326,7 +326,7 @@ def create_request(barcode, transaction_type, loanable_items, verbose=False, transaction_location_pid=user_location, transaction_user_pid=user_pid, transaction_date=transaction_date, - pickup_location_pid=get_random_pickup_location(patron.pid), + pickup_location_pid=get_random_pickup_location(patron.pid, item), document_pid=item.replace_refs()['document']['pid'], ) return item['barcode'] @@ -373,9 +373,12 @@ def get_loanable_items(patron_type_pid): yield item -def get_random_pickup_location(patron_pid): +def get_random_pickup_location(patron_pid, item): """Find a qualified pickup location.""" - pickup_locations_pids = list(Location.get_pickup_location_pids(patron_pid)) + pickup_locations_pids = list(Location.get_pickup_location_pids( + patron_pid=patron_pid, + item_pid=item.pid + )) return random.choice(pickup_locations_pids) diff --git a/rero_ils/modules/loans/listener.py b/rero_ils/modules/loans/listener.py index f6503b7ab7..579f61cd0d 100644 --- a/rero_ils/modules/loans/listener.py +++ b/rero_ils/modules/loans/listener.py @@ -20,7 +20,10 @@ from invenio_circulation.proxies import current_circulation from ..items.api import Item +from ..items.models import ItemStatus from ..loans.api import Loan +from ..locations.api import Location +from ..notifications.utils import send_notification_to_location def enrich_loan_data(sender, json=None, record=None, index=None, @@ -41,10 +44,22 @@ def enrich_loan_data(sender, json=None, record=None, index=None, def listener_loan_state_changed(_, prev_loan, loan, trigger): """Create notification based on loan state changes.""" if loan.get('state') == 'PENDING': + # create notification to requester item_pid = loan.get('item_pid') checkedout_loan_pid = Item.get_loan_pid_with_item_on_loan(item_pid) if checkedout_loan_pid: checked_out_loan = Loan.get_record_by_pid(checkedout_loan_pid) checked_out_loan.create_notification(notification_type='recall') + # send notification to location if needed + # Notification should be sent only if the item is on shelf without + # previous pending loan and item location assign 'send_notification' + # to true + item = Item.get_record_by_pid(item_pid) + item_location = Location.get_record_by_pid(item.location_pid) + if item_location \ + and item_location.get('send_notification', False) \ + and item.status == ItemStatus.ON_SHELF \ + and item.number_of_requests() == 0: + send_notification_to_location(loan, item, item_location) elif loan.get('state') == 'ITEM_AT_DESK': loan.create_notification(notification_type='availability') diff --git a/rero_ils/modules/loans/utils.py b/rero_ils/modules/loans/utils.py index 705c89f222..0a191c7b59 100644 --- a/rero_ils/modules/loans/utils.py +++ b/rero_ils/modules/loans/utils.py @@ -24,6 +24,7 @@ from ..circ_policies.api import CircPolicy from ..items.api import Item from ..libraries.api import Library +from ..locations.api import Location from ..patrons.api import Patron @@ -153,7 +154,24 @@ def is_item_available_for_checkout(item_pid): def can_be_requested(loan): """Check if record can be requested.""" + # TODO : Should use + # ``` item = Item.get_record_by_pid(loan.get('item_pid') + # can, reasons = item.can(ItemCirculationActions.REQUEST, loan=loan) + # return can + # ``` + # But this usage cause a lot of problem with invenio-circulation because + # it seems this function only answer the question "Is the item potentially + # requestable" and not "Is the item is really requestable". + if not loan.get('item_pid'): raise Exception('Transaction on document is not implemented.') + # 1) Check if circulation_policy allows request policy = get_circ_policy(loan) - return policy.get('allow_requests') + if not policy.get('allow_requests'): + return False + # 2) Check if location allows request + location = Location.get_record_by_pid(loan.location_pid) + if not location or not location.get('allow_request'): + return False + # All checks are successful, the request is allowed + return True diff --git a/rero_ils/modules/locations/api.py b/rero_ils/modules/locations/api.py index 9d57445867..ba1fb769f9 100644 --- a/rero_ils/modules/locations/api.py +++ b/rero_ils/modules/locations/api.py @@ -79,20 +79,23 @@ def extended_validation(self, **kwargs): return True @classmethod - def get_pickup_location_pids(cls, patron_pid=None): + def get_pickup_location_pids(cls, patron_pid=None, item_pid=None): """Return pickup locations.""" from ..patrons.api import Patron - from ..patron_types.api import PatronType - search = LocationsSearch()\ - .filter('term', is_pickup=True) + from ..items.api import Item + search = LocationsSearch() + + if item_pid: + loc = Item.get_record_by_pid(item_pid).get_location() + if loc.restrict_pickup_to: + search = search.filter('terms', pid=loc.restrict_pickup_to) + + search = search.filter('term', is_pickup=True) + if patron_pid: - patron = Patron.get_record_by_pid(patron_pid) - ptty_pid = patron.replace_refs()['patron_type']['pid'] - org_pid = PatronType.get_record_by_pid( - ptty_pid).replace_refs()['organisation']['pid'] - search = search.filter( - 'term', - organisation__pid=org_pid) + org_pid = Patron.get_record_by_pid(patron_pid).organisation_pid + search = search.filter('term', organisation__pid=org_pid) + locations = search.source(['pid']).scan() for location in locations: yield location.pid @@ -139,6 +142,26 @@ def organisation_pid(self): library = Library.get_record_by_pid(self.library_pid) return library.organisation_pid + @property + def restrict_pickup_to(self): + """Get restriction pickup location pid of location.""" + return [ + location['pid'] + for location in self.replace_refs().get('restrict_pickup_to', []) + ] + + @classmethod + def allow_request(cls, item, **kwargs): + """Check if an item can be requested regarding its location. + + :param item : the item to check + :param kwargs : addition arguments + :return a tuple with True|False and reasons to disallow if False. + """ + if item and not item.get_location().get('allow_request', False): + return False, ["Item location disallows request."] + return True, [] + class LocationsIndexer(IlsRecordsIndexer): """Holdings indexing class.""" diff --git a/rero_ils/modules/locations/jsonschemas/locations/location-v0.0.1.json b/rero_ils/modules/locations/jsonschemas/locations/location-v0.0.1.json index f2a5c4c019..875897a5cb 100644 --- a/rero_ils/modules/locations/jsonschemas/locations/location-v0.0.1.json +++ b/rero_ils/modules/locations/jsonschemas/locations/location-v0.0.1.json @@ -9,14 +9,19 @@ "pid", "code", "name", - "library" + "library", + "allow_request" ], "propertiesOrder": [ "name", "code", "is_online", "is_pickup", - "pickup_name" + "pickup_name", + "allow_request", + "send_notification", + "notification_email", + "restrict_pickup_to" ], "properties": { "$schema": { @@ -127,6 +132,85 @@ "pattern": "^https://ils.rero.ch/api/libraries/.*?$" } } + }, + "allow_request": { + "title": "Allow request", + "type": "boolean", + "description": "If enabled, it allows requests for items linked to this location.", + "default": true + }, + "send_notification": { + "title": "Send notification", + "description": "Send a email notification when an item of the location has been requested.", + "type": "boolean", + "default": false, + "form": { + "hideExpression": "model.allow_request === false", + "expressionProperties": { + "templateOptions.required": "model.allow_request === true" + } + } + }, + "notification_email": { + "title": "Notification email", + "description": "Specify a generic email address used for notification.", + "type": "string", + "format": "email", + "pattern": "^\\w+([\\.-]?\\w+)*@\\w+([\\.-]?\\w+)*(\\.\\w{2,})+$", + "minLength": 6, + "form": { + "hideExpression": "model.allow_request === false || model.send_notification === false", + "expressionProperties": { + "templateOptions.required": "model.send_notification === true && model.allow_request === true" + }, + "templateOptions": { + "addonLeft": { + "class": "fa fa-at" + } + }, + "validation": { + "messages": { + "pattern": "Email should have at least one @ and one ." + } + } + } + }, + "restrict_pickup_to": { + "title": "Restrict pickup to", + "description": "Select locations where items linked to the current location could be requested.", + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "object", + "title": "Location", + "properties": { + "$ref": { + "title": "Location URI", + "type": "string", + "pattern": "^https://ils.rero.ch/api/locations/.*?$", + "form": { + "remoteOptions": { + "type": "locations", + "query": "is_pickup:true", + "labelField": "pickup_name" + } + } + } + } + }, + "form": { + "wrappers": [ + "toggle-switch" + ], + "templateOptions": { + "label": "", + "toggle-switch": { + "label": "Restrict pickup to", + "description": "If enabled, restrict pickup to specific pickup locations." + } + } + } } } } diff --git a/rero_ils/modules/locations/mappings/v6/locations/location-v0.0.1.json b/rero_ils/modules/locations/mappings/v6/locations/location-v0.0.1.json index 5c4a49f452..86b8ce13b4 100644 --- a/rero_ils/modules/locations/mappings/v6/locations/location-v0.0.1.json +++ b/rero_ils/modules/locations/mappings/v6/locations/location-v0.0.1.json @@ -44,6 +44,19 @@ } } }, + "notification_email": { + "type": "keyword" + }, + "allow_request": { + "type": "boolean" + }, + "restrict_pickup_to": { + "properties": { + "pid": { + "type": "keyword" + } + } + }, "_created": { "type": "date" }, diff --git a/rero_ils/modules/notifications/dispatcher.py b/rero_ils/modules/notifications/dispatcher.py index 98f159050e..401b14e9d3 100644 --- a/rero_ils/modules/notifications/dispatcher.py +++ b/rero_ils/modules/notifications/dispatcher.py @@ -26,46 +26,44 @@ from ..locations.api import Location -class Dispatcher(): +class Dispatcher: """Dispatcher notifications class.""" - def __init__(self): - """Init dispatcher.""" - self.channels_function = { - 'email': self.send_email, - 'sms': self.send_sms, - 'whatsapp': self.send_whatsapp, - 'mail': self.send_mail - } - def dispatch_notification(self, notification=None, verbose=False): """Dispatch the notification.""" + + def not_yet_implemented(*args): + """Do nothing placeholder for a notification.""" + return + if notification: data = notification.replace_pids_and_refs() - communication_channel = \ - data['loan']['patron']['communication_channel'] - if communication_channel not in self.channels_function: - raise ValueError( - 'Unknown communication channel: {channel}'.format( - channel=communication_channel - ) - ) - # call the function from channels_function - self.channels_function.get(communication_channel)(data) + communication_switcher = { + 'email': Dispatcher.send_mail, + # 'sms': not_yet_implemented + # 'telepathy': self.madness_mind + # ... + } + dispatcher_function = communication_switcher.get( + data['loan']['patron']['communication_channel'], + not_yet_implemented + ) + dispatcher_function(data) notification = notification.update_process_date() if verbose: current_app.logger.info( ('Notification: {pid} chanel: {chanel} type:' '{type} loan: {lpid}').format( pid=notification['pid'], - chanel=communication_channel, + chanel=data['loan']['patron']['communication_channel'], type=notification['notification_type'], lpid=data['loan']['pid'] ) ) return notification - def send_email(self, data): + @staticmethod + def send_mail(data): """Send the notification by email.""" notification_type = data.get('notification_type') language = data['loan']['patron']['communication_language'] @@ -89,19 +87,4 @@ def send_email(self, data): text = msg.body.split('\n') msg.subject = text[0] msg.body = '\n'.join(text[1:]) - try: - task_send_email.run(msg.__dict__) - except Exception as error: - raise(error) - - def send_sms(self, data): - """Send the notification by sms.""" - # TODO: add send sms code - - def send_whatsapp(self, data): - """Send the notification by whatsapp.""" - # TODO: add send whatsapp code - - def send_mail(self, data): - """Send the notification by mail.""" - # TODO: add send mail code + task_send_email.run(msg.__dict__) diff --git a/rero_ils/modules/notifications/templates/email/others/location_notification.txt b/rero_ils/modules/notifications/templates/email/others/location_notification.txt new file mode 100644 index 0000000000..ec869a2351 --- /dev/null +++ b/rero_ils/modules/notifications/templates/email/others/location_notification.txt @@ -0,0 +1,14 @@ +The item [{{ item.barcode }}] has been requested +An item has been requested. See below for information about this item. + +Pickup location : {{ pickup_location.library.name }} -- {{ pickup_location.name }} +Request date : {{ loan.transaction_date }} +Item barcode : {{ item.barcode }} +Item call number : {{ item.call_number }} +Item type : {{ item.item_type.name }} +Document title : {{ document.title_text }} / {{ document.responsibility_statement }} + + +Patron name : {{ patron.formatted_name }} +Patron email : {{ patron.email }} +Patron barcode : {{ patron.get('barcode') }} diff --git a/rero_ils/modules/notifications/utils.py b/rero_ils/modules/notifications/utils.py new file mode 100644 index 0000000000..f4ee583d81 --- /dev/null +++ b/rero_ils/modules/notifications/utils.py @@ -0,0 +1,98 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2019 RERO +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +"""Notification utils.""" +from datetime import timezone + +import ciso8601 +from flask_security.utils import config_value +from invenio_mail.api import TemplatedMessage +from invenio_mail.tasks import send_email + +from rero_ils.modules.documents.api import Document +from rero_ils.modules.documents.views import create_title_responsibilites +from rero_ils.modules.item_types.api import ItemType +from rero_ils.modules.items.api import Item +from rero_ils.modules.locations.api import Location +from rero_ils.modules.patrons.api import Patron + + +def _build_notification_email_context(loan, item, location): + """Build the context used by the send_notification_to_location function. + + :param loan : the loan for which build context + :param item : the item for which build context + :param location : the item location + """ + document_pid = Item.get_document_pid_by_item_pid(loan.item_pid) + document = Document.get_record_by_pid(document_pid) + pickup_location = Location.get_record_by_pid( + loan.get('pickup_location_pid') + ) + patron = Patron.get_record_by_pid(loan.patron_pid) + ctx = { + 'loan': loan.replace_refs().dumps(), + 'item': item.replace_refs().dumps(), + 'document': document.replace_refs().dumps(), + 'pickup_location': pickup_location, + 'item_location': location.dumps(), + 'patron': patron + } + library = pickup_location.get_library() + ctx['pickup_location']['library'] = library + ctx['item']['item_type'] = \ + ItemType.get_record_by_pid(item.item_type_pid) + titles = [title for title in ctx['document'].get('title', []) + if title['type'] == 'bf:Title'] + ctx['document']['title_text'] = \ + next(iter(titles or []), {}).get('_text') + responsibility_statement = create_title_responsibilites( + document.get('responsibilityStatement', []) + ) + ctx['document']['responsibility_statement'] = \ + next(iter(responsibility_statement or []), '') + trans_date = ciso8601.parse_datetime(loan.get('transaction_date')) + trans_date = trans_date\ + .replace(tzinfo=timezone.utc)\ + .astimezone(tz=library.get_timezone()) + ctx['loan']['transaction_date'] = \ + trans_date.strftime("%d.%m.%Y - %H:%M:%S") + return ctx + + +def send_notification_to_location(loan, item, location): + """Send a notification to the location defined email. + + :param loan: the loan to be parsed + :param item: the requested item + :param location: the location to inform + """ + if not location.get('send_notification', False) \ + or not location.get('notification_email'): + return + template = 'email/others/location_notification.txt' + recipient = location.get('notification_email') + msg = TemplatedMessage( + template_body=template, + sender=config_value('EMAIL_SENDER'), + recipients=[recipient], + ctx=_build_notification_email_context(loan, item, location) + ) + text = msg.body.split('\n') + msg.subject = text[0] + msg.body = '\n'.join(text[1:]) + send_email.run(msg.__dict__) diff --git a/rero_ils/modules/patrons/api.py b/rero_ils/modules/patrons/api.py index 7139aee886..b2b4b1cd72 100644 --- a/rero_ils/modules/patrons/api.py +++ b/rero_ils/modules/patrons/api.py @@ -259,6 +259,28 @@ def get_patron_by_barcode(cls, barcode=None): except StopIteration: return None + @classmethod + def can_request(cls, item, **kwargs): + """Check if a paton can request an item. + + :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. + """ + 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 + # 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')) + # a blocked patron can't request any item + if patron.get('blocked', False): + return False, ['Patron is blocked'] + return True, [] + @classmethod def patrons_with_obsolete_subscription_pids(cls, end_date=None): """Search about patrons with obsolete subscription.""" @@ -307,6 +329,16 @@ def initial(self): return initial + @property + def formatted_name(self): + """Return the best possible human readable patron name.""" + name_parts = [ + self.get('last_name', '').strip(), + self.get('first_name', '').strip() + ] + name_parts = [part for part in name_parts if part] # remove empty part + return ', '.join(name_parts) + @property def patron_type_pid(self): """Shortcut for patron type pid.""" diff --git a/tests/api/test_availability.py b/tests/api/test_availability.py index dfb879cc6b..7f3e44cbca 100644 --- a/tests/api/test_availability.py +++ b/tests/api/test_availability.py @@ -15,23 +15,31 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +"""Tests availability.""" + +from copy import deepcopy + import mock from flask import url_for from invenio_accounts.testutils import login_user_via_session from utils import get_json, postdata +from rero_ils.modules.documents.views import can_request from rero_ils.modules.holdings.api import Holding from rero_ils.modules.items.api import Item from rero_ils.modules.items.models import ItemStatus from rero_ils.modules.items.views import item_availability_text from rero_ils.modules.loans.api import LoanAction +from rero_ils.modules.locations.api import Location +from rero_ils.modules.utils import get_ref_for_pid def test_item_can_request( client, document, holding_lib_martigny, item_lib_martigny, librarian_martigny_no_email, lib_martigny, patron_martigny_no_email, circulation_policies, - patron_type_children_martigny): + patron_type_children_martigny, loc_public_martigny_data, + system_librarian_martigny_no_email, item_lib_martigny_data): """Test item can request API.""" # test no logged user res = client.get( @@ -44,6 +52,9 @@ def test_item_can_request( ) assert res.status_code == 401 + result = can_request(item_lib_martigny) + assert not result + login_user_via_session(client, librarian_martigny_no_email.user) # valid test res = client.get( @@ -56,7 +67,20 @@ def test_item_can_request( ) assert res.status_code == 200 data = get_json(res) - assert data.get('can_request') + assert data.get('can') + + # test no valid -- patron doesn't have correct role + res = client.get( + url_for( + 'api_item.can_request', + item_pid=item_lib_martigny.pid, + library_pid=lib_martigny.pid, + patron_barcode=system_librarian_martigny_no_email.get('barcode') + ) + ) + assert res.status_code == 200 + data = get_json(res) + assert not data.get('can') # test no valid item res = client.get( @@ -67,9 +91,7 @@ def test_item_can_request( patron_barcode=patron_martigny_no_email.get('barcode') ) ) - assert res.status_code == 200 - data = get_json(res) - assert not data.get('can_request') + assert res.status_code == 404 # test no valid library res = client.get( @@ -80,9 +102,7 @@ def test_item_can_request( patron_barcode=patron_martigny_no_email.get('barcode') ) ) - assert res.status_code == 200 - data = get_json(res) - assert not data.get('can_request') + assert res.status_code == 404 # test no valid patron res = client.get( @@ -93,9 +113,7 @@ def test_item_can_request( patron_barcode='no_barcode' ) ) - assert res.status_code == 200 - data = get_json(res) - assert not data.get('can_request') + assert res.status_code == 404 # test no valid item status item_lib_martigny['status'] = ItemStatus.MISSING @@ -110,10 +128,48 @@ def test_item_can_request( ) assert res.status_code == 200 data = get_json(res) - assert not data.get('can_request') + assert not data.get('can') item_lib_martigny['status'] = ItemStatus.ON_SHELF item_lib_martigny.update(item_lib_martigny, dbcommit=True, reindex=True) + # Location :: allow_request == false + # create a new location and set 'allow_request' to false. Assign a new + # item to this location. Chek if this item can be requested : it couldn't + # with 'Item location doesn't allow request' reason. + new_location = deepcopy(loc_public_martigny_data) + del new_location['pid'] + new_location['allow_request'] = False + new_location = Location.create(new_location, dbcommit=True, reindex=True) + assert new_location + new_item = deepcopy(item_lib_martigny_data) + del new_item['pid'] + new_item['barcode'] = 'dummy_barcode_allow_request' + new_item['location']['$ref'] = get_ref_for_pid(Location, new_location.pid) + new_item = Item.create(new_item, dbcommit=True, reindex=True) + assert new_item + + res = client.get(url_for('api_item.can_request', item_pid=new_item.pid)) + assert res.status_code == 200 + data = get_json(res) + assert not data.get('can') + + # remove created data + item_url = url_for( + 'invenio_records_rest.item_item', + pid_value=new_item.pid + ) + hold_url = url_for( + 'invenio_records_rest.hold_item', + pid_value=new_item.holding_pid + ) + loc_url = url_for( + 'invenio_records_rest.loc_item', + pid_value=new_location.pid + ) + client.delete(item_url) + client.delete(hold_url) + client.delete(loc_url) + def test_item_holding_document_availability( client, document, lib_martigny, @@ -121,7 +177,8 @@ def test_item_holding_document_availability( item_lib_martigny, item2_lib_martigny, librarian_martigny_no_email, librarian_saxon_no_email, patron_martigny_no_email, patron2_martigny_no_email, - loc_public_saxon, circulation_policies): + loc_public_saxon, circulation_policies, ebook_1_data, + item_lib_martigny_data): """Test item, holding and document availability.""" assert item_availablity_status( client, item_lib_martigny.pid, librarian_martigny_no_email.user) @@ -132,7 +189,7 @@ def test_item_holding_document_availability( client, holding_lib_martigny.pid, librarian_martigny_no_email.user) assert holding_lib_martigny.get_holding_loan_conditions() == 'standard' assert document.is_available(view_code='global') - assert document_availablity_status( + assert document_availability_status( client, document.pid, librarian_martigny_no_email.user) # login as patron @@ -169,7 +226,7 @@ def test_item_holding_document_availability( client, holding_lib_martigny.pid, librarian_martigny_no_email.user) assert holding_lib_martigny.get_holding_loan_conditions() == 'standard' assert document.is_available('global') - assert document_availablity_status( + assert document_availability_status( client, document.pid, librarian_martigny_no_email.user) # validate request @@ -194,7 +251,7 @@ def test_item_holding_document_availability( client, holding_lib_martigny.pid, librarian_martigny_no_email.user) assert holding_lib_martigny.get_holding_loan_conditions() == 'standard' assert document.is_available('global') - assert document_availablity_status( + assert document_availability_status( client, document.pid, librarian_martigny_no_email.user) login_user_via_session(client, librarian_saxon_no_email.user) # receive @@ -219,7 +276,7 @@ def test_item_holding_document_availability( client, holding_lib_martigny.pid, librarian_saxon_no_email.user) assert holding_lib_martigny.get_holding_loan_conditions() == 'standard' assert document.is_available('global') - assert document_availablity_status( + assert document_availability_status( client, document.pid, librarian_martigny_no_email.user) # checkout res, _ = postdata( @@ -242,7 +299,7 @@ def test_item_holding_document_availability( client, holding_lib_martigny.pid, librarian_saxon_no_email.user) assert holding_lib_martigny.get_holding_loan_conditions() == 'standard' assert document.is_available('global') - assert document_availablity_status( + assert document_availability_status( client, document.pid, librarian_martigny_no_email.user) # test can not request item already checked out to patron @@ -306,7 +363,7 @@ class locale: client, holding_lib_martigny.pid, librarian_martigny_no_email.user) assert holding_lib_martigny.get_holding_loan_conditions() == 'standard' assert not document.is_available('global') - assert not document_availablity_status( + assert not document_availability_status( client, document.pid, librarian_martigny_no_email.user) @@ -338,7 +395,7 @@ def holding_availablity_status(client, pid, user): return data.get('availability') -def document_availablity_status(client, pid, user): +def document_availability_status(client, pid, user): """Returns document availability.""" login_user_via_session(client, user) res = client.get( @@ -351,3 +408,55 @@ def document_availablity_status(client, pid, user): assert res.status_code == 200 data = get_json(res) return data.get('availability') + + +def test_availability_cipo_allow_request( + client, librarian_martigny_no_email, item_lib_martigny, + item_type_standard_martigny, patron_martigny_no_email, + circ_policy_short_martigny): + """Test availability is cipo disallow request.""" + login_user_via_session(client, librarian_martigny_no_email.user) + + # update the cipo to disallow request + cipo = circ_policy_short_martigny + cipo['allow_requests'] = False + cipo.update(cipo.dumps(), dbcommit=True, reindex=True) + + res = client.get( + url_for( + 'api_item.can_request', + item_pid=item_lib_martigny.pid, + patron_barcode=patron_martigny_no_email.get('barcode') + ) + ) + assert res.status_code == 200 + data = get_json(res) + assert not data.get('can') + + # reset the cipo + cipo['allow_requests'] = True + cipo.update(cipo.dumps(), dbcommit=True, reindex=True) + + +def test_document_availability_failed(client, librarian2_martigny_no_email): + """Test document availability with dummy data should failed.""" + login_user_via_session(client, librarian2_martigny_no_email.user) + res = client.get( + url_for( + 'api_documents.document_availability', + document_pid='dummy_pid' + ) + ) + assert res.status_code == 404 + + +def test_item_availability_failed(client, librarian2_martigny_no_email): + """Test item availability with dummy data should failed.""" + login_user_via_session(client, librarian2_martigny_no_email.user) + res = client.get( + url_for( + 'api_item.item_availability', + item_pid='dummy_pid' + ) + ) + assert res.status_code == 404 diff --git a/tests/api/test_items_rest_views.py b/tests/api/test_items_rest_views.py index 8a168fc107..c2a7f282c0 100644 --- a/tests/api/test_items_rest_views.py +++ b/tests/api/test_items_rest_views.py @@ -752,3 +752,27 @@ def test_update_loan_pickup_location( ) ) assert res.status_code == 403 + + +def test_item_pickup_location( + client, librarian_martigny_no_email, item2_lib_martigny): + """Test get item pickup locations.""" + login_user_via_session(client, librarian_martigny_no_email.user) + # test with dummy data will return 404 + res = client.get( + url_for( + 'api_item.get_pickup_locations', + item_pid='dummy_pid' + ) + ) + assert res.status_code == 404 + # test with an existing item + res = client.get( + url_for( + 'api_item.get_pickup_locations', + item_pid=item2_lib_martigny.pid + ) + ) + assert res.status_code == 200 + data = get_json(res) + assert 'locations' in data diff --git a/tests/api/test_loans_rest.py b/tests/api/test_loans_rest.py index e3d76362bf..0cebe4df0a 100644 --- a/tests/api/test_loans_rest.py +++ b/tests/api/test_loans_rest.py @@ -36,6 +36,7 @@ get_last_transaction_loc_for_item, get_loans_by_patron_pid, \ get_overdue_loans from rero_ils.modules.loans.utils import can_be_requested +from rero_ils.modules.locations.api import LocationsSearch from rero_ils.modules.notifications.api import NotificationsSearch, \ number_of_reminders_sent @@ -96,7 +97,8 @@ def test_loans_logged_permissions(client, loan_pending_martigny, def test_loan_utils(client, patron_martigny_no_email, patron2_martigny_no_email, circulation_policies, - loan_pending_martigny, item_lib_martigny): + loan_pending_martigny, item_lib_martigny, + loc_public_martigny): """Test loan utils.""" loan_metadata = dict(item_lib_martigny) if 'item_pid' not in loan_metadata: @@ -129,6 +131,25 @@ def test_loan_utils(client, patron_martigny_no_email, with pytest.raises(IlsRecordError.PidDoesNotExist): new_loan.organisation_pid + new_loan = deepcopy(loan_pending_martigny) + assert can_be_requested(new_loan) + loc_public_martigny['allow_request'] = False + loc_public_martigny.update( + loc_public_martigny, + dbcommit=True, + reindex=True + ) + flush_index(LocationsSearch.Meta.index) + assert not can_be_requested(new_loan) + + loc_public_martigny['allow_request'] = True + loc_public_martigny.update( + loc_public_martigny, + dbcommit=True, + reindex=True + ) + flush_index(LocationsSearch.Meta.index) + def test_due_soon_loans(client, librarian_martigny_no_email, patron_martigny_no_email, loc_public_martigny, @@ -262,12 +283,20 @@ def test_checkout_item_transit(client, item2_lib_martigny, librarian_saxon_no_email, patron_martigny_no_email, loc_public_saxon, + loc_public_martigny, circulation_policies): """Test checkout of an item in transit.""" assert item2_lib_martigny.available # request login_user_via_session(client, librarian_martigny_no_email.user) + loc_public_martigny['notification_email'] = 'dummy_email@fake.domain' + loc_public_martigny['send_notification'] = True + loc_public_martigny.update( + loc_public_martigny.dumps(), + dbcommit=True, + reindex=True + ) res, data = postdata( client, @@ -286,6 +315,15 @@ def test_checkout_item_transit(client, item2_lib_martigny, loan = Loan.get_record_by_pid(loan_pid) assert loan.get('state') == 'PENDING' + # reset the location + del loc_public_martigny['notification_email'] + del loc_public_martigny['send_notification'] + loc_public_martigny.update( + loc_public_martigny.dumps(), + dbcommit=True, + reindex=True + ) + # validate request res, _ = postdata( client, diff --git a/tests/api/test_locations_rest.py b/tests/api/test_locations_rest.py index 375544de5b..30eaa7dbe7 100644 --- a/tests/api/test_locations_rest.py +++ b/tests/api/test_locations_rest.py @@ -24,10 +24,69 @@ import pytest from flask import url_for from invenio_accounts.testutils import login_user_via_session -from utils import VerifyRecordPermissionPatch, get_json, postdata, \ - to_relative_url +from utils import VerifyRecordPermissionPatch, flush_index, get_json, \ + postdata, to_relative_url +from rero_ils.modules.documents.views import item_library_pickup_locations from rero_ils.modules.errors import RecordValidationError +from rero_ils.modules.locations.api import Location, LocationsSearch + + +def test_location_pickup_locations(locations, patron_martigny_no_email, + patron_sion_no_email, loc_public_martigny, + item2_lib_martigny): + """Test for pickup locations.""" + + # At the beginning, if we load all locations from fixtures, there are 4 + # pickup locations (loc1, loc3, loc5, loc7) + pickup_locations = Location.get_pickup_location_pids() + assert set(pickup_locations) == set(['loc1', 'loc3', 'loc5', 'loc7']) + + # check pickup restrictions by patron_pid + pickup_locations = Location.get_pickup_location_pids( + patron_pid=patron_martigny_no_email.pid) + assert set(pickup_locations) == set(['loc1', 'loc3', 'loc5']) + pickup_locations = Location.get_pickup_location_pids( + patron_pid=patron_sion_no_email.pid) + assert set(pickup_locations) == set(['loc7']) + + # check pickup restrictions by item_barcode + # * update `loc1` to restrict_pickup_to 'loc3' and 'loc6' + # --> 'loc6' isn't a pickup location... it's just for test + loc_public_martigny['restrict_pickup_to'] = [ + {'$ref': 'https://ils.rero.ch/api/locations/loc3'}, + {'$ref': 'https://ils.rero.ch/api/locations/loc6'}, + ] + loc_public_martigny.update( + loc_public_martigny, + dbcommit=True, + reindex=True + ) + flush_index(LocationsSearch.Meta.index) + pickup_locations = Location.get_pickup_location_pids( + item_pid=item2_lib_martigny.pid) + assert set(pickup_locations) == set(['loc3']) + + pickup_locations = Location.get_pickup_location_pids( + patron_pid=patron_sion_no_email.pid, + item_pid=item2_lib_martigny.pid) + assert set(pickup_locations) == set([]) + + # check document.views::item_library_pickup_locations + # As we limit pickup to two specific location, this tests will also + # return only these two records instead of all pickups for the + # organisation + picks = item_library_pickup_locations(item2_lib_martigny) + assert len(picks) == 2 + + # reset the location to default value before leaving + del loc_public_martigny['restrict_pickup_to'] + loc_public_martigny.update( + loc_public_martigny, + dbcommit=True, + reindex=True + ) + flush_index(LocationsSearch.Meta.index) def test_locations_permissions(client, loc_public_martigny, json_header): @@ -98,7 +157,7 @@ def test_locations_get(client, loc_public_martigny): data = get_json(res) result = data['hits']['hits'][0]['metadata'] # organisation has been added during the indexing - del(result['organisation']) + del (result['organisation']) assert result == location.replace_refs() @@ -174,13 +233,8 @@ def test_location_can_delete(client, item_lib_martigny, loc_public_martigny): assert 'links' in reasons -def test_filtered_locations_get( - client, librarian_martigny_no_email, loc_public_martigny, - loc_restricted_martigny, loc_public_saxon, loc_restricted_saxon, - loc_public_fully, loc_restricted_fully, - librarian_sion_no_email, - loc_public_sion, loc_restricted_sion -): +def test_filtered_locations_get(client, librarian_martigny_no_email, + librarian_sion_no_email, locations): """Test location filter by organisation.""" # Martigny login_user_via_session(client, librarian_martigny_no_email.user) @@ -189,7 +243,7 @@ def test_filtered_locations_get( res = client.get(list_url) assert res.status_code == 200 data = get_json(res) - assert data['hits']['total'] == 6 + assert data['hits']['total'] == 9 # Sion login_user_via_session(client, librarian_sion_no_email.user) @@ -198,7 +252,7 @@ def test_filtered_locations_get( res = client.get(list_url) assert res.status_code == 200 data = get_json(res) - assert data['hits']['total'] == 2 + assert data['hits']['total'] == 4 def test_location_secure_api(client, json_header, loc_public_martigny, diff --git a/tests/api/test_patrons_blocked.py b/tests/api/test_patrons_blocked.py index 76e17654c3..03754c5c1d 100644 --- a/tests/api/test_patrons_blocked.py +++ b/tests/api/test_patrons_blocked.py @@ -21,6 +21,9 @@ from invenio_accounts.testutils import login_user_via_session from utils import get_json +from rero_ils.modules.loans.api import Loan +from rero_ils.modules.loans.utils import can_be_requested + def test_blocked_field_exists( client, @@ -84,7 +87,7 @@ def test_blocked_patron_cannot_request(client, ) assert res.status_code == 200 data = get_json(res) - assert data.get('can_request', {}) is False + assert not data['can'] # Check with valid patron res = client.get( @@ -97,4 +100,12 @@ def test_blocked_patron_cannot_request(client, ) assert res.status_code == 200 data = get_json(res) - assert data.get('can_request', {}) is True + assert data['can'] + + # Create "virtual" Loan (not registered) + loan = Loan({ + 'item_pid': item_lib_martigny.pid, + 'library_pid': lib_martigny.pid, + 'patron_pid': patron3_martigny_no_email.pid + }) + assert not can_be_requested(loan) diff --git a/tests/api/test_patrons_rest.py b/tests/api/test_patrons_rest.py index c1c0af6a00..5da7b6a855 100644 --- a/tests/api/test_patrons_rest.py +++ b/tests/api/test_patrons_rest.py @@ -43,6 +43,7 @@ def test_patrons_shortcuts( del new_patron['patron_type'] assert not new_patron.patron_type_pid assert not new_patron.organisation_pid + assert new_patron.formatted_name == "Roduit, Louis" def test_filtered_patrons_get( diff --git a/tests/data/data.json b/tests/data/data.json index c070a54dc4..28b99bad71 100644 --- a/tests/data/data.json +++ b/tests/data/data.json @@ -533,7 +533,8 @@ "$ref": "https://ils.rero.ch/api/libraries/lib1" }, "is_pickup": true, - "pickup_name": "MARTIGNY-PUBLIC: Public Space" + "pickup_name": "MARTIGNY-PUBLIC: Public Space", + "allow_request": true }, "loc2": { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -542,7 +543,8 @@ "pid": "loc2", "library": { "$ref": "https://ils.rero.ch/api/libraries/lib1" - } + }, + "allow_request": true }, "loc3": { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -553,7 +555,8 @@ "$ref": "https://ils.rero.ch/api/libraries/lib2" }, "is_pickup": true, - "pickup_name": "SAXON-PUBLIC: Public Space" + "pickup_name": "SAXON-PUBLIC: Public Space", + "allow_request": true }, "loc4": { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -562,7 +565,8 @@ "pid": "loc4", "library": { "$ref": "https://ils.rero.ch/api/libraries/lib2" - } + }, + "allow_request": true }, "loc5": { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -573,7 +577,8 @@ "$ref": "https://ils.rero.ch/api/libraries/lib3" }, "is_pickup": true, - "pickup_name": "FULLY-PUBLIC: Public Space" + "pickup_name": "FULLY-PUBLIC: Public Space", + "allow_request": true }, "loc6": { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -582,7 +587,8 @@ "pid": "loc6", "library": { "$ref": "https://ils.rero.ch/api/libraries/lib3" - } + }, + "allow_request": true }, "loc7": { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -593,7 +599,8 @@ "$ref": "https://ils.rero.ch/api/libraries/lib4" }, "is_pickup": true, - "pickup_name": "SION-PUBLIC: Public Space" + "pickup_name": "SION-PUBLIC: Public Space", + "allow_request": true }, "loc8": { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -602,7 +609,8 @@ "pid": "loc8", "library": { "$ref": "https://ils.rero.ch/api/libraries/lib4" - } + }, + "allow_request": true }, "loc9": { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -612,7 +620,8 @@ "library": { "$ref": "https://ils.rero.ch/api/libraries/lib1" }, - "is_online": true + "is_online": true, + "allow_request": true }, "loc10": { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -622,7 +631,8 @@ "library": { "$ref": "https://ils.rero.ch/api/libraries/lib2" }, - "is_online": true + "is_online": true, + "allow_request": true }, "loc11": { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -632,7 +642,8 @@ "library": { "$ref": "https://ils.rero.ch/api/libraries/lib3" }, - "is_online": true + "is_online": true, + "allow_request": true }, "loc12": { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -642,7 +653,8 @@ "library": { "$ref": "https://ils.rero.ch/api/libraries/lib4" }, - "is_online": true + "is_online": true, + "allow_request": true }, "loc13": { "$schema": "https://ils.rero.ch/schema/locations/location-v0.0.1.json", @@ -652,7 +664,8 @@ "library": { "$ref": "https://ils.rero.ch/api/libraries/lib5" }, - "is_online": true + "is_online": true, + "allow_request": true }, "itty1": { "$schema": "https://ils.rero.ch/schema/item_types/item_type-v0.0.1.json", @@ -2092,7 +2105,8 @@ "roles": [ "system_librarian", "librarian" - ] + ], + "barcode": "sys_ptrn1" }, "ptrn2": { "$schema": "https://ils.rero.ch/schema/patrons/patron-v0.0.1.json", diff --git a/tests/fixtures/organisations.py b/tests/fixtures/organisations.py index 620cde1f28..7bf1ae0788 100644 --- a/tests/fixtures/organisations.py +++ b/tests/fixtures/organisations.py @@ -247,6 +247,24 @@ def loc_online_aproz_data(data): return deepcopy(data.get('loc13')) +@pytest.fixture(scope="module") +def locations(loc_public_martigny, loc_restricted_martigny, + loc_public_saxon, loc_restricted_saxon, + loc_public_fully, loc_restricted_fully, + loc_public_sion, loc_restricted_sion, + loc_online_martigny, loc_online_saxon, + loc_online_fully, loc_online_sion, loc_online_aproz): + """Create all locations.""" + return [ + loc_public_martigny, loc_restricted_martigny, + loc_public_saxon, loc_restricted_saxon, + loc_public_fully, loc_restricted_fully, + loc_public_sion, loc_restricted_sion, + loc_online_martigny, loc_online_saxon, + loc_online_fully, loc_online_sion, loc_online_aproz + ] + + @pytest.fixture(scope="module") def loc_public_martigny(app, lib_martigny, loc_public_martigny_data): """Create public space location for Martigny ville.""" diff --git a/tests/ui/notifications/test_notifications_api.py b/tests/ui/notifications/test_notifications_api.py index 9e64e21d20..843f876153 100644 --- a/tests/ui/notifications/test_notifications_api.py +++ b/tests/ui/notifications/test_notifications_api.py @@ -21,7 +21,6 @@ from copy import deepcopy -import pytest from utils import get_mapping from rero_ils.modules.notifications.api import Notification, \ @@ -129,6 +128,4 @@ def update_process_date(self): Dispatcher().dispatch_notification(notification=notification, verbose=True) notification = DummyNotification('XXXX') - with pytest.raises(ValueError): - Dispatcher().dispatch_notification(notification=notification, - verbose=True) + Dispatcher().dispatch_notification(notification=notification, verbose=True) diff --git a/tests/ui/patrons/test_patrons_ui.py b/tests/ui/patrons/test_patrons_ui.py index 9fad4c741d..22eacf179f 100644 --- a/tests/ui/patrons/test_patrons_ui.py +++ b/tests/ui/patrons/test_patrons_ui.py @@ -110,6 +110,12 @@ def test_patrons_logged_user(client, librarian_martigny_no_email): assert not data.get('metadata') assert data.get('settings').get('language') + login_user_via_session(client, librarian_martigny_no_email.user) + res = client.get(url_for('patrons.logged_user', resolve=1)) + assert res.status_code == 200 + data = get_json(res) + assert 'organisation' in data['metadata']['library'] + class current_i18n: class locale: language = 'fr'