circulation: support non-circulating libraries

The transaction locations of a library is selected in the list of pickup
locations of the library.  This causes issues with libraries that have
no pickup location setted.

With this commit, for libraries with no pickup locations the system
considers the first library location as transaction location for
circulation transactions. These libraries can now checkout items
directly from other libraries and once checked-in back they go
in_transit.
It is still mandatory to setup library hours and notification
settings before checkout items.

* Upgrades to invenio-circulation v1.0.0a34.
* Supports checkout of items of libraries with no opening hours externally.
* Fixes problem when in transit flash message was always displaying the pickup library.
* Fixes situation A of the issue 2419.
* Supports checkout of items of libraries with no pickup locations.
* Closes #2367.
* Adds a non-circulating library to units testing.

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

pyproject.toml

@@ -30,7 +30,7 @@ uwsgitop = ">=0.11"
 
 ## Third party invenio modules used by RERO ILS
 invenio-oaiharvester = {tag = "v1.0.0a4", git = "https://github.com/inveniosoftware/invenio-oaiharvester.git"}
-invenio-circulation = {tag = "v1.0.0a33", git = "https://github.com/inveniosoftware/invenio-circulation.git"}
+invenio-circulation = {tag = "v1.0.0a34", git = "https://github.com/inveniosoftware/invenio-circulation.git"}
 
 ## Invenio 3.4 base modules used by RERO ILS
 # same as invenio metadata extras without invenio-search-ui

rero_ils/config.py

@@ -67,7 +67,8 @@
 from .modules.items.api import Item
 from .modules.items.models import ItemCirculationAction, ItemIssueStatus
 from .modules.items.permissions import ItemPermission
-from .modules.items.utils import item_location_retriever
+from .modules.items.utils import item_location_retriever, \
+    same_location_validator
 from .modules.libraries.api import Library
 from .modules.libraries.permissions import LibraryPermission
 from .modules.loans.api import Loan, LoanState
@@ -2598,6 +2599,9 @@ def _(x):
 
 CIRCULATION_LOAN_LOCATIONS_VALIDATION = \
     validate_item_pickup_transaction_locations
+
+CIRCULATION_SAME_LOCATION_VALIDATOR = same_location_validator
+
 """Validates the item, pickup and transaction locations of pending loans."""
 
 # This is needed for absolute URL (url_for)

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/items/api/record.py

@@ -484,12 +484,15 @@ def organisation_view(self):
         return organisation['view_code']
 
     def get_owning_pickup_location_pid(self):
-        """Returns the pickup location for the item owning location.
+        """Returns the pickup location pid for the item owning location.
 
-        :return the pid of the item owning item location.
+        case when library has no pickup location defined, we return the item
+        owning location pid.
+
+        :return location pid.
         """
         library = self.get_library()
-        return library.get_pickup_location_pid()
+        return library.get_pickup_location_pid() or self.location_pid
 
     @property
     def notes(self):

rero_ils/modules/items/utils.py

@@ -17,6 +17,8 @@
 
 """Item utils."""
 
+from rero_ils.modules.locations.api import LocationsSearch
+
 
 def item_pid_to_object(item_pid):
     """Build an item_pid object from a given item pid.
@@ -50,6 +52,22 @@ def item_location_retriever(item_pid):
         return item.get_owning_pickup_location_pid()
 
 
+def same_location_validator(item_pid, input_location_pid):
+    """Validate that item and transaction location are in same library.
+
+    :param item_pid: the item_pid object
+    :type input_location_pid: object
+    :return: true if in same library otherwise false
+    :rtype: boolean
+    """
+    from rero_ils.modules.items.api import ItemsSearch
+    lib_from_loc = LocationsSearch().get_record_by_pid(
+        input_location_pid).library.pid
+    lib_from_item = ItemsSearch().get_record_by_pid(
+        item_pid.get('value')).library.pid
+    return lib_from_loc == lib_from_item
+
+
 def exists_available_item(items=[]):
     """Check if at least one item of the list are available.
 

rero_ils/modules/libraries/api.py

@@ -98,24 +98,40 @@ def get_organisation(self):
         from ..organisations.api import Organisation
         return Organisation.get_record_by_pid(self.organisation_pid)
 
-    def pickup_location_query(self):
+    def _pickup_location_query(self):
         """Search the location index for pickup locations."""
-        return LocationsSearch().filter(
-            'term', library__pid=self.pid).filter(
-                'term', is_pickup=True).source(['pid']).scan()
+        return LocationsSearch() \
+            .filter('term', library__pid=self.pid) \
+            .filter('term', is_pickup=True) \
+            .source(['pid']) \
+            .scan()
+
+    def location_pids(self):
+        """Return a generator of ES Hits of all pids of 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():
+        for location in self._pickup_location_query():
             yield location.pid
 
     def get_pickup_location_pid(self):
-        """Returns libraries first pickup location pid."""
+        """Returns one picup location pid for a library."""
         try:
-            return next(self.pickup_location_query()).pid
+            return next(self._pickup_location_query()).pid
         except StopIteration:
             return None
 
+    def get_transaction_location_pid(self):
+        """Returns one pickup or one transaction location pid for a library."""
+        try:
+            return next(self._pickup_location_query()).pid
+        except StopIteration:
+            return next(self.location_pids()).pid
+
     def _is_betweentimes(self, time_to_test, times):
         """Test if time is between times."""
         times_open = False

rero_ils/modules/loans/api.py

@@ -141,10 +141,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
         )
@@ -154,7 +160,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():
@@ -642,6 +648,16 @@ def location_pid(self):
             'library_pid'
         )
 
+    @property
+    def pickup_location_pid(self):
+        """Get loan pickup_location PID."""
+        return self.get('pickup_location_pid')
+
+    @property
+    def transaction_location_pid(self):
+        """Get loan transaction_location PID."""
+        return self.get('transaction_location_pid')
+
     @property
     def get_overdue_fees(self):
         """Get all overdue fees based based on incremental fees setting.
@@ -732,7 +748,7 @@ def dumps_for_circulation(self):
         data['rank'] = item.patron_request_rank(patron)
         if item.status == ItemStatus.IN_TRANSIT:
             destination_loc_pid = item.location_pid
-            if LoanState.ITEM_IN_TRANSIT_FOR_PICKUP:
+            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()