circulation: allow checkout from never open lib

- Fixes infinite loop when checking out an item from library without
open days but with open exception hours.
- Changes `_has_is_open` to `has_open_day` function. It now returns
only returns True for exception dates without specified non-standard
hours.
- Fixes LibraryNeverOpen error not being passed to monitoring.
- Fixes checkout not possible from library without opening hours.
- Closes #2419.

Co-Authored-by: Pascal Repond <pascal.repond@rero.ch>
Co-Authored-by: Johnny Mariéthoz <Johnny.Mariethoz@rero.ch>

rero_ils/modules/items/api/circulation.py

@@ -45,6 +45,7 @@
 from ...errors import NoCirculationAction
 from ...item_types.api import ItemType
 from ...libraries.api import Library
+from ...libraries.exceptions import LibraryNeverOpen
 from ...loans.api import Loan, get_last_transaction_loc_for_item, \
     get_request_by_item_pid_by_patron_pid
 from ...loans.models import LoanAction, LoanState
@@ -420,10 +421,15 @@ def checkout(self, current_loan, **kwargs):
                 transaction_library_pid = self.library_pid
             library = Library.get_record_by_pid(transaction_library_pid)
             if not library.is_open(action_params['end_date'], True):
-                new_end_date = library.next_open(action_params['end_date'])
-                new_end_date = new_end_date.astimezone()\
-                    .replace(microsecond=0).isoformat()
-                action_params['end_date'] = new_end_date
+                try:
+                    new_end_date = library.next_open(action_params['end_date'])
+                    new_end_date = new_end_date.astimezone()\
+                        .replace(microsecond=0).isoformat()
+                    action_params['end_date'] = new_end_date
+                except LibraryNeverOpen:
+                    # If library has no open dates, keep the default due date
+                    # to avoid circulation errors
+                    pass
         # Call invenio_circulation for 'checkout' trigger
         loan = current_circulation.circulation.trigger(
             current_loan,

rero_ils/modules/libraries/api.py

@@ -167,15 +167,18 @@ def _is_betweentimes(self, time_to_test, times):
                                             (time_to_test <= end_time))
         return times_open
 
-    def _has_is_open(self):
-        """Test if library has opening days."""
+    def _has_open_day(self):
+        """Test if library has opening days with standard times."""
         if opening_hours := self.get('opening_hours'):
             for opening_hour in opening_hours:
                 if opening_hour['is_open']:
                     return True
         if exception_dates := self.get('exception_dates'):
             for exception_date in exception_dates:
-                if exception_date['is_open']:
+                # ignore exception open dates if they have non-standard hours
+                # a non-standard open day is not considered open for circ
+                if exception_date['is_open'] \
+                        and not exception_date.get('times'):
                     return True
         return False
 
@@ -241,7 +244,7 @@ def is_open(self, date=None, day_only=False):
         #   is into this periods (depending of `day_only` method argument).
         day_name = date.strftime("%A").lower()
         regular_rule = [
-            rule for rule in self['opening_hours']
+            rule for rule in self.get('opening_hours', [])
             if rule['day'] == day_name
         ]
         if regular_rule:
@@ -279,8 +282,10 @@ def _get_opening_hour_by_day(self, day_name):
     def next_open(self, date=None, previous=False, ensure=False):
         """Get next open day."""
         date = date or datetime.now(pytz.utc)
-        if not self._has_is_open():
-            raise LibraryNeverOpen
+        if not self._has_open_day():
+            raise LibraryNeverOpen(
+                f'No open days found for library (pid: {self.pid})'
+                )
         if isinstance(date, str):
             date = parser.parse(date)
         add_day = -1 if previous else 1

rero_ils/modules/loans/utils.py

@@ -26,6 +26,7 @@
 from rero_ils.modules.circ_policies.api import CircPolicy
 from rero_ils.modules.items.api import Item
 from rero_ils.modules.libraries.api import Library
+from rero_ils.modules.libraries.exceptions import LibraryNeverOpen
 from rero_ils.modules.locations.api import Location
 from rero_ils.modules.patrons.api import Patron
 from rero_ils.modules.utils import get_ref_for_pid
@@ -83,17 +84,21 @@ def get_default_loan_duration(loan, initial_loan):
     due_date_eve = now_in_library_timezone \
         + timedelta(days=policy.get('checkout_duration', 0)) \
         - timedelta(days=1)
-    next_open_date = library.next_open(date=due_date_eve)
-    # all libraries are closed at 23h59
-    # the next_open returns UTC.
-    end_date_in_library_timezone = next_open_date.astimezone(
-        library.get_timezone()).replace(
-            hour=23,
-            minute=59,
-            second=0,
-            microsecond=0
-        )
-    return end_date_in_library_timezone - now_in_library_timezone
+    try:
+        next_open_date = library.next_open(date=due_date_eve)
+        # all libraries are closed at 23h59
+        # the next_open returns UTC.
+        end_date_in_library_timezone = next_open_date.astimezone(
+            library.get_timezone()).replace(
+                hour=23,
+                minute=59,
+                second=0,
+                microsecond=0
+            )
+        return end_date_in_library_timezone - now_in_library_timezone
+    except LibraryNeverOpen:
+        # if the library has no open day, use standard loan duration from cipo
+        return due_date_eve - now_in_library_timezone
 
 
 def get_extension_params(loan=None, initial_loan=None, parameter_name=None):

tests/api/libraries/test_libraries_rest.py

@@ -147,13 +147,21 @@ def test_non_circulating_libraries(
 
 def test_library_never_open(lib_sion):
     """Test library with no opening hours."""
-    assert lib_sion._has_is_open()
+    assert lib_sion._has_open_day()
     assert lib_sion.next_open()
 
     del lib_sion['opening_hours']
+    # Add a standard open day in the exceptions data
+    lib_sion['exception_dates'].append(
+        {
+            'is_open': True,
+            'start_date': '2022-06-15',
+            'title': 'Ouverture exceptionnelle'
+        }
+    )
     lib_sion.update(lib_sion, dbcommit=True, reindex=True)
 
-    assert lib_sion._has_is_open()
+    assert lib_sion._has_open_day()
 
     del lib_sion['exception_dates']
     lib_sion.update(lib_sion, dbcommit=True, reindex=True)

tests/ui/circulation/test_actions_checkout.py

@@ -31,6 +31,66 @@
 from rero_ils.modules.loans.models import LoanAction, LoanState
 
 
+def test_checkout_library_never_open(
+        circulation_policies,
+        patron_martigny,
+        lib_martigny,
+        item_lib_martigny,
+        loc_public_martigny,
+        librarian_martigny
+        ):
+    """Test checkout from a library without opening hours."""
+
+    # Test checkout if library has no open days but has exception days/hours
+    del lib_martigny['opening_hours']
+    lib_martigny.commit()
+
+    data = deepcopy(item_lib_martigny)
+    data.pop('barcode')
+    data.setdefault('status', ItemStatus.ON_SHELF)
+    created_item = Item.create(
+        data=data, dbcommit=True, reindex=True, delete_pid=True)
+
+    params = {
+        'patron_pid': patron_martigny.pid,
+        'transaction_location_pid': loc_public_martigny.pid,
+        'transaction_user_pid': librarian_martigny.pid,
+        'pickup_location_pid': loc_public_martigny.pid
+    }
+    onloan_item, actions = created_item.checkout(**params)
+    loan = Loan.get_record_by_pid(actions[LoanAction.CHECKOUT].get('pid'))
+
+    # Check loan is ITEM_ON_LOAN and item is ON_LOAN
+    assert onloan_item.status == ItemStatus.ON_LOAN
+    assert loan['state'] == LoanState.ITEM_ON_LOAN
+
+    # Test checkout if library has no open days and no exception days/hours
+    del lib_martigny['exception_dates']
+    lib_martigny.commit()
+
+    data = deepcopy(item_lib_martigny)
+    data.pop('barcode')
+    data.setdefault('status', ItemStatus.ON_SHELF)
+    created_item = Item.create(
+        data=data, dbcommit=True, reindex=True, delete_pid=True)
+
+    params = {
+        'patron_pid': patron_martigny.pid,
+        'transaction_location_pid': loc_public_martigny.pid,
+        'transaction_user_pid': librarian_martigny.pid,
+        'pickup_location_pid': loc_public_martigny.pid
+    }
+    onloan_item, actions = created_item.checkout(**params)
+    loan = Loan.get_record_by_pid(actions[LoanAction.CHECKOUT].get('pid'))
+
+    # Check loan is ITEM_ON_LOAN and item is ON_LOAN
+    assert onloan_item.status == ItemStatus.ON_LOAN
+    assert loan['state'] == LoanState.ITEM_ON_LOAN
+
+    from invenio_db import db
+    db.session.rollback()
+
+
 def test_checkout_on_item_on_shelf(
         circulation_policies,
         patron_martigny,