circulation: allow checkout from never open lib

- Fixes infinite loop when checking out an item from library without open days but with open exception days. - Fixes LibraryNeverOpen error not being passed to monitoring. - Closes rero#2419. Co-Authored-by: Pascal Repond <pascal.repond@rero.ch> Co-Authored-by: Johnny Mariéthoz <Johnny.Mariethoz@rero.ch>
PascalRepond · Jun 15, 2023 · 70a9177 · 70a9177
1 parent eeca0e2
commit 70a9177
Show file tree

Hide file tree

Showing 5 changed files with 151 additions and 12 deletions.
diff --git a/rero_ils/modules/items/api/circulation.py b/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,

diff --git a/rero_ils/modules/libraries/api.py b/rero_ils/modules/libraries/api.py
@@ -175,7 +175,7 @@ def _has_is_open(self):
                     return True
         if exception_dates := self.get('exception_dates'):
             for exception_date in exception_dates:
-                if exception_date['is_open']:
+                if exception_date['is_open'] and not exception_date.get('times'):
                     return True
         return False
 
@@ -241,7 +241,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:
@@ -280,7 +280,7 @@ 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
+            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

diff --git a/rero_ils/modules/loans/utils.py b/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,20 @@ 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(
+    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
+        return end_date_in_library_timezone - now_in_library_timezone
+    except LibraryNeverOpen:
+        return due_date_eve - now_in_library_timezone
 
 
 def get_extension_params(loan=None, initial_loan=None, parameter_name=None):

diff --git a/tests/api/circulation/test_actions_views_checkout.py b/tests/api/circulation/test_actions_views_checkout.py
@@ -20,6 +20,7 @@
 
 import ciso8601
 from invenio_accounts.testutils import login_user_via_session
+from invenio_db import db
 from utils import postdata
 
 from rero_ils.modules.items.models import ItemStatus
@@ -155,3 +156,71 @@ def test_checkout(
     transaction_end_date = ciso8601.parse_datetime(transaction_end_date)
     next_open_date = lib_martigny.next_open(next_saturday)
     assert next_open_date.date() == transaction_end_date.date()
+
+    # Test checkout from library with no opening hours
+    res, _ = postdata(
+        client,
+        'api_item.checkin',
+        dict(
+            item_pid=item.pid,
+            transaction_library_pid=lib_martigny.pid,
+            transaction_user_pid=librarian_martigny.pid
+        )
+    )
+    assert res.status_code == 200
+    assert item.status == ItemStatus.ON_SHELF
+
+    lib_martigny['opening_hours'] = [
+        {
+            "day": "monday",
+            "is_open": False,
+            "times": []
+        },
+        {
+            "day": "tuesday",
+            "is_open": False,
+            "times": []
+        },
+        {
+            "day": "wednesday",
+            "is_open": False,
+            "times": []
+        },
+        {
+            "day": "thursday",
+            "is_open": False,
+            "times": []
+        },
+        {
+            "day": "friday",
+            "is_open": False,
+            "times": []
+        },
+        {
+            "day": "saturday",
+            "is_open": False,
+            "times": []
+        },
+        {
+            "day": "sunday",
+            "is_open": False,
+            "times": []
+        }
+    ]
+    db.session.commit()
+
+    params = dict(
+        item_pid=item.pid,
+        patron_pid=patron_martigny.pid,
+        transaction_user_pid=librarian_martigny.pid,
+        transaction_location_pid=loc_public_martigny.pid
+    )
+
+    res, _ = postdata(
+        client,
+        'api_item.checkout',
+        params
+    )
+
+    assert res.status_code == 200
+    db.session.rollback()
diff --git a/tests/ui/circulation/test_actions_checkout.py b/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."""
+
+    # Check 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 if library has no open days but has exception hours
+    # 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
+
+    # Check 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,