items: delete checked-in provisional items

* Adds a daily task to delete checked-in provisional items having not associated fees.

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

* Migration instructions:

This task need to be enabled for the production instance
- invenio reroils scheduler enable_tasks -n delete-provisional-items -v

rero_ils/config.py

@@ -385,6 +385,11 @@ def _(x):
         # Every week on Saturday at 22:22 UTC,
         'enabled': False
     },
+    'delete-provisional-items': {
+        'task': 'rero_ils.modules.items.tasks.delete_provisional_items',
+        'schedule': crontab(minute=0, hour=3),  # Every day at 03:00 UTC,
+        'enabled': False,
+    },
     # 'mef-harvester': {
     #     'task': 'rero_ils.modules.apiharvester.tasks.harvest_records',
     #     'schedule': timedelta(minutes=60),

rero_ils/modules/items/tasks.py

@@ -23,11 +23,45 @@
 from flask import current_app
 
 from .api import Item
+from .utils import get_provisional_items_pids_candidate_to_delete
 from ..api import IlsRecordError
 from ..holdings.api import Holding
 from ..utils import extracted_data_from_ref, set_timestamp
 
 
+@shared_task()
+def delete_provisional_items(verbose=True):
+    """Delete checked-in provisional items.
+
+    For the list of candidates item pids to delete, this method tries to delete
+    each item. The item will not be deleted if the method `can_delete` is True.
+    The reasons not to delete a provisional item are the same as other items:
+    1. no active loans
+    2. no fees
+
+    :param verbose: is the task should be verbose.
+    """
+    count_items, deleted_items = 0, 0
+    for count_items, item_pid in enumerate(
+            get_provisional_items_pids_candidate_to_delete(), 1):
+        try:
+            item = Item.get_record_by_pid(item_pid)
+            item.delete(item, dbcommit=True, delindex=True)
+            deleted_items += 1
+        except IlsRecordError.NotDeleted:
+            pass
+        except Exception as error:
+            current_app.logger.error(error)
+
+    msg_dict = {
+        'number_of_candidate_items_to_delete': count_items,
+        'numner_of_deleted_items': deleted_items
+    }
+    set_timestamp('claims-creation', **msg_dict)
+
+    return msg_dict
+
+
 @shared_task
 def process_late_claimed_issues(
     expected_issues_to_late=True, create_first_claim=True,

rero_ils/modules/items/utils.py

@@ -17,7 +17,10 @@
 
 """Item utils."""
 
+
+from rero_ils.modules.items.models import ItemStatus, TypeOfItem
 from rero_ils.modules.locations.api import LocationsSearch
+from rero_ils.modules.patron_transactions.api import PatronTransactionsSearch
 
 
 def item_pid_to_object(item_pid):
@@ -85,3 +88,29 @@ def exists_available_item(items=[]):
         if item.available:
             return True
     return False
+
+
+def get_provisional_items_pids_candidate_to_delete():
+    """Returns checked-in provisional items pids.
+
+    Returns list of candidate provisional items pids to delete, based on the
+    status of the item. Filtering by the status `ItemStatus.ON_SHELF` removes
+    items with active loans. in addition, remove items with active fees.
+    :return an item pid generator.
+    """
+    from rero_ils.modules.items.api import ItemsSearch
+
+    # query ES index for open fees
+    query_fees = PatronTransactionsSearch()\
+        .filter('term', status='open')\
+        .filter('range', total_amount={'gt': 0})\
+        .source('item')
+    # list of item pids with open fees
+    item_pids_with_fees = [hit.item.pid for hit in query_fees.scan()]
+    query = ItemsSearch()\
+        .filter('term', type=TypeOfItem.PROVISIONAL) \
+        .filter('terms', status=[ItemStatus.ON_SHELF]) \
+        .exclude('terms', pid=item_pids_with_fees)\
+        .source('pid')
+    for hit in query.scan():
+        yield hit.pid

scripts/setup

@@ -524,7 +524,7 @@ then
 else
     eval ${PREFIX} invenio reroils scheduler enable_tasks -n scheduler-timestamp -n bulk-indexer -n anonymize-loans -n claims-creation -n accounts -n clear_and_renew_subscriptions -n collect-stats -v
     eval ${PREFIX} invenio reroils scheduler enable_tasks -n notification-creation -n notification-dispatch-availability -n notification-dispatch-recall -n find-contribution -v
-    eval ${PREFIX} invenio reroils scheduler enable_tasks -n cancel-expired-request -v
+    eval ${PREFIX} invenio reroils scheduler enable_tasks -n cancel-expired-request -n delete-provisional-items -v
     info_msg "For ebooks harvesting run:"
     msg "\tinvenio reroils oaiharvester harvest -n ebooks -a max=100 -q"
 fi

tests/api/holdings/test_provisional_items.py

@@ -20,14 +20,20 @@
 
 from __future__ import absolute_import, print_function
 
+from datetime import datetime, timezone
+
 from flask import url_for
 from invenio_accounts.testutils import login_user_via_session
 from utils import get_json, postdata
 
 from rero_ils.modules.items.api import Item
 from rero_ils.modules.items.models import ItemStatus, TypeOfItem
+from rero_ils.modules.items.tasks import delete_provisional_items
+from rero_ils.modules.items.utils import \
+    get_provisional_items_pids_candidate_to_delete
 from rero_ils.modules.loans.api import Loan
 from rero_ils.modules.loans.models import LoanAction, LoanState
+from rero_ils.modules.patron_transactions.api import PatronTransaction
 from rero_ils.modules.utils import get_ref_for_pid
 
 
@@ -102,7 +108,8 @@ def test_provisional_items_creation(client, document, org_martigny,
 
 def test_holding_requests(client, patron_martigny, loc_public_martigny,
                           circulation_policies, librarian_martigny,
-                          holding_lib_martigny_w_patterns, lib_martigny):
+                          holding_lib_martigny_w_patterns, lib_martigny,
+                          item_lib_martigny, org_martigny):
     """Test holding patron request."""
     login_user_via_session(client, patron_martigny.user)
     holding = holding_lib_martigny_w_patterns
@@ -171,8 +178,6 @@ def test_holding_requests(client, patron_martigny, loc_public_martigny,
     assert res.status_code == 200
     item = Item.get_record_by_pid(item.pid)
     assert item.status == ItemStatus.ON_SHELF
-    # TODO: add additional tests for the task to delete provisional items with
-    # no active loans.
 
     # test requests made by a librarian
     # test fails when there are missing parameters
@@ -235,3 +240,34 @@ def test_holding_requests(client, patron_martigny, loc_public_martigny,
     assert item_2.holding_pid == holding.pid
     assert item_2.get('enumerationAndChronology') == description
     assert item_2.pid != item.pid
+
+    all_item_pids = [pid for pid in Item.get_all_pids()]
+    assert all_item_pids
+
+    # test delete provisional items with no active fees/loans
+    report = delete_provisional_items()
+    assert report.get('numner_of_deleted_items')
+    assert report.get('number_of_candidate_items_to_delete')
+    # assert that not deleted items are either having loans/fees or not
+    # provisional items
+    left_item_pids = [pid for pid in Item.get_all_pids()]
+    assert left_item_pids
+    for pid in left_item_pids:
+        record = Item.get_record_by_pid(pid)
+        can, _ = record.can_delete
+        assert not can or record.get('type') != TypeOfItem.PROVISIONAL
+    # item_2 has pending loans then it should not be removed
+    assert item_2.pid in left_item_pids
+    assert item_2.pid in get_provisional_items_pids_candidate_to_delete()
+    # add fee to item_2 and make sure it will not be candidate at the deletion.
+    data = {
+        'loan': {'$ref': get_ref_for_pid('loanid', loan_2.pid)},
+        'patron': {'$ref': get_ref_for_pid('patrons', patron_martigny.pid)},
+        'organisation': {'$ref': get_ref_for_pid('org', org_martigny.pid)},
+        'status': 'open',
+        'total_amount': 0.6,
+        'type': 'overdue',
+        'creation_date': datetime.now(timezone.utc).isoformat()
+    }
+    PatronTransaction.create(data, dbcommit=True, reindex=True)
+    assert item_2.pid not in get_provisional_items_pids_candidate_to_delete()