Skip to content

Commit 2982f86

Browse files
pi-sigmaswrichards
authored andcommitted
[#2920] Fix check for natural keys duplicates in ZGW imports
- The check for natural keys duplicates produced false positives because the same `omschrijving` can be used for different types of ZGW objects (e.g. "Afgehandel" for status and resulaat). The check for duplicate natural keys is now scoped to the type of ZGW configuration.
1 parent 3c302d7 commit 2982f86

File tree

2 files changed

+117
-46
lines changed

2 files changed

+117
-46
lines changed

src/open_inwoner/openzaak/import_export.py

+4-3
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ def from_jsonl_stream_or_string(cls, stream_or_string: IO | str) -> Self:
334334

335335
rows_successfully_processed = 0
336336
import_errors = []
337-
natural_keys_seen = set()
337+
natural_keys_seen = defaultdict(set)
338338
for line in cls._lines_iter_from_jsonl_stream_or_string(stream_or_string):
339339
try:
340340
(deserialized_object,) = serializers.deserialize(
@@ -351,10 +351,11 @@ def from_jsonl_stream_or_string(cls, stream_or_string: IO | str) -> Self:
351351
import_errors.append(error)
352352
else:
353353
source_config = deserialized_object.object
354-
if (natural_key := source_config.natural_key()) in natural_keys_seen:
354+
natural_key = source_config.natural_key()
355+
if natural_key in natural_keys_seen[source_config.__class__.__name__]:
355356
import_errors.append(ZGWImportError.from_jsonl(line))
356357
continue
357-
natural_keys_seen.add(natural_key)
358+
natural_keys_seen[source_config.__class__.__name__].add(natural_key)
358359
try:
359360
match source_config:
360361
case CatalogusConfig():

src/open_inwoner/openzaak/tests/test_import_export.py

+113-43
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,7 @@
55
from django.core.files.storage.memory import InMemoryStorage
66
from django.test import TestCase
77

8-
from open_inwoner.openzaak.import_export import (
9-
CatalogusConfigExport,
10-
CatalogusConfigImport,
11-
)
8+
from open_inwoner.openzaak.import_export import ZGWConfigExport, ZGWConfigImport
129
from open_inwoner.openzaak.models import (
1310
CatalogusConfig,
1411
ZaakTypeConfig,
@@ -29,7 +26,7 @@
2926

3027

3128
class ZGWExportImportMockData:
32-
def __init__(self, count=0):
29+
def __init__(self, count=0, with_dupes=False):
3330
self.original_url = f"https://foo.{count}.maykinmedia.nl"
3431
self.original_uuid = "a1591906-3368-470a-a957-4b8634c275a1"
3532
self.service = ServiceFactory(slug=f"service-{count}")
@@ -81,6 +78,14 @@ def __init__(self, count=0):
8178
omschrijving="informatieobject",
8279
zaaktype_uuids=[self.original_uuid],
8380
)
81+
if with_dupes:
82+
self.ztc_resultaat_2 = ZaakTypeResultaatTypeConfigFactory(
83+
zaaktype_config=self.ztc,
84+
resultaattype_url=self.original_url,
85+
omschrijving="status omschrijving", # test dupes across models
86+
zaaktype_uuids=[self.original_uuid],
87+
description="",
88+
)
8489

8590

8691
class ExportObjectTests(TestCase):
@@ -94,17 +99,17 @@ def test_export_only_accepts_queryset(self):
9499
for arg in (list(), set(), tuple(), None, "", CatalogusConfig.objects.first()):
95100
with self.subTest(f"Default factory should not accept {arg}"):
96101
with self.assertRaises(TypeError):
97-
CatalogusConfigExport.from_catalogus_configs(arg)
102+
ZGWConfigExport.from_catalogus_configs(arg)
98103

99104
def test_export_only_accepts_queryset_of_correct_type(self):
100105
with self.assertRaises(ValueError):
101-
CatalogusConfigExport.from_catalogus_configs(ZaakTypeConfig.objects.all())
106+
ZGWConfigExport.from_catalogus_configs(ZaakTypeConfig.objects.all())
102107

103108
def test_equality_operator(self):
104-
export_a = CatalogusConfigExport.from_catalogus_configs(
109+
export_a = ZGWConfigExport.from_catalogus_configs(
105110
CatalogusConfig.objects.filter(pk=self.mocks[0].catalogus.pk)
106111
)
107-
export_b = CatalogusConfigExport.from_catalogus_configs(
112+
export_b = ZGWConfigExport.from_catalogus_configs(
108113
CatalogusConfig.objects.filter(pk=self.mocks[1].catalogus.pk)
109114
)
110115

@@ -113,14 +118,14 @@ def test_equality_operator(self):
113118
self.assertFalse(export_a == export_b)
114119

115120
def test_only_models_related_to_exported_catalogus_config_are_included(self):
116-
export = CatalogusConfigExport.from_catalogus_configs(
121+
export = ZGWConfigExport.from_catalogus_configs(
117122
CatalogusConfig.objects.filter(pk=self.mocks[0].catalogus.pk)
118123
)
119124

120125
for export_field, mock_field in zip(
121126
(
122127
"catalogus_configs",
123-
"zaak_type_configs",
128+
"zaaktype_configs",
124129
"zaak_status_type_configs",
125130
"zaak_resultaat_type_configs",
126131
"zaak_informatie_object_type_configs",
@@ -148,6 +153,79 @@ def test_only_models_related_to_exported_catalogus_config_are_included(self):
148153
)
149154

150155

156+
class ZaakTypeConfigExportTest(TestCase):
157+
def setUp(self):
158+
self.mocks = ZGWExportImportMockData(0)
159+
160+
def test_export_zaaktype_configs(self):
161+
export = ZGWConfigExport.from_zaaktype_configs(
162+
ZaakTypeConfig.objects.filter(pk=self.mocks.ztc.pk)
163+
)
164+
rows = export.as_dicts()
165+
166+
expected = [
167+
{
168+
"model": "openzaak.zaaktypeconfig",
169+
"fields": {
170+
"urls": '["https://foo.0.maykinmedia.nl"]',
171+
"catalogus": ["DM-0", "123456789"],
172+
"identificatie": "ztc-id-a-0",
173+
"omschrijving": "zaaktypeconfig",
174+
"notify_status_changes": False,
175+
"description": "",
176+
"external_document_upload_url": "",
177+
"document_upload_enabled": False,
178+
"contact_form_enabled": False,
179+
"contact_subject_code": "",
180+
"relevante_zaakperiode": None,
181+
},
182+
},
183+
{
184+
"model": "openzaak.zaaktypeinformatieobjecttypeconfig",
185+
"fields": {
186+
"zaaktype_config": ["ztc-id-a-0", "DM-0", "123456789"],
187+
"informatieobjecttype_url": "https://foo.0.maykinmedia.nl",
188+
"omschrijving": "informatieobject",
189+
"zaaktype_uuids": '["a1591906-3368-470a-a957-4b8634c275a1"]',
190+
"document_upload_enabled": False,
191+
"document_notification_enabled": False,
192+
},
193+
},
194+
{
195+
"model": "openzaak.zaaktypestatustypeconfig",
196+
"fields": {
197+
"zaaktype_config": ["ztc-id-a-0", "DM-0", "123456789"],
198+
"statustype_url": "https://foo.0.maykinmedia.nl",
199+
"omschrijving": "status omschrijving",
200+
"statustekst": "",
201+
"zaaktype_uuids": '["a1591906-3368-470a-a957-4b8634c275a1"]',
202+
"status_indicator": "",
203+
"status_indicator_text": "",
204+
"document_upload_description": "",
205+
"description": "status",
206+
"notify_status_change": True,
207+
"action_required": False,
208+
"document_upload_enabled": True,
209+
"call_to_action_url": "",
210+
"call_to_action_text": "",
211+
"case_link_text": "",
212+
},
213+
},
214+
{
215+
"model": "openzaak.zaaktyperesultaattypeconfig",
216+
"fields": {
217+
"zaaktype_config": ["ztc-id-a-0", "DM-0", "123456789"],
218+
"resultaattype_url": "https://foo.0.maykinmedia.nl",
219+
"omschrijving": "resultaat",
220+
"zaaktype_uuids": '["a1591906-3368-470a-a957-4b8634c275a1"]',
221+
"description": "",
222+
},
223+
},
224+
]
225+
226+
self.assertEqual(rows, expected)
227+
228+
151229
class TestCatalogusExport(TestCase):
152230
def setUp(self):
153231
self.mocks = [
@@ -156,7 +234,7 @@ def setUp(self):
156234
]
157235

158236
def test_export_catalogus_configs(self):
159-
export = CatalogusConfigExport.from_catalogus_configs(
237+
export = ZGWConfigExport.from_catalogus_configs(
160238
CatalogusConfig.objects.filter(pk=self.mocks[0].catalogus.pk)
161239
)
162240
rows = export.as_dicts()
@@ -233,9 +311,7 @@ def test_export_catalogus_configs(self):
233311
self.assertEqual(rows, expected)
234312

235313
def test_export_catalogus_configs_as_jsonl(self):
236-
export = CatalogusConfigExport.from_catalogus_configs(
237-
CatalogusConfig.objects.all()
238-
)
314+
export = ZGWConfigExport.from_catalogus_configs(CatalogusConfig.objects.all())
239315
rows = list(export.as_jsonl_iter())
240316

241317
expected = [
@@ -276,6 +352,7 @@ def setUp(self):
276352
]
277353
self.json_dupes = [
278354
'{"model": "openzaak.zaaktypestatustypeconfig", "fields": {"zaaktype_config": ["ztc-id-a-0", "DM-0", "123456789"], "statustype_url": "https://bar.maykinmedia.nl", "omschrijving": "status omschrijving", "statustekst": "statustekst nieuw", "zaaktype_uuids": "[]", "status_indicator": "", "status_indicator_text": "", "document_upload_description": "", "description": "status", "notify_status_change": true, "action_required": false, "document_upload_enabled": true, "call_to_action_url": "", "call_to_action_text": "", "case_link_text": ""}}',
355+
'{"model": "openzaak.zaaktyperesultaattypeconfig", "fields": {"zaaktype_config": ["ztc-id-a-0", "DM-0", "123456789"], "resultaattype_url": "https://bar.maykinmedia.nl", "omschrijving": "status omschrijving", "zaaktype_uuids": "[]", "description": "description new"}}',
279356
]
280357
self.jsonl = "\n".join(self.json_lines)
281358
self.jsonl_with_dupes = "\n".join(self.json_lines + self.json_dupes)
@@ -284,14 +361,14 @@ def test_import_jsonl_update_success(self):
284361
mocks = ZGWExportImportMockData()
285362
self.storage.save("import.jsonl", io.StringIO(self.jsonl))
286363

287-
import_result = CatalogusConfigImport.import_from_jsonl_file_in_django_storage(
364+
import_result = ZGWConfigImport.import_from_jsonl_file_in_django_storage(
288365
"import.jsonl", self.storage
289366
)
290367

291368
# check import
292369
self.assertEqual(
293370
import_result,
294-
CatalogusConfigImport(
371+
ZGWConfigImport(
295372
total_rows_processed=5,
296373
catalogus_configs_imported=1,
297374
zaaktype_configs_imported=1,
@@ -371,7 +448,7 @@ def test_import_jsonl_missing_statustype_config(self):
371448
# we use `asdict` and replace the Exceptions with string representations
372449
# because for Exceptions raised from within dataclasses, equality ==/is identity
373450
import_result = dataclasses.asdict(
374-
CatalogusConfigImport.import_from_jsonl_file_in_django_storage(
451+
ZGWConfigImport.import_from_jsonl_file_in_django_storage(
375452
"import.jsonl", self.storage
376453
)
377454
)
@@ -380,7 +457,7 @@ def test_import_jsonl_missing_statustype_config(self):
380457
"ZaakTypeConfig identificatie = 'ztc-id-a-0'"
381458
)
382459
import_expected = dataclasses.asdict(
383-
CatalogusConfigImport(
460+
ZGWConfigImport(
384461
total_rows_processed=6,
385462
catalogus_configs_imported=1,
386463
zaaktype_configs_imported=1,
@@ -417,7 +494,7 @@ def test_import_jsonl_update_statustype_config_missing_zt_config(self):
417494
# we use `asdict` and replace the Exceptions with string representations
418495
# because for Exceptions raised from within dataclasses, equality ==/is identity
419496
import_result = dataclasses.asdict(
420-
CatalogusConfigImport.import_from_jsonl_file_in_django_storage(
497+
ZGWConfigImport.import_from_jsonl_file_in_django_storage(
421498
"import.jsonl", self.storage
422499
)
423500
)
@@ -426,7 +503,7 @@ def test_import_jsonl_update_statustype_config_missing_zt_config(self):
426503
"ZaakTypeConfig identificatie = 'bogus'"
427504
)
428505
import_expected = dataclasses.asdict(
429-
CatalogusConfigImport(
506+
ZGWConfigImport(
430507
total_rows_processed=6,
431508
catalogus_configs_imported=1,
432509
zaaktype_configs_imported=1,
@@ -463,7 +540,7 @@ def test_import_jsonl_update_reports_duplicate_db_records(self):
463540
# we use `asdict` and replace the Exceptions with string representations
464541
# because for Exceptions raised from within dataclasses, equality ==/is identity
465542
import_result = dataclasses.asdict(
466-
CatalogusConfigImport.import_from_jsonl_file_in_django_storage(
543+
ZGWConfigImport.import_from_jsonl_file_in_django_storage(
467544
"import.jsonl", self.storage
468545
)
469546
)
@@ -472,7 +549,7 @@ def test_import_jsonl_update_reports_duplicate_db_records(self):
472549
"ZaakTypeConfig identificatie = 'ztc-id-a-0'"
473550
)
474551
import_expected = dataclasses.asdict(
475-
CatalogusConfigImport(
552+
ZGWConfigImport(
476553
total_rows_processed=5,
477554
catalogus_configs_imported=1,
478555
zaaktype_configs_imported=1,
@@ -489,14 +566,14 @@ def test_import_jsonl_update_reports_duplicate_db_records(self):
489566
self.assertEqual(import_result, import_expected)
490567

491568
def test_import_jsonl_update_reports_duplicate_natural_keys_in_upload_file(self):
492-
mocks = ZGWExportImportMockData()
569+
mocks = ZGWExportImportMockData(with_dupes=True)
493570

494571
self.storage.save("import.jsonl", io.StringIO(self.jsonl_with_dupes))
495572

496573
# we use `asdict` and replace the Exceptions with string representations
497574
# because for Exceptions raised from within dataclasses, equality ==/is identity
498575
import_result = dataclasses.asdict(
499-
CatalogusConfigImport.import_from_jsonl_file_in_django_storage(
576+
ZGWConfigImport.import_from_jsonl_file_in_django_storage(
500577
"import.jsonl", self.storage
501578
)
502579
)
@@ -505,13 +582,14 @@ def test_import_jsonl_update_reports_duplicate_natural_keys_in_upload_file(self)
505582
"natural keys: omschrijving = 'status omschrijving', ZaakTypeConfig identificatie = 'ztc-id-a-0'"
506583
)
507584
import_expected = dataclasses.asdict(
508-
CatalogusConfigImport(
509-
total_rows_processed=6,
585+
ZGWConfigImport(
586+
total_rows_processed=7,
510587
catalogus_configs_imported=1,
511588
zaaktype_configs_imported=1,
512589
zaak_informatie_object_type_configs_imported=1,
513590
zaak_status_type_configs_imported=1,
514-
zaak_resultaat_type_configs_imported=1,
591+
# resultaat_type_config with "status omschrijving" should be imported
592+
zaak_resultaat_type_configs_imported=2,
515593
import_errors=[expected_error],
516594
),
517595
)
@@ -538,9 +616,7 @@ def test_import_jsonl_fails_with_catalogus_domein_rsin_mismatch(self):
538616
with self.assertLogs(
539617
logger="open_inwoner.openzaak.import_export", level="ERROR"
540618
) as cm:
541-
import_result = CatalogusConfigImport.from_jsonl_stream_or_string(
542-
import_line
543-
)
619+
import_result = ZGWConfigImport.from_jsonl_stream_or_string(import_line)
544620
self.assertEqual(
545621
cm.output,
546622
[
@@ -567,7 +643,7 @@ def test_import_jsonl_fails_with_catalogus_domein_rsin_mismatch(self):
567643
def test_bad_import_types(self):
568644
for bad_type in (set(), list(), b""):
569645
with self.assertRaises(ValueError):
570-
CatalogusConfigImport.from_jsonl_stream_or_string(bad_type)
646+
ZGWConfigImport.from_jsonl_stream_or_string(bad_type)
571647

572648
def test_valid_input_types_are_accepted(self):
573649
ZGWExportImportMockData()
@@ -578,10 +654,10 @@ def test_valid_input_types_are_accepted(self):
578654
self.jsonl,
579655
):
580656
with self.subTest(f"Input type {type(input)}"):
581-
import_result = CatalogusConfigImport.from_jsonl_stream_or_string(input)
657+
import_result = ZGWConfigImport.from_jsonl_stream_or_string(input)
582658
self.assertEqual(
583659
import_result,
584-
CatalogusConfigImport(
660+
ZGWConfigImport(
585661
total_rows_processed=5,
586662
catalogus_configs_imported=1,
587663
zaaktype_configs_imported=1,
@@ -597,9 +673,7 @@ def test_import_is_atomic(self):
597673
bad_jsonl = self.jsonl + "\n" + bad_line
598674

599675
with self.assertRaises(KeyError):
600-
CatalogusConfigImport.from_jsonl_stream_or_string(
601-
stream_or_string=bad_jsonl
602-
)
676+
ZGWConfigImport.from_jsonl_stream_or_string(stream_or_string=bad_jsonl)
603677

604678
counts = (
605679
CatalogusConfig.objects.count(),
@@ -622,11 +696,7 @@ def setUp(self):
622696
ZGWExportImportMockData()
623697

624698
def test_exports_can_be_imported(self):
625-
export = CatalogusConfigExport.from_catalogus_configs(
626-
CatalogusConfig.objects.all()
627-
)
628-
import_result = CatalogusConfigImport.from_jsonl_stream_or_string(
629-
export.as_jsonl()
630-
)
699+
export = ZGWConfigExport.from_catalogus_configs(CatalogusConfig.objects.all())
700+
import_result = ZGWConfigImport.from_jsonl_stream_or_string(export.as_jsonl())
631701

632702
self.assertEqual(import_result.total_rows_processed, 5)

0 commit comments

Comments
 (0)