From 56da678ab78d1fb89bd8f03bdbb361c1982f06a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20Marie=CC=81thoz?= Date: Tue, 22 Aug 2023 09:44:00 +0200 Subject: [PATCH 1/4] circulation: optimize the response time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Removes useless informations. * Optimizes the computation of the additional informations. * Create dumper classes for items and loans. Co-Authored-by: Johnny Mariéthoz --- rero_ils/modules/items/api/circulation.py | 31 +-------- rero_ils/modules/items/dumpers.py | 34 ++++++++++ rero_ils/modules/items/views/api_views.py | 14 ++-- rero_ils/modules/loans/api.py | 38 ----------- rero_ils/modules/loans/dumpers.py | 81 +++++++++++++++++++++++ tests/ui/items/test_items_dumpers.py | 38 ++++++++++- tests/ui/loans/test_loans_dumpers.py | 32 +++++++++ 7 files changed, 194 insertions(+), 74 deletions(-) create mode 100644 rero_ils/modules/loans/dumpers.py create mode 100644 tests/ui/loans/test_loans_dumpers.py diff --git a/rero_ils/modules/items/api/circulation.py b/rero_ils/modules/items/api/circulation.py index 9a367c38c7..4ae7deeb58 100644 --- a/rero_ils/modules/items/api/circulation.py +++ b/rero_ils/modules/items/api/circulation.py @@ -42,7 +42,6 @@ from ..models import ItemCirculationAction, ItemIssueStatus, ItemStatus from ..utils import item_pid_to_object from ...circ_policies.api import CircPolicy -from ...documents.api import Document from ...errors import NoCirculationAction from ...item_types.api import ItemType from ...libraries.api import Library @@ -853,32 +852,6 @@ def prior_checkout_actions(self, action_params): break return action_params, actions - def dumps_for_circulation(self, sort_by=None): - """Enhance item information for api_views.""" - item = self.replace_refs() - data = item.dumps() - - document = Document.get_record_by_pid(item['document']['pid']) - doc_data = document.dumps() - data['document']['title'] = doc_data['title'] - - location = Location.get_record_by_pid(item['location']['pid']) - loc_data = location.dumps() - data['location']['name'] = loc_data['name'] - organisation = self.get_organisation() - data['location']['organisation'] = { - 'pid': organisation.get('pid'), - 'name': organisation.get('name') - } - data['actions'] = list(self.actions) - data['available'] = self.available - - # data['number_of_requests'] = self.number_of_requests() - for loan in self.get_requests(sort_by=sort_by): - data.setdefault('pending_loans', - []).append(loan.dumps_for_circulation()) - return data - @classmethod def get_loans_by_item_pid(cls, item_pid): """Return any loan loans for item.""" @@ -1314,14 +1287,14 @@ def available(self): """ if self.get('_masked', False): return False - if self.item_has_active_loan_or_request() > 0: - return False if self.circulation_category.get('negative_availability'): return False if self.temp_item_type_negative_availability: return False if self.is_issue and self.issue_status != ItemIssueStatus.RECEIVED: return False + if self.item_has_active_loan_or_request() > 0: + return False return True @property diff --git a/rero_ils/modules/items/dumpers.py b/rero_ils/modules/items/dumpers.py index a22b472b39..4244b86b4c 100644 --- a/rero_ils/modules/items/dumpers.py +++ b/rero_ils/modules/items/dumpers.py @@ -17,16 +17,21 @@ # along with this program. If not, see . """Items dumpers.""" +from copy import deepcopy from invenio_records.dumpers import Dumper as InvenioRecordsDumper from rero_ils.modules.commons.exceptions import MissingDataException +from rero_ils.modules.documents.api import Document from rero_ils.modules.documents.dumpers import \ TitleDumper as DocumentTitleDumper from rero_ils.modules.holdings.api import Holding from rero_ils.modules.holdings.dumpers import ClaimIssueHoldingDumper from rero_ils.modules.libraries.dumpers import \ LibrarySerialClaimNotificationDumper +from rero_ils.modules.loans.dumpers import \ + CirculationDumper as LoanCirculationDumper +from rero_ils.modules.locations.api import Location from rero_ils.modules.vendors.dumpers import VendorClaimIssueNotificationDumper @@ -103,3 +108,32 @@ def dump(self, record, data): 'claim_counter': record.claims_count }) return {k: v for k, v in data.items() if v is not None} + + +class CirculationActionDumper(InvenioRecordsDumper): + """Item issue dumper for circulation actions.""" + + def dump(self, record, data): + """Dump an item for circulation actions.""" + item = record.replace_refs() + data = deepcopy(dict(item)) + document = Document.get_record_by_pid(item['document']['pid']) + doc_data = document.dumps() + data['document']['title'] = doc_data['title'] + + location = Location.get_record_by_pid(item['location']['pid']) + loc_data = deepcopy(dict(location)) + data['location']['name'] = loc_data['name'] + # TODO: check if it is required + data['location']['organisation'] = { + 'pid': record.organisation_pid + } + data['actions'] = list(record.actions) + + # only the first request is used by the UI + requests = record.get_requests(sort_by='_created') + if first_request := next(requests, None): + data['pending_loans'] = [ + first_request.dumps(LoanCirculationDumper()) + ] + return data diff --git a/rero_ils/modules/items/views/api_views.py b/rero_ils/modules/items/views/api_views.py index 81cf2f4f2a..38856bb69e 100644 --- a/rero_ils/modules/items/views/api_views.py +++ b/rero_ils/modules/items/views/api_views.py @@ -41,6 +41,8 @@ NoCirculationActionIsPermitted from rero_ils.modules.libraries.api import Library from rero_ils.modules.loans.api import Loan +from rero_ils.modules.loans.dumpers import \ + CirculationDumper as LoanCirculationDumper from rero_ils.modules.operation_logs.api import OperationLogsSearch from rero_ils.modules.operation_logs.permissions import \ search_action as op_log_search_action @@ -48,7 +50,7 @@ from rero_ils.permissions import request_item_permission from ..api import Item -from ..dumpers import ClaimIssueNotificationDumper +from ..dumpers import CirculationActionDumper, ClaimIssueNotificationDumper from ..models import ItemCirculationAction from ..permissions import late_issue_management as late_issue_management_action from ..utils import get_recipient_suggestions, item_pid_to_object @@ -144,10 +146,11 @@ def decorated_view(*args, **kwargs): func(item, data, *args, **kwargs) for action, loan in action_applied.items(): if loan: - action_applied[action] = loan.dumps_for_circulation() + action_applied[action] = loan.dumps( + LoanCirculationDumper()) return jsonify({ - 'metadata': item_data.dumps_for_circulation(), + 'metadata': item_data.dumps(CirculationActionDumper()), 'action_applied': action_applied }) except NoCirculationAction as error: @@ -422,8 +425,9 @@ def item(item_barcode): abort(404) loan = get_loan_for_item(item_pid_to_object(item.pid)) if loan: - loan = Loan.get_record_by_pid(loan.get('pid')).dumps_for_circulation() - item_dumps = item.dumps_for_circulation() + loan = Loan.get_record_by_pid( + loan.get('pid')).dumps(LoanCirculationDumper()) + item_dumps = item.dumps(CirculationActionDumper()) if patron_pid := flask_request.args.get('patron_pid'): patron = Patron.get_record_by_pid(patron_pid) diff --git a/rero_ils/modules/loans/api.py b/rero_ils/modules/loans/api.py index 70e0ceed47..27bf8b909b 100644 --- a/rero_ils/modules/loans/api.py +++ b/rero_ils/modules/loans/api.py @@ -724,44 +724,6 @@ def get_overdue_fees(self): break return fees - def dumps_for_circulation(self): - """Dumps for circulation.""" - loan = self.replace_refs() - data = loan.dumps() - data['creation_date'] = self.created - patron = Patron.get_record_by_pid(loan['patron_pid']) - - # Add patron informations - ptrn_data = patron.dumps() - data['patron'] = {} - data['patron']['barcode'] = ptrn_data['patron']['barcode'] - data['patron']['name'] = ', '.join(( - ptrn_data['last_name'], ptrn_data['first_name'])) - if loan.get('pickup_location_pid'): - location = Location.get_record_by_pid(loan['pickup_location_pid']) - data['pickup_location'] = { - 'name': location.get('name'), - 'library_name': location.get_library().get('name') - } - - # Always add item destination readable information if item state is - # 'in transit' ; much easier to know these informations for UI ! - item = self.item - data['rank'] = item.patron_request_rank(patron) - if item.status == ItemStatus.IN_TRANSIT: - destination_loc_pid = item.location_pid - if loan.get('state') == LoanState.ITEM_IN_TRANSIT_FOR_PICKUP: - destination_loc_pid = self.get('pickup_location_pid') - destination_loc = Location.get_record_by_pid(destination_loc_pid) - destination_lib = destination_loc.get_library() - data['item_destination'] = { - 'location_name': destination_loc.get('name'), - 'location_code': destination_loc.get('code'), - 'library_name': destination_lib.get('name'), - 'library_code': destination_lib.get('code') - } - return data - def is_notified(self, notification_type=None, counter=0): """Check if a notification already exists for a loan by type.""" trans_date = ciso8601.parse_datetime(self.get('transaction_date')) diff --git a/rero_ils/modules/loans/dumpers.py b/rero_ils/modules/loans/dumpers.py new file mode 100644 index 0000000000..7e85e6056a --- /dev/null +++ b/rero_ils/modules/loans/dumpers.py @@ -0,0 +1,81 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2019-2023 RERO +# Copyright (C) 2019-2023 UCLouvain +# +# 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 . + +"""Loans dumpers.""" + +from copy import deepcopy + +from invenio_records.dumpers import Dumper as InvenioRecordsDumper + +from rero_ils.modules.items.models import ItemStatus +from rero_ils.modules.locations.api import Location +from rero_ils.modules.patrons.api import PatronsSearch + +from .models import LoanState + + +class CirculationDumper(InvenioRecordsDumper): + """Item issue dumper for circulation actions.""" + + def dump(self, record, data): + """Dump an loan for circulation.""" + data = deepcopy(dict(record)) + # used only for pending + data['creation_date'] = record.created + + ptrn_query = PatronsSearch()\ + .source(['patron', 'first_name', 'last_name'])\ + .filter('term', pid=record['patron_pid']) + if ptrn_data := next(ptrn_query.scan(), None): + data['patron'] = {} + data['patron']['barcode'] = ptrn_data.patron.barcode.pop() + data['patron']['name'] = ', '.join(( + ptrn_data.last_name, ptrn_data.first_name)) + + # only for pending requests + if record.get('pickup_location_pid') \ + and record.get('state') == LoanState.PENDING: + location = Location.get_record_by_pid( + record.get('pickup_location_pid')) + data['pickup_location'] = { + 'name': location.get('name'), + 'library_name': location.get_library().get('name') + } + + # Always add item destination readable information if item state is + # 'in transit' ; much easier to know these informations for UI ! + item = record.item + if item.status == ItemStatus.IN_TRANSIT: + destination_loc_pid = item.location_pid + if record.get('state') == LoanState.ITEM_IN_TRANSIT_FOR_PICKUP: + destination_loc_pid = record.get('pickup_location_pid') + # can be already computed + if library_name := data.get( + 'pickup_location', {}).get('library_name'): + data['item_destination'] = { + 'library_name': library_name + } + # do nothing is already done + if not data.get('item_destination'): + destination_loc = Location.get_record_by_pid( + destination_loc_pid) + destination_lib = destination_loc.get_library() + data['item_destination'] = { + 'library_name': destination_lib.get('name') + } + return data diff --git a/tests/ui/items/test_items_dumpers.py b/tests/ui/items/test_items_dumpers.py index ec2a1905c0..271c128aa7 100644 --- a/tests/ui/items/test_items_dumpers.py +++ b/tests/ui/items/test_items_dumpers.py @@ -20,15 +20,49 @@ from copy import deepcopy import pytest +from utils import item_record_to_a_specific_loan_state from rero_ils.modules.commons.exceptions import MissingDataException from rero_ils.modules.holdings.api import Holding -from rero_ils.modules.items.dumpers import ClaimIssueNotificationDumper, \ - ItemCirculationDumper +from rero_ils.modules.items.dumpers import CirculationActionDumper, \ + ClaimIssueNotificationDumper, ItemCirculationDumper from rero_ils.modules.items.models import TypeOfItem +from rero_ils.modules.loans.models import LoanState from rero_ils.modules.utils import get_ref_for_pid +def test_item_action_circulation_dumper( + item_lib_martigny, patron_martigny, loc_public_martigny, + librarian_martigny, circulation_policies): + """Test item circulation action dumper.""" + params = { + 'patron_pid': patron_martigny.pid, + 'transaction_location_pid': loc_public_martigny.pid, + 'transaction_user_pid': librarian_martigny.pid, + 'pickup_location_pid': loc_public_martigny.pid, + } + item, _ = item_record_to_a_specific_loan_state( + item=item_lib_martigny, loan_state=LoanState.PENDING, + params=params, copy_item=True) + data = item.dumps(CirculationActionDumper()) + # $ref resolution + assert data['library']['pid'] + + # document title + assert data['document']['title'] + + # location name + assert data['location']['name'] + + # organisation pid + assert data['location']['organisation']['pid'] + # actions + assert data['actions'] + + # pending loans + assert len(data['pending_loans']) == 1 + + def test_item_circulation_dumper(item_lib_martigny): """Test item circulation dumper.""" item = item_lib_martigny diff --git a/tests/ui/loans/test_loans_dumpers.py b/tests/ui/loans/test_loans_dumpers.py new file mode 100644 index 0000000000..79595cfcd3 --- /dev/null +++ b/tests/ui/loans/test_loans_dumpers.py @@ -0,0 +1,32 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2019-2023 RERO +# Copyright (C) 2019-2023 UCLouvain +# +# 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 . + +"""Loans Record dumper tests.""" + +from rero_ils.modules.loans.dumpers import CirculationDumper + + +def test_loan_circulation_dumper(loan_pending_martigny): + """Test loan circulation action dumper.""" + data = loan_pending_martigny.dumps(CirculationDumper()) + assert data['state'] + assert data['creation_date'] + assert 'name' in data['patron'] + assert 'barcode' in data['patron'] + assert 'name' in data['pickup_location'] + assert 'library_name' in data['pickup_location'] From f9b07c0f24d510015b14701be1cafcfa72c20d08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20Marie=CC=81thoz?= Date: Wed, 23 Aug 2023 11:30:39 +0200 Subject: [PATCH 2/4] circulation: adds availibility API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Adds holding availibility API. * Uniformize documents and items availibility API. * Removes availibity from the documents, holdings and items JSON serializers. Co-Authored-by: Johnny Mariéthoz --- pyproject.toml | 2 +- rero_ils/modules/documents/api_views.py | 52 ++++++++++++++++++ .../modules/documents/serializers/json.py | 2 - rero_ils/modules/documents/views.py | 43 +++++---------- rero_ils/modules/holdings/api_views.py | 10 ++++ rero_ils/modules/holdings/serializers.py | 2 - rero_ils/modules/items/serializers/json.py | 12 ----- rero_ils/modules/items/views/api_views.py | 24 ++++++--- tests/api/holdings/test_holdings_rest.py | 1 - tests/api/items/test_items_serializer.py | 9 ---- tests/api/test_availability.py | 53 ++++++++++++++++--- 11 files changed, 139 insertions(+), 71 deletions(-) create mode 100644 rero_ils/modules/documents/api_views.py diff --git a/pyproject.toml b/pyproject.toml index 71209ca4db..3d2825f57f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -161,7 +161,7 @@ rero-ils = "rero_ils.modules.ext:REROILSAPP" acq_accounts = "rero_ils.modules.acquisition.acq_accounts.views:api_blueprint" acq_orders = "rero_ils.modules.acquisition.acq_orders.views:api_blueprint" acq_receipts = "rero_ils.modules.acquisition.acq_receipts.views:api_blueprint" -api_documents = "rero_ils.modules.documents.views:api_blueprint" +documents = "rero_ils.modules.documents.api_views:api_blueprint" circ_policies = "rero_ils.modules.circ_policies.views:blueprint" local_entities = "rero_ils.modules.entities.local_entities.views:api_blueprint" remote_entities = "rero_ils.modules.entities.remote_entities.views:api_blueprint" diff --git a/rero_ils/modules/documents/api_views.py b/rero_ils/modules/documents/api_views.py new file mode 100644 index 0000000000..d1a6b2eed9 --- /dev/null +++ b/rero_ils/modules/documents/api_views.py @@ -0,0 +1,52 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2019-2023 RERO +# Copyright (C) 2019-2023 UCLouvain +# +# 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 . + +"""Blueprint for document api.""" + +from flask import Blueprint, abort, jsonify +from flask import request as flask_request + +from .api import Document +from .utils import get_remote_cover +from ..utils import cached + +api_blueprint = Blueprint( + 'api_documents', + __name__, + url_prefix='/document' +) + + +@api_blueprint.route('/cover/') +@cached(timeout=300, query_string=True) +def cover(isbn): + """Document cover service.""" + return jsonify(get_remote_cover(isbn)) + + +@api_blueprint.route('//availability', methods=['GET']) +def document_availability(pid): + """HTTP GET request for document availability.""" + if not Document.record_pid_exists(pid): + abort(404) + view_code = flask_request.args.get('view_code') + if not view_code: + view_code = 'global' + return jsonify({ + 'available': Document.is_available(pid, view_code) + }) diff --git a/rero_ils/modules/documents/serializers/json.py b/rero_ils/modules/documents/serializers/json.py index eec4848149..c25d06bb4c 100644 --- a/rero_ils/modules/documents/serializers/json.py +++ b/rero_ils/modules/documents/serializers/json.py @@ -21,7 +21,6 @@ from flask import current_app, json, request, stream_with_context from werkzeug.local import LocalProxy -from rero_ils.modules.documents.api import Document from rero_ils.modules.documents.utils import process_i18n_literal_fields from rero_ils.modules.documents.views import create_title_alternate_graphic, \ create_title_responsibilites, create_title_variants @@ -80,7 +79,6 @@ def _postprocess_search_hit(self, hit: dict) -> None: metadata = hit.get('metadata', {}) pid = metadata.get('pid') - metadata['available'] = Document.is_available(pid, view_code) titles = metadata.get('title', []) if text_title := TitleExtension.format_text( titles, with_subtitle=False diff --git a/rero_ils/modules/documents/views.py b/rero_ils/modules/documents/views.py index 2c3bf8c3c0..647f7dc3ed 100644 --- a/rero_ils/modules/documents/views.py +++ b/rero_ils/modules/documents/views.py @@ -24,9 +24,9 @@ import click from elasticsearch_dsl.query import Q -from flask import Blueprint, abort, current_app, jsonify, render_template -from flask import request as flask_request +from flask import Blueprint, current_app, render_template from flask import url_for +from flask import Blueprint, current_app, render_template from flask_babelex import gettext as _ from flask_login import current_user from invenio_records_ui.signals import record_viewed @@ -49,6 +49,16 @@ from .utils import display_alternate_graphic_first, get_remote_cover, \ title_format_text, title_format_text_alternate_graphic, \ title_variant_format_text +from ..collections.api import CollectionsSearch +from ..entities.api import Entity +from ..entities.models import EntityType +from ..holdings.models import HoldingNoteTypes +from ..items.models import ItemCirculationAction +from ..libraries.api import Library +from ..locations.api import Location +from ..organisations.api import Organisation +from ..patrons.api import current_patrons +from ..utils import extracted_data_from_ref def doc_item_view_method(pid, record, template=None, **kwargs): @@ -69,8 +79,6 @@ def doc_item_view_method(pid, record, template=None, **kwargs): if viewcode != current_app.config.get('RERO_ILS_SEARCH_GLOBAL_VIEW_CODE'): organisation = Organisation.get_record_by_viewcode(viewcode) - record['available'] = Document.is_available(record.pid, viewcode) - # build provision activity ProvisionActivitiesExtension().post_dump(record={}, data=record) @@ -104,19 +112,6 @@ def doc_item_view_method(pid, record, template=None, **kwargs): ) -api_blueprint = Blueprint( - 'api_documents', - __name__ -) - - -@api_blueprint.route('/cover/') -@cached(timeout=300, query_string=True) -def cover(isbn): - """Document cover service.""" - return jsonify(get_remote_cover(isbn)) - - blueprint = Blueprint( 'documents', __name__, @@ -486,20 +481,6 @@ def work_access_point(work_access_point): return wap -@api_blueprint.route('/availabilty/', methods=['GET']) -def document_availability(document_pid): - """HTTP GET request for document availability.""" - view_code = flask_request.args.get('view_code') - if not view_code: - view_code = 'global' - document = Document.get_record_by_pid(document_pid) - if not document: - abort(404) - return jsonify({ - 'availability': Document.is_available(document_pid, view_code) - }) - - @blueprint.app_template_filter() def create_publication_statement(provision_activity): """Create publication statement from place, agent and date values.""" diff --git a/rero_ils/modules/holdings/api_views.py b/rero_ils/modules/holdings/api_views.py index 1f694b46fe..7a638f498c 100644 --- a/rero_ils/modules/holdings/api_views.py +++ b/rero_ils/modules/holdings/api_views.py @@ -308,3 +308,13 @@ def get_pickup_locations(holding_pid): return jsonify({ 'locations': locations }) + + +@api_blueprint.route('//availability', methods=['GET']) +def holding_availability(pid): + """HTTP GET request for holding availability.""" + if holding := Holding.get_record_by_pid(pid): + return jsonify({ + 'available': holding.available + }) + abort(404) diff --git a/rero_ils/modules/holdings/serializers.py b/rero_ils/modules/holdings/serializers.py index 4e4d452847..5e6c27eb13 100644 --- a/rero_ils/modules/holdings/serializers.py +++ b/rero_ils/modules/holdings/serializers.py @@ -40,8 +40,6 @@ def _postprocess_search_hit(self, hit: dict) -> None: """ metadata = hit.get('metadata', {}) record = Holding.get_record_by_pid(metadata.get('pid')) - # available - metadata['available'] = record.available # Circulation category circ_category_pid = metadata['circulation_category']['pid'] circ_category = self.get_resource(ItemType, circ_category_pid) diff --git a/rero_ils/modules/items/serializers/json.py b/rero_ils/modules/items/serializers/json.py index 67b7f0af61..b752f97ecc 100644 --- a/rero_ils/modules/items/serializers/json.py +++ b/rero_ils/modules/items/serializers/json.py @@ -22,7 +22,6 @@ from rero_ils.modules.documents.extensions import TitleExtension from rero_ils.modules.item_types.api import ItemTypesSearch from rero_ils.modules.items.api import Item -from rero_ils.modules.items.models import ItemStatus from rero_ils.modules.libraries.api import LibrariesSearch from rero_ils.modules.locations.api import LocationsSearch from rero_ils.modules.organisations.api import OrganisationsSearch @@ -53,17 +52,6 @@ def _set_item_type_circulation_information(metadata, pid): document['title'], with_subtitle=True) item = self.get_resource(Item, metadata.get('pid')) - metadata['available'] = item.available - metadata['availability'] = { - 'available': metadata['available'], - 'status': metadata['status'], - 'display_text': item.availability_text, - 'request': item.number_of_requests() - } - if not metadata['available'] \ - and metadata['status'] == ItemStatus.ON_LOAN: - metadata['availability']['due_date'] = \ - item.get_item_end_date(format=None) # Item in collection if collection := item.in_collection(): diff --git a/rero_ils/modules/items/views/api_views.py b/rero_ils/modules/items/views/api_views.py index 38856bb69e..5fb3717a4a 100644 --- a/rero_ils/modules/items/views/api_views.py +++ b/rero_ils/modules/items/views/api_views.py @@ -51,7 +51,7 @@ from ..api import Item from ..dumpers import CirculationActionDumper, ClaimIssueNotificationDumper -from ..models import ItemCirculationAction +from ..models import ItemCirculationAction, ItemStatus from ..permissions import late_issue_management as late_issue_management_action from ..utils import get_recipient_suggestions, item_pid_to_object from ...commons.exceptions import MissingDataException @@ -61,6 +61,7 @@ __name__, url_prefix='/item' ) + blueprint = Blueprint( 'items', __name__ @@ -464,15 +465,24 @@ def item(item_barcode): }) -@api_blueprint.route('//availability', methods=['GET']) -@check_authentication +@api_blueprint.route('//availability', methods=['GET']) @jsonify_error -def item_availability(item_pid): +def item_availability(pid): """HTTP GET request for item availability.""" - record = Item.get_record_by_pid(item_pid) - if not record: + item = Item.get_record_by_pid(pid) + if not item: abort(404) - return jsonify({'availability': record.available}) + data = dict(available=item.available) + if flask_request.args.get('more_info'): + extra = { + 'status': item['status'], + 'circulation_message': item.availability_text, + 'number_of_request': item.number_of_requests() + } + if not data['available'] and extra['status'] == ItemStatus.ON_LOAN: + extra['due_date'] = item.get_item_end_date(format=None) + data |= extra + return jsonify(data) @api_blueprint.route('//can_request', methods=['GET']) diff --git a/tests/api/holdings/test_holdings_rest.py b/tests/api/holdings/test_holdings_rest.py index c262a443d6..0010ef5ce9 100644 --- a/tests/api/holdings/test_holdings_rest.py +++ b/tests/api/holdings/test_holdings_rest.py @@ -91,7 +91,6 @@ def test_holdings_get( assert res.status_code == 200 data = get_json(res) hit = data['hits']['hits'][0]['metadata'] - assert hit['available'] def test_filtered_holdings_get( diff --git a/tests/api/items/test_items_serializer.py b/tests/api/items/test_items_serializer.py index 77710c27db..fab7f8de89 100644 --- a/tests/api/items/test_items_serializer.py +++ b/tests/api/items/test_items_serializer.py @@ -17,8 +17,6 @@ """Tests REST items Serializer.""" -from datetime import datetime - from flask import url_for from utils import get_csv, login_user @@ -75,13 +73,6 @@ def test_serializers( list_url = url_for('invenio_records_rest.item_list') response = client.get(list_url, headers=rero_json_header) data = response.json['hits']['hits'] - # check the due date: it should be an ISO date - item_with_due_date = [ - res['metadata'] for res in data - if res['metadata'].get('availability', {}).get('due_date') - ][0] - due_date = item_with_due_date['availability']['due_date'] - assert datetime.fromisoformat(due_date).year is not None assert response.status_code == 200 list_url = url_for('api_item.inventory_search') diff --git a/tests/api/test_availability.py b/tests/api/test_availability.py index 449b233d7a..e859124a5d 100644 --- a/tests/api/test_availability.py +++ b/tests/api/test_availability.py @@ -384,12 +384,12 @@ def item_availablity_status(client, pid, user): res = client.get( url_for( 'api_item.item_availability', - item_pid=pid, + pid=pid, ) ) assert res.status_code == 200 data = get_json(res) - return data.get('availability') + return data.get('available') def document_availability_status(client, pid, user): @@ -398,13 +398,13 @@ def document_availability_status(client, pid, user): res = client.get( url_for( 'api_documents.document_availability', - document_pid=pid, + pid=pid, view_code='global' ) ) assert res.status_code == 200 data = get_json(res) - return data.get('availability') + return data.get('available') def test_availability_cipo_allow_request( @@ -442,7 +442,7 @@ def test_document_availability_failed(client, librarian2_martigny): res = client.get( url_for( 'api_documents.document_availability', - document_pid='dummy_pid' + pid='dummy_pid' ) ) assert res.status_code == 404 @@ -454,7 +454,48 @@ def test_item_availability_failed(client, librarian2_martigny): res = client.get( url_for( 'api_item.item_availability', - item_pid='dummy_pid' + pid='dummy_pid' ) ) assert res.status_code == 404 + + +def test_item_availability_extra(client, item_lib_sion): + """Test item availability with an extra parameters.""" + res = client.get( + url_for( + 'api_item.item_availability', + pid=item_lib_sion.pid + ) + ) + assert list(res.json.keys()) == ['available'] + + res = client.get( + url_for( + 'api_item.item_availability', + pid=item_lib_sion.pid, + more_info=1 + ) + ) + assert list(res.json.keys()) == \ + ['available', 'circulation_message', 'number_of_request', 'status'] + + +def test_holding_availability(client, holding_lib_martigny): + """Test holding availability endpoint.""" + res = client.get( + url_for( + 'api_holding.holding_availability', + pid='dummy_pid' + ) + ) + assert res.status_code == 404 + + res = client.get( + url_for( + 'api_holding.holding_availability', + pid=holding_lib_martigny.pid + ) + ) + assert res.status_code == 200 + assert 'available' in res.json From 4ccdc6042368b4f92927a2ef20ec0a86af3050de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20Marie=CC=81thoz?= Date: Thu, 24 Aug 2023 11:59:55 +0200 Subject: [PATCH 3/4] document: add text for the partOf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Adds a text version of the partOf document as done in the list of results. Co-Authored-by: Johnny Mariéthoz --- rero_ils/modules/documents/serializers/json.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/rero_ils/modules/documents/serializers/json.py b/rero_ils/modules/documents/serializers/json.py index c25d06bb4c..db7fa1592f 100644 --- a/rero_ils/modules/documents/serializers/json.py +++ b/rero_ils/modules/documents/serializers/json.py @@ -30,6 +30,7 @@ from rero_ils.modules.serializers import JSONSerializer from ..dumpers import document_replace_refs_dumper +from ..dumpers.indexer import IndexerDumper from ..extensions import TitleExtension GLOBAL_VIEW_CODE = LocalProxy(lambda: current_app.config.get( @@ -53,6 +54,7 @@ def preprocess_record(self, pid, record, links_factory=None, **kwargs): """Prepare a record and persistent identifier for serialization.""" rec = record + # TODO: uses dumpers # build responsibility data for display purpose responsibility_statement = rec.get('responsibilityStatement', []) if responsibilities := create_title_responsibilites( @@ -70,8 +72,18 @@ def preprocess_record(self, pid, record, links_factory=None, **kwargs): if variant_titles := create_title_variants(titles): rec['ui_title_variants'] = variant_titles - return super().preprocess_record( + + data = super().preprocess_record( pid=pid, record=rec, links_factory=links_factory, kwargs=kwargs) + metadata = data['metadata'] + resolve = request.args.get( + 'resolve', + default=False, + type=lambda v: v.lower() in ['true', '1'] + ) + if request and resolve: + IndexerDumper()._process_host_document(None, metadata) + return data def _postprocess_search_hit(self, hit: dict) -> None: """Post-process each hit of a search result.""" From 85c8e210fead9af4e24e05e20420cfc9a6e28d34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20Marie=CC=81thoz?= Date: Wed, 23 Aug 2023 08:42:07 +0200 Subject: [PATCH 4/4] documents: optimize availability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Uses Elasticsearch to compute the documents, items and holdings availability. - Search only one organisation when an organisation is retrived using a view code. - Renames the available property for items and holdings into a `is_available` method. Co-Authored-by: Johnny Mariéthoz --- pyproject.toml | 1 - rero_ils/modules/documents/api.py | 136 +++++++++++++++++++--- rero_ils/modules/documents/views.py | 6 +- rero_ils/modules/holdings/api.py | 77 +++++++++--- rero_ils/modules/holdings/api_views.py | 2 +- rero_ils/modules/items/api/api.py | 33 ++++++ rero_ils/modules/items/api/circulation.py | 29 +++-- rero_ils/modules/items/utils.py | 2 +- rero_ils/modules/items/views/api_views.py | 2 +- rero_ils/modules/loans/api.py | 11 ++ tests/api/items/test_items_rest_views.py | 2 +- tests/api/loans/test_loans_rest.py | 16 +-- tests/api/test_availability.py | 56 +++++---- tests/ui/documents/test_documents_api.py | 15 --- tests/ui/holdings/test_holdings_api.py | 7 +- tests/ui/items/test_items_api.py | 8 +- 16 files changed, 295 insertions(+), 108 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3d2825f57f..1497b6e928 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -165,7 +165,6 @@ documents = "rero_ils.modules.documents.api_views:api_blueprint" circ_policies = "rero_ils.modules.circ_policies.views:blueprint" local_entities = "rero_ils.modules.entities.local_entities.views:api_blueprint" remote_entities = "rero_ils.modules.entities.remote_entities.views:api_blueprint" -documents = "rero_ils.modules.documents.views:api_blueprint" holdings = "rero_ils.modules.holdings.api_views:api_blueprint" imports = "rero_ils.modules.imports.views:api_blueprint" item_types = "rero_ils.modules.item_types.views:blueprint" diff --git a/rero_ils/modules/documents/api.py b/rero_ils/modules/documents/api.py index 96bd65e5b5..052273a9dd 100644 --- a/rero_ils/modules/documents/api.py +++ b/rero_ils/modules/documents/api.py @@ -151,27 +151,125 @@ def _validate(self, **kwargs): return json @classmethod - def is_available(cls, pid, view_code, raise_exception=False): - """Get availability for document.""" - from ..holdings.api import Holding + def get_n_available_holdings(cls, pid, org_pid=None): + """Get the number of available and electronic holdings. + + :param pid: str - the document pid. + :param org_pid: str - the organisation pid. + :returns: int, int - the number of available and electronic holdings. + """ + from rero_ils.modules.holdings.api import HoldingsSearch + + # create the holding search query + holding_query = HoldingsSearch().available_query() + + # filter by the current document + filters = Q('term', document__pid=pid) + + # filter by organisation + if org_pid: + filters &= Q('term', organisation__pid=org_pid) + holding_query = holding_query.filter(filters) + + # get the number of electronic holdings + n_electronic_holdings = holding_query\ + .filter('term', holdings_type='electronic') + + return holding_query.count(), n_electronic_holdings.count() + + @classmethod + def get_available_item_pids(cls, pid, org_pid=None): + """Get the list of the available item pids. + + :param pid: str - the document pid. + :param org_pid: str - the organisation pid. + :returns: [str] - the list of the available item pids. + """ + from rero_ils.modules.items.api import ItemsSearch + + # create the item query + items_query = ItemsSearch().available_query() + + # filter by the current document + filters = Q('term', document__pid=pid) + + # filter by organisation + if org_pid: + filters &= Q('term', organisation__pid=org_pid) + + return [ + hit.pid for hit in items_query.filter(filters).source('pid').scan() + ] + + @classmethod + def get_item_pids_with_active_loan(cls, pid, org_pid=None): + """Get the list of items pids that have active loans. + + :param pid: str - the document pid. + :param org_pid: str - the organisation pid. + :returns: [str] - the list of the item pids having active loans. + """ + from rero_ils.modules.loans.api import LoansSearch + + loan_query = LoansSearch().unavailable_query() + + # filter by the current document + filters = Q('term', document_pid=pid) + + # filter by organisation + if org_pid: + filters &= Q('term', organisation__pid=org_pid) + + loan_query = loan_query.filter(filters) + + return [ + hit.item_pid.value for hit in loan_query.source('item_pid').scan() + ] + + @classmethod + def is_available(cls, pid, view_code=None): + """Get availability for document. + + Note: if the logic has to be changed here please check also for items + and holdings availability. + + :param pid: str - document pid value. + :param view_code: str - the view code. + """ + # get the organisation pid corresponding to the view code + org_pid = None if view_code != current_app.config.get( 'RERO_ILS_SEARCH_GLOBAL_VIEW_CODE'): - view_id = Organisation.get_record_by_viewcode(view_code)['pid'] - holding_pids = Holding.get_holdings_pid_by_document_pid_by_org( - pid, view_id) - else: - holding_pids = Holding.get_holdings_pid_by_document_pid(pid) - for holding_pid in holding_pids: - if holding := Holding.get_record_by_pid(holding_pid): - if holding.available: - return True - else: - msg = f'No holding: {holding_pid} in DB ' \ - f'for document: {pid}' - current_app.logger.error(msg) - if raise_exception: - raise ValueError(msg) - return False + org_pid = Organisation.get_record_by_viewcode(view_code)['pid'] + + # -------------- Holdings -------------------- + # get the number of available and electronic holdings + n_available_holdings, n_electronic_holdings = \ + cls.get_n_available_holdings(pid, org_pid) + + # available if an electronic holding exists + if n_electronic_holdings: + return True + + # unavailable if no holdings exists + if not n_available_holdings: + return False + + # -------------- Items -------------------- + # get the available item pids + available_item_pids = cls.get_available_item_pids(pid, org_pid) + + # unavailable if no items exists + if not available_item_pids: + return False + + # --------------- Loans ------------------- + # get item pids that have active loans + unavailable_item_pids = \ + cls.get_item_pids_with_active_loan(pid, org_pid) + + # available if at least one item don't have active loan + return bool(set(available_item_pids) - set(unavailable_item_pids)) @property def harvested(self): diff --git a/rero_ils/modules/documents/views.py b/rero_ils/modules/documents/views.py index 647f7dc3ed..4860d1a30e 100644 --- a/rero_ils/modules/documents/views.py +++ b/rero_ils/modules/documents/views.py @@ -24,9 +24,7 @@ import click from elasticsearch_dsl.query import Q -from flask import Blueprint, current_app, render_template -from flask import url_for -from flask import Blueprint, current_app, render_template +from flask import Blueprint, current_app, render_template, url_for from flask_babelex import gettext as _ from flask_login import current_user from invenio_records_ui.signals import record_viewed @@ -41,7 +39,7 @@ from rero_ils.modules.locations.api import Location from rero_ils.modules.organisations.api import Organisation from rero_ils.modules.patrons.api import current_patrons -from rero_ils.modules.utils import cached, extracted_data_from_ref +from rero_ils.modules.utils import extracted_data_from_ref from .api import Document, DocumentsSearch from .extensions import EditionStatementExtension, \ diff --git a/rero_ils/modules/holdings/api.py b/rero_ils/modules/holdings/api.py index 8071357d66..ba4ffea9e0 100644 --- a/rero_ils/modules/holdings/api.py +++ b/rero_ils/modules/holdings/api.py @@ -84,6 +84,14 @@ class Meta: default_filter = None + def available_query(self): + """Base query for holding availability. + + :returns: a filtered Elasticsearch query. + """ + # should not masked + return self.exclude('term', _masked=True) + class Holding(IlsRecord): """Holding class.""" @@ -260,14 +268,65 @@ def vendor(self): if self.get('vendor'): return extracted_data_from_ref(self.get('vendor'), data='record') - @property - def available(self): - """Get availability for holding.""" + def get_available_item_pids(self): + """Get the list of the available item pids. + + :returns: [str] - the list of the available item pids. + """ + from rero_ils.modules.items.api import ItemsSearch + items_query = ItemsSearch().available_query() + filters = Q('term', holding__pid=self.pid) + return [ + hit.pid for hit in items_query.filter(filters).source('pid').scan() + ] + + def get_item_pids_with_active_loan(self, item_pids): + """Get the list of items pids that have active loans. + + :param item_pids: [str] - the list of the item pids. + :returns: the list of the item pids having active loans. + """ + from rero_ils.modules.loans.api import LoansSearch + + loan_query = LoansSearch().unavailable_query() + + # the loans corresponding to the given item pids + loan_query = loan_query.filter(Q('terms', item_pid__value=item_pids)) + + return [ + hit.item_pid.value for hit in loan_query.source('item_pid').scan() + ] + + def is_available(self): + """Get availability for the current holding. + + Note: if the logic has to be changed here please check also for items + and documents availability. + """ + # -------------- Holdings -------------------- + # unavailable if masked + if self.get('_masked', False): + return False + + # available if the holding is electronic if self.is_electronic: return True - if self.get('_masked', False): + + # -------------- Items -------------------- + # get available item pids + available_item_pids = self.get_available_item_pids() + + # unavailable if no item exists + if not available_item_pids: return False - return self._exists_available_child() + + # --------------- Loans ------------------- + # get item pids that have active loans + unavailable_item_pids = \ + self.get_item_pids_with_active_loan(available_item_pids) + + # available if at least one item don't have active loan + return bool(set(available_item_pids) - set(unavailable_item_pids)) @property def max_number_of_claims(self): @@ -303,14 +362,6 @@ def get_note(self, note_type): if note.get('type') == note_type] return next(iter(notes), None) - def _exists_available_child(self): - """Check if at least one child of this holding is available.""" - for pid in Item.get_items_pid_by_holding_pid(self.pid): - item = Item.get_record_by_pid(pid) - if item.available: - return True - return False - @property def get_items_count_by_holding_pid(self): """Returns items count from holding pid.""" diff --git a/rero_ils/modules/holdings/api_views.py b/rero_ils/modules/holdings/api_views.py index 7a638f498c..92194bf9fa 100644 --- a/rero_ils/modules/holdings/api_views.py +++ b/rero_ils/modules/holdings/api_views.py @@ -315,6 +315,6 @@ def holding_availability(pid): """HTTP GET request for holding availability.""" if holding := Holding.get_record_by_pid(pid): return jsonify({ - 'available': holding.available + 'available': holding.is_available() }) abort(404) diff --git a/rero_ils/modules/items/api/api.py b/rero_ils/modules/items/api/api.py index 3bf1022487..337a10ebe7 100644 --- a/rero_ils/modules/items/api/api.py +++ b/rero_ils/modules/items/api/api.py @@ -21,12 +21,14 @@ from functools import partial from elasticsearch.exceptions import NotFoundError +from elasticsearch_dsl import Q from invenio_search import current_search_client from rero_ils.modules.api import IlsRecordError, IlsRecordsIndexer, \ IlsRecordsSearch from rero_ils.modules.documents.api import DocumentsSearch from rero_ils.modules.fetchers import id_fetcher +from rero_ils.modules.item_types.api import ItemTypesSearch from rero_ils.modules.minters import id_minter from rero_ils.modules.organisations.api import Organisation from rero_ils.modules.patrons.api import current_librarian @@ -62,6 +64,37 @@ class Meta: default_filter = None + def available_query(self): + """Base elasticsearch query to compute availability. + + :returns: elasticsearch query. + """ + must_not_filters = [ + # should not be masked + Q('term', _masked=True), + # if issue the status should be received + Q('exists', field='issue') & ~Q('term', issue__status='received') + ] + + # negative availability item types + not_available_item_types = [ + hit.pid for hit in ItemTypesSearch() + .source('pid') + .filter('term', negative_availability=True) + .scan() + ] + if not_available_item_types: + # negative availability item type and not temporary item types + has_items_filters = \ + Q('terms', item_type__pid=not_available_item_types) + has_items_filters &= ~Q('exists', field='temporary_item_type') + # temporary item types with negative availability + has_items_filters |= Q( + 'terms', temporary_item_type__pid=not_available_item_types) + # add to the must not filters + must_not_filters.append(has_items_filters) + return self.filter(Q('bool', must_not=must_not_filters)) + class Item(ItemCirculation, ItemIssue): """Item class.""" diff --git a/rero_ils/modules/items/api/circulation.py b/rero_ils/modules/items/api/circulation.py index 4ae7deeb58..9c6e8659cc 100644 --- a/rero_ils/modules/items/api/circulation.py +++ b/rero_ils/modules/items/api/circulation.py @@ -1276,26 +1276,23 @@ def get_loan_states_for_an_item(self): ]).params(preserve_order=True).source(['state']) return list(dict.fromkeys([result.state for result in search.scan()])) - @property - def available(self): + def is_available(self): """Get availability for item. - An item is 'available' if there are no related request/active_loan and - if the related circulation category doesn't specify a negative - availability. - All masked items are considered as unavailable. + Note: if the logic has to be changed here please check also for + documents and holdings availability. """ - if self.get('_masked', False): - return False - if self.circulation_category.get('negative_availability'): - return False - if self.temp_item_type_negative_availability: - return False - if self.is_issue and self.issue_status != ItemIssueStatus.RECEIVED: - return False - if self.item_has_active_loan_or_request() > 0: + from ..api import ItemsSearch + + items_query = ItemsSearch().available_query() + + # check item availability + if not items_query.filter('term', pid=self.pid).count(): return False - return True + + # --------------- Loans ------------------- + # unavailable if the current item has active loans + return not self.item_has_active_loan_or_request() @property def availability_text(self): diff --git a/rero_ils/modules/items/utils.py b/rero_ils/modules/items/utils.py index 0b3057588b..2d6474d8b6 100644 --- a/rero_ils/modules/items/utils.py +++ b/rero_ils/modules/items/utils.py @@ -89,7 +89,7 @@ def exists_available_item(items=None): item = Item.get_record_by_pid(item) if not isinstance(item, Item): raise ValueError('All items should be Item resource.') - if item.available: + if item.is_available(): return True return False diff --git a/rero_ils/modules/items/views/api_views.py b/rero_ils/modules/items/views/api_views.py index 5fb3717a4a..b8409172b2 100644 --- a/rero_ils/modules/items/views/api_views.py +++ b/rero_ils/modules/items/views/api_views.py @@ -472,7 +472,7 @@ def item_availability(pid): item = Item.get_record_by_pid(pid) if not item: abort(404) - data = dict(available=item.available) + data = dict(available=item.is_available()) if flask_request.args.get('more_info'): extra = { 'status': item['status'], diff --git a/rero_ils/modules/loans/api.py b/rero_ils/modules/loans/api.py index 27bf8b909b..ea9b543c4a 100644 --- a/rero_ils/modules/loans/api.py +++ b/rero_ils/modules/loans/api.py @@ -72,6 +72,17 @@ class Meta: default_filter = None + def unavailable_query(self): + """Base query to compute item availability. + + Unavailable if some active loans exists. + + :returns: an elasticsearch query + """ + states = [LoanState.PENDING] + \ + current_app.config['CIRCULATION_STATES_LOAN_ACTIVE'] + return self.filter('terms', state=states) + class Loan(IlsRecord): """Loan class.""" diff --git a/tests/api/items/test_items_rest_views.py b/tests/api/items/test_items_rest_views.py index 54d3f2cb23..29b678e29d 100644 --- a/tests/api/items/test_items_rest_views.py +++ b/tests/api/items/test_items_rest_views.py @@ -42,7 +42,7 @@ def test_item_dumps(client, item_lib_martigny, org_martigny, assert res.status_code == 200 item_es = Item(get_json(res).get('metadata')) - assert item_es.available + assert item_es.is_available() assert item_es.organisation_pid == org_martigny.pid diff --git a/tests/api/loans/test_loans_rest.py b/tests/api/loans/test_loans_rest.py index 6a30e6f112..bfe80232dd 100644 --- a/tests/api/loans/test_loans_rest.py +++ b/tests/api/loans/test_loans_rest.py @@ -209,7 +209,7 @@ def test_due_soon_loans(client, librarian_martigny, can, reasons = item.can_delete assert can assert reasons == {} - assert item.available + assert item.is_available() assert not get_last_transaction_loc_for_item(item_pid) assert not item.patron_has_an_active_loan_on_item(patron_martigny) @@ -420,7 +420,7 @@ def test_checkout_item_transit(client, mailbox, item2_lib_martigny, loc_public_martigny, circulation_policies): """Test checkout of an item in transit.""" - assert item2_lib_martigny.available + assert item2_lib_martigny.is_available() mailbox.clear() # request @@ -447,7 +447,7 @@ def test_checkout_item_transit(client, mailbox, item2_lib_martigny, assert res.status_code == 200 actions = data.get('action_applied') loan_pid = actions[LoanAction.REQUEST].get('pid') - assert not item2_lib_martigny.available + assert not item2_lib_martigny.is_available() assert len(mailbox) == 1 assert mailbox[-1].recipients == [ @@ -477,9 +477,9 @@ def test_checkout_item_transit(client, mailbox, item2_lib_martigny, ) ) assert res.status_code == 200 - assert not item2_lib_martigny.available + assert not item2_lib_martigny.is_available() item = Item.get_record_by_pid(item2_lib_martigny.pid) - assert not item.available + assert not item.is_available() loan = Loan.get_record_by_pid(loan_pid) assert loan['state'] == LoanState.ITEM_IN_TRANSIT_FOR_PICKUP @@ -497,9 +497,9 @@ def test_checkout_item_transit(client, mailbox, item2_lib_martigny, ) ) assert res.status_code == 200 - assert not item2_lib_martigny.available + assert not item2_lib_martigny.is_available() item = Item.get_record_by_pid(item2_lib_martigny.pid) - assert not item.available + assert not item.is_available() loan_before_checkout = get_loan_for_item(item_pid_to_object(item.pid)) assert loan_before_checkout.get('state') == LoanState.ITEM_AT_DESK @@ -640,7 +640,7 @@ def test_librarian_request_on_blocked_user( patron3_martigny_blocked, circulation_policies): """Librarian request on blocked user returns a specific 403 message.""" - assert item_lib_martigny.available + assert item_lib_martigny.is_available() # request login_user_via_session(client, librarian_martigny.user) diff --git a/tests/api/test_availability.py b/tests/api/test_availability.py index e859124a5d..c7a446e9d6 100644 --- a/tests/api/test_availability.py +++ b/tests/api/test_availability.py @@ -202,8 +202,8 @@ def test_item_holding_document_availability( """Test item, holding and document availability.""" assert item_availablity_status( client, item_lib_martigny.pid, librarian_martigny.user) - assert item_lib_martigny.available - assert holding_lib_martigny.available + assert item_lib_martigny.is_available() + assert holding_lib_martigny.is_available() assert holding_lib_martigny.get_holding_loan_conditions() == 'standard' assert Document.is_available(document.pid, view_code='global') assert document_availability_status( @@ -235,11 +235,11 @@ def test_item_holding_document_availability( assert res.status_code == 200 actions = data.get('action_applied') loan_pid = actions[LoanAction.REQUEST].get('pid') - assert not item_lib_martigny.available + assert not item_lib_martigny.is_available() assert not item_availablity_status( client, item_lib_martigny.pid, librarian_martigny.user) holding = Holding.get_record_by_pid(holding_lib_martigny.pid) - assert holding.available + assert holding.is_available() assert holding_lib_martigny.get_holding_loan_conditions() == 'standard' assert Document.is_available(document.pid, 'global') assert document_availability_status( @@ -257,12 +257,12 @@ def test_item_holding_document_availability( ) ) assert res.status_code == 200 - assert not item_lib_martigny.available + assert not item_lib_martigny.is_available() assert not item_availablity_status( client, item_lib_martigny.pid, librarian_martigny.user) - assert not item_lib_martigny.available + assert not item_lib_martigny.is_available() holding = Holding.get_record_by_pid(holding_lib_martigny.pid) - assert holding.available + assert holding.is_available() assert holding_lib_martigny.get_holding_loan_conditions() == 'standard' assert Document.is_available(document.pid, 'global') assert document_availability_status( @@ -280,13 +280,13 @@ def test_item_holding_document_availability( ) ) assert res.status_code == 200 - assert not item_lib_martigny.available + assert not item_lib_martigny.is_available() assert not item_availablity_status( client, item_lib_martigny.pid, librarian_saxon.user) item = Item.get_record_by_pid(item_lib_martigny.pid) - assert not item.available + assert not item.is_available() holding = Holding.get_record_by_pid(holding_lib_martigny.pid) - assert holding.available + assert holding.is_available() assert holding_lib_martigny.get_holding_loan_conditions() == 'standard' assert Document.is_available(document.pid, 'global') assert document_availability_status( @@ -305,20 +305,20 @@ def test_item_holding_document_availability( assert res.status_code == 200 item = Item.get_record_by_pid(item_lib_martigny.pid) - assert not item.available + assert not item.is_available() assert not item_availablity_status( client, item.pid, librarian_martigny.user) holding = Holding.get_record_by_pid(holding_lib_martigny.pid) - assert holding.available + assert holding.is_available() assert holding_lib_martigny.get_holding_loan_conditions() == 'standard' assert Document.is_available(document.pid, 'global') assert document_availability_status( client, document.pid, librarian_martigny.user) - # masked item isn't available + # masked item isn't.is_available() item['_masked'] = True item = item.update(item, dbcommit=True, reindex=True) - assert not item.available + assert not item.is_available() del item['_masked'] item.update(item, dbcommit=True, reindex=True) @@ -367,11 +367,11 @@ def test_item_holding_document_availability( ) ) assert res.status_code == 200 - assert not item2_lib_martigny.available + assert not item2_lib_martigny.is_available() assert not item_availablity_status( client, item2_lib_martigny.pid, librarian_martigny.user) holding = Holding.get_record_by_pid(holding_lib_martigny.pid) - assert not holding.available + assert not holding.is_available() assert holding_lib_martigny.get_holding_loan_conditions() == 'standard' assert not Document.is_available(document.pid, 'global') assert not document_availability_status( @@ -380,7 +380,6 @@ def test_item_holding_document_availability( def item_availablity_status(client, pid, user): """Returns item availability.""" - login_user_via_session(client, user) res = client.get( url_for( 'api_item.item_availability', @@ -394,7 +393,6 @@ def item_availablity_status(client, pid, user): def document_availability_status(client, pid, user): """Returns document availability.""" - login_user_via_session(client, user) res = client.get( url_for( 'api_documents.document_availability', @@ -436,9 +434,9 @@ def test_availability_cipo_allow_request( cipo.update(cipo.dumps(), dbcommit=True, reindex=True) -def test_document_availability_failed(client, librarian2_martigny): +def test_document_availability_failed( + client, item_lib_martigny, document_with_issn, org_martigny): """Test document availability with dummy data should failed.""" - login_user_via_session(client, librarian2_martigny.user) res = client.get( url_for( 'api_documents.document_availability', @@ -446,11 +444,27 @@ def test_document_availability_failed(client, librarian2_martigny): ) ) assert res.status_code == 404 + res = client.get( + url_for( + 'api_documents.document_availability', + pid=document_with_issn.pid + ) + ) + assert res.status_code == 200 + assert not res.json.get('available') + res = client.get( + url_for( + 'api_documents.document_availability', + pid=document_with_issn.pid, + view_code=org_martigny['code'] + ) + ) + assert res.status_code == 200 + assert not res.json.get('available') def test_item_availability_failed(client, librarian2_martigny): """Test item availability with dummy data should failed.""" - login_user_via_session(client, librarian2_martigny.user) res = client.get( url_for( 'api_item.item_availability', diff --git a/tests/ui/documents/test_documents_api.py b/tests/ui/documents/test_documents_api.py index 0ee5a57f2a..2ac96c389f 100644 --- a/tests/ui/documents/test_documents_api.py +++ b/tests/ui/documents/test_documents_api.py @@ -40,21 +40,6 @@ from rero_ils.modules.tasks import process_bulk_queue -def test_document_properties(document): - """Test document properties.""" - # As no item/holding are loaded into this test, `is_available` should - # return false/raise an error. - assert not Document.is_available(document.pid, 'global') - - with mock.patch( - 'rero_ils.modules.holdings.api.Holding.' - 'get_holdings_pid_by_document_pid', - mock.MagicMock(return_value=['not_exists']) - ): - with pytest.raises(ValueError): - Document.is_available(document.pid, 'global', raise_exception=True) - - def test_document_create(db, document_data_tmp): """Test document creation.""" ptty = Document.create(document_data_tmp, delete_pid=True) diff --git a/tests/ui/holdings/test_holdings_api.py b/tests/ui/holdings/test_holdings_api.py index efef507ca5..0e136928c4 100644 --- a/tests/ui/holdings/test_holdings_api.py +++ b/tests/ui/holdings/test_holdings_api.py @@ -72,9 +72,10 @@ def test_holding_availability(holding_lib_sion_electronic, holding_lib_martigny, item_lib_martigny): """Test holding availability.""" # An electronic holding is always available despite if no item are linked - assert holding_lib_sion_electronic.available + assert holding_lib_sion_electronic.is_available() # The availability of other holdings type depends on children availability - assert holding_lib_martigny.available == item_lib_martigny.available + assert holding_lib_martigny.is_available() == \ + item_lib_martigny.is_available() def test_holding_extended_validation( @@ -162,4 +163,4 @@ def test_holdings_properties(holding_lib_martigny_w_patterns, vendor_martigny): assert holding.days_before_next_claim == 7 holding['_masked'] = True - assert not holding.available + assert not holding.is_available() diff --git a/tests/ui/items/test_items_api.py b/tests/ui/items/test_items_api.py index 796b659834..249a13634e 100644 --- a/tests/ui/items/test_items_api.py +++ b/tests/ui/items/test_items_api.py @@ -246,13 +246,13 @@ def test_items_availability(item_type_missing_martigny, item = Item.create(item_data, dbcommit=True, reindex=True) # test the availability and availability_text - assert not item.available + assert not item.is_available() assert len(item.availability_text) == \ len(item_type_missing_martigny.get('displayed_status', [])) + 1 del item['temporary_item_type'] item = item.update(item, dbcommit=True, reindex=True) - assert item.available + assert item.is_available() assert len(item.availability_text) == 1 # only default value # test availability and availability_text for an issue @@ -265,12 +265,12 @@ def test_items_availability(item_type_missing_martigny, 'expected_date': '1970-01-01' } item = item.update(item, dbcommit=True, reindex=True) - assert item.available + assert item.is_available() assert item.availability_text[0]['label'] == item.status item['issue']['status'] = ItemIssueStatus.LATE item = item.update(item, dbcommit=True, reindex=True) - assert not item.available + assert not item.is_available() assert item.availability_text[0]['label'] == ItemIssueStatus.LATE # delete the created item