holdings: place a circulation request

When patron places a request on a holdings record, patron
must provide two required information: pickup_location and
description of the issue.
At the receiving of the request, system attempts to create
a new item of type `provisional`, attach it to the holdings
record and finally reserve it to the patron.
This exact same behaviour executed for requests placed by
librarian on behalf of the patron.

Co-Authored-by: Aly Badr <[email protected]>

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)

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']

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."""

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