circulation: manage non-circulating libraries

In older versions, the transaction locations of a library are
selected from the list of pickup locations of the library.
This causes a problem for libraries have not setup a pickup location.

With this commit, for libraries with no pickup locations
the system considers the first library location as transaction
location for circulation transactions.

* Fixes problem when checkout of items of libraries with no opening hours was no possible.
* Allows requests on items of non-circulating libraries at external locations.
* Closes rero#2367
* Adds a non-circulating library to units testing.

Co-Authored-by: Aly Badr <aly.badr@rero.ch>

rero_ils/modules/items/api/circulation.py

@@ -174,6 +174,7 @@ def complete_action_missing_params(
         kwargs['transaction_date'] = datetime.utcnow().isoformat()
         document_pid = extracted_data_from_ref(item.get('document'))
         kwargs.setdefault('document_pid', document_pid)
+        # set the transaction location for the circulation transaction
         transaction_location_pid = kwargs.get(
             'transaction_location_pid', None)
         if not transaction_location_pid:
@@ -182,8 +183,17 @@ def complete_action_missing_params(
             if transaction_library_pid is not None:
                 lib = Library.get_record_by_pid(transaction_library_pid)
                 kwargs['transaction_location_pid'] = \
-                    lib.get_pickup_location_pid()
-
+                    lib.get_transaction_location_pid()
+        # set the pickup_location_pid field if not found for loans that are
+        # ready for checkout.
+        if not kwargs.get('pickup_location_pid') and \
+            loan.get('state') in \
+                [
+                    LoanState.CREATED,
+                    LoanState.ITEM_AT_DESK
+                ]:
+            kwargs['pickup_location_pid'] = \
+                kwargs.get('transaction_location_pid')
         return loan, kwargs
 
     def checkin_item_on_shelf(self, loans_list, **kwargs):

rero_ils/modules/libraries/api.py

@@ -104,6 +104,11 @@ def pickup_location_query(self):
             'term', library__pid=self.pid).filter(
                 'term', is_pickup=True).source(['pid']).scan()
 
+    def library_locations(self):
+        """Return list of all pids of the library locations."""
+        return LocationsSearch().filter(
+            'term', library__pid=self.pid).source(['pid']).scan()
+
     def get_pickup_locations_pids(self):
         """Returns libraries all pickup locations pids."""
         for location in self.pickup_location_query():
@@ -116,6 +121,13 @@ def get_pickup_location_pid(self):
         except StopIteration:
             return None
 
+    def get_transaction_location_pid(self):
+        """Returns libraries first transaction location pid."""
+        try:
+            return next(self.pickup_location_query()).pid
+        except StopIteration:
+            return next(self.library_locations()).pid
+
     def _is_betweentimes(self, time_to_test, times):
         """Test if time is between times."""
         times_open = False

rero_ils/modules/loans/api.py

@@ -137,10 +137,16 @@ def can_extend(cls, item, **kwargs):
                 return True, []
         if loan.get('state') != LoanState.ITEM_ON_LOAN:
             return False, [_('The loan cannot be extended')]
+        # The parameters for the renewal is calculated based on the transaction
+        # library and not the owning library.
+        transaction_library_pid = Location\
+            .get_record_by_pid(loan['transaction_location_pid'])\
+            .get_library().get('pid')
+
         patron = Patron.get_record_by_pid(loan.get('patron_pid'))
         cipo = CircPolicy.provide_circ_policy(
             item.organisation_pid,
-            item.library_pid,
+            transaction_library_pid,
             patron.patron_type_pid,
             item.item_type_circulation_category_pid
         )
@@ -150,7 +156,7 @@ def can_extend(cls, item, **kwargs):
                 extend_loan_data_is_valid(
                     loan.get('end_date'),
                     cipo.get('renewal_duration'),
-                    item.library_pid
+                    transaction_library_pid
                )):
             return False, [_('Circulation policies disallows the operation.')]
         if item.number_of_requests():

tests/api/circulation/test_library_with_no_circulation.py

@@ -0,0 +1,107 @@
+# -*- coding: utf-8 -*-
+#
+# RERO ILS
+# Copyright (C) 2020 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 <http://www.gnu.org/licenses/>.
+
+"""Tests REST checkout API methods in library with no circulation."""
+
+from invenio_accounts.testutils import login_user_via_session
+from utils import postdata
+
+from rero_ils.modules.loans.api import Loan, LoanState
+
+
+def test_requesting_item_from_non_circulating_library(
+        client, librarian_martigny, lib_martigny, lib_martigny_bourg,
+        patron_martigny, loc_public_martigny, loc_public_martigny_bourg,
+        item_lib_martigny_bourg, circulation_policies,
+        librarian_martigny_bourg):
+    """Test requests at non circulating library."""
+    # TEST: a librarian from an external library can request and item from a
+    # non-circulation library to be picked-up at his own library.
+    login_user_via_session(client, librarian_martigny.user)
+    res, data = postdata(
+        client,
+        'api_item.librarian_request',
+        dict(
+            item_pid=item_lib_martigny_bourg.pid,
+            patron_pid=patron_martigny.pid,
+            pickup_location_pid=loc_public_martigny.pid,
+            transaction_library_pid=lib_martigny_bourg.pid,
+            transaction_user_pid=librarian_martigny.pid
+        )
+    )
+    assert res.status_code == 200
+    loan = data.get('metadata').get('pending_loans')[0]
+    assert loan.get('transaction_location_pid') == \
+        loc_public_martigny_bourg.pid
+    assert loan.get('pickup_location_pid') == loc_public_martigny.pid
+
+    # non-circulating library send items to requesting library
+    login_user_via_session(client, librarian_martigny_bourg.user)
+    res, data = postdata(
+        client,
+        'api_item.validate_request',
+        dict(
+            pid=loan.get('pid'),
+            transaction_library_pid=lib_martigny_bourg.pid,
+            transaction_user_pid=librarian_martigny_bourg.pid
+        )
+    )
+    assert res.status_code == 200
+    loan = data.get('metadata').get('pending_loans')[0]
+    assert loan.get('transaction_location_pid') == \
+        loc_public_martigny_bourg.pid
+    assert loan.get('pickup_location_pid') == loc_public_martigny.pid
+    assert loan.get('state') == LoanState.ITEM_IN_TRANSIT_FOR_PICKUP
+
+    # requesting library receives an item from non-circulating library.
+    login_user_via_session(client, librarian_martigny.user)
+    res, data = postdata(
+        client,
+        'api_item.receive',
+        dict(
+            pid=loan.get('pid'),
+            transaction_library_pid=lib_martigny.pid,
+            transaction_user_pid=librarian_martigny.pid
+        )
+    )
+    assert res.status_code == 200
+    loan = data.get('metadata').get('pending_loans')[0]
+    assert loan.get('transaction_location_pid') == \
+        loc_public_martigny.pid
+    assert loan.get('pickup_location_pid') == loc_public_martigny.pid
+    assert loan.get('state') == LoanState.ITEM_AT_DESK
+
+    # checkout item to requested patron
+    login_user_via_session(client, librarian_martigny.user)
+    params = dict(
+        item_pid=item_lib_martigny_bourg.pid,
+        patron_pid=patron_martigny.pid,
+        transaction_user_pid=librarian_martigny.pid,
+        transaction_library_pid=lib_martigny.pid,
+    )
+    res, data = postdata(
+        client,
+        'api_item.checkout',
+        params
+    )
+    assert res.status_code == 200
+    loan_pid = data.get('action_applied').get('checkout').get('pid')
+    loan = Loan.get_record_by_pid(loan_pid)
+    assert loan.get('transaction_location_pid') == \
+        loc_public_martigny.pid
+    assert loan.get('pickup_location_pid') == loc_public_martigny.pid
+    assert loan.get('state') == LoanState.ITEM_ON_LOAN

tests/api/libraries/test_libraries_rest.py

@@ -145,9 +145,14 @@ def test_libraries_post_put_delete(client, lib_martigny_data, json_header):
     assert res.status_code == 410
 
 
-def test_library_no_pickup(lib_sion):
-    """Test library with no pick_up location."""
+def test_non_circulating_libraries(
+        lib_sion, lib_martigny, lib_martigny_bourg, loc_public_martigny,
+        loc_public_martigny_bourg):
+    """Test pickup vs transaction locations."""
     assert not lib_sion.get_pickup_location_pid()
+    assert not lib_martigny_bourg.get_pickup_location_pid()
+    assert lib_martigny.get_pickup_location_pid()
+    assert lib_martigny_bourg.get_transaction_location_pid()
 
 
 def test_library_never_open(lib_sion):
@@ -186,7 +191,7 @@ def test_filtered_libraries_get(
     res = client.get(list_url)
     assert res.status_code == 200
     data = get_json(res)
-    assert data['hits']['total']['value'] == 3
+    assert data['hits']['total']['value'] == 4
 
     # Sion
     login_user_via_session(client, librarian_sion.user)

tests/data/data.json

@@ -810,6 +810,49 @@
     ],
     "communication_language": "fre"
   },
+  "lib7": {
+    "$schema": "https://bib.rero.ch/schemas/libraries/library-v0.0.1.json",
+    "address": "Ave de la gare, Martigny 1920",
+    "code": "MARTIGNYBOURG",
+    "email": "reroilstest+martignybourg@gmail.com",
+    "name": "Library of Martigny-bourg",
+    "organisation": {
+      "$ref": "https://bib.rero.ch/api/organisations/org1"
+    },
+    "pid": "lib7",
+    "notification_settings": [
+      {
+        "type": "due_soon",
+        "email": "reroilstest+martignybourg@gmail.com"
+      },
+      {
+        "type": "overdue",
+        "email": "reroilstest+martignybourg@gmail.com"
+      },
+      {
+        "type": "recall",
+        "email": "reroilstest+martignybourg@gmail.com"
+      },
+      {
+        "type": "availability",
+        "email": "reroilstest+martignybourg@gmail.com",
+        "delay": 0
+      },
+      {
+        "type": "request",
+        "email": "reroilstest+martignybourg@gmail.com"
+      },
+      {
+        "type": "transit_notice",
+        "email": "reroilstest+martignybourg@gmail.com"
+      },
+      {
+        "type": "booking",
+        "email": "reroilstest+martignybourg@gmail.com"
+      }
+    ],
+    "communication_language": "fre"
+  },
   "loc1": {
     "$schema": "https://bib.rero.ch/schemas/locations/location-v0.0.1.json",
     "code": "MARTIGNY-PUBLIC",
@@ -965,6 +1008,17 @@
     "pickup_name": "SAILLON-PUBLIC: Public Space",
     "allow_request": true
   },
+  "loc15": {
+    "$schema": "https://bib.rero.ch/schemas/locations/location-v0.0.1.json",
+    "code": "MARTIGNY-BOURG-PUBLIC",
+    "name": "Martigny Bourg Library Public Space",
+    "pid": "loc15",
+    "library": {
+      "$ref": "https://bib.rero.ch/api/libraries/lib7"
+    },
+    "is_pickup": false,
+    "allow_request": true
+  },
   "itty1": {
     "$schema": "https://bib.rero.ch/schemas/item_types/item_type-v0.0.1.json",
     "name": "standard",
@@ -3262,6 +3316,32 @@
     },
     "status": "on_shelf"
   },
+  "item10": {
+    "$schema": "https://bib.rero.ch/schemas/items/item-v0.0.1.json",
+    "pid": "item10",
+    "barcode": "123410",
+    "type": "standard",
+    "document": {
+      "$ref": "https://bib.rero.ch/api/documents/doc1"
+    },
+    "call_number": "000010",
+    "location": {
+      "$ref": "https://bib.rero.ch/api/locations/loc15"
+    },
+    "library": {
+      "$ref": "https://bib.rero.ch/api/libraries/lib7"
+    },
+    "organisation": {
+      "$ref": "https://bib.rero.ch/api/organisations/org1"
+    },
+    "item_type": {
+      "$ref": "https://bib.rero.ch/api/item_types/itty1"
+    },
+    "status": "on_shelf",
+    "url": "https://lipda.mediatheque.ch/CH-000019-X:223156.file",
+    "pac_code": "0_frozen_collection",
+    "price": 15.2
+  },
   "ptrn1": {
     "$schema": "https://bib.rero.ch/schemas/patrons/patron-v0.0.1.json",
     "pid": "ptrn1",
@@ -3575,6 +3655,27 @@
       "communication_language": "fre"
     }
   },
+  "ptrn13": {
+    "$schema": "https://bib.rero.ch/schemas/patrons/patron-v0.0.1.json",
+    "pid": "ptrn13",
+    "first_name": "Michelle",
+    "last_name": "Bonvin",
+    "street": "Avenue de la Gare, 4",
+    "postal_code": "1920",
+    "city": "Martigny",
+    "birth_date": "1975-02-07",
+    "libraries": [
+      {
+        "$ref": "https://bib.rero.ch/api/libraries/lib7"
+      }
+    ],
+    "username": "michelle",
+    "email": "mbonvin@gmail.com",
+    "home_phone": "+41324993599",
+    "roles": [
+      "librarian"
+    ]
+  },
   "dummy_notif": {
     "$schema": "https://bib.rero.ch/schemas/notifications/notification-v0.0.1.json",
     "pid": "notif1",

tests/fixtures/circulation.py

@@ -117,6 +117,30 @@ def librarian_martigny(
     yield create_patron(data)
 
 
+# ------------ Org: Martigny, Lib: Martigny-Bourg, Librarian 1 ----------
+@pytest.fixture(scope="module")
+def librarian_martigny_bourg_data(data):
+    """Load Martigny librarian data."""
+    return deepcopy(data.get('ptrn13'))
+
+
+@pytest.fixture(scope="function")
+def librarian_martigny_bourg_data_tmp(data):
+    """Load Martigny librarian data scope function."""
+    return deepcopy(data.get('ptrn13'))
+
+
+@pytest.fixture(scope="module")
+def librarian_martigny_bourg(
+        app,
+        roles,
+        lib_martigny_bourg,
+        librarian_martigny_bourg_data):
+    """Create Martigny bourg librarian record."""
+    data = librarian_martigny_bourg_data
+    yield create_patron(data)
+
+
 # ------------ Org: Martigny, Lib: Martigny, Librarian 2 ----------
 @pytest.fixture(scope="module")
 def librarian2_martigny_data(data):