circulation: fix item status after a check-in

* Overwrites bugged invenio-circulation check-in transitions.
* Closes #780

Co-Authored-by: Johnny Mariéthoz <[email protected]>
Co-Authored-by: Aly Badr <[email protected]>

rero_ils/config.py

@@ -34,9 +34,8 @@
     CIRCULATION_LOAN_MINTER, CIRCULATION_LOAN_PID_TYPE
 from invenio_circulation.search.api import LoansSearch
 from invenio_circulation.transitions.transitions import CreatedToPending, \
-    ItemAtDeskToItemOnLoan, ItemInTransitHouseToItemReturned, \
-    ItemOnLoanToItemInTransitHouse, ItemOnLoanToItemOnLoan, \
-    ItemOnLoanToItemReturned, PendingToItemAtDesk, \
+    ItemAtDeskToItemOnLoan, ItemOnLoanToItemInTransitHouse, \
+    ItemOnLoanToItemOnLoan, PendingToItemAtDesk, \
     PendingToItemInTransitPickup, ToItemOnLoan
 from invenio_records_rest.facets import terms_filter
 from invenio_records_rest.utils import allow_all, deny_all
@@ -66,6 +65,8 @@
 from .modules.loans.api import Loan
 from .modules.loans.permissions import can_list_loan_factory, \
     can_read_loan_factory
+from .modules.loans.transitions import ItemInTransitHouseToItemReturned, \
+    ItemOnLoanToItemReturned
 from .modules.loans.utils import can_be_requested, get_default_loan_duration, \
     get_extension_params, is_item_available_for_checkout, \
     loan_satisfy_circ_policies

rero_ils/modules/loans/transitions.py

@@ -0,0 +1,84 @@
+# -*- coding: utf-8 -*-
+#
+# RERO ILS
+# Copyright (C) 2019 RERO
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, version 3 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+"""REROILS Circulation custom transitions."""
+
+
+from invenio_circulation.api import get_document_pid_by_item_pid, \
+    get_pending_loans_by_doc_pid
+from invenio_circulation.proxies import current_circulation
+from invenio_circulation.transitions.base import Transition
+from invenio_circulation.transitions.transitions import _ensure_same_location,\
+    ensure_same_item
+from invenio_db import db
+
+from ..documents.api import Document
+
+
+def _update_document_pending_request_for_item(item_pid, **kwargs):
+    """Update pending loans on a Document with no Item attached yet."""
+    document_pid = get_document_pid_by_item_pid(item_pid)
+    # check if document has no other items
+    document = Document.get_record_by_pid(document_pid)
+    if document.get_number_of_items() == 1:
+        for pending_loan in get_pending_loans_by_doc_pid(document_pid):
+            pending_loan['item_pid'] = item_pid
+            pending_loan.commit()
+            db.session.commit()
+            current_circulation.loan_indexer.index(pending_loan)
+
+
+class ItemInTransitHouseToItemReturned(Transition):
+    """Check-in action when returning an item to its belonging location."""
+
+    @ensure_same_item
+    def before(self, loan, **kwargs):
+        """Validate check-in action."""
+        super(ItemInTransitHouseToItemReturned, self).before(loan, **kwargs)
+
+        _ensure_same_location(loan['item_pid'],
+                              loan['transaction_location_pid'],
+                              self.dest,
+                              error_msg="Item should be in transit to house. ")
+
+    def after(self, loan):
+        """Convert dates to string before saving loan."""
+        super(ItemInTransitHouseToItemReturned, self).after(loan)
+        _update_document_pending_request_for_item(loan['item_pid'])
+
+
+class ItemOnLoanToItemReturned(Transition):
+    """Check-in action when returning an item to its belonging location."""
+
+    @ensure_same_item
+    def before(self, loan, **kwargs):
+        """Validate check-in action."""
+        super(ItemOnLoanToItemReturned, self).before(loan, **kwargs)
+
+        _ensure_same_location(loan['item_pid'],
+                              loan['transaction_location_pid'],
+                              self.dest,
+                              error_msg="Item should be in transit to house. ")
+
+        # set end loan date as transaction date when completing loan
+        loan['end_date'] = loan['transaction_date']
+
+    def after(self, loan):
+        """Convert dates to string before saving loan."""
+        loan['end_date'] = loan['end_date'].isoformat()
+        super(ItemOnLoanToItemReturned, self).after(loan)
+        _update_document_pending_request_for_item(loan['item_pid'])

tests/api/test_circ_bug.py

@@ -0,0 +1,180 @@
+# -*- coding: utf-8 -*-
+#
+# RERO ILS
+# Copyright (C) 2019 RERO
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, version 3 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+"""Tests invenio circulation bug when document has items attached."""
+
+
+from invenio_accounts.testutils import login_user_via_session
+from utils import postdata
+from rero_ils.modules.loans.api import LoanAction
+
+
+def test_document_with_one_item_attached_bug(
+        client, librarian_martigny_no_email, patron_martigny_no_email,
+        patron2_martigny_no_email, loc_public_martigny,
+        item_type_standard_martigny, item_lib_martigny, json_header,
+        circulation_policies, lib_martigny):
+    """Test document with one item."""
+    login_user_via_session(client, librarian_martigny_no_email.user)
+
+    # checkout first item1 to patron
+    res, data = postdata(
+        client,
+        'api_item.checkout',
+        dict(
+            item_pid=item_lib_martigny.pid,
+            patron_pid=patron_martigny_no_email.pid,
+            transaction_library_pid=lib_martigny.pid
+        )
+    )
+    assert res.status_code == 200
+
+    actions = data.get('action_applied')
+    loan_pid = actions[LoanAction.CHECKOUT].get('pid')
+
+    # request first item by patron2
+    res, data = postdata(
+        client,
+        'api_item.librarian_request',
+        dict(
+            item_pid=item_lib_martigny.pid,
+            pickup_location_pid=loc_public_martigny.pid,
+            patron_pid=patron2_martigny_no_email.pid
+        )
+    )
+    assert res.status_code == 200
+    actions = data.get('action_applied')
+    loan2_pid = actions[LoanAction.REQUEST].get('pid')
+
+    # checkin the first item
+    res, data = postdata(
+        client,
+        'api_item.checkin',
+        dict(
+            item_pid=item_lib_martigny.pid,
+            pid=loan_pid
+        )
+    )
+    assert res.status_code == 200
+
+    assert item_lib_martigny.number_of_requests() == 1
+
+    res, data = postdata(
+        client,
+        'api_item.cancel_loan',
+        dict(
+            item_pid=item_lib_martigny.pid,
+            pid=loan2_pid
+        )
+    )
+    assert res.status_code == 200
+    assert item_lib_martigny.number_of_requests() == 0
+
+
+def test_document_with_items_attached_bug(client, librarian_martigny_no_email,
+                                          patron_martigny_no_email,
+                                          patron2_martigny_no_email,
+                                          item2_lib_martigny,
+                                          loc_public_martigny,
+                                          item_type_standard_martigny,
+                                          item_lib_martigny, json_header,
+                                          circulation_policies, lib_martigny):
+    """Test document with multiple items."""
+    login_user_via_session(client, librarian_martigny_no_email.user)
+
+    # checkout first item1 to patron
+    res, data = postdata(
+        client,
+        'api_item.checkout',
+        dict(
+            item_pid=item_lib_martigny.pid,
+            patron_pid=patron_martigny_no_email.pid,
+            transaction_library_pid=lib_martigny.pid
+        )
+    )
+    assert res.status_code == 200
+
+    actions = data.get('action_applied')
+    loan_pid = actions[LoanAction.CHECKOUT].get('pid')
+
+    # checkout second item2 to patron
+    res, data = postdata(
+        client,
+        'api_item.checkout',
+        dict(
+            item_pid=item2_lib_martigny.pid,
+            patron_pid=patron_martigny_no_email.pid,
+            transaction_library_pid=lib_martigny.pid
+        )
+    )
+    assert res.status_code == 200
+    actions = data.get('action_applied')
+    loan2_pid = actions[LoanAction.CHECKOUT].get('pid')
+
+    # request first item by patron2
+    res, data = postdata(
+        client,
+        'api_item.librarian_request',
+        dict(
+            item_pid=item_lib_martigny.pid,
+            pickup_location_pid=loc_public_martigny.pid,
+            patron_pid=patron2_martigny_no_email.pid
+        )
+    )
+    assert res.status_code == 200
+
+    # request second item by patron2
+    res, data = postdata(
+        client,
+        'api_item.librarian_request',
+        dict(
+            item_pid=item2_lib_martigny.pid,
+            pickup_location_pid=loc_public_martigny.pid,
+            patron_pid=patron2_martigny_no_email.pid
+        )
+    )
+    assert res.status_code == 200
+    assert item_lib_martigny.number_of_requests() == 1
+    assert item2_lib_martigny.number_of_requests() == 1
+
+    # checkin the first item
+    res, data = postdata(
+        client,
+        'api_item.checkin',
+        dict(
+            item_pid=item_lib_martigny.pid,
+            pid=loan_pid
+        )
+    )
+    assert res.status_code == 200
+
+    assert item_lib_martigny.number_of_requests() == 1
+    assert item2_lib_martigny.number_of_requests() == 1
+
+    # checkin the second item
+    res, data = postdata(
+        client,
+        'api_item.checkin',
+        dict(
+            item_pid=item2_lib_martigny.pid,
+            pid=loan2_pid
+        )
+    )
+    assert res.status_code == 200
+
+    assert item_lib_martigny.number_of_requests() == 1
+    assert item2_lib_martigny.number_of_requests() == 1