Skip to content

Commit 41819da

Browse files
committed
claims: manage missing data for claim notification
* Closes #3379. * Checks if all requires data are available to render the claim notification content. If not, return a specific exception to specify the missing data. Co-Authored-by: Renaud Michotte <[email protected]>
1 parent ab158f1 commit 41819da

File tree

7 files changed

+86
-17
lines changed

7 files changed

+86
-17
lines changed

rero_ils/modules/commons/exceptions.py

+20-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121

2222
class RecordNotFound(Exception):
23-
"""Record con't be found into Invenio."""
23+
"""Record can't be found into Invenio."""
2424

2525
def __init__(self, record_cls, record_pid):
2626
"""Initialization method.
@@ -31,6 +31,24 @@ def __init__(self, record_cls, record_pid):
3131
self.record_cls = record_cls
3232
self.record_pid = record_pid
3333

34-
def __str__(self):
34+
def __repr__(self):
3535
"""String representation of the exception."""
3636
return f'{self.record_cls.__name__}#{self.record_pid} not found'
37+
38+
39+
class MissingDataException(KeyError):
40+
"""Exception when a data is missing."""
41+
42+
def __init__(self, missing_data):
43+
"""Initialization method.
44+
45+
:param missing_data: list of missing data field names.
46+
:type missing_data: str|list<str>
47+
"""
48+
if not isinstance(missing_data, list):
49+
missing_data = [missing_data]
50+
self.missing_data = missing_data
51+
52+
def __repr__(self):
53+
"""String representation of the exception."""
54+
return f'Missing data :: {", ".join(self.missing_data)}'

rero_ils/modules/items/dumpers.py

+12-7
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
from invenio_records.dumpers import Dumper as InvenioRecordsDumper
2222

23+
from rero_ils.modules.commons.exceptions import MissingDataException
2324
from rero_ils.modules.documents.dumpers import \
2425
TitleDumper as DocumentTitleDumper
2526
from rero_ils.modules.holdings.api import Holding
@@ -82,19 +83,23 @@ class ClaimIssueNotificationDumper(InvenioRecordsDumper):
8283

8384
def dump(self, record, data):
8485
"""Dump an item issue for claim notification generation."""
85-
assert record.is_issue, 'record must be an `ItemIssue` resource'
86-
assert (holding := record.holding), \
87-
'record must be related to an existing `Holding`'
86+
if not record.is_issue:
87+
raise TypeError('record must be an `ItemIssue` resource')
88+
if not (holding := record.holding):
89+
raise MissingDataException('item.holding')
90+
if not (vendor := holding.vendor):
91+
raise MissingDataException('item.holding.vendor')
8892

8993
data.update({
90-
'vendor': holding.vendor.dumps(
94+
'vendor': vendor.dumps(
9195
dumper=VendorClaimIssueNotificationDumper()),
92-
'document': holding.document.dumps(dumper=DocumentTitleDumper()),
96+
'document': holding.document.dumps(
97+
dumper=DocumentTitleDumper()),
9398
'library': holding.library.dumps(
9499
dumper=LibrarySerialClaimNotificationDumper()),
95-
'holdings': holding.dumps(dumper=ClaimIssueHoldingDumper()),
100+
'holdings': holding.dumps(
101+
dumper=ClaimIssueHoldingDumper()),
96102
'enumerationAndChronology': record.enumerationAndChronology,
97103
'claim_counter': record.claims_count
98104
})
99-
100105
return {k: v for k, v in data.items() if v is not None}

rero_ils/modules/items/views/api_views.py

+6-1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
from ..models import ItemCirculationAction
5353
from ..permissions import late_issue_management as late_issue_management_action
5454
from ..utils import get_recipient_suggestions, item_pid_to_object
55+
from ...commons.exceptions import MissingDataException
5556

5657
api_blueprint = Blueprint(
5758
'api_item',
@@ -572,7 +573,11 @@ def claim_notification_preview(item_pid):
572573
if not record.is_issue:
573574
abort(400, 'Item isn\'t an issue')
574575

575-
issue_data = record.dumps(dumper=ClaimIssueNotificationDumper())
576+
try:
577+
issue_data = record.dumps(dumper=ClaimIssueNotificationDumper())
578+
except (TypeError, MissingDataException) as exp:
579+
abort(500, str(exp))
580+
576581
# update the claims issue counter ::
577582
# As this is preview for next claim, we need to add 1 to the returned
578583
# claim counter

rero_ils/modules/libraries/dumpers.py

+4
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
from invenio_records.dumpers import Dumper as InvenioRecordsDumper
2222

23+
from rero_ils.modules.commons.exceptions import MissingDataException
2324
from rero_ils.modules.libraries.models import LibraryAddressType
2425

2526

@@ -55,6 +56,9 @@ def dump(self, record, data):
5556
:param record: The record to dump.
5657
:param data: The initial dump data passed in by ``record.dumps()``.
5758
"""
59+
if 'serial_acquisition_settings' not in record:
60+
raise MissingDataException('library.serial_acquisition_settings')
61+
5862
data.update({
5963
'name': record.get('name'),
6064
'address': record.get_address(LibraryAddressType.MAIN_ADDRESS),

tests/api/items/test_items_issue.py

+11
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@
2323
from utils import VerifyRecordPermissionPatch, flush_index, get_csv, \
2424
get_json, parse_csv, postdata
2525

26+
from rero_ils.modules.commons.exceptions import MissingDataException
2627
from rero_ils.modules.holdings.api import Holding
2728
from rero_ils.modules.items.api import Item
29+
from rero_ils.modules.items.dumpers import ClaimIssueNotificationDumper
2830
from rero_ils.modules.notifications.api import Notification, \
2931
NotificationsSearch
3032
from rero_ils.modules.notifications.models import RecipientType
@@ -90,6 +92,7 @@ def test_issues_claim_notifications(
9092
# 1) call with unknown item --> return 404
9193
# 2) call with a standard item --> return 400
9294
# 3) simulate a template rendering error --> return 500
95+
# 4) missing data --> return 500
9396
# 4) call with an issue item --> return 200
9497
for pid, ret_code in [('dummy_pid', 404), (item.pid, 400)]:
9598
url = url_for('api_item.claim_notification_preview', item_pid=pid)
@@ -104,6 +107,14 @@ def test_issues_claim_notifications(
104107
assert response.status_code == 500
105108
assert 'my_error' in response.json['message']
106109

110+
with mock.patch.object(
111+
ClaimIssueNotificationDumper, 'dump',
112+
mock.MagicMock(side_effect=MissingDataException('Test!'))
113+
):
114+
response = client.get(url)
115+
assert response.status_code == 500
116+
assert 'Test!' in response.json['message']
117+
107118
response = client.get(url)
108119
assert response.status_code == 200
109120
assert all(field in response.json

tests/ui/items/test_items_dumpers.py

+25-1
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,14 @@
1919
"""Items Record dumper tests."""
2020
from copy import deepcopy
2121

22+
import pytest
23+
24+
from rero_ils.modules.commons.exceptions import MissingDataException
2225
from rero_ils.modules.holdings.api import Holding
23-
from rero_ils.modules.items.dumpers import ItemCirculationDumper
26+
from rero_ils.modules.items.dumpers import ClaimIssueNotificationDumper, \
27+
ItemCirculationDumper
28+
from rero_ils.modules.items.models import TypeOfItem
29+
from rero_ils.modules.utils import get_ref_for_pid
2430

2531

2632
def test_item_circulation_dumper(item_lib_martigny):
@@ -52,3 +58,21 @@ def test_item_circulation_dumper(item_lib_martigny):
5258

5359
# RESET HOLDING RECORD
5460
holdings.update(original_holding_data, dbcommit=True, reindex=True)
61+
62+
63+
def test_claim_issue_dumper(item_lib_martigny):
64+
"""Test claim issue notification dumper."""
65+
with pytest.raises(TypeError):
66+
item_lib_martigny.dumps(dumper=ClaimIssueNotificationDumper())
67+
68+
item_lib_martigny['type'] = TypeOfItem.ISSUE
69+
holding = item_lib_martigny.holding
70+
holding.pop('vendor', None)
71+
with pytest.raises(MissingDataException) as exc:
72+
item_lib_martigny.dumps(dumper=ClaimIssueNotificationDumper())
73+
assert 'item.holding.vendor' in str(exc)
74+
75+
item_lib_martigny['holding']['$ref'] = get_ref_for_pid('hold', 'dummy')
76+
with pytest.raises(MissingDataException) as exc:
77+
item_lib_martigny.dumps(dumper=ClaimIssueNotificationDumper())
78+
assert 'item.holding' in str(exc)

tests/ui/libraries/test_libraries_dumpers.py

+8-6
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,15 @@
1717
# along with this program. If not, see <http://www.gnu.org/licenses/>.
1818

1919
"""Library Record dumpers."""
20+
import pytest
21+
22+
from rero_ils.modules.commons.exceptions import MissingDataException
2023
from rero_ils.modules.libraries.dumpers import \
2124
LibraryAcquisitionNotificationDumper, LibrarySerialClaimNotificationDumper
2225

2326

24-
def test_library_serial_claim_dumper(lib_martigny, lib_saxon):
25-
"""Test serial claim library dumper."""
27+
def test_library_serial_dumpers(lib_martigny, lib_saxon):
28+
"""Test serial library dumpers."""
2629

2730
# Acquisition dumper
2831
dump_data = lib_martigny.dumps(LibraryAcquisitionNotificationDumper())
@@ -37,7 +40,6 @@ def test_library_serial_claim_dumper(lib_martigny, lib_saxon):
3740
assert data['address']
3841
assert data['shipping_informations']
3942
assert data['billing_informations']
40-
data = lib_saxon.dumps(LibrarySerialClaimNotificationDumper())
41-
assert data['address']
42-
assert 'shipping_informations' not in data
43-
assert 'billing_informations' not in data
43+
with pytest.raises(MissingDataException) as exc:
44+
lib_saxon.dumps(LibrarySerialClaimNotificationDumper())
45+
assert 'library.serial_acquisition_settings' in str(exc)

0 commit comments

Comments
 (0)