diff --git a/src/open_inwoner/openzaak/managers.py b/src/open_inwoner/openzaak/managers.py index 0a5fa4806c..7e1f8bc759 100644 --- a/src/open_inwoner/openzaak/managers.py +++ b/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,20 +124,15 @@ 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( @@ -150,16 +140,8 @@ def filter_case_type(self, case_type: ZaakType): ) 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, ) diff --git a/src/open_inwoner/openzaak/migrations/0052_zaaktypeconfig_catalogus_is_required.py b/src/open_inwoner/openzaak/migrations/0052_zaaktypeconfig_catalogus_is_required.py new file mode 100644 index 0000000000..3447e4c686 --- /dev/null +++ b/src/open_inwoner/openzaak/migrations/0052_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", "0051_drop_root_zgw_fields"), + ] + + 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", + ), + ), + ] diff --git a/src/open_inwoner/openzaak/models.py b/src/open_inwoner/openzaak/models.py index 0948a594cd..3c50514598 100644 --- a/src/open_inwoner/openzaak/models.py +++ b/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 _ @@ -311,7 +311,6 @@ class ZaakTypeConfig(models.Model): catalogus = models.ForeignKey( "openzaak.CatalogusConfig", on_delete=models.CASCADE, - null=True, ) identificatie = models.CharField( @@ -381,12 +380,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), ), ] diff --git a/src/open_inwoner/openzaak/tests/test_migrations.py b/src/open_inwoner/openzaak/tests/test_migrations.py index 128ee04526..8d8b299da6 100644 --- a/src/open_inwoner/openzaak/tests/test_migrations.py +++ b/src/open_inwoner/openzaak/tests/test_migrations.py @@ -1,7 +1,14 @@ +from django.db import DataError, connection +from django.db.migrations.executor import MigrationExecutor +from django.test import TestCase + from zgw_consumers.constants import APITypes from open_inwoner.openzaak.tests.factories import ServiceFactory -from open_inwoner.utils.tests.test_migrations import TestSuccessfulMigrations +from open_inwoner.utils.tests.test_migrations import ( + TestFailingMigrations, + TestSuccessfulMigrations, +) class TestMultiZGWBackendMigrations(TestSuccessfulMigrations): @@ -66,3 +73,24 @@ def test_migration_0048_to_0051_multi_zgw_backend(self): expected, msg="Service config should have been moved to a new ZGWApiGroupConfig", ) + + +class TestMakeZaakTypeConfigCatalogusRequired(TestFailingMigrations): + migrate_from = "0051_drop_root_zgw_fields" + migrate_to = "0052_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.", + ) diff --git a/src/open_inwoner/openzaak/tests/test_models.py b/src/open_inwoner/openzaak/tests/test_models.py index 40e09252d7..a00df1b939 100644 --- a/src/open_inwoner/openzaak/tests/test_models.py +++ b/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): diff --git a/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py b/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py index 0e19929fba..07e19a1d8f 100644 --- a/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py +++ b/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,7 +425,6 @@ 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 @@ -474,7 +432,6 @@ def test_user_status_notifications_disabled(self, m, mock_handle: Mock): user.save() ztc = ZaakTypeConfigFactory.create( - catalogus=None, identificatie=data.zaak_type["identificatie"], # set this to notify notify_status_changes=True, @@ -498,7 +455,6 @@ 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 @@ -506,9 +462,9 @@ def test_action_required_notifications_cannot_be_disabled( 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,7 +484,6 @@ 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 @@ -536,7 +491,6 @@ def test_user_bad_email(self, m, mock_handle: Mock): 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,