From db1e4cbd6faf8ec5b9c293b11e2533acec240cac Mon Sep 17 00:00:00 2001 From: Aly Badr Date: Fri, 7 Feb 2020 09:00:48 +0100 Subject: [PATCH 1/2] items: can request api for an item * Removes duplicate fixtures function. Co-Authored-by: Aly Badr --- rero_ils/modules/items/api_views.py | 14 ++++ rero_ils/modules/items/utils.py | 71 +++++++++++++++++++ tests/api/test_availability.py | 106 +++++++++++++++++++++++++++- tests/api/test_items_rest.py | 30 +++++++- tests/fixtures/circulation.py | 18 ----- 5 files changed, 217 insertions(+), 22 deletions(-) create mode 100644 rero_ils/modules/items/utils.py diff --git a/rero_ils/modules/items/api_views.py b/rero_ils/modules/items/api_views.py index e3c7194a11..aab1badd14 100644 --- a/rero_ils/modules/items/api_views.py +++ b/rero_ils/modules/items/api_views.py @@ -29,6 +29,7 @@ from werkzeug.exceptions import NotFound from .api import Item +from .utils import is_librarian_can_request_item_for_patron from ..circ_policies.api import CircPolicy from ..libraries.api import Library from ..loans.api import Loan @@ -323,3 +324,16 @@ def item_availability(item_pid): return jsonify({ 'availability': item.available }) + + +@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) diff --git a/rero_ils/modules/items/utils.py b/rero_ils/modules/items/utils.py new file mode 100644 index 0000000000..59cb27a2d0 --- /dev/null +++ b/rero_ils/modules/items/utils.py @@ -0,0 +1,71 @@ +# -*- 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 . + +"""Item utils.""" + +from flask import jsonify +from flask_babelex import gettext as _ + +from .api import Item, ItemStatus +from ..libraries.api import Library +from ..loans.api import Loan +from ..loans.utils import can_be_requested +from ..patrons.api import Patron + + +def jsonify_response(response=False, reason=None): + """Jsonify api response.""" + return jsonify({ + 'can_request': response, + 'reason': reason + }) + + +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. + + required_parameters: item_pid, library_pid, patron_barcode + """ + 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=_( + 'Circulation policies do not allow request on this item.')) + 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.')) diff --git a/tests/api/test_availability.py b/tests/api/test_availability.py index 5d69f39d7c..e548fa6863 100644 --- a/tests/api/test_availability.py +++ b/tests/api/test_availability.py @@ -21,13 +21,101 @@ from utils import get_json, postdata from rero_ils.modules.holdings.api import Holding -from rero_ils.modules.items.api import Item +from rero_ils.modules.items.api import Item, ItemStatus from rero_ils.modules.items.views import item_availability_text from rero_ils.modules.loans.api import LoanAction +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): + """Test item can request API.""" + # test no logged user + res = client.get( + url_for( + 'api_item.can_request', + item_pid=item_lib_martigny.pid, + library_pid=lib_martigny.pid, + patron_barcode=patron_martigny_no_email.get('barcode') + ) + ) + assert res.status_code == 401 + + login_user_via_session(client, librarian_martigny_no_email.user) + # valid test + res = client.get( + url_for( + 'api_item.can_request', + item_pid=item_lib_martigny.pid, + library_pid=lib_martigny.pid, + patron_barcode=patron_martigny_no_email.get('barcode') + ) + ) + assert res.status_code == 200 + data = get_json(res) + assert data.get('can_request') + + # test no valid item + res = client.get( + url_for( + 'api_item.can_request', + item_pid='no_item', + library_pid=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_request') + + # test no valid library + res = client.get( + url_for( + 'api_item.can_request', + item_pid=item_lib_martigny.pid, + library_pid='no_library', + patron_barcode=patron_martigny_no_email.get('barcode') + ) + ) + assert res.status_code == 200 + data = get_json(res) + assert not data.get('can_request') + + # test no valid patron + res = client.get( + url_for( + 'api_item.can_request', + item_pid=item_lib_martigny.pid, + library_pid=lib_martigny.pid, + patron_barcode='no_barcode' + ) + ) + assert res.status_code == 200 + data = get_json(res) + assert not data.get('can_request') + + # test no valid item status + item_lib_martigny['status'] = ItemStatus.MISSING + item_lib_martigny.update(item_lib_martigny, dbcommit=True, reindex=True) + res = client.get( + url_for( + 'api_item.can_request', + item_pid=item_lib_martigny.pid, + library_pid=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_request') + item_lib_martigny['status'] = ItemStatus.ON_SHELF + item_lib_martigny.update(item_lib_martigny, dbcommit=True, reindex=True) + + def test_item_holding_document_availability( - client, document, + client, document, lib_martigny, holding_lib_martigny, item_lib_martigny, item2_lib_martigny, librarian_martigny_no_email, librarian_saxon_no_email, @@ -82,6 +170,7 @@ def test_item_holding_document_availability( assert document.is_available('global') assert document_availablity_status( client, document.pid, librarian_martigny_no_email.user) + # validate request res, _ = postdata( client, @@ -155,6 +244,19 @@ def test_item_holding_document_availability( assert document_availablity_status( client, document.pid, librarian_martigny_no_email.user) + # test can not request item already checked out to patron + res = client.get( + url_for( + 'api_item.can_request', + item_pid=item_lib_martigny.pid, + library_pid=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_request') + class current_i18n: class locale: language = 'en' diff --git a/tests/api/test_items_rest.py b/tests/api/test_items_rest.py index a994cf97f5..151fb46228 100644 --- a/tests/api/test_items_rest.py +++ b/tests/api/test_items_rest.py @@ -402,7 +402,7 @@ def test_checkout_organisation_policy(client, lib_martigny, def test_items_requests(client, librarian_martigny_no_email, patron_martigny_no_email, loc_public_martigny, - item_type_standard_martigny, + item_type_standard_martigny, lib_martigny, item_lib_martigny, json_header, circulation_policies): """Test requesting an item and validation.""" @@ -437,6 +437,19 @@ def test_items_requests(client, librarian_martigny_no_email, assert item.patron_request_rank(patron.get('barcode')) == 1 assert item.is_requested_by_patron(patron.get('barcode')) + # test can not request item already requested to patron + res = client.get( + url_for( + 'api_item.can_request', + item_pid=item_pid, + library_pid=lib_martigny.pid, + patron_barcode=patron.get('barcode') + ) + ) + assert res.status_code == 200 + data = get_json(res) + assert not data.get('can_request') + # checkout res, data = postdata( client, @@ -961,7 +974,7 @@ def test_items_no_extend(client, librarian_martigny_no_email, def test_items_deny_requests(client, librarian_martigny_no_email, patron_martigny_no_email, loc_public_martigny, - item_type_standard_martigny, + item_type_standard_martigny, lib_martigny, item_lib_martigny, json_header, circ_policy_short_martigny): """Test items when requests are denied.""" @@ -990,6 +1003,19 @@ def test_items_deny_requests(client, librarian_martigny_no_email, ) assert res.status_code == 403 + # test can request because of a circulation policy does not allow request + res = client.get( + url_for( + 'api_item.can_request', + item_pid=item_pid, + library_pid=lib_martigny.pid, + patron_barcode=patron.get('barcode') + ) + ) + assert res.status_code == 200 + data = get_json(res) + assert not data.get('can_request') + circ_policy_short_martigny['allow_requests'] = True circ_policy_short_martigny.update( data=circ_policy_short_martigny, diff --git a/tests/fixtures/circulation.py b/tests/fixtures/circulation.py index fd5d753cc8..c5c17efb5f 100644 --- a/tests/fixtures/circulation.py +++ b/tests/fixtures/circulation.py @@ -406,24 +406,6 @@ def system_librarian_sion_no_email( return ptrn -# ------------ Org: Sion, Lib: Sion, Librarian ---------- -@pytest.fixture(scope="module") -@mock.patch('rero_ils.modules.patrons.api.send_reset_password_instructions') -def patron_martigny_no_email( - app, - roles, - patron_type_children_martigny, - patron_martigny_data): - """Create Martigny patron without sending reset password instruction.""" - ptrn = Patron.create( - data=patron_martigny_data, - delete_pid=False, - dbcommit=True, - reindex=True) - flush_index(PatronsSearch.Meta.index) - return ptrn - - # ------------ Org: Sion, Lib: Sion, Librarian ---------- @pytest.fixture(scope="module") def librarian_sion_data(data): From 455551195bc5273f25fcc2034eb6784202f7d923 Mon Sep 17 00:00:00 2001 From: Bertrand Zuchuat Date: Fri, 7 Feb 2020 06:33:57 +0100 Subject: [PATCH 2/2] items: can request api for an item * Removes duplicate fixtures function. * Adds sort on pickup_name location Co-Authored-by: Bertrand Zuchuat Co-Authored-by: Aly Badr --- rero_ils/config.py | 4 ++ rero_ils/modules/items/api_views.py | 46 ++++++++++++++++++- rero_ils/modules/items/utils.py | 71 ----------------------------- 3 files changed, 48 insertions(+), 73 deletions(-) delete mode 100644 rero_ils/modules/items/utils.py diff --git a/rero_ils/config.py b/rero_ils/config.py index dbb4ef8fc6..bef6749d23 100644 --- a/rero_ils/config.py +++ b/rero_ils/config.py @@ -1208,6 +1208,10 @@ def _(x): fields=['name'], title='Location name', default_order='asc' ) +RECORDS_REST_SORT_OPTIONS['locations']['pickup_name'] = dict( + fields=['pickup_name'], title='Pickup Location name', + default_order='asc' +) RECORDS_REST_DEFAULT_SORT['locations'] = dict( query='bestmatch', noquery='name') diff --git a/rero_ils/modules/items/api_views.py b/rero_ils/modules/items/api_views.py index aab1badd14..89441108a5 100644 --- a/rero_ils/modules/items/api_views.py +++ b/rero_ils/modules/items/api_views.py @@ -28,11 +28,11 @@ from invenio_circulation.errors import CirculationException from werkzeug.exceptions import NotFound -from .api import Item -from .utils import is_librarian_can_request_item_for_patron +from .api import Item, ItemStatus from ..circ_policies.api import CircPolicy 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 @@ -337,3 +337,45 @@ def can_request(item_pid, library_pid, patron_barcode): """ return is_librarian_can_request_item_for_patron( item_pid, library_pid, patron_barcode) + + +def jsonify_response(response=False, reason=None): + """Jsonify api response.""" + return jsonify({ + 'can_request': response, + 'reason': reason + }) + + +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. + + required_parameters: item_pid, library_pid, patron_barcode + """ + 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='Circulation policies do not allow request on this item.') + 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.') diff --git a/rero_ils/modules/items/utils.py b/rero_ils/modules/items/utils.py deleted file mode 100644 index 59cb27a2d0..0000000000 --- a/rero_ils/modules/items/utils.py +++ /dev/null @@ -1,71 +0,0 @@ -# -*- 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 . - -"""Item utils.""" - -from flask import jsonify -from flask_babelex import gettext as _ - -from .api import Item, ItemStatus -from ..libraries.api import Library -from ..loans.api import Loan -from ..loans.utils import can_be_requested -from ..patrons.api import Patron - - -def jsonify_response(response=False, reason=None): - """Jsonify api response.""" - return jsonify({ - 'can_request': response, - 'reason': reason - }) - - -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. - - required_parameters: item_pid, library_pid, patron_barcode - """ - 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=_( - 'Circulation policies do not allow request on this item.')) - 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.'))