Merge pull request #1262 from maykinmedia/tasks/2564-catalogus-requir…

…ed-on-zaaktypeconfig

[#2564] Mark catalogus as a required field on ZaakTypeconfig

src/open_inwoner/openzaak/managers.py

@@ -86,20 +86,15 @@ def record_if_unique_notification(
 
 class ZaakTypeInformatieObjectTypeConfigQueryset(models.QuerySet):
     def filter_catalogus(self, case_type: ZaakType):
-        if case_type.catalogus:
-            # support both url and resolved dataclass
-            catalogus_url = (
-                case_type.catalogus
-                if isinstance(case_type.catalogus, str)
-                else case_type.catalogus.url
-            )
-            return self.filter(
-                zaaktype_config__catalogus__url=catalogus_url,
-            )
-        else:
-            return self.filter(
-                zaaktype_config__catalogus__isnull=True,
-            )
+        # support both url and resolved dataclass
+        catalogus_url = (
+            case_type.catalogus
+            if isinstance(case_type.catalogus, str)
+            else case_type.catalogus.url
+        )
+        return self.filter(
+            zaaktype_config__catalogus__url=catalogus_url,
+        )
 
     def filter_case_type(self, case_type: ZaakType):
         return self.filter_catalogus(case_type).filter(
@@ -129,37 +124,24 @@ def get_for_case_and_info_type(
 
 class ZaakTypeConfigQueryset(models.QuerySet):
     def filter_catalogus(self, case_type: ZaakType):
-        if case_type.catalogus:
-            # support both url and resolved dataclass
-            catalogus_url = (
-                case_type.catalogus
-                if isinstance(case_type.catalogus, str)
-                else case_type.catalogus.url
-            )
-            return self.filter(
-                catalogus__url=catalogus_url,
-            )
-        else:
-            return self.filter(
-                catalogus__isnull=True,
-            )
+        # support both url and resolved dataclass
+        catalogus_url = (
+            case_type.catalogus
+            if isinstance(case_type.catalogus, str)
+            else case_type.catalogus.url
+        )
+        return self.filter(
+            catalogus__url=catalogus_url,
+        )
 
     def filter_case_type(self, case_type: ZaakType):
         return self.filter_catalogus(case_type).filter(
             identificatie=case_type.identificatie,
         )
 
     def filter_from_str(self, catalogus_url: str, case_type_identificatie: str):
-        qs = self
-        if catalogus_url:
-            qs = qs.filter(
-                catalogus__url=catalogus_url,
-            )
-        else:
-            qs = qs.filter(
-                catalogus__isnull=True,
-            )
-        return qs.filter(
+        return self.filter(
+            catalogus__url=catalogus_url,
             identificatie=case_type_identificatie,
         )
 

src/open_inwoner/openzaak/migrations/0053_zaaktypeconfig_catalogus_is_required.py

@@ -0,0 +1,55 @@
+# Generated by Django 4.2.11 on 2024-06-18 12:04
+
+from django.db import DataError, migrations, models
+import django.db.models.deletion
+
+
+def validate_no_missing_catalogus_fields(apps, schema_editor):
+    ZaakTypeConfig = apps.get_model("openzaak", "ZaakTypeConfig")
+
+    if rows_with_missing_catalogus := ZaakTypeConfig.objects.filter(
+        catalogus__isnull=True
+    ).count():
+        raise DataError(
+            f"Your database contains {rows_with_missing_catalogus} ZaakTypeConfig"
+            " row(s) with a missing `catalogus` field. This field is now required:"
+            " please manually update all the affected rows or re-sync your ZGW"
+            " objects to ensure the field is included."
+        )
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("openzaak", "0052_add_catalogusconfig_service"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            validate_no_missing_catalogus_fields,
+            reverse_code=lambda *args, **kwargs: None,
+        ),
+        migrations.AlterField(
+            model_name="zaaktypeconfig",
+            name="catalogus",
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.CASCADE,
+                to="openzaak.catalogusconfig",
+            ),
+        ),
+        migrations.RemoveConstraint(
+            model_name="zaaktypeconfig",
+            name="unique_identificatie_in_catalogus",
+        ),
+        migrations.RemoveConstraint(
+            model_name="zaaktypeconfig",
+            name="unique_identificatie_without_catalogus",
+        ),
+        migrations.AddConstraint(
+            model_name="zaaktypeconfig",
+            constraint=models.UniqueConstraint(
+                fields=("catalogus", "identificatie"),
+                name="unique_identificatie_in_catalogus",
+            ),
+        ),
+    ]

src/open_inwoner/openzaak/models.py

@@ -2,7 +2,7 @@
 from datetime import timedelta
 
 from django.db import models, transaction
-from django.db.models import Q, UniqueConstraint
+from django.db.models import UniqueConstraint
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
 
@@ -319,7 +319,6 @@ class ZaakTypeConfig(models.Model):
     catalogus = models.ForeignKey(
         "openzaak.CatalogusConfig",
         on_delete=models.CASCADE,
-        null=True,
     )
 
     identificatie = models.CharField(
@@ -389,12 +388,6 @@ class Meta:
             UniqueConstraint(
                 name="unique_identificatie_in_catalogus",
                 fields=["catalogus", "identificatie"],
-                condition=Q(catalogus__isnull=False),
-            ),
-            UniqueConstraint(
-                name="unique_identificatie_without_catalogus",
-                fields=["identificatie"],
-                condition=Q(catalogus__isnull=True),
             ),
         ]
 

src/open_inwoner/openzaak/tests/test_migrations.py

@@ -151,3 +151,24 @@ def test_migration_0051_to_0052_raises_for_multiple_api_groups(self):
             " are multiple instances configured. Please (temporarily) ensure you have only a single"
             " ZGWApiGroupConfig configured, then run this migration again.",
         )
+
+
+class TestMakeZaakTypeConfigCatalogusRequired(TestFailingMigrations):
+    migrate_from = "0052_add_catalogusconfig_service"
+    migrate_to = "0053_zaaktypeconfig_catalogus_is_required"
+    app = "openzaak"
+
+    def setUpBeforeMigration(self, apps):
+        ZaakTypeConfig = apps.get_model("openzaak", "ZaakTypeConfig")
+        ZaakTypeConfig.objects.create(urls=[], catalogus=None, identificatie="foobar")
+
+    def test_migration_0051_to_0052_raises_with_descriptive_exception_message(self):
+        with self.assertRaises(DataError) as cm:
+            self.attempt_migration()
+
+        self.assertEqual(
+            str(cm.exception),
+            "Your database contains 1 ZaakTypeConfig row(s) with a missing `catalogus` field."
+            " This field is now required: please manually update all the affected rows or re-sync"
+            " your ZGW objects to ensure the field is included.",
+        )

src/open_inwoner/openzaak/tests/test_models.py

@@ -47,26 +47,6 @@ def test_queryset_filter_case_type_with_catalog(self):
         actual = list(ZaakTypeConfig.objects.filter_case_type(case_type))
         self.assertEqual(actual, [zaak_type_config])
 
-    def test_queryset_filter_case_type_without_catalog(self):
-        zaak_type_config = ZaakTypeConfigFactory(
-            catalogus=None,
-            identificatie="AAAA",
-        )
-        case_type = factory(
-            ZaakType,
-            generate_oas_component(
-                "ztc",
-                "schemas/ZaakType",
-                uuid="aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
-                url=f"{CATALOGI_ROOT}zaaktype/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
-                catalogus=None,
-                identificatie="AAAA",
-            ),
-        )
-
-        actual = list(ZaakTypeConfig.objects.filter_case_type(case_type))
-        self.assertEqual(actual, [zaak_type_config])
-
 
 class ZaakTypeInformatieObjectTypeConfigFactoryModelTestCase(TestCase):
     def test_queryset_filter_case_type_with_catalog(self):
@@ -115,49 +95,6 @@ def test_queryset_filter_case_type_with_catalog(self):
         )
         self.assertEqual(actual, [a1])
 
-    def test_queryset_filter_case_type_without_catalog(self):
-        zaak_type_config = ZaakTypeConfigFactory(
-            catalogus=None,
-            identificatie="AAAA",
-        )
-        a1 = ZaakTypeInformatieObjectTypeConfigFactory(
-            zaaktype_config=zaak_type_config,
-            informatieobjecttype_url="https://example.com/v1/infoobject/a1",
-            zaaktype_uuids=["aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"],
-        )
-        a2 = ZaakTypeInformatieObjectTypeConfigFactory(
-            zaaktype_config=zaak_type_config,
-            informatieobjecttype_url="https://example.com/v1/infoobject/a2",
-            zaaktype_uuids=[],
-        )
-        b = ZaakTypeInformatieObjectTypeConfigFactory(
-            zaaktype_config=zaak_type_config,
-            informatieobjecttype_url="https://example.com/v1/infoobject/bbb",
-            zaaktype_uuids=["aaaaaaaa-aaaa-bbbb-aaaa-aaaaaaaaaaaa"],
-        )
-        c = ZaakTypeInformatieObjectTypeConfigFactory(
-            zaaktype_config__catalogus=None,
-            zaaktype_config__identificatie="CCC",
-            informatieobjecttype_url="https://example.com/v1/infoobject/bbb",
-            zaaktype_uuids=["aaaaaaaa-aaaa-bbbb-aaaa-aaaaaaaaaaaa"],
-        )
-        case_type = factory(
-            ZaakType,
-            generate_oas_component(
-                "ztc",
-                "schemas/ZaakType",
-                uuid="aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
-                url=f"{CATALOGI_ROOT}zaaktype/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
-                catalogus=None,
-                identificatie="AAAA",
-            ),
-        )
-
-        actual = list(
-            ZaakTypeInformatieObjectTypeConfig.objects.filter_case_type(case_type)
-        )
-        self.assertEqual(actual, [a1])
-
 
 class UserCaseStatusNotificationTests(TestCase):
     def test_status_has_received_similar_notes_within(self):

src/open_inwoner/openzaak/tests/test_notification_zaak_status.py

@@ -281,7 +281,6 @@ def test_status_bails_when_skip_informeren_is_set_and_no_zaaktypeconfig_is_found
         oz_config.save()
 
         data = MockAPIData()
-        data.zaak_type["catalogus"] = None
         data.install_mocks(m)
 
         handle_zaken_notification(data.status_notification)
@@ -370,46 +369,6 @@ def test_status_handle_notification_status_type_config_notify_false(
             level=logging.INFO,
         )
 
-    def test_status_handle_notification_when_skip_informeren_is_set_and_zaaktypeconfig_is_found_from_zaaktype_none_catalog(
-        self, m, mock_handle: Mock
-    ):
-        oz_config = OpenZaakConfig.get_solo()
-        oz_config.skip_notification_statustype_informeren = True
-        oz_config.save()
-
-        data = MockAPIData()
-        data.zaak_type["catalogus"] = None
-        data.install_mocks(m)
-
-        ztc = ZaakTypeConfigFactory.create(
-            catalogus=None,
-            identificatie=data.zaak_type["identificatie"],
-            # set this to notify
-            notify_status_changes=True,
-        )
-        ZaakTypeStatusTypeConfigFactory.create(
-            zaaktype_config=ztc,
-            omschrijving=data.status_type_final["omschrijving"],
-            statustype_url=data.status_type_final["url"],
-            notify_status_change=True,
-        )
-
-        handle_zaken_notification(data.status_notification)
-
-        mock_handle.assert_called_once()
-
-        # check call arguments
-        args = mock_handle.call_args.args
-        self.assertEqual(args[0], data.user_initiator)
-        self.assertEqual(args[1].url, data.zaak["url"])
-        self.assertEqual(args[2].url, data.status_final["url"])
-
-        self.assertTimelineLog(
-            "accepted status notification: attempt informing users ",
-            lookup=Lookups.startswith,
-            level=logging.INFO,
-        )
-
     def test_status_bails_when_skip_informeren_is_set_and_zaaktypeconfig_is_found_but_not_set(
         self, m, mock_handle: Mock
     ):
@@ -466,15 +425,13 @@ def test_user_status_notifications_disabled(self, m, mock_handle: Mock):
         oz_config.save()
 
         data = MockAPIData()
-        data.zaak_type["catalogus"] = None
         data.install_mocks(m)
 
         user = data.user_initiator
         user.cases_notifications = False  # opt-out
         user.save()
 
         ztc = ZaakTypeConfigFactory.create(
-            catalogus=None,
             identificatie=data.zaak_type["identificatie"],
             # set this to notify
             notify_status_changes=True,
@@ -498,17 +455,16 @@ def test_action_required_notifications_cannot_be_disabled(
         oz_config.save()
 
         data = MockAPIData()
-        data.zaak_type["catalogus"] = None
         data.install_mocks(m)
 
         user = data.user_initiator
         user.cases_notifications = False  # opt-out
         user.save()
 
         ztc = ZaakTypeConfigFactory.create(
-            catalogus=None,
             identificatie=data.zaak_type["identificatie"],
             notify_status_changes=True,
+            catalogus__url=data.zaak_type["catalogus"],
         )
         ZaakTypeStatusTypeConfigFactory.create(
             zaaktype_config=ztc,
@@ -528,15 +484,13 @@ def test_user_bad_email(self, m, mock_handle: Mock):
         oz_config.save()
 
         data = MockAPIData()
-        data.zaak_type["catalogus"] = None
         data.install_mocks(m)
 
         user = data.user_initiator
         user.email = "user@example.org"
         user.save()
 
         ztc = ZaakTypeConfigFactory.create(
-            catalogus=None,
             identificatie=data.zaak_type["identificatie"],
             # set this to notify
             notify_status_changes=True,
@@ -578,7 +532,6 @@ def test_handle_status_update(self, mock_send: Mock, mock_feed_hook: Mock):
         status_final.statustype = factory(StatusType, data.status_type_final)
 
         ztc = ZaakTypeConfigFactory.create(
-            catalogus=None,
             identificatie=data.zaak_type["identificatie"],
             # set this to notify
             notify_status_changes=True,
@@ -710,7 +663,6 @@ def test_action_required_template(self, mock_send: Mock, mock_feed_hook: Mock):
         status.statustype = factory(StatusType, data.status_type_final)
 
         ztc = ZaakTypeConfigFactory.create(
-            catalogus=None,
             identificatie=data.zaak_type["identificatie"],
             # set this to notify
             notify_status_changes=True,