diff --git a/rero_ils/modules/holdings/api_views.py b/rero_ils/modules/holdings/api_views.py index bd038f38bc..a8cad70765 100644 --- a/rero_ils/modules/holdings/api_views.py +++ b/rero_ils/modules/holdings/api_views.py @@ -22,12 +22,22 @@ from functools import wraps +from elasticsearch import exceptions from flask import Blueprint, abort, current_app, jsonify from flask import request as flask_request +from invenio_circulation.errors import CirculationException, \ + MissingRequiredParameterError from invenio_db import db from jinja2.exceptions import TemplateSyntaxError, UndefinedError from werkzeug.exceptions import NotFound, Unauthorized +from rero_ils.modules.errors import NoCirculationActionIsPermitted +from rero_ils.modules.items.api import Item +from rero_ils.modules.items.models import ItemStatus +from rero_ils.modules.items.views.api_views import \ + check_authentication_for_request, check_logged_user_authentication +from rero_ils.modules.patrons.api import Patron +from rero_ils.modules.utils import get_ref_for_pid from rero_ils.modules.views import check_authentication from .api import Holding @@ -139,3 +149,99 @@ def receive_regular_issue(holding_pid): abort(400) # the created item of type issue is returned return jsonify({'issue': issue}) + + +def do_holding_jsonify_action(func): + """Jsonify loan actions for holdings methods. + + This method for the circulation actions that required access to the holding + object before executing the invenio-circulation logic. + """ + @wraps(func) + def decorated_view(*args, **kwargs): + try: + data = flask_request.get_json() + description = data.pop('description') + except KeyError: + # The description parameter is missing. + abort(400, str('missing description parameter.')) + + try: + holding_pid = data.pop('holding_pid', None) + holding = Holding.get_record_by_pid(holding_pid) + if not holding: + abort(404, 'Holding not found') + # create a provisional item + item_metadata = { + 'type': 'provisional', + 'document': { + '$ref': get_ref_for_pid('doc', holding.document_pid)}, + 'location': { + '$ref': get_ref_for_pid('loc', holding.location_pid)}, + 'item_type': {'$ref': get_ref_for_pid( + 'itty', holding.circulation_category_pid)}, + 'enumerationAndChronology': description, + 'status': ItemStatus.ON_SHELF, + 'holding': {'$ref': get_ref_for_pid('hold', holding.pid)} + } + item = Item.create(item_metadata, dbcommit=True, reindex=True) + + _, action_applied = func(holding, item, data, *args, **kwargs) + return jsonify({ + 'action_applied': action_applied + }) + except NoCirculationActionIsPermitted: + # The circulation specs do not allow updates on some loan states. + return jsonify({'status': 'error: Forbidden'}), 403 + except MissingRequiredParameterError as error: + # Return error 400 when there is a missing required parameter + abort(400, str(error)) + except CirculationException as error: + abort(403, error.description or str(error)) + except NotFound as error: + raise error + except exceptions.RequestError as error: + # missing required parameters + return jsonify({'status': f'error: {error}'}), 400 + except Exception as error: + # TODO: need to know what type of exception and document there. + # raise error + current_app.logger.error(str(error)) + return jsonify({'status': f'error: {error}'}), 400 + return decorated_view + + +@api_blueprint.route('/patron_request', methods=['POST']) +@check_logged_user_authentication +@check_authentication_for_request +@do_holding_jsonify_action +def patron_request(holding, item, data): + """HTTP POST request for Holding request action by a patron. + + required_parameters: + holding_pid, + pickup_location_pid, + description + """ + patron_pid = Patron.get_current_patron(holding).pid + data['patron_pid'] = patron_pid + data['transaction_user_pid'] = patron_pid + data['transaction_location_pid'] = data['pickup_location_pid'] + return item.request(**data) + + +@api_blueprint.route('/request', methods=['POST']) +@check_authentication +@do_holding_jsonify_action +def librarian_request(holding, item, data): + """HTTP POST request for Holding request action. + + required_parameters: + holding_pid, + pickup_location_pid, + description, + patron_pid, + transaction_location_pid or transaction_library_pid, + transaction_user_pid + """ + return item.request(**data) diff --git a/rero_ils/modules/items/views/api_views.py b/rero_ils/modules/items/views/api_views.py index 5cdb13ff07..4f205b20b3 100644 --- a/rero_ils/modules/items/views/api_views.py +++ b/rero_ils/modules/items/views/api_views.py @@ -38,8 +38,7 @@ from rero_ils.modules.items.utils import item_pid_to_object from rero_ils.modules.libraries.api import Library from rero_ils.modules.loans.api import Loan -from rero_ils.modules.patrons.api import Patron, current_librarian, \ - current_patrons +from rero_ils.modules.patrons.api import Patron, current_librarian from rero_ils.modules.views import check_authentication from rero_ils.permissions import request_item_permission @@ -168,12 +167,7 @@ def patron_request(item, data): pickup_location_pid """ # get the patron account of the same org of the location pid - def get_patron(item): - for ptrn in current_patrons: - if ptrn.organisation_pid == item.organisation_pid: - return ptrn - - patron_pid = get_patron(item).pid + patron_pid = Patron.get_current_patron(item).pid data['patron_pid'] = patron_pid data['transaction_user_pid'] = patron_pid data['transaction_location_pid'] = data['pickup_location_pid'] diff --git a/rero_ils/modules/patrons/api.py b/rero_ils/modules/patrons/api.py index b2241a5d20..548cb6424f 100644 --- a/rero_ils/modules/patrons/api.py +++ b/rero_ils/modules/patrons/api.py @@ -769,6 +769,16 @@ def set_keep_history(self, keep_history, dbcommit=True, reindex=True): self.reindex() PatronsSearch.flush_and_refresh() + def get_current_patron(record): + """Return the patron account belongs to record's organisation. + + :param record - a valid rero_ils resource/object. + :returns: The patron record linked to the organisation. + """ + for ptrn in current_patrons: + if ptrn.organisation_pid == record.organisation_pid: + return ptrn + class PatronsIndexer(IlsRecordsIndexer): """Holdings indexing class.""" diff --git a/tests/api/holdings/test_provisional_items.py b/tests/api/holdings/test_provisional_items.py index 3a2c2d3c89..b8ab32cf21 100644 --- a/tests/api/holdings/test_provisional_items.py +++ b/tests/api/holdings/test_provisional_items.py @@ -22,10 +22,12 @@ from flask import url_for from invenio_accounts.testutils import login_user_via_session -from utils import get_json +from utils import get_json, postdata from rero_ils.modules.items.api import Item from rero_ils.modules.items.models import ItemStatus, TypeOfItem +from rero_ils.modules.loans.api import Loan +from rero_ils.modules.loans.models import LoanAction, LoanState from rero_ils.modules.utils import get_ref_for_pid @@ -96,3 +98,140 @@ def test_provisional_items_creation(client, document, org_martigny, items = data['hits']['hits'] assert len(items) == 1 assert items[0]['metadata']['pid'] == item_lib_martigny.pid + + +def test_holding_requests(client, patron_martigny, loc_public_martigny, + circulation_policies, librarian_martigny, + holding_lib_martigny_w_patterns, lib_martigny): + """Test holding patron request.""" + login_user_via_session(client, patron_martigny.user) + holding = holding_lib_martigny_w_patterns + description = 'Year: 2000 / volume: 15 / number: 22 / pages: 11-12' + # test fails when there is a missing description or holding_pid + res, data = postdata( + client, + 'api_holding.patron_request', + dict( + holding_pid=holding.pid, + pickup_location_pid=loc_public_martigny.pid + ) + ) + assert res.status_code == 400 + res, data = postdata( + client, + 'api_holding.patron_request', + dict( + description=description, + pickup_location_pid=loc_public_martigny.pid + ) + ) + assert res.status_code == 404 + # test passes when all required parameters are given + res, data = postdata( + client, + 'api_holding.patron_request', + dict( + holding_pid=holding.pid, + pickup_location_pid=loc_public_martigny.pid, + description=description + ) + ) + assert res.status_code == 200 + loan = Loan.get_record_by_pid( + data.get('action_applied')[LoanAction.REQUEST].get('pid')) + assert loan.state == LoanState.PENDING + item = Item.get_record_by_pid(loan.item_pid) + assert item.get('type') == TypeOfItem.PROVISIONAL + assert item.status == ItemStatus.ON_SHELF + assert item.holding_pid == holding.pid + assert item.get('enumerationAndChronology') == description + # checkout the item to the requested patron + login_user_via_session(client, librarian_martigny.user) + res, data = postdata(client, 'api_item.checkout', dict( + item_pid=item.pid, + patron_pid=patron_martigny.pid, + transaction_location_pid=loc_public_martigny.pid, + transaction_user_pid=librarian_martigny.pid, + )) + assert res.status_code == 200 + loan_pid = data.get('action_applied')[LoanAction.CHECKOUT].get('pid') + assert loan_pid == loan.pid + item = Item.get_record_by_pid(item.pid) + assert item.status == ItemStatus.ON_LOAN + # return the item at the owning library + res, data = postdata( + client, + 'api_item.checkin', + dict( + item_pid=item.pid, + transaction_location_pid=loc_public_martigny.pid, + transaction_user_pid=librarian_martigny.pid + ) + ) + assert res.status_code == 200 + item = Item.get_record_by_pid(item.pid) + assert item.status == ItemStatus.ON_SHELF + # TODO: add additional tests for the task to delete provisional items with + # no active loans. + + # test requests made by a librarian + # test fails when there are missing parameters + res, data = postdata( + client, + 'api_holding.librarian_request', + dict( + holding_pid=holding.pid, + pickup_location_pid=loc_public_martigny.pid, + description=description, + transaction_library_pid=lib_martigny.pid, + transaction_user_pid=librarian_martigny.pid + ) + ) + assert res.status_code == 400 + res, data = postdata( + client, + 'api_holding.librarian_request', + dict( + holding_pid=holding.pid, + pickup_location_pid=loc_public_martigny.pid, + description=description, + patron_pid=patron_martigny.pid, + transaction_library_pid=lib_martigny.pid + ) + ) + assert res.status_code == 400 + res, data = postdata( + client, + 'api_holding.librarian_request', + dict( + holding_pid=holding.pid, + pickup_location_pid=loc_public_martigny.pid, + patron_pid=patron_martigny.pid, + transaction_library_pid=lib_martigny.pid, + transaction_user_pid=librarian_martigny.pid + ) + ) + assert res.status_code == 400 + # test passes when all required parameters are given + res, data = postdata( + client, + 'api_holding.librarian_request', + dict( + holding_pid=holding.pid, + pickup_location_pid=loc_public_martigny.pid, + description=description, + patron_pid=patron_martigny.pid, + transaction_library_pid=lib_martigny.pid, + transaction_user_pid=librarian_martigny.pid + ) + ) + assert res.status_code == 200 + loan_2 = Loan.get_record_by_pid( + data.get('action_applied')[LoanAction.REQUEST].get('pid')) + assert loan_2.state == LoanState.PENDING + item_2 = Item.get_record_by_pid(loan_2.item_pid) + assert item_2.get('type') == TypeOfItem.PROVISIONAL + assert item_2.status == ItemStatus.ON_SHELF + assert item_2.holding_pid == holding.pid + assert item_2.get('enumerationAndChronology') == description + assert item_2.pid != item.pid