loans: add parameter to sort pending loans

* Adds sort order parameter to the item.requested_loans.library_pid api.
* Adds sort order parameter to the item.loans.patron_barcode api.
* Improves notification fixtures to have due_soon and recall records.
* Fixes item fixtures problem.

Co-Authored-by: Aly Badr <[email protected]>

rero_ils/modules/items/api.py

@@ -295,7 +295,7 @@ def item_link_to_holding(self):
         self.commit()
         self.dbcommit(reindex=True, forceindex=True)
 
-    def dumps_for_circulation(self):
+    def dumps_for_circulation(self, sort_by=None):
         """Enhance item information for api_views."""
         item = self.replace_refs()
         data = item.dumps()
@@ -315,7 +315,7 @@ def dumps_for_circulation(self):
         data['actions'] = list(self.actions)
         data['available'] = self.available
         # data['number_of_requests'] = self.number_of_requests()
-        for loan in self.get_requests():
+        for loan in self.get_requests(sort_by=sort_by):
             data.setdefault('pending_loans',
                             []).append(loan.dumps_for_circulation())
         return data
@@ -394,44 +394,59 @@ def get_item_by_barcode(cls, barcode=None):
             return None
 
     @classmethod
-    def get_pendings_loans(cls, library_pid):
-        """Returns list of pending loand for a given library."""
-        # check library exists
+    def get_pendings_loans(cls, library_pid=None, sort_by='transaction_date'):
+        """Return list of sorted pending loans for a given library.
+
+        default sort is set to transaction_date
+        """
+        # check if library exists
         lib = Library.get_record_by_pid(library_pid)
         if not lib:
             raise Exception('Invalid Library PID')
-
-        results = current_circulation.loan_search\
+        # the '-' prefix means a desc order.
+        sort_by = sort_by or 'transaction_date'
+        order_by = 'asc'
+        if sort_by.startswith('-'):
+            sort_by = sort_by[1:]
+            order_by = 'desc'
+        search = current_circulation.loan_search\
             .source(['pid'])\
             .params(preserve_order=True)\
             .filter('term', state='PENDING')\
             .filter('term', library_pid=library_pid)\
-            .sort({'transaction_date': {'order': 'asc'}})\
-            .scan()
+            .sort({sort_by: {"order": order_by}})
+        results = search.scan()
         for loan in results:
             yield Loan.get_record_by_pid(loan.pid)
 
     @classmethod
-    def get_checked_out_loans(cls, patron_pid):
-        """Returns checked out loans for a given patron."""
+    def get_checked_out_loans(
+            cls, patron_pid=None, sort_by='transaction_date'):
+        """Returns sorted checked out loans for a given patron."""
         # check library exists
         patron = Patron.get_record_by_pid(patron_pid)
         if not patron:
             raise InvalidRecordID('Invalid Patron PID')
-        results = current_circulation.loan_search\
-            .source(['pid'])\
+        # the '-' prefix means a desc order.
+        sort_by = sort_by or 'transaction_date'
+        order_by = 'asc'
+        if sort_by.startswith('-'):
+            sort_by = sort_by[1:]
+            order_by = 'desc'
+
+        results = current_circulation.loan_search.source(['pid'])\
             .params(preserve_order=True)\
             .filter('term', state='ITEM_ON_LOAN')\
             .filter('term', patron_pid=patron_pid)\
-            .sort({'transaction_date': {'order': 'asc'}})\
-            .scan()
+            .sort({sort_by: {"order": order_by}}).scan()
         for loan in results:
             yield Loan.get_record_by_pid(loan.pid)
 
     @classmethod
-    def get_checked_out_items(cls, patron_pid):
-        """Return checked out items for a given patron."""
-        loans = cls.get_checked_out_loans(patron_pid)
+    def get_checked_out_items(cls, patron_pid=None, sort_by=None):
+        """Return sorted checked out items for a given patron."""
+        loans = cls.get_checked_out_loans(
+            patron_pid=patron_pid, sort_by=sort_by)
         returned_item_pids = []
         for loan in loans:
             item_pid = loan.get('item_pid')
@@ -441,23 +456,32 @@ def get_checked_out_items(cls, patron_pid):
                 returned_item_pids.append(item_pid)
                 yield item, loan
 
-    def get_requests(self):
-        """Return any pending, item_on_transit, item_at_desk loans."""
+    def get_requests(self, sort_by=None):
+        """Return sorted pending, item_on_transit, item_at_desk loans.
+
+        default sort is transaction_date.
+        """
         search = search_by_pid(
             item_pid=self.pid, filter_states=[
                 'PENDING',
                 'ITEM_AT_DESK',
                 'ITEM_IN_TRANSIT_FOR_PICKUP'
-            ]).params(preserve_order=True)\
-            .source(['pid'])\
-            .sort({'transaction_date': {'order': 'asc'}})
+            ]).params(preserve_order=True).source(['pid'])
+        order_by = 'asc'
+        sort_by = sort_by or 'transaction_date'
+        if sort_by.startswith('-'):
+            sort_by = sort_by[1:]
+            order_by = 'desc'
+        search = search.sort({sort_by: {'order': order_by}})
         for result in search.scan():
             yield Loan.get_record_by_pid(result.pid)
 
     @classmethod
-    def get_requests_to_validate(cls, library_pid):
+    def get_requests_to_validate(
+            cls, library_pid=None, sort_by=None):
         """Returns list of requests to validate for a given library."""
-        loans = cls.get_pendings_loans(library_pid)
+        loans = cls.get_pendings_loans(
+            library_pid=library_pid, sort_by=sort_by)
         returned_item_pids = []
         for loan in loans:
             item_pid = loan.get('item_pid')

rero_ils/modules/items/api_views.py

@@ -215,12 +215,14 @@ def extend_loan(item, data):
 @check_authentication
 @jsonify_error
 def requested_loans(library_pid):
-    """HTTP GET request for requested loans for a library."""
-    items_loans = Item.get_requests_to_validate(library_pid)
+    """HTTP GET request for sorted requested loans for a library."""
+    sort_by = flask_request.args.get('sort')
+    items_loans = Item.get_requests_to_validate(
+        library_pid=library_pid, sort_by=sort_by)
     metadata = []
     for item, loan in items_loans:
         metadata.append({
-            'item': item.dumps_for_circulation(),
+            'item': item.dumps_for_circulation(sort_by=sort_by),
             'loan': loan.dumps_for_circulation()
         })
     return jsonify({
@@ -235,11 +237,13 @@ def requested_loans(library_pid):
 @check_authentication
 @jsonify_error
 def loans(patron_pid):
-    """HTTP GET request for requested loans for a library."""
-    items_loans = Item.get_checked_out_items(patron_pid)
+    """HTTP GET request for sorted loans for a patron pid."""
+    sort_by = flask_request.args.get('sort')
+    items_loans = Item.get_checked_out_items(
+        patron_pid=patron_pid, sort_by=sort_by)
     metadata = []
     for item, loan in items_loans:
-        item_dumps = item.dumps_for_circulation()
+        item_dumps = item.dumps_for_circulation(sort_by=sort_by)
         metadata.append({
             'item': item_dumps,
             'loan': loan.dumps_for_circulation()

rero_ils/modules/loans/cli.py

@@ -164,16 +164,25 @@ def create_loan(barcode, transaction_type, loanable_items, verbose=False,
         )
         loan = get_loan_for_item(item.pid)
         loan_pid = loan.get('pid')
+        loan = Loan.get_record_by_pid(loan_pid)
         if transaction_type == 'overdue':
-            loan = Loan.get_record_by_pid(loan_pid)
+            end_date = datetime.now(timezone.utc) - timedelta(days=2)
+            loan['end_date'] = end_date.isoformat()
+            loan.update(
+                loan,
+                dbcommit=True,
+                reindex=True
+            )
+            loan.create_notification(notification_type='due_soon')
+
             end_date = datetime.now(timezone.utc) - timedelta(days=70)
             loan['end_date'] = end_date.isoformat()
             loan.update(
                 loan,
                 dbcommit=True,
                 reindex=True
             )
-            notif = loan.create_notification(notification_type='overdue')
+            loan.create_notification(notification_type='overdue')
 
         elif transaction_type == 'extended':
             user_pid, user_location = \
@@ -206,6 +215,7 @@ def create_loan(barcode, transaction_type, loanable_items, verbose=False,
                         requested_patron.pid),
                     document_pid=item.replace_refs()['document']['pid'],
                 )
+                loan.create_notification(notification_type='recall')
         return item['barcode']
     except Exception as err:
         if verbose:

rero_ils/modules/notifications/tasks.py

@@ -29,17 +29,17 @@ def create_over_and_due_soon_notifications(overdue=True, due_soon=True):
     """Creates due_soon and overdue notifications."""
     no_over_due_loans = 0
     no_due_soon_loans = 0
+    if due_soon:
+        due_soon_loans = get_due_soon_loans()
+        for loan in due_soon_loans:
+            loan.create_notification(notification_type='due_soon')
+            no_due_soon_loans += 1
     if overdue:
         over_due_loans = get_overdue_loans()
 
         for loan in over_due_loans:
             loan.create_notification(notification_type='overdue')
             no_over_due_loans += 1
-    if due_soon:
-        due_soon_loans = get_due_soon_loans()
-        for loan in due_soon_loans:
-            loan.create_notification(notification_type='due_soon')
-            no_due_soon_loans += 1
 
     return 'created {no_over_due_loans} overdue loans, '\
         '{no_due_soon_loans} due soon loans'.format(

tests/api/test_items_rest_views.py

@@ -536,3 +536,159 @@ def test_item_secure_api_delete(client, item_lib_saxon,
     res = client.delete(record_url)
     # sys_librarian can delete items in other libraries in same org.
     assert res.status_code == 204
+
+
+def test_pending_loans_order(client, librarian_martigny_no_email,
+                             patron_martigny_no_email, loc_public_martigny,
+                             item_type_standard_martigny,
+                             item2_lib_martigny, json_header,
+                             patron2_martigny_no_email, patron_sion_no_email,
+                             circulation_policies):
+    """Test sort of pending loans."""
+    login_user_via_session(client, librarian_martigny_no_email.user)
+    library_pid = librarian_martigny_no_email.replace_refs()['library']['pid']
+
+    res, _ = postdata(
+        client,
+        'api_item.librarian_request',
+        dict(
+            item_pid=item2_lib_martigny.pid,
+            patron_pid=patron_sion_no_email.pid,
+            pickup_location_pid=loc_public_martigny.pid
+        )
+    )
+
+    res, _ = postdata(
+        client,
+        'api_item.librarian_request',
+        dict(
+            item_pid=item2_lib_martigny.pid,
+            patron_pid=patron_martigny_no_email.pid,
+            pickup_location_pid=loc_public_martigny.pid
+        )
+    )
+    assert res.status_code == 200
+
+    res, _ = postdata(
+        client,
+        'api_item.librarian_request',
+        dict(
+            item_pid=item2_lib_martigny.pid,
+            patron_pid=patron2_martigny_no_email.pid,
+            pickup_location_pid=loc_public_martigny.pid
+        )
+    )
+    assert res.status_code == 200
+
+    # sort by pid asc
+    res = client.get(
+        url_for(
+            'api_item.requested_loans', library_pid=library_pid,
+            sort='pid'))
+    assert res.status_code == 200
+    data = get_json(res)
+    loans = data['hits']['hits'][0]['item']['pending_loans']
+    assert loans[2]['pid'] > loans[1]['pid'] > loans[0]['pid']
+
+    # sort by pid desc
+    res = client.get(
+        url_for(
+            'api_item.requested_loans', library_pid=library_pid,
+            sort='-pid'))
+    assert res.status_code == 200
+    data = get_json(res)
+    loans = data['hits']['hits'][0]['item']['pending_loans']
+    assert loans[2]['pid'] < loans[1]['pid'] < loans[0]['pid']
+
+    # sort by transaction desc
+    res = client.get(
+        url_for(
+            'api_item.requested_loans', library_pid=library_pid,
+            sort='-transaction_date'))
+    assert res.status_code == 200
+    data = get_json(res)
+    loans = data['hits']['hits'][0]['item']['pending_loans']
+    assert loans[2]['pid'] < loans[1]['pid'] < loans[0]['pid']
+
+    # sort by patron_pid asc
+    res = client.get(
+        url_for(
+            'api_item.requested_loans', library_pid=library_pid,
+            sort='patron_pid'))
+    assert res.status_code == 200
+    data = get_json(res)
+    loans = data['hits']['hits'][0]['item']['pending_loans']
+    assert loans[0]['patron_pid'] == patron_sion_no_email.pid
+    assert loans[1]['patron_pid'] == patron_martigny_no_email.pid
+    assert loans[2]['patron_pid'] == patron2_martigny_no_email.pid
+
+    # sort by invalid field
+    res = client.get(
+        url_for(
+            'api_item.requested_loans', library_pid=library_pid,
+            sort='does not exist'))
+    assert res.status_code == 500
+    data = get_json(res)
+    assert 'RequestError(400' in data['status']
+
+
+def test_patron_checkouts_order(client, librarian_martigny_no_email,
+                                patron_martigny_no_email, loc_public_martigny,
+                                item_type_standard_martigny,
+                                item3_lib_martigny, json_header,
+                                item2_lib_martigny,
+                                circulation_policies):
+    """Test sort of checkout loans."""
+    login_user_via_session(client, librarian_martigny_no_email.user)
+    res, _ = postdata(
+        client,
+        'api_item.checkout',
+        dict(
+            item_pid=item3_lib_martigny.pid,
+            patron_pid=patron_martigny_no_email.pid
+        ),
+    )
+    assert res.status_code == 200
+
+    res, _ = postdata(
+        client,
+        'api_item.checkout',
+        dict(
+            item_pid=item2_lib_martigny.pid,
+            patron_pid=patron_martigny_no_email.pid
+        ),
+    )
+    assert res.status_code == 200
+
+    # sort by transaction_date asc
+    res = client.get(
+        url_for(
+            'api_item.loans', patron_pid=patron_martigny_no_email.pid,
+            sort='transaction_date'))
+    assert res.status_code == 200
+    data = get_json(res)
+    items = data['hits']['hits']
+
+    assert items[0]['item']['pid'] == item3_lib_martigny.pid
+    assert items[1]['item']['pid'] == item2_lib_martigny.pid
+
+    # sort by transaction_date desc
+    res = client.get(
+        url_for(
+            'api_item.loans', patron_pid=patron_martigny_no_email.pid,
+            sort='-transaction_date'))
+    assert res.status_code == 200
+    data = get_json(res)
+    items = data['hits']['hits']
+
+    assert items[0]['item']['pid'] == item2_lib_martigny.pid
+    assert items[1]['item']['pid'] == item3_lib_martigny.pid
+
+    # sort by invalid field
+    res = client.get(
+        url_for(
+            'api_item.loans', patron_pid=patron_martigny_no_email.pid,
+            sort='does not exist'))
+    assert res.status_code == 500
+    data = get_json(res)
+    assert 'RequestError(400' in data['status']