circulation: adapt reroils

Adapts reroils after the upgrade to invenio-circulation v1.0.0a21.

The two transitions ItemOnLoanToItemReturned and ItemOnLoanToItemReturned are
temporarily updated and inserted into reroils until a fix for the issue
inveniosoftware/invenio-circulation#127 is available.

This PR expects the fix for the ITEM_AT_DESK problem in the current version
of invenio-circulation.

Some timezone units testing are disabled until a fix is given in a later PR.

Co-Authored-by: Aly Badr <aly.badr@rero.ch>

rero_ils/config.py

@@ -57,6 +57,7 @@
 from .modules.items.api import Item
 from .modules.items.permissions import can_create_item_factory, \
     can_update_delete_item_factory
+from .modules.items.utils import item_exists, item_location_retriever
 from .modules.libraries.api import Library
 from .modules.libraries.permissions import can_update_library_factory
 from .modules.loans.api import Loan
@@ -66,6 +67,7 @@
     ItemOnLoanToItemReturned
 from .modules.loans.utils import can_be_requested, get_default_loan_duration, \
     get_extension_params, is_item_available_for_checkout, \
+    loan_build_document_ref, loan_build_item_ref, loan_build_patron_ref, \
     loan_satisfy_circ_policies
 from .modules.locations.api import Location
 from .modules.locations.permissions import can_create_location_factory, \
@@ -1586,18 +1588,20 @@ def _(x):
 }
 
 #: Invenio circulation configuration.
-CIRCULATION_ITEM_EXISTS = Item.get_record_by_pid
+CIRCULATION_ITEM_EXISTS = item_exists
 CIRCULATION_PATRON_EXISTS = Patron.get_record_by_pid
 
-CIRCULATION_ITEM_LOCATION_RETRIEVER = Item.item_location_retriever
+CIRCULATION_ITEM_LOCATION_RETRIEVER = item_location_retriever
 CIRCULATION_DOCUMENT_RETRIEVER_FROM_ITEM = \
-    Item.get_document_pid_by_item_pid
+    Item.get_document_pid_by_item_pid_object
 CIRCULATION_ITEMS_RETRIEVER_FROM_DOCUMENT = Item.get_items_pid_by_document_pid
 
 CIRCULATION_DOCUMENT_EXISTS = Document.get_record_by_pid
-CIRCULATION_ITEM_REF_BUILDER = Loan.loan_build_item_ref
-CIRCULATION_PATRON_REF_BUILDER = Loan.loan_build_patron_ref
-CIRCULATION_DOCUMENT_REF_BUILDER = Loan.loan_build_document_ref
+
+CIRCULATION_ITEM_REF_BUILDER = loan_build_item_ref
+CIRCULATION_PATRON_REF_BUILDER = loan_build_patron_ref
+CIRCULATION_DOCUMENT_REF_BUILDER = loan_build_document_ref
+
 CIRCULATION_TRANSACTION_LOCATION_VALIDATOR = \
     Location.transaction_location_validator
 CIRCULATION_TRANSACTION_USER_VALIDATOR = \
@@ -1673,7 +1677,10 @@ def _(x):
         dict(dest='CANCELLED', trigger='cancel', transition=ToCancelled)
     ],
     'ITEM_IN_TRANSIT_FOR_PICKUP': [
-        dict(dest='ITEM_AT_DESK', trigger='receive'),
+        dict(
+            dest='ITEM_AT_DESK',
+            trigger='receive'
+        ),
         dict(dest='CANCELLED', trigger='cancel', transition=ToCancelled)
     ],
     'ITEM_ON_LOAN': [

rero_ils/modules/documents/views.py

@@ -40,6 +40,7 @@
     title_variant_format_text
 from ..holdings.api import Holding
 from ..items.api import Item, ItemStatus
+from ..items.utils import item_pid_to_object
 from ..libraries.api import Library
 from ..loans.api import Loan
 from ..loans.utils import can_be_requested
@@ -208,8 +209,7 @@ def can_request(item):
                     item.get_library().replace_refs()['organisation']['pid']:
                 # Complete metadata before Loan creation
                 loan_metadata = dict(item)
-                if 'item_pid' not in loan_metadata:
-                    loan_metadata['item_pid'] = item.pid
+                loan_metadata['item_pid'] = item_pid_to_object(item.pid)
                 if 'patron_pid' not in loan_metadata:
                     loan_metadata['patron_pid'] = patron.pid
                 # Create "virtual" Loan (not registered)

rero_ils/modules/items/api.py

@@ -32,6 +32,7 @@
 from invenio_search import current_search
 
 from .models import ItemIdentifier, ItemStatus
+from .utils import item_pid_to_object
 from ..api import IlsRecord, IlsRecordError, IlsRecordsIndexer, \
     IlsRecordsSearch
 from ..circ_policies.api import CircPolicy
@@ -110,7 +111,7 @@ def wrapper(item, *args, **kwargs):
 
             if not loan:
                 data = {
-                    'item_pid': item.pid,
+                    'item_pid': item_pid_to_object(item.pid),
                     'patron_pid': patron_pid
                 }
                 loan = Loan.create(data, dbcommit=True, reindex=True)
@@ -119,7 +120,7 @@ def wrapper(item, *args, **kwargs):
                 description="Parameter 'pid' is required")
 
         # set missing parameters
-        kwargs['item_pid'] = item.pid
+        kwargs['item_pid'] = item_pid_to_object(item.pid)
         kwargs['patron_pid'] = loan.get('patron_pid')
         kwargs['pid'] = loan.pid
         # TODO: case when user want to have his own transaction date
@@ -263,6 +264,7 @@ def dumps_for_circulation(self, sort_by=None):
         }
         data['actions'] = list(self.actions)
         data['available'] = self.available
+
         # data['number_of_requests'] = self.number_of_requests()
         for loan in self.get_requests(sort_by=sort_by):
             data.setdefault('pending_loans',
@@ -285,34 +287,57 @@ def holding_pid(self):
             return self.replace_refs()['holding']['pid']
         return None
 
+    @property
+    def document_pid(self):
+        """Shortcut for item document pid."""
+        if self.replace_refs().get('document'):
+            return self.replace_refs()['document']['pid']
+        return None
+
     @classmethod
     def get_document_pid_by_item_pid(cls, item_pid):
         """Returns document pid from item pid."""
         item = cls.get_record_by_pid(item_pid).replace_refs()
         return item.get('document', {}).get('pid')
 
+    @classmethod
+    def get_document_pid_by_item_pid_object(cls, item_pid):
+        """Returns document pid from item pid.
+
+        :param item_pid: the item_pid object
+        :type item_pid: object
+        :return: the document pid
+        :rtype: str
+        """
+        item = cls.get_record_by_pid(item_pid.get('value')).replace_refs()
+        return item.get('document', {}).get('pid')
+
     @classmethod
     def get_items_pid_by_document_pid(cls, document_pid):
         """Returns item pisd from document pid."""
         results = ItemsSearch()\
             .filter('term', document__pid=document_pid)\
             .source(['pid']).scan()
         for item in results:
-            yield item.pid
+            yield item_pid_to_object(item.pid)
 
     @classmethod
     def get_loans_by_item_pid(cls, item_pid):
         """Return any loan loans for item."""
-        results = current_circulation.loan_search_cls.filter(
-            'term', item_pid=item_pid).source(includes='pid').scan()
+        item_pid_object = item_pid_to_object(item_pid)
+        search_cls = current_circulation.loan_search_cls
+        search = search_cls().filter(
+            'term', item_pid__value=item_pid_object['value']).filter(
+            'term', item_pid__type=item_pid_object['type'])
+        results = search.source(includes='pid').scan()
         for loan in results:
             yield Loan.get_record_by_pid(loan.pid)
 
     @classmethod
     def get_loan_pid_with_item_on_loan(cls, item_pid):
         """Returns loan pid for checked out item."""
-        search = search_by_pid(
-            item_pid=item_pid, filter_states=['ITEM_ON_LOAN'])
+        search = search_by_pid(item_pid=item_pid_to_object(
+            item_pid), filter_states=['ITEM_ON_LOAN'])
         results = search.source(['pid']).scan()
         try:
             return next(results).pid
@@ -323,7 +348,7 @@ def get_loan_pid_with_item_on_loan(cls, item_pid):
     def get_loan_pid_with_item_in_transit(cls, item_pid):
         """Returns loan pi for in_transit item."""
         search = search_by_pid(
-            item_pid=item_pid, filter_states=[
+            item_pid=item_pid_to_object(item_pid), filter_states=[
                 "ITEM_IN_TRANSIT_FOR_PICKUP",
                 "ITEM_IN_TRANSIT_TO_HOUSE"])
         results = search.source(['pid']).scan()
@@ -358,13 +383,14 @@ def get_pendings_loans(cls, library_pid=None, sort_by='transaction_date'):
         if sort_by.startswith('-'):
             sort_by = sort_by[1:]
             order_by = 'desc'
-        search = current_circulation.loan_search_cls\
-            .source(['pid'])\
-            .params(preserve_order=True)\
-            .filter('term', state='PENDING')\
-            .filter('term', library_pid=library_pid)\
-            .sort({sort_by: {"order": order_by}})
-        results = search.scan()
+
+        search_cls = current_circulation.loan_search_cls
+        search = search_cls().params(preserve_order=True).filter(
+            'term', state='PENDING').filter(
+            'term', library_pid=library_pid).sort(
+                {sort_by: {"order": order_by}})
+        results = search.source(includes='pid').scan()
+
         for loan in results:
             yield Loan.get_record_by_pid(loan.pid)
 
@@ -383,11 +409,13 @@ def get_checked_out_loans(
             sort_by = sort_by[1:]
             order_by = 'desc'
 
-        results = current_circulation.loan_search_cls.source(['pid'])\
-            .params(preserve_order=True)\
-            .filter('term', state='ITEM_ON_LOAN')\
-            .filter('term', patron_pid=patron_pid)\
-            .sort({sort_by: {"order": order_by}}).scan()
+        search_cls = current_circulation.loan_search_cls
+        search = search_cls().params(preserve_order=True).filter(
+            'term', state='ITEM_ON_LOAN').filter(
+            'term', patron_pid=patron_pid).sort(
+                {sort_by: {"order": order_by}})
+        results = search.source(includes='pid').scan()
+
         for loan in results:
             yield Loan.get_record_by_pid(loan.pid)
 
@@ -398,7 +426,7 @@ def get_checked_out_items(cls, patron_pid=None, sort_by=None):
             patron_pid=patron_pid, sort_by=sort_by)
         returned_item_pids = []
         for loan in loans:
-            item_pid = loan.get('item_pid')
+            item_pid = loan.get('item_pid').get('value')
             item = Item.get_record_by_pid(item_pid)
             if item.status == ItemStatus.ON_LOAN and \
                     item_pid not in returned_item_pids:
@@ -411,7 +439,7 @@ def get_requests(self, sort_by=None):
         default sort is transaction_date.
         """
         search = search_by_pid(
-            item_pid=self.pid, filter_states=[
+            item_pid=item_pid_to_object(self.pid), filter_states=[
                 'PENDING',
                 'ITEM_AT_DESK',
                 'ITEM_IN_TRANSIT_FOR_PICKUP'
@@ -433,7 +461,7 @@ def get_requests_to_validate(
             library_pid=library_pid, sort_by=sort_by)
         returned_item_pids = []
         for loan in loans:
-            item_pid = loan.get('item_pid')
+            item_pid = loan.get('item_pid').get('value')
             item = Item.get_record_by_pid(item_pid)
             if item.status == ItemStatus.ON_SHELF and \
                     item_pid not in returned_item_pids:
@@ -599,7 +627,7 @@ def action_filter(self, action, loan):
     def actions(self):
         """Get all available actions."""
         transitions = current_app.config.get('CIRCULATION_LOAN_TRANSITIONS')
-        loan = get_loan_for_item(self.pid)
+        loan = get_loan_for_item(item_pid_to_object(self.pid))
         actions = set()
         if loan:
             for transition in transitions.get(loan.get('state')):
@@ -636,7 +664,7 @@ def actions(self):
 
     def status_update(self, dbcommit=False, reindex=False, forceindex=False):
         """Update item status."""
-        loan = get_loan_for_item(self.pid)
+        loan = get_loan_for_item(item_pid_to_object(self.pid))
         if loan:
             self['status'] = self.statuses[loan.get('state')]
         else:
@@ -653,7 +681,7 @@ def available(self):
 
     def get_item_end_date(self, format='short_date'):
         """Get item due date for a given item."""
-        loan = get_loan_for_item(self.pid)
+        loan = get_loan_for_item(item_pid_to_object(self.pid))
         if loan:
             end_date = loan['end_date']
             due_date = format_date_filter(
@@ -666,7 +694,7 @@ def get_item_end_date(self, format='short_date'):
 
     def get_extension_count(self):
         """Get item renewal count."""
-        loan = get_loan_for_item(self.pid)
+        loan = get_loan_for_item(item_pid_to_object(self.pid))
         if loan:
             return loan.get('extension_count', 0)
         return 0
@@ -702,35 +730,18 @@ def is_loaned_to_patron(self, patron_barcode):
         """Check if the item is loaned by a given patron."""
         patron = Patron.get_patron_by_barcode(patron_barcode)
         if patron:
-            states = ['CREATED"', 'PENDING'] + \
+            states = ['CREATED', 'PENDING'] + \
                 current_app.config['CIRCULATION_STATES_LOAN_ACTIVE']
             search = search_by_patron_item_or_document(
                 patron_pid=patron.pid,
-                item_pid=self.pid,
+                item_pid=item_pid_to_object(self.pid),
                 document_pid=self.document_pid,
                 filter_states=states,
             )
             search_result = search.execute()
             return search_result.hits.total > 0
         return False
 
-    @classmethod
-    def item_location_retriever(cls, item_pid, **kwargs):
-        """Get item selflocation or the transaction location of the.
-
-        last loan.
-        """
-        # TODO: for requests we probably need the transation_location_pid
-        #       to deal with multiple pickup locations for a library
-        item = cls.get_record_by_pid(item_pid)
-        if item:
-            # TODO: this will be useful for the very specific rero use cases
-
-            # last_location = item.get_last_location()
-            # if last_location:
-            #     return last_location.pid
-            return item.get_owning_pickup_location_pid()
-
     @add_loans_parameters_and_flush_indexes
     def validate_request(self, current_loan, **kwargs):
         """Validate item request."""
@@ -821,7 +832,7 @@ def prior_checkout_actions(self, action_params):
                 actions.update(cancel_actions)
                 del action_params['pid']
         else:
-            loan = get_loan_for_item(self.pid)
+            loan = get_loan_for_item(item_pid_to_object(self.pid))
             if (loan and loan.get('state') != 'ITEM_AT_DESK'):
                 item, cancel_actions = self.cancel_loan(pid=loan.get('pid'))
                 actions.update(cancel_actions)
@@ -930,7 +941,7 @@ def return_missing(self):
     def get_number_of_loans(self):
         """Get number of loans."""
         search = search_by_pid(
-            item_pid=self.pid,
+            item_pid=item_pid_to_object(self.pid),
             exclude_states=[
                 'CANCELLED',
                 'ITEM_RETURNED',
@@ -969,11 +980,10 @@ def organisation_view(self):
 
     def item_has_active_loan_or_request(self):
         """Return True if active loan or a request found for item."""
-        item_object = {'value': self.pid, 'type': 'item'}
         states = ['PENDING'] + \
             current_app.config['CIRCULATION_STATES_LOAN_ACTIVE']
         search = search_by_pid(
-            item_pid=item_object,
+            item_pid=item_pid_to_object(self.pid),
             filter_states=states,
         )
         search_result = search.execute()

rero_ils/modules/items/api_views.py

@@ -29,6 +29,7 @@
 from werkzeug.exceptions import NotFound
 
 from .api import Item, ItemStatus
+from .utils import item_pid_to_object
 from ..circ_policies.api import CircPolicy
 from ..libraries.api import Library
 from ..loans.api import Loan
@@ -229,6 +230,7 @@ def requested_loans(library_pid):
         library_pid=library_pid, sort_by=sort_by)
     metadata = []
     for item, loan in items_loans:
+        a = loan.dumps_for_circulation()
         metadata.append({
             'item': item.dumps_for_circulation(sort_by=sort_by),
             'loan': loan.dumps_for_circulation()
@@ -272,7 +274,7 @@ def item(item_barcode):
     item = Item.get_item_by_barcode(item_barcode)
     if not item:
         abort(404)
-    loan = get_loan_for_item(item.pid)
+    loan = get_loan_for_item(item_pid_to_object(item.pid))
     if loan:
         loan = Loan.get_record_by_pid(loan.get('pid')).dumps_for_circulation()
     item_dumps = item.dumps_for_circulation()
@@ -364,11 +366,13 @@ def is_librarian_can_request_item_for_patron(
         return jsonify_response(reason='Library not found.')
     # Create a loan
     loan = Loan({
-        'patron_pid': patron.pid, 'item_pid': item.pid,
+        'patron_pid': patron.pid,
+        'item_pid': item_pid_to_object(item.pid),
         'library_pid': library_pid})
     if not can_be_requested(loan):
         return jsonify_response(
             reason='Request not allowed by the circulation policy.')
+
     if item.status != ItemStatus.MISSING:
         loaned_to_patron = item.is_loaned_to_patron(patron_barcode)
         if loaned_to_patron: