loans: improve due date timezone consideration

* Closes #599 by using correct timezone for due date (it takes into
account opening hours and exception dates correctly)
* tests due date timezone
* adds get_timezone() on Library that returns BABEL_DEFAULT_TIMEZONE as default
* changes due date process to take into account Library timezone
* adds library_pid property on Loan objects

Co-Authored-by: Olivier DOSSMANN <[email protected]>

.travis.yml

@@ -53,6 +53,8 @@ env:
     - E2E_OUTPUT=base64
     # Enable end-to-end tests
     - E2E=no
+    # Enable Europe/Zurich timezone
+    - TZ="Europe/Zurich"
   matrix:
     - REQUIREMENTS="--deploy" E2E=yes
     - REQUIREMENTS=""

rero_ils/filter.py

@@ -25,8 +25,8 @@
 
 
 def format_date_filter(date_str, format='medium', locale='en'):
-    """Format the date."""
-    form_date = dateparser.parse(str(date_str))
+    """Format the date to the given locale."""
+    form_date = dateparser.parse(str(date_str), locales=[locale, 'en'])
     if format == 'full':
         format = "EEEE, d. MMMM y"
     elif format == 'medium':

rero_ils/modules/documents/views.py

@@ -39,6 +39,7 @@
 from ..holdings.api import Holding
 from ..items.api import Item, ItemStatus
 from ..libraries.api import Library
+from ..loans.api import Loan
 from ..loans.utils import can_be_requested
 from ..locations.api import Location
 from ..organisations.api import Organisation
@@ -203,11 +204,14 @@ def can_request(item):
             if 'patron' in patron.get('roles') and \
                 patron.get_organisation()['pid'] == \
                     item.get_library().replace_refs()['organisation']['pid']:
-                # TODO: Virtual Loan
-                loan = {
-                    'item_pid': item.pid,
-                    'patron_pid': patron.pid
-                }
+                # Complete metadata before Loan creation
+                loan_metadata = dict(item)
+                if 'item_pid' not in loan_metadata:
+                    loan_metadata['item_pid'] = item.pid
+                if 'patron_pid' not in loan_metadata:
+                    loan_metadata['patron_pid'] = patron.pid
+                # Create "virtual" Loan (not registered)
+                loan = Loan(loan_metadata)
                 if not can_be_requested(loan):
                     return False
                 patron_barcode = patron.get('barcode')

rero_ils/modules/items/views.py

@@ -21,8 +21,6 @@
 
 from functools import wraps
 
-import pytz
-from dateutil import parser
 from flask import Blueprint, current_app, flash, jsonify, redirect, \
     render_template, url_for
 from flask_babelex import gettext as _
@@ -35,7 +33,6 @@
 from ..item_types.api import ItemType
 from ..libraries.api import Library
 from ..locations.api import Location
-from ...filter import format_date_filter
 from ...permissions import request_item_permission
 
 
@@ -131,9 +128,7 @@ def item_availability_text(item):
     else:
         text = ''
         if item.status == ItemStatus.ON_LOAN:
-            due_date = pytz.utc.localize(parser.parse(
-                item.get_item_end_date()))
-            due_date = format_date_filter(due_date, format='short_date')
+            due_date = item.get_item_end_date(format='short_date')
             text = '{msg} {date}'.format(
                 msg=_('due until'),
                 date=due_date)

rero_ils/modules/libraries/api.py

@@ -17,9 +17,10 @@
 
 """API for manipulating libraries."""
 
-from datetime import datetime, timedelta, timezone
+from datetime import datetime, timedelta
 from functools import partial
 
+import pytz
 from dateutil import parser
 from dateutil.rrule import FREQNAMES, rrule
 from invenio_search.api import RecordsSearch
@@ -110,10 +111,13 @@ def _is_betweentimes(self, time_to_test, times):
 
     def _is_in_period(self, datetime_to_test, exception_date, day_only):
         """Test if date is period."""
-        start_date = date_string_to_utc(exception_date['start_date'])
+        start_date = exception_date['start_date']
+        if isinstance(exception_date['start_date'], str):
+            start_date = date_string_to_utc(start_date)
         end_date = exception_date.get('end_date')
         if end_date:
-            end_date = date_string_to_utc(end_date)
+            if isinstance(end_date, str):
+                end_date = date_string_to_utc(end_date)
             is_in_period = (
                 datetime_to_test.date() - start_date.date()
             ).days >= 0
@@ -150,10 +154,11 @@ def _has_exception(self, _open, date, exception_dates,
         """Test the day has an exception."""
         exception = _open
         for exception_date in exception_dates:
-            start_date = date_string_to_utc(exception_date['start_date'])
+            if isinstance(exception_date['start_date'], str):
+                start_date = date_string_to_utc(exception_date['start_date'])
             repeat = exception_date.get('repeat')
             if _open:
-                # test for exceptios closed
+                # test for closed exceptions
                 if not exception_date['is_open']:
                     has_period, is_in_period = self._is_in_period(
                         date,
@@ -171,7 +176,7 @@ def _has_exception(self, _open, date, exception_dates,
                     if not exception:
                         return False
             else:
-                # test for exceptions opened
+                # test for opened exceptions
                 if exception_date['is_open']:
                     if self._is_in_repeat(date, start_date, repeat):
                         exception = True
@@ -197,12 +202,16 @@ def _has_is_open(self):
                     return True
         return False
 
-    def is_open(self, date=datetime.now(timezone.utc), day_only=False):
+    def is_open(self, date=datetime.now(pytz.utc), day_only=False):
         """Test library is open."""
         _open = False
 
+        # Change date to be aware and with timezone.
         if isinstance(date, str):
             date = date_string_to_utc(date)
+        if isinstance(date, datetime):
+            if date.tzinfo is None:
+                date = date.replace(tzinfo=pytz.utc)
         day_name = date.strftime("%A").lower()
         for opening_hour in self['opening_hours']:
             if day_name == opening_hour['day']:
@@ -225,7 +234,7 @@ def is_open(self, date=datetime.now(timezone.utc), day_only=False):
                 times_open = not times_open
         return times_open
 
-    def next_open(self, date=datetime.now(timezone.utc), previous=False):
+    def next_open(self, date=datetime.now(pytz.utc), previous=False):
         """Get next open day."""
         if not self._has_is_open():
             raise LibraryNeverOpen
@@ -239,8 +248,8 @@ def next_open(self, date=datetime.now(timezone.utc), previous=False):
             date += timedelta(days=add_day)
         return date
 
-    def count_open(self, start_date=datetime.now(timezone.utc),
-                   end_date=datetime.now(timezone.utc), day_only=False):
+    def count_open(self, start_date=datetime.now(pytz.utc),
+                   end_date=datetime.now(pytz.utc), day_only=False):
         """Get next open day."""
         if isinstance(start_date, str):
             start_date = date_string_to_utc(start_date)
@@ -255,7 +264,7 @@ def count_open(self, start_date=datetime.now(timezone.utc),
             start_date += timedelta(days=1)
         return count
 
-    def in_working_days(self, count, date=datetime.now(timezone.utc)):
+    def in_working_days(self, count, date=datetime.now(pytz.utc)):
         """Get date for given working days."""
         counting = 1
         if isinstance(date, str):
@@ -297,3 +306,9 @@ def reasons_not_to_delete(self):
         if links:
             cannot_delete['links'] = links
         return cannot_delete
+
+    def get_timezone(self):
+        """Get library timezone. By default use BABEL_DEFAULT_TIMEZONE."""
+        # TODO: get timezone regarding Library address
+        default = pytz.timezone('Europe/Zurich')
+        return default

rero_ils/modules/loans/api.py

@@ -149,6 +149,22 @@ def organisation_pid(self):
             return item.organisation_pid
         return None
 
+    @property
+    def library_pid(self):
+        """Get library PID regarding transaction location PID or location."""
+        from ..items.api import Item
+
+        location_pid = self.get('transaction_location_pid')
+        item_pid = self.get('item_pid')
+
+        if not location_pid and item_pid:
+            item = Item.get_record_by_pid(item_pid)
+            return item.holding_library_pid
+        elif location_pid:
+            loc = Location.get_record_by_pid(location_pid)
+            return loc.library_pid
+        return None
+
     def dumps_for_circulation(self):
         """Dumps for circulation."""
         loan = self.replace_refs()

rero_ils/modules/loans/utils.py

@@ -24,20 +24,14 @@
 from ..circ_policies.api import CircPolicy
 from ..items.api import Item
 from ..libraries.api import Library
-from ..locations.api import Location
 from ..patrons.api import Patron
 
 
 def get_circ_policy(loan):
     """Return a circ policy for loan."""
     item = Item.get_record_by_pid(loan.get('item_pid'))
     holding_circulation_category = item.holding_circulation_category_pid
-    transaction_location_pid = loan.get('transaction_location_pid')
-    if not transaction_location_pid:
-        library_pid = item.holding_library_pid
-    else:
-        library_pid = \
-            Location.get_record_by_pid(transaction_location_pid).library_pid
+    library_pid = loan.library_pid
     patron = Patron.get_record_by_pid(loan.get('patron_pid'))
     patron_type_pid = patron.patron_type_pid
 
@@ -50,27 +44,31 @@ def get_circ_policy(loan):
 
 def get_default_loan_duration(loan):
     """Return calculated checkout duration in number of days."""
+    # TODO: case when 'now' is not sysdate.
+    now = datetime.utcnow()
+
+    # Get library (to check opening hours and get timezone)
+    library = Library.get_record_by_pid(loan.library_pid)
+
+    # Process difference between now and end of day in term of hours/minutes
+    #   - use hours and minutes from now
+    #   - check regarding end of day (eod), 23:59
+    #   - correct the hours/date regarding library timezone
+    eod = timedelta(hours=23, minutes=59)
+    aware_eod = eod - library.get_timezone().utcoffset(now, is_dst=True)
+    time_to_eod = aware_eod - timedelta(hours=now.hour, minutes=now.minute)
+
+    # Due date should be defined differently from checkout_duration
+    # For that we use:
+    #   - expected due date (now + checkout_duration)
+    #   - next library open date (the eve of exepected due date is used)
+    # We finally make the difference between next library open date and now.
+    # We apply a correction for hour/minute to be 23:59 (end of day).
     policy = get_circ_policy(loan)
-    # TODO: case when start_date is not sysdate.
-    start_date = datetime.now(timezone.utc)
-    time_to_end_of_day = timedelta(hours=23, minutes=59) - \
-        timedelta(hours=start_date.hour, minutes=start_date.minute)
-    transaction_location_pid = loan.get('transaction_location_pid')
-    if not transaction_location_pid:
-        library_pid = Item.get_record_by_pid(loan.item_pid).holding_library_pid
-    else:
-        library_pid = \
-            Location.get_record_by_pid(transaction_location_pid).library_pid
-
-    library = Library.get_record_by_pid(library_pid)
-    # invenio-circulation due_date.
-    due_date = start_date + timedelta(days=policy.get('checkout_duration'))
-    # rero_ils due_date, considering library opening_hours and exception_dates.
-    # next_open: -1 to check first the due date not the days.
-    open_after_due_date = library.next_open(date=due_date - timedelta(days=1))
-    new_duration = open_after_due_date - start_date
-
-    return timedelta(days=new_duration.days) + time_to_end_of_day
+    due_date_eve = now + timedelta(days=policy.get('checkout_duration')) - \
+        timedelta(days=1)
+    next_open_date = library.next_open(date=due_date_eve)
+    return timedelta(days=(next_open_date - now).days) + time_to_eod
 
 
 def get_extension_params(loan=None, parameter_name=None):
@@ -81,19 +79,17 @@ def get_extension_params(loan=None, parameter_name=None):
         'max_count': policy.get('number_renewals'),
         'duration_default': policy.get('renewal_duration')
     }
-    current_date = datetime.now(timezone.utc)
-    time_to_end_of_day = timedelta(hours=23, minutes=59) - \
-        timedelta(hours=current_date.hour, minutes=current_date.minute)
-
-    transaction_location_pid = loan.get('transaction_location_pid')
-    if not transaction_location_pid:
-        library_pid = Item.get_record_by_pid(loan.item_pid).holding_library_pid
-    else:
-        library_pid = \
-            Location.get_record_by_pid(transaction_location_pid).library_pid
-    library = Library.get_record_by_pid(library_pid)
 
-    calculated_due_date = current_date + timedelta(
+    # Get library (to check opening hours)
+    library = Library.get_record_by_pid(loan.library_pid)
+
+    now = datetime.utcnow()
+    # Fix end of day regarding Library timezone
+    eod = timedelta(hours=23, minutes=59)
+    aware_eod = eod - library.get_timezone().utcoffset(now, is_dst=True)
+    time_to_eod = aware_eod - timedelta(hours=now.hour, minutes=now.minute)
+
+    calculated_due_date = now + timedelta(
         days=policy.get('renewal_duration'))
 
     first_open_date = library.next_open(
@@ -102,9 +98,9 @@ def get_extension_params(loan=None, parameter_name=None):
     if first_open_date.date() < end_date.date():
         params['max_count'] = 0
 
-    new_duration = first_open_date - current_date
+    new_duration = first_open_date - now
     params['duration_default'] = \
-        timedelta(days=new_duration.days) + time_to_end_of_day
+        timedelta(days=new_duration.days) + time_to_eod
 
     return params.get(parameter_name)