acquisition: delete receipt and its lines

* Deletes all receipt lines when attempting to delete a receipt.
* Reindexes corresponding order lines after deletion.
* Fixes expenditures amount calculation.
* Fixes JSON receipt JSON schema problems.

Co-Authored-by: Aly Badr <aly.badr@rero.ch>
Co-Authored-by: Renaud Michotte <renaud.michotte@gmail.com>

rero_ils/modules/acq_accounts/api.py

@@ -219,26 +219,30 @@ def expenditure_amount(self):
         results = search.execute()
         lines_expenditure = results.aggregations.sum_receipt_lines.value
 
-        recpt_expenditure = 0
+        receipt_expenditure = 0
         search = AcqReceiptsSearch() \
-            .filter('nested',
-                    path='amount_adjustments',
-                    query=Q(
-                        'bool',
-                        must=[Q('match',
-                                amount_adjustments__acq_account__pid=self.pid)]
-                    ))
+            .filter(
+                'nested',
+                path='amount_adjustments',
+                query=Q(
+                    'bool', must=[
+                        Q('match',
+                          amount_adjustments__acq_account__pid=self.pid)
+                    ]
+                )
+            )
         for hit in search.scan():
-            recpt_expenditure += sum(
-                [a.amount for a in hit.amount_adjustments])
-        self_amount = lines_expenditure + recpt_expenditure
+            receipt_expenditure += sum([
+                adjustment.amount for adjustment in hit.amount_adjustments
+                if adjustment.acq_account.pid == self.pid
+            ])
+        self_amount = lines_expenditure + receipt_expenditure
 
         # Expenditure of children accounts
         query = AcqAccountsSearch().filter('term', parent__pid=self.pid)
         query.aggs.metric('total', 'sum', field='expenditure_amount.total')
         results = query.execute()
         children_amount = results.aggregations.total.value
-
         return round(self_amount, 2), round(children_amount, 2)
 
     @property

rero_ils/modules/acq_order_lines/api.py

@@ -266,22 +266,21 @@ class AcqOrderLinesIndexer(IlsRecordsIndexer):
 
     record_cls = AcqOrderLine
 
+    @staticmethod
+    def _reindex_related_resources(record):
+        record.order.reindex()
+        record.account.reindex()
+
     def index(self, record):
         """Index an Acquisition Order Line and update total amount of order."""
-        from ..acq_orders.api import AcqOrder
         return_value = super().index(record)
-        order = AcqOrder.get_record_by_pid(record.order_pid)
-        order.reindex()
-        record.account.reindex()
+        AcqOrderLinesIndexer._reindex_related_resources(record)
         return return_value
 
     def delete(self, record):
         """Delete a Acquisition Order Line and update total amount of order."""
-        from ..acq_orders.api import AcqOrder
         return_value = super().delete(record)
-        order = AcqOrder.get_record_by_pid(record.order_pid)
-        order.reindex()
-        record.account.reindex()
+        AcqOrderLinesIndexer._reindex_related_resources(record)
         return return_value
 
     def bulk_index(self, record_id_iterator):

rero_ils/modules/acq_receipt_lines/api.py

@@ -187,14 +187,15 @@ class AcqReceiptLinesIndexer(IlsRecordsIndexer):
     record_cls = AcqReceiptLine
 
     def index(self, record):
-        """Index an acq receipt line record.
-
-        Reindex the parent acq receipt, acq_order and acq_account records.
-        """
-        from rero_ils.modules.acq_order_lines.api import AcqOrderLine
+        """Index an AcqReceiptLine line record."""
         return_value = super().index(record)
-        order_line = AcqOrderLine.get_record_by_pid(record.order_line_pid)
-        order_line.reindex()
-        order_line.order.reindex()
-        order_line.account.reindex()
+        # The reindexing of the parent order_line will also fired the
+        # indexing of the parent order and related account
+        record.order_line.reindex()
+        return return_value
+
+    def delete(self, record):
+        """Delete an AcqReceiptLine record from indexer."""
+        return_value = super().delete(record)
+        record.order_line.reindex()
         return return_value

rero_ils/modules/acq_receipts/api.py

@@ -24,6 +24,7 @@
 from rero_ils.modules.acq_receipt_lines.api import AcqReceiptLine, \
     AcqReceiptLinesSearch
 
+from .extensions import AcqReceiptExtension
 from .models import AcqReceiptIdentifier, AcqReceiptLineCreationStatus, \
     AcqReceiptMetadata
 from ..api import IlsRecord, IlsRecordsIndexer, IlsRecordsSearch
@@ -66,6 +67,10 @@ class AcqReceipt(IlsRecord):
     provider = AcqReceiptProvider
     model_cls = AcqReceiptMetadata
 
+    _extensions = [
+        AcqReceiptExtension(),
+    ]
+
     @classmethod
     def create(cls, data, id_=None, delete_pid=False,
                dbcommit=True, reindex=True, **kwargs):
@@ -74,14 +79,18 @@ def create(cls, data, id_=None, delete_pid=False,
         :param data: a dict data to create the record.
         :param dbcommit: if True call dbcommit, make the change effective
                          in db.
-        :param redindex: reindex the record.
+        :param reindex: reindex the record.
         :param id_ - UUID, it would be generated if it is not given
         :param delete_pid - remove the pid present in the data if True
         :returns: the created record
         """
         cls._build_additional_refs(data)
         record = super().create(
             data, id_, delete_pid, dbcommit, reindex, **kwargs)
+        # reindex the related account if necessary
+        if reindex:
+            for account in record.get_adjustment_accounts():
+                account.reindex()
         return record
 
     def update(self, data, commit=True, dbcommit=True, reindex=True):
@@ -91,13 +100,26 @@ def update(self, data, commit=True, dbcommit=True, reindex=True):
         :param commit: if True push the db transaction.
         :param dbcommit: if True call dbcommit, make the change effective
                          in db.
-        :param redindex: reindex the record.
+        :param reindex: reindex the record.
         :returns: the updated record
         """
+        # We need to manage the indexing of related adjustment accounts to
+        # ensure than expenditure amount of theses accounts are correct. To do
+        # that, we need to get the accounts BEFORE changes and AFTER changes (
+        # in the case of we delete adjustment) to create a python set. Each
+        # entry of this set should be reindex.
+
+        original_accounts = self.get_adjustment_accounts()
         new_data = deepcopy(dict(self))
         new_data.update(data)
         self._build_additional_refs(new_data)
-        super().update(new_data, commit, dbcommit, reindex)
+        record = super().update(new_data, commit, dbcommit, reindex)
+        if reindex:
+            AcqReceiptsSearch().flush_and_refresh()
+        new_accounts = record.get_adjustment_accounts()
+        accounts_set = original_accounts.union(new_accounts)
+        for account in accounts_set:
+            account.reindex()
         return self
 
     @classmethod
@@ -119,7 +141,7 @@ def create_receipt_lines(self, receipt_lines=None, dbcommit=True,
         :param receipt_lines: a list of dicts to create the records.
         :param dbcommit: if True call dbcommit, make the change effective
                          in db.
-        :param redindex: reindex the record.
+        :param reindex: reindex the record.
         :returns: a list containing the given data to build the receipt line
                   with a `status` field, either `success` or `failure`.
                   In case of `success`, the created record is returned.
@@ -141,7 +163,7 @@ def create_receipt_lines(self, receipt_lines=None, dbcommit=True,
                 record['receipt_line'] = line
             except Exception as error:
                 record['status'] = AcqReceiptLineCreationStatus.FAILURE
-                record['error_message'] = str(error.message)
+                record['error_message'] = str(error)
             created_receipt_lines.append(record)
 
         return created_receipt_lines
@@ -168,9 +190,9 @@ def get_links_to_me(self, get_pids=False):
     def reasons_not_to_delete(self):
         """Get reasons not to delete receipt."""
         cannot_delete = {}
-        links = self.get_links_to_me()
-        if links:
-            cannot_delete['links'] = links
+        # Note : linked receipt lines aren't yet a reason to keep the record.
+        #        These lines will be deleted with the record.
+        # TODO :: add a reason if order is concluded (rollovered or invoiced)
         return cannot_delete
 
     # GETTER & SETTER =========================================================
@@ -272,8 +294,21 @@ def get_note(self, note_type):
         ]
         return next(iter(note), None)
 
+    def get_adjustment_accounts(self):
+        """Get the list of adjustment account pid related to this receipt."""
+        return set([
+            extracted_data_from_ref(adj.get('acq_account'), data='record')
+            for adj in self.amount_adjustments
+        ])
+
 
 class AcqReceiptsIndexer(IlsRecordsIndexer):
     """Indexing documents in Elasticsearch."""
 
     record_cls = AcqReceipt
+
+    def delete(self, record):
+        """Delete a AcqReceipt from indexer."""
+        super().delete(record)
+        for account in record.get_adjustment_accounts():
+            account.reindex()

rero_ils/modules/acq_receipts/extensions.py

@@ -0,0 +1,35 @@
+# -*- coding: utf-8 -*-
+#
+# RERO ILS
+# Copyright (C) 2021 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/>.
+
+"""Acquisition Receipt record extensions."""
+
+from invenio_records.extensions import RecordExtension
+
+
+class AcqReceiptExtension(RecordExtension):
+    """Defines hooks about API functions calls for AcqReceipt."""
+
+    def pre_delete(self, record, force=False):
+        """Called before a record is deleted.
+
+        :param record: the record metadata.
+        :param force: force the deleting of the record.
+        """
+        # For receipts, we are allowed to delete all of its receipt lines
+        # without further checks.
+        for line in record.get_receipt_lines():
+            line.delete(force=force, delindex=True)

rero_ils/modules/acq_receipts/jsonschemas/acq_receipts/acq_receipt-v0.0.1.json

@@ -5,15 +5,15 @@
   "description": "JSON schema for Acquisition Receipt.",
   "additionalProperties": false,
   "propertiesOrder": [
-    "acq_order",
     "exchange_rate",
     "amount_adjustments",
     "notes"
   ],
   "required": [
     "$schema",
     "pid",
-    "acq_order"
+    "acq_order",
+    "exchange_rate"
   ],
   "properties": {
     "$schema": {
@@ -39,24 +39,15 @@
         "$ref": {
           "title": "Order",
           "type": "string",
-          "pattern": "^https://bib.rero.ch/api/acq_orders/.*?$",
-          "form": {
-            "focus": true,
-            "remoteOptions": {
-              "type": "acq_orders"
-            },
-            "placeholder": "Choose an order",
-            "templateOptions": {
-              "label": ""
-            }
-          }
+          "pattern": "^https://bib.rero.ch/api/acq_orders/.*?$"
         }
       }
     },
     "exchange_rate": {
       "title": "Exchange rate",
       "type": "number",
       "exclusiveMinimum": 0,
+      "default": 1,
       "form": {
         "hide": true,
         "navigation": {
@@ -116,13 +107,19 @@
             }
           }
         }
+      },
+      "form": {
+        "hide": true,
+        "navigation": {
+          "essential": true
+        }
       }
     },
     "notes": {
       "title": "Notes",
       "type": "array",
       "minItems": 0,
-      "maxItems": 2,
+      "maxItems": 1,
       "items": {
         "type": "object",
         "additionalProperties": false,
@@ -168,6 +165,10 @@
         }
       },
       "form": {
+        "hide": true,
+        "navigation": {
+          "essential": true
+        },
         "templateOptions": {
           "wrappers": [
             "card"

tests/api/acq_receipts/test_acq_receipts_rest.py

@@ -158,9 +158,11 @@ def test_acq_receipts_can_delete(
         client, document, acq_receipt_fiction_martigny,
         acq_receipt_line_1_fiction_martigny):
     """Test can delete an acq receipt."""
+    # We can delete an AcqReceipt even if some children AcqReceiptLines exists
+    # because they will be cascading deleted if we delete the parent AcqReceipt
     can, reasons = acq_receipt_fiction_martigny.can_delete
-    assert not can
-    assert reasons['links']['acq_receipt_lines']
+    assert can
+    assert 'acq_receipt_lines' not in reasons.get('links', {})
 
 
 def test_filtered_acq_receipts_get(