From 6ed7e177eaebbd9249a6d4b051f0bfabcd907bf1 Mon Sep 17 00:00:00 2001 From: Calum Mackervoy Date: Wed, 25 Sep 2024 15:09:43 +0200 Subject: [PATCH] fix(fixtures/django/14_prescribers.json): re-type accepted orgs typed Other refactor(admin): authorization_status is a more reliable field feat(PrescriberOrganization): CheckConstraint for restricting Other orgs validation fix: invalidated tests wording changes wording change --- itou/fixtures/django/14_prescribers.json | 76 +++++++++---------- itou/prescribers/admin.py | 2 +- itou/prescribers/admin_forms.py | 9 +-- ...revent_validated_authorization_if_other.py | 23 ++++++ itou/prescribers/models.py | 10 ++- tests/prescribers/tests.py | 54 +++---------- tests/www/prescribers_views/test_list.py | 6 +- 7 files changed, 89 insertions(+), 91 deletions(-) create mode 100644 itou/prescribers/migrations/0006_prescriberorganization_prevent_validated_authorization_if_other.py diff --git a/itou/fixtures/django/14_prescribers.json b/itou/fixtures/django/14_prescribers.json index 7792275e80..686d6e089a 100644 --- a/itou/fixtures/django/14_prescribers.json +++ b/itou/fixtures/django/14_prescribers.json @@ -20,7 +20,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - ARRAS", "phone": "0311452459", "post_code": "62005", @@ -53,7 +53,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - BETHUNE", "phone": "+33 5 71 65 67 13", "post_code": "62400", @@ -86,7 +86,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - BOULOGNE-SUR-MER", "phone": "03 42 73 78 12", "post_code": "62203", @@ -119,7 +119,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - CALAIS", "phone": "+33 (0)4 76 92 87 19", "post_code": "62101", @@ -152,7 +152,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - HENIN-BEAUMONT", "phone": "0426751993", "post_code": "62252", @@ -185,7 +185,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - LIEVIN", "phone": "+33 6 42 26 43 76", "post_code": "62803", @@ -218,7 +218,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - MONTREUIL", "phone": "+33 (0)4 99 28 11 81", "post_code": "62170", @@ -251,7 +251,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - SAINT-OMER", "phone": "0803709003", "post_code": "62503", @@ -284,7 +284,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "ADEFI", "phone": "05 41 53 48 90", "post_code": "62130", @@ -317,7 +317,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - HAGUENAU", "phone": "06 35 88 85 94", "post_code": "67500", @@ -350,7 +350,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - MOLSHEIM", "phone": "0448777732", "post_code": "67120", @@ -383,7 +383,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - SAVERNE", "phone": "+33 1 00 72 66 32", "post_code": "67700", @@ -416,7 +416,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - SCHILTIGHEIM", "phone": "+33 1 22 93 83 85", "post_code": "67300", @@ -449,7 +449,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - SELESTAT", "phone": "+33 (0)8 08 75 37 68", "post_code": "67600", @@ -482,7 +482,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - STRASBOURG", "phone": "+33 8 01 92 82 29", "post_code": "67000", @@ -515,7 +515,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - AUBERVILLIERS", "phone": "0698714866", "post_code": "93300", @@ -548,7 +548,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - AULNAY-SOUS-BOIS", "phone": "0636387618", "post_code": "93600", @@ -581,7 +581,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - BOBIGNY", "phone": "01 33 51 31 58", "post_code": "93000", @@ -614,7 +614,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - BONDY", "phone": "0656754389", "post_code": "93140", @@ -647,7 +647,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - CLICHY-SOUS-BOIS", "phone": "01 37 15 81 89", "post_code": "93390", @@ -680,7 +680,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - EPINAY-SUR-SEINE", "phone": "01 15 77 19 46", "post_code": "93800", @@ -713,7 +713,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - LA COURNEUVE", "phone": "0134219615", "post_code": "93120", @@ -746,7 +746,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - MONTREUIL", "phone": "+33 6 28 68 52 55", "post_code": "93100", @@ -779,7 +779,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - NOISY-LE-GRAND", "phone": "+33 (0)3 32 75 11 67", "post_code": "93160", @@ -812,7 +812,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - PANTIN", "phone": "05 90 06 89 62", "post_code": "93500", @@ -845,7 +845,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - ROSNY-SOUS-BOIS", "phone": "+33 (0)5 89 96 97 91", "post_code": "93110", @@ -878,7 +878,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - SAINT-DENIS", "phone": "0216983908", "post_code": "93200", @@ -911,7 +911,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - SEVRAN", "phone": "+33 4 65 98 67 47", "post_code": "93270", @@ -944,7 +944,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "ML", "name": "MISSION LOCALE - VILLEMOMBLE", "phone": "0586491420", "post_code": "93250", @@ -977,7 +977,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "PJJ", "name": "DIRECTION TERRITORIALE DE LA PROTECTION JUDICIAIRE DE LA JEUNESSE - 67", "phone": "08 04 60 54 63", "post_code": "67081", @@ -1010,7 +1010,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "PJJ", "name": "DIRECTION TERRITORIALE DE LA PROTECTION JUDICIAIRE DE LA JEUNESSE - 62", "phone": "08 04 06 75 34", "post_code": "62000", @@ -1043,7 +1043,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "PJJ", "name": "DIRECTION TERRITORIALE DE LA PROTECTION JUDICIAIRE DE LA JEUNESSE - 93", "phone": "+33 5 51 35 28 49", "post_code": "93692", @@ -1076,7 +1076,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "SPIP", "name": "SERVICE PENITENTIAIRE D'INSERTION ET DE PROBATION - 67", "phone": "0805057444", "post_code": "68000", @@ -1109,7 +1109,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "SPIP", "name": "SERVICE PENITENTIAIRE D'INSERTION ET DE PROBATION - 62", "phone": "+33 2 87 27 48 37", "post_code": "62031", @@ -1142,7 +1142,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "SPIP", "name": "SERVICE PENITENTIAIRE D'INSERTION ET DE PROBATION - 93", "phone": "+33 5 03 44 85 44", "post_code": "93200", @@ -1175,7 +1175,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "CAP_EMPLOI", "name": "CAP EMPLOI - 62", "phone": "+33 5 43 78 27 45", "post_code": "62000", @@ -1208,7 +1208,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "CAP_EMPLOI", "name": "CAP EMPLOI - 93", "phone": "02 77 30 09 63", "post_code": "93600", @@ -1241,7 +1241,7 @@ "is_authorized": true, "is_brsa": false, "is_head_office": false, - "kind": "Autre", + "kind": "CAP_EMPLOI", "name": "CAP EMPLOI - 67", "phone": "+33 (0)1 68 93 60 86", "post_code": "68000", diff --git a/itou/prescribers/admin.py b/itou/prescribers/admin.py index f74c7499a9..de0d4f64fa 100644 --- a/itou/prescribers/admin.py +++ b/itou/prescribers/admin.py @@ -225,7 +225,7 @@ def response_change(self, request, obj): if request.user.is_superuser or obj.has_pending_authorization() or obj.has_refused_authorization(): # Organizations typed as "Other" cannot be marked valid if obj.kind == PrescriberOrganizationKind.OTHER: - msg = "Pour habiliter cette organisation, vous devez sélectionner un type différent de “Autre”" + msg = "Pour habiliter cette organisation, vous devez sélectionner un type différent de “Autre”." self.message_user(request, msg, messages.ERROR) return HttpResponseRedirect(request.get_full_path()) obj.is_authorized = True diff --git a/itou/prescribers/admin_forms.py b/itou/prescribers/admin_forms.py index 6ecf5f90a4..c76b3ef9bb 100644 --- a/itou/prescribers/admin_forms.py +++ b/itou/prescribers/admin_forms.py @@ -1,6 +1,6 @@ from django import forms -from itou.prescribers.enums import PrescriberOrganizationKind +from itou.prescribers.enums import PrescriberAuthorizationStatus, PrescriberOrganizationKind from itou.prescribers.models import PrescriberOrganization @@ -23,10 +23,9 @@ def clean(self): cleaned_data = super().clean() if ( cleaned_data.get("kind") == PrescriberOrganizationKind.OTHER - and self.instance.is_authorized - # TODO(calummackervoy): included to permit cleaning existing rows. We can later remove this - and self.instance.kind != PrescriberOrganizationKind.OTHER + and self.instance.authorization_status == PrescriberAuthorizationStatus.VALIDATED ): raise forms.ValidationError( - "Cette organisation a été habilitée. Vous devez sélectionner un type différent de “Autre”" + "Cette organisation a été habilitée. Vous devez sélectionner un type différent de “Autre”." ) + return cleaned_data diff --git a/itou/prescribers/migrations/0006_prescriberorganization_prevent_validated_authorization_if_other.py b/itou/prescribers/migrations/0006_prescriberorganization_prevent_validated_authorization_if_other.py new file mode 100644 index 0000000000..8bc5023d5b --- /dev/null +++ b/itou/prescribers/migrations/0006_prescriberorganization_prevent_validated_authorization_if_other.py @@ -0,0 +1,23 @@ +# Generated by Django 5.0.9 on 2024-09-26 09:53 + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("cities", "0001_initial"), + ("prescribers", "0005_alter_prescribermembership_updated_by_and_more"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AddConstraint( + model_name="prescriberorganization", + constraint=models.CheckConstraint( + check=models.Q(("authorization_status", "VALIDATED"), ("kind", "Autre"), _negated=True), + name="prevent_validated_authorization_if_other", + violation_error_message='Une organisation habilitée ne peut pas être de type "Autre".', + ), + ), + ] diff --git a/itou/prescribers/models.py b/itou/prescribers/models.py index 5651ea13f1..c8662f0fc0 100644 --- a/itou/prescribers/models.py +++ b/itou/prescribers/models.py @@ -206,7 +206,15 @@ class Meta: "Une organisation habilitée délégataire d'un Conseil Départemental " "doit être conventionnée pour le suivi des BRSA." ), - ) + ), + models.CheckConstraint( + name="prevent_validated_authorization_if_other", + check=~models.Q( + authorization_status=PrescriberAuthorizationStatus.VALIDATED, + kind=PrescriberOrganizationKind.OTHER, + ), + violation_error_message=('Une organisation habilitée ne peut pas être de type "Autre".'), + ), ] def save(self, *args, **kwargs): diff --git a/tests/prescribers/tests.py b/tests/prescribers/tests.py index 9dba570945..bd2110a781 100644 --- a/tests/prescribers/tests.py +++ b/tests/prescribers/tests.py @@ -44,7 +44,7 @@ def test_get_accredited_orgs_for(self): accredited_org = PrescriberOrganizationFactory( authorized=True, department=departmental_council_org.department, - kind=PrescriberOrganizationKind.OTHER, + kind=PrescriberOrganizationKind.ODC, is_brsa=True, ) @@ -307,46 +307,6 @@ def test_refuse_prescriber_habilitation_by_superuser(self): assert updated_prescriber_organization.authorization_updated_by == self.superuser assert updated_prescriber_organization.authorization_status == PrescriberAuthorizationStatus.REFUSED - # TODO(calummackervoy): this test can be removed once there are no longer approved organizations of type "Other" - # approving such organizations is now blocked - def test_refuse_prescriber_habilitation_kind_other(self): - # An OTHER organization should not have an authorization, which means we sometime have to refuse it - - self.client.force_login(self.superuser) - - prescriber_organization = PrescriberOrganizationFactory( - authorized=True, - siret="83987278500010", - department="14", - post_code="14000", - authorization_updated_at=datetime.now(tz=get_current_timezone()), - kind=PrescriberOrganizationKind.OTHER, - ) - - url = reverse("admin:prescribers_prescriberorganization_change", args=[prescriber_organization.pk]) - response = self.client.get(url) - self.assertContains(response, self.REFUSE_BUTTON_LABEL) - - post_data = { - "id": prescriber_organization.pk, - "post_code": prescriber_organization.post_code, - "department": prescriber_organization.department, - "kind": prescriber_organization.kind, - "name": prescriber_organization.name, - "siret": prescriber_organization.siret, - "_authorization_action_refuse": "Refuser+l'habilitation", - **self.FORMSETS_PAYLOAD, - } - - response = self.client.post(url, data=post_data) - assert response.status_code == 302 - - updated_prescriber_organization = PrescriberOrganization.objects.get(pk=prescriber_organization.pk) - assert not updated_prescriber_organization.is_authorized - assert updated_prescriber_organization.kind == PrescriberOrganizationKind.OTHER - assert updated_prescriber_organization.authorization_updated_by == self.superuser - assert updated_prescriber_organization.authorization_status == PrescriberAuthorizationStatus.REFUSED - def test_refuse_prescriber_habilitation_error(self): self.client.force_login(self.superuser) @@ -711,7 +671,7 @@ def test_prevent_prescriber_habilitation_organization_type_other(self): [ messages.Message( messages.ERROR, - "Pour habiliter cette organisation, vous devez sélectionner un type différent de “Autre”", + "Pour habiliter cette organisation, vous devez sélectionner un type différent de “Autre”.", ) ], ) @@ -754,7 +714,7 @@ def test_prevent_setting_prescriber_organization_to_other_once_accepted(self): response = self.client.post(url, data=post_data) assert response.status_code == 200 - expected_msg = "Cette organisation a été habilitée. Vous devez sélectionner un type différent de “Autre”" + expected_msg = "Cette organisation a été habilitée. Vous devez sélectionner un type différent de “Autre”." assert len(response.context["errors"]) == 1 assert response.context["errors"][0] == [expected_msg] @@ -859,6 +819,14 @@ def test_validated_odc_is_brsa_constraint(): PrescriberOrganization.objects.filter(pk=organization.pk).update(is_brsa=False) +def test_prevent_validated_authorization_if_other_constraint(): + organization = PrescriberOrganizationFactory(kind=PrescriberOrganizationKind.OTHER) + with pytest.raises(IntegrityError): + PrescriberOrganization.objects.filter(pk=organization.pk).update( + authorization_status=PrescriberAuthorizationStatus.VALIDATED + ) + + def test_deactivate_last_admin(admin_client): organization = PrescriberOrganizationWithMembershipFactory() membership = organization.memberships.first() diff --git a/tests/www/prescribers_views/test_list.py b/tests/www/prescribers_views/test_list.py index 960a68759d..4a5944c83a 100644 --- a/tests/www/prescribers_views/test_list.py +++ b/tests/www/prescribers_views/test_list.py @@ -27,13 +27,13 @@ def test_list_accredited_organizations(client): accredited_org = PrescriberOrganizationFactory( authorized=True, department=organization.department, - kind=PrescriberOrganizationKind.OTHER, + kind=PrescriberOrganizationKind.ODC, is_brsa=True, ) accredited_org_from_other_department = PrescriberOrganizationFactory( authorized=True, department=random.choice(tuple(other_departments)), - kind=PrescriberOrganizationKind.OTHER, + kind=PrescriberOrganizationKind.ODC, is_brsa=True, ) non_authorized_org = PrescriberOrganizationFactory( @@ -44,7 +44,7 @@ def test_list_accredited_organizations(client): authorized_but_not_brsa_org = PrescriberOrganizationFactory( authorized=True, department=organization.department, - kind=PrescriberOrganizationKind.OTHER, + kind=PrescriberOrganizationKind.PE, is_brsa=False, )