Skip to content

Commit 48d91f2

Browse files
rerowepPascalRepond
andcommitted
feat(documents): allow deletion when acq order lines are terminal
* Closes rero#3225. * Add module-level constant `ACQ_ORDER_LINE_BLOCKING_STATUSES` (APPROVED, ORDERED, PARTIALLY_RECEIVED) in `documents/api.py` to avoid reallocating the list on every call to `get_links_to_me()`. * Filter the `acq_order_lines` link count by these blocking statuses so that order lines in a terminal state (RECEIVED, CANCELLED) no longer prevent document deletion. * Guard the receipt-line ES dumper against a missing document: when the document has already been deleted, fall back to the pid extracted from the `$ref` instead of raising an error. * Guard the order-line ES dumper the same way, for consistency. * Override `replace_refs()` in `AcqOrderLine` to resolve the document reference manually before delegating to the parent: if the document has been deleted the lazy `JsonRef` proxy would raise a `JsonRefError` during `deepcopy` inside the base dumper, making any edit to a receipt-line (or other resource triggering order-line re-indexation) fail. Follows the same pattern already used in `Template.replace_refs()`. * Extend the reception-workflow test with `get_links_to_me()` assertions at each stage of the workflow, and verify that once all order lines are terminal the document reports no blocking links and no reasons not to delete. Co-Authored-by: Peter Weber <peter.weber@rero.ch> Co-Authored-by: Pascal Repond <pascal.repond@rero.ch>
1 parent d29392b commit 48d91f2

File tree

5 files changed

+125
-22
lines changed

5 files changed

+125
-22
lines changed

rero_ils/modules/acquisition/acq_order_lines/api.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# -*- coding: utf-8 -*-
22
#
33
# RERO ILS
4-
# Copyright (C) 2019-2022 RERO
4+
# Copyright (C) 2019-2026 RERO
55
# Copyright (C) 2019-2022 UCLouvain
66
#
77
# This program is free software: you can redistribute it and/or modify
@@ -82,11 +82,19 @@ def extended_validation(self, **kwargs):
8282
8383
:return: False if
8484
- notes array has multiple notes with same type
85+
- an update would leave the line non-cancelled while its document
86+
has been deleted
8587
"""
8688
# NOTES fields testing
8789
note_types = [note.get("type") for note in self.get("notes", [])]
8890
if len(note_types) != len(set(note_types)):
8991
return _("Cannot have multiple notes of the same type.")
92+
# Deleted-document guard: a document may be deleted once all its order
93+
# lines reach a terminal status (RECEIVED or CANCELLED). Prevent any
94+
# subsequent update that would re-activate the line (remove or unset
95+
# `is_cancelled`) while the document no longer exists.
96+
if self.updated and not self.get("is_cancelled") and not self.document:
97+
return _("Cannot save an active order line for a deleted document.")
9098
from rero_ils.modules.acquisition.acq_orders.api import AcqOrder
9199
from rero_ils.modules.acquisition.acq_orders.models import AcqOrderStatus
92100

@@ -143,6 +151,26 @@ def _build_total_amount(cls, data):
143151
"""Build total amount for order line."""
144152
data["total_amount"] = data["amount"] * data["quantity"]
145153

154+
def replace_refs(self):
155+
"""Replace $ref with real data, handling deleted documents gracefully.
156+
157+
The document linked to an order line may have been deleted (allowed
158+
when all order lines reach a terminal status). To avoid a
159+
``JsonRefError`` during indexing, we resolve the document ref manually
160+
before delegating to the parent, then restore the original ref.
161+
"""
162+
doc_ref = self.pop("document", None)
163+
try:
164+
data = super().replace_refs()
165+
finally:
166+
if doc_ref:
167+
self["document"] = doc_ref
168+
if doc_ref:
169+
doc_pid = extracted_data_from_ref(doc_ref)
170+
# Mimic what the standard jsonresolver returns: {pid, type}.
171+
data["document"] = {"pid": doc_pid, "type": "doc"}
172+
return data
173+
146174
# GETTER & SETTER =========================================================
147175
# * Define some properties as shortcut to quickly access object attrs.
148176
# * Defines some getter methods to access specific object values.

rero_ils/modules/acquisition/acq_order_lines/dumpers.py

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# -*- coding: utf-8 -*-
22
#
33
# RERO ILS
4-
# Copyright (C) 2019-2022 RERO
4+
# Copyright (C) 2019-2026 RERO
55
# Copyright (C) 2019-2022 UCLouvain
66
#
77
# This program is free software: you can redistribute it and/or modify
@@ -59,15 +59,18 @@ def dump(self, record, data):
5959
# Add document information's: pid, formatted title and ISBN
6060
# identifiers (remove None values from document metadata)
6161
document = record.document
62-
identifiers = document.get_identifiers(filters=[IdentifierType.ISBN], with_alternatives=True)
63-
identifiers = [identifier.normalize() for identifier in identifiers]
64-
65-
data["document"] = {
66-
"pid": document.pid,
67-
"title": TitleExtension.format_text(document.get("title", [])),
68-
"identifiers": identifiers,
69-
}
70-
data["document"] = {k: v for k, v in data["document"].items() if v}
62+
if document:
63+
identifiers = document.get_identifiers(filters=[IdentifierType.ISBN], with_alternatives=True)
64+
identifiers = [identifier.normalize() for identifier in identifiers]
65+
data["document"] = {
66+
"pid": document.pid,
67+
"title": TitleExtension.format_text(document.get("title", [])),
68+
"identifiers": identifiers,
69+
}
70+
data["document"] = {k: v for k, v in data["document"].items() if v}
71+
else:
72+
# Document has been deleted; keep only the pid extracted from the $ref
73+
data["document"] = {"pid": record.document_pid}
7174
return data
7275

7376

rero_ils/modules/acquisition/acq_receipt_lines/dumpers.py

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,19 @@ def dump(self, record, data):
5050
# Add document information's: pid, formatted title and ISBN identifiers
5151
# (remove None values from document metadata)
5252
document = order_line.document
53-
identifiers = document.get_identifiers(filters=[IdentifierType.ISBN], with_alternatives=True)
54-
identifiers = [identifier.normalize() for identifier in identifiers]
55-
data["document"] = {
56-
"pid": document.pid,
57-
"title": TitleExtension.format_text(document.get("title", [])),
58-
"identifiers": identifiers,
59-
}
60-
data["document"] = {k: v for k, v in data["document"].items() if v}
53+
if document:
54+
identifiers = document.get_identifiers(filters=[IdentifierType.ISBN], with_alternatives=True)
55+
identifiers = [identifier.normalize() for identifier in identifiers]
56+
data["document"] = {
57+
k: v
58+
for k, v in {
59+
"pid": document.pid,
60+
"title": TitleExtension.format_text(document.get("title", [])),
61+
"identifiers": identifiers,
62+
}.items()
63+
if v
64+
}
65+
else:
66+
# Document has been deleted; keep only the pid extracted from the $ref
67+
data["document"] = {"pid": order_line.document_pid}
6168
return data

rero_ils/modules/documents/api.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# -*- coding: utf-8 -*-
22
#
33
# RERO ILS
4-
# Copyright (C) 2019-2023 RERO
4+
# Copyright (C) 2019-2026 RERO
55
# Copyright (C) 2019-2023 UCLouvain
66
#
77
# This program is free software: you can redistribute it and/or modify
@@ -29,6 +29,7 @@
2929
from jsonschema.exceptions import ValidationError
3030

3131
from rero_ils.modules.acquisition.acq_order_lines.api import AcqOrderLinesSearch
32+
from rero_ils.modules.acquisition.acq_order_lines.models import AcqOrderLineStatus
3233
from rero_ils.modules.api import IlsRecord, IlsRecordsIndexer, IlsRecordsSearch
3334
from rero_ils.modules.commons.identifiers import IdentifierFactory, IdentifierType
3435
from rero_ils.modules.documents.tasks import reindex_document_items
@@ -57,6 +58,12 @@
5758
# fetcher
5859
document_id_fetcher = partial(id_fetcher, provider=DocumentProvider)
5960

61+
ACQ_ORDER_LINE_BLOCKING_STATUSES = [
62+
AcqOrderLineStatus.APPROVED,
63+
AcqOrderLineStatus.ORDERED,
64+
AcqOrderLineStatus.PARTIALLY_RECEIVED,
65+
]
66+
6067

6168
class DocumentsSearch(IlsRecordsSearch):
6269
"""DocumentsSearch."""
@@ -313,8 +320,14 @@ def get_links_to_me(self, get_pids=False):
313320
exclude_states=[LoanState.CANCELLED, LoanState.ITEM_RETURNED],
314321
)
315322
file_query = self.get_records_files_query().source()
316-
acq_order_lines_query = AcqOrderLinesSearch().filter("term", document__pid=self.pid)
323+
324+
acq_order_lines_query = (
325+
AcqOrderLinesSearch()
326+
.filter("term", document__pid=self.pid)
327+
.filter("terms", status=ACQ_ORDER_LINE_BLOCKING_STATUSES)
328+
)
317329
local_fields_query = LocalFieldsSearch().get_local_fields(self.provider.pid_type, self.pid)
330+
318331
relation_types = {
319332
"partOf": "partOf.document.pid",
320333
"supplement": "supplement.pid",

tests/api/acquisition/test_acquisition_reception_workflow.py

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# -*- coding: utf-8 -*-
22
#
33
# RERO ILS
4-
# Copyright (C) 2021 RERO
4+
# Copyright (C) 2021-2026 RERO
55
#
66
# This program is free software: you can redistribute it and/or modify
77
# it under the terms of the GNU Affero General Public License as published by
@@ -19,14 +19,18 @@
1919

2020
from unittest import mock
2121

22+
import pytest
2223
from flask import url_for
2324
from invenio_accounts.testutils import login_user_via_session
25+
from jsonschema.exceptions import ValidationError
2426

2527
from rero_ils.modules.acquisition.acq_order_lines.api import AcqOrderLine
28+
from rero_ils.modules.acquisition.acq_order_lines.dumpers import AcqOrderLineESDumper
2629
from rero_ils.modules.acquisition.acq_order_lines.models import AcqOrderLineStatus
2730
from rero_ils.modules.acquisition.acq_orders.api import AcqOrder
2831
from rero_ils.modules.acquisition.acq_orders.models import AcqOrderStatus
2932
from rero_ils.modules.acquisition.acq_receipt_lines.api import AcqReceiptLinesSearch
33+
from rero_ils.modules.acquisition.acq_receipt_lines.dumpers import AcqReceiptLineESDumper
3034
from rero_ils.modules.acquisition.acq_receipts.api import (
3135
AcqReceipt,
3236
AcqReceiptLine,
@@ -35,6 +39,7 @@
3539
from rero_ils.modules.acquisition.acq_receipts.models import (
3640
AcqReceiptLineCreationStatus,
3741
)
42+
from rero_ils.modules.documents.api import Document
3843
from rero_ils.modules.notifications.api import Notification
3944
from rero_ils.modules.notifications.models import (
4045
NotificationChannel,
@@ -64,6 +69,7 @@ def test_acquisition_reception_workflow(
6469
document,
6570
):
6671
"""Test complete acquisition workflow."""
72+
assert "acq_order_lines" not in document.get_links_to_me()
6773

6874
def assert_account_data(accounts):
6975
"""Assert account informations."""
@@ -286,6 +292,8 @@ def assert_account_data(accounts):
286292
}
287293
assert_account_data(manual_controls)
288294

295+
assert document.get_links_to_me() == {"acq_order_lines": 6}
296+
289297
# STEP 3 :: UPDATE ORDER LINES
290298
# * Cancel some order lines and change some quantities --> make sure
291299
# calculations still good
@@ -363,6 +371,8 @@ def assert_account_data(accounts):
363371
assert order_line_1.unreceived_quantity == 5
364372
assert order_line_1.status == AcqOrderLineStatus.APPROVED
365373

374+
assert document.get_links_to_me() == {"acq_order_lines": 4}
375+
366376
# STEP 4 :: SEND THE ORDER
367377
# * Test send order and make sure statuses are up to date.
368378
# - check order lines (status, order-date)
@@ -611,6 +621,48 @@ def assert_account_data(accounts):
611621
}
612622
assert_account_data(manual_controls)
613623

624+
# All order lines are now RECEIVED or CANCELLED (no blocking statuses remain),
625+
# so the document should have no links preventing deletion.
626+
assert "acq_order_lines" not in document.get_links_to_me()
627+
assert document.reasons_not_to_delete() == {}
628+
629+
# STEP 7.5: DOCUMENT DELETION + RE-INDEXATION WITH DELETED DOCUMENT
630+
# * Delete the document to validate the deletion path and
631+
# exercise the guards:
632+
# - AcqOrderLine.replace_refs(): must not raise JsonRefError when
633+
# the order line is re-indexed after document deletion.
634+
# - AcqOrderLineESDumper: must fall back to {"pid": ...} only.
635+
# - AcqReceiptLineESDumper: same fallback for receipt-line ES dumps.
636+
doc_pid = document.pid
637+
url = url_for("invenio_records_rest.doc_item", pid_value=doc_pid)
638+
res = client.delete(url)
639+
assert res.status_code == 204
640+
assert Document.get_record_by_pid(doc_pid) is None
641+
642+
# replace_refs() must return the document stub without raising JsonRefError.
643+
terminal_line = AcqOrderLine.get_record_by_pid(order_line_5.pid)
644+
replaced = terminal_line.replace_refs()
645+
assert replaced["document"] == {"pid": doc_pid, "type": "doc"}
646+
647+
# AcqOrderLineESDumper must fall back to pid-only when document is absent.
648+
dumped_ol = terminal_line.dumps(dumper=AcqOrderLineESDumper())
649+
assert dumped_ol["document"] == {"pid": doc_pid}
650+
651+
# AcqReceiptLineESDumper must also fall back to pid-only.
652+
receipt_line = next(receipt_2.get_receipt_lines())
653+
dumped_rl = receipt_line.dumps(dumper=AcqReceiptLineESDumper())
654+
assert dumped_rl["document"] == {"pid": doc_pid}
655+
656+
# Full re-indexation (the _prepare_record / replace_refs path) must not raise.
657+
terminal_line.reindex()
658+
659+
# extended_validation must reject any attempt to un-cancel a CANCELLED
660+
# order line whose document is gone (deleted-document guard).
661+
cancelled_line = AcqOrderLine.get_record_by_pid(order_line_3.pid)
662+
assert cancelled_line.get("is_cancelled") # sanity: must be CANCELLED
663+
with pytest.raises(ValidationError, match="deleted document"):
664+
cancelled_line.update({"is_cancelled": False})
665+
614666
# TEST 8: DELETE RECEIPTS
615667
# * Delete the second receipt. This will also delete the related receipt
616668
# lines. The order status must remain to PARTIALLY_RECEIVED.

0 commit comments

Comments
 (0)