diff --git a/mesads/app/admin/ads.py b/mesads/app/admin/ads.py index 6e062489..5dd6d526 100644 --- a/mesads/app/admin/ads.py +++ b/mesads/app/admin/ads.py @@ -148,13 +148,6 @@ def has_delete_permission(self, request, obj=None): "prefecture", "owner_name", "owner_siret", - # Rewrite titles to limit the width of the column. - admin.display(description="Carte pro. tit.")( - lambda ads: ads.owner_license_number or "-" - ), - admin.display(description="Exploitée par titulaire ?", boolean=True)( - lambda ads: ads.used_by_owner - ), "ads_users", ) @@ -175,7 +168,6 @@ def has_delete_permission(self, request, obj=None): list_filter = [ ADSPeriodListFilter, - "used_by_owner", "adsuser__status", ADSUsersCount, "attribution_type", diff --git a/mesads/app/forms.py b/mesads/app/forms.py index dbf301e1..73497bea 100644 --- a/mesads/app/forms.py +++ b/mesads/app/forms.py @@ -105,16 +105,7 @@ class Meta: "owner_phone", "owner_mobile", "owner_email", - "used_by_owner", - "owner_license_number", ) - widgets = { - # used_by_owner has null=True because the field is not set for new - # ADS, but we don't want to allow null values for old ADS. Use a - # BooleanSelect instead of the default NullBooleanSelect. - # Note, we could use a checkbox instead of a select. - "used_by_owner": BooleanSelect(), - } def __init__(self, epci=None, *args, **kwargs): super().__init__(*args, **kwargs) @@ -164,7 +155,11 @@ def _should_delete_form(self, form): ADSUserFormSet = inlineformset_factory( ADS, ADSUser, - fields=("status", "name", "siret", "license_number"), + # It is important to leave "deleted_at" in the fields, otherwise + # django.db.models.constraints.CheckConstraint.validate() will silently skip + # the evaluation of the constraints using this field. + # Does it suck? Yes. Did I spend 2 days to figure it out? Also yes. + fields=("status", "name", "siret", "license_number", "deleted_at"), can_delete=True, extra=0, formset=AutoDeleteADSUserFormSet, diff --git a/mesads/app/management/commands/fix_ads_user_and_used_by_owner.py b/mesads/app/management/commands/fix_ads_user_and_used_by_owner.py deleted file mode 100644 index f568f3fa..00000000 --- a/mesads/app/management/commands/fix_ads_user_and_used_by_owner.py +++ /dev/null @@ -1,54 +0,0 @@ -from django.core.management.base import BaseCommand -from django.db.models import Count - -from mesads.app.models import ADS - - -class Command(BaseCommand): - def handle(self, **opts): - handled = {} - q = ( - ADS.objects.filter(used_by_owner=True) - .annotate(count=Count("adsuser")) - .filter(count__gt=0, adsuser__deleted_at__isnull=True) - ) - - for ads in q: - done = False - if ads.id in handled: - continue - - handled[ads.id] = { - "ads": ads, - "emails": [ - req.user.email - for req in ads.ads_manager.adsmanagerrequest_set.all() - ], - } - - if ads.owner_siret: - users = ads.adsuser_set.all() - if len(users) == 1: - if users[0].siret == ads.owner_siret: - users[0].delete() - done = True - else: - if ( - ads.owner_license_number == users[0].license_number - or not users[0].license_number - ): - users[0].license_number = ads.owner_license_number - ads.owner_license_number = "" - - if not done: - ads.used_by_owner = False - - try: - ads.save() - except Exception: - print("oops, ads %s" % ads.id) - - for item in handled.values(): - if not item["emails"]: - continue - print("%s;%s" % (item["ads"].id, ";".join(item["emails"]))) diff --git a/mesads/app/management/commands/import_ads.py b/mesads/app/management/commands/import_ads.py index 019c94c5..601a6ce5 100644 --- a/mesads/app/management/commands/import_ads.py +++ b/mesads/app/management/commands/import_ads.py @@ -290,6 +290,9 @@ def load_ads(self, cols, override=False): self.excel.idx("email du titulaire"), ) + raise RuntimeError( + "The field ADS.used_by_owner has been removed. Please update this code if you need to import ADS." + ) ads.used_by_owner = self.parse_bool( cols, self.excel.idx("ads exploitée par son titulaire"), diff --git a/mesads/app/migrations/0073_alter_adsuser_status.py b/mesads/app/migrations/0073_alter_adsuser_status.py new file mode 100644 index 00000000..5e49ecc9 --- /dev/null +++ b/mesads/app/migrations/0073_alter_adsuser_status.py @@ -0,0 +1,35 @@ +# Generated by Django 4.1.9 on 2024-03-11 09:46 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("app", "0072_alter_ads_owner_name"), + ] + + operations = [ + migrations.AlterField( + model_name="adsuser", + name="status", + field=models.CharField( + blank=True, + choices=[ + ( + "titulaire_exploitant", + "Le titulaire de l'ADS (personne physique)", + ), + ( + "legal_representative", + "Le représentant légal de la société titulaire de l'ADS (gérant ou président non salarié)", + ), + ("salarie", "Salarié du titulaire de l'ADS"), + ("cooperateur", "Le locataire-coopérateur de l'ADS"), + ("locataire_gerant", "Le locataire-gérant de l'ADS"), + ("autre", "Autre"), + ], + max_length=255, + verbose_name="Modalité d'exploitation de l'ADS", + ), + ), + ] diff --git a/mesads/app/migrations/0074_fix_adsuser.py b/mesads/app/migrations/0074_fix_adsuser.py new file mode 100644 index 00000000..9d64821b --- /dev/null +++ b/mesads/app/migrations/0074_fix_adsuser.py @@ -0,0 +1,36 @@ +# Generated by Django 4.1.9 on 2024-03-11 10:43 + +from django.db import migrations + + +def upgrade(apps, schema_editor): + """Fix ADSUser data, see https://trello.com/c/dG9fG0wC""" + + ADS = apps.get_model("app", "ADS") + ADSUser = apps.get_model("app", "ADSUser") + + for ads in ADS.objects.filter(used_by_owner=True): + ads_user = ADSUser( + ads=ads, + status="titulaire_exploitant", + license_number=ads.owner_license_number, + name="", + siret="", + ) + ads_user.save() + + for ads in ADS.objects.filter(used_by_owner=False): + for ads_user in ads.adsuser_set.all(): + if ads_user.status in ("titulaire_exploitant", "autre"): + ads_user.status = "legal_representative" + ads_user.save() + + +class Migration(migrations.Migration): + dependencies = [ + ("app", "0073_alter_adsuser_status"), + ] + + operations = [ + migrations.RunPython(upgrade), + ] diff --git a/mesads/app/migrations/0075_remove_ads_used_by_owner_null_for_new_ads_and_more.py b/mesads/app/migrations/0075_remove_ads_used_by_owner_null_for_new_ads_and_more.py new file mode 100644 index 00000000..533bad70 --- /dev/null +++ b/mesads/app/migrations/0075_remove_ads_used_by_owner_null_for_new_ads_and_more.py @@ -0,0 +1,50 @@ +# Generated by Django 4.1.9 on 2024-03-11 10:53 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("app", "0074_fix_adsuser"), + ] + + operations = [ + migrations.RemoveConstraint( + model_name="ads", + name="used_by_owner_null_for_new_ads", + ), + migrations.RemoveConstraint( + model_name="ads", + name="owner_license_number_empty_if_not_used_by_owner", + ), + migrations.RemoveField( + model_name="ads", + name="owner_license_number", + ), + migrations.RemoveField( + model_name="ads", + name="used_by_owner", + ), + migrations.AlterField( + model_name="adsuser", + name="status", + field=models.CharField( + blank=True, + choices=[ + ( + "titulaire_exploitant", + "Le titulaire de l'ADS (personne physique)", + ), + ( + "legal_representative", + "Le représentant légal de la société titulaire de l'ADS (gérant ou président non salarié)", + ), + ("salarie", "Salarié du titulaire de l'ADS"), + ("cooperateur", "Le locataire-coopérateur de l'ADS"), + ("locataire_gerant", "Le locataire-gérant de l'ADS"), + ], + max_length=255, + verbose_name="Modalité d'exploitation de l'ADS", + ), + ), + ] diff --git a/mesads/app/migrations/0076_adsuser_only_one_titulaire_exploitant.py b/mesads/app/migrations/0076_adsuser_only_one_titulaire_exploitant.py new file mode 100644 index 00000000..87177b0e --- /dev/null +++ b/mesads/app/migrations/0076_adsuser_only_one_titulaire_exploitant.py @@ -0,0 +1,23 @@ +# Generated by Django 4.1.9 on 2024-03-11 11:03 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("app", "0075_remove_ads_used_by_owner_null_for_new_ads_and_more"), + ] + + operations = [ + migrations.AddConstraint( + model_name="adsuser", + constraint=models.UniqueConstraint( + condition=models.Q( + ("deleted_at__isnull", True), ("status", "titulaire_exploitant") + ), + fields=("ads", "status"), + name="only_one_titulaire_exploitant", + violation_error_message="Il ne peut y avoir qu'un seul titulaire par ADS.", + ), + ), + ] diff --git a/mesads/app/migrations/0077_adsuser_name_empty_for_titulaire_exploitant_and_more.py b/mesads/app/migrations/0077_adsuser_name_empty_for_titulaire_exploitant_and_more.py new file mode 100644 index 00000000..cbe6b2a3 --- /dev/null +++ b/mesads/app/migrations/0077_adsuser_name_empty_for_titulaire_exploitant_and_more.py @@ -0,0 +1,38 @@ +# Generated by Django 4.1.9 on 2024-03-11 14:58 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("app", "0076_adsuser_only_one_titulaire_exploitant"), + ] + + operations = [ + migrations.AddConstraint( + model_name="adsuser", + constraint=models.CheckConstraint( + check=models.Q( + ("deleted_at__isnull", False), + models.Q(("name", ""), ("status", "titulaire_exploitant")), + models.Q(("status", "titulaire_exploitant"), _negated=True), + _connector="OR", + ), + name="name_empty_for_titulaire_exploitant", + violation_error_message="Le nom du conducteur ne peut être renseigné que s'il ne s'agit pas du titulaire de l'ADS.", + ), + ), + migrations.AddConstraint( + model_name="adsuser", + constraint=models.CheckConstraint( + check=models.Q( + ("deleted_at__isnull", False), + models.Q(("siret", ""), ("status", "titulaire_exploitant")), + models.Q(("status", "titulaire_exploitant"), _negated=True), + _connector="OR", + ), + name="siret_empty_for_titulaire_exploitant", + violation_error_message="Le SIRET du conducteur ne peut être renseigné que s'il ne s'agit pas du titulaire de l'ADS.", + ), + ), + ] diff --git a/mesads/app/migrations/0078_fix_adsuser_legal_representative.py b/mesads/app/migrations/0078_fix_adsuser_legal_representative.py new file mode 100644 index 00000000..13cf5505 --- /dev/null +++ b/mesads/app/migrations/0078_fix_adsuser_legal_representative.py @@ -0,0 +1,40 @@ +# Generated by Django 4.1.9 on 2024-03-11 15:01 + +from django.db import migrations + + +def upgrade(apps, schema_editor): + """Fix ADSUser data, see https://trello.com/c/dG9fG0wC""" + ADSUser = apps.get_model("app", "ADSUser") + + for ads_user in ADSUser.objects.filter(status="legal_representative").all(): + if ads_user.siret == "": + continue + + # ADSUser siret and ADS siret are similar, remove the siret from ADSUser + if ads_user.ads.owner_siret == ads_user.siret: + ads_user.siret = "" + ads_user.save() + continue + + # Values are different. Discard the SIRET from ADSUser, assuming it is wrong + if ads_user.ads.owner_siret != "": + ads_user.siret = "" + ads_user.save() + continue + + # ADS.owner_siret is empty, let's use the one from ADSUser + ads_user.ads.owner_siret = ads_user.siret + ads_user.ads.save() + ads_user.siret = "" + ads_user.save() + + +class Migration(migrations.Migration): + dependencies = [ + ("app", "0077_adsuser_name_empty_for_titulaire_exploitant_and_more"), + ] + + operations = [ + migrations.RunPython(upgrade), + ] diff --git a/mesads/app/migrations/0079_fix_adsuser_salarie.py b/mesads/app/migrations/0079_fix_adsuser_salarie.py new file mode 100644 index 00000000..b8f043a5 --- /dev/null +++ b/mesads/app/migrations/0079_fix_adsuser_salarie.py @@ -0,0 +1,22 @@ +# Generated by Django 4.1.9 on 2024-03-11 15:27 + +from django.db import migrations + + +def upgrade(apps, schema_editor): + """Fix ADSUser data, see https://trello.com/c/dG9fG0wC""" + ADSUser = apps.get_model("app", "ADSUser") + + for ads_user in ADSUser.objects.filter(status="salarie").exclude(siret="").all(): + ads_user.siret = "" + ads_user.save() + + +class Migration(migrations.Migration): + dependencies = [ + ("app", "0078_fix_adsuser_legal_representative"), + ] + + operations = [ + migrations.RunPython(upgrade), + ] diff --git a/mesads/app/migrations/0080_remove_adsuser_siret_empty_for_titulaire_exploitant_and_more.py b/mesads/app/migrations/0080_remove_adsuser_siret_empty_for_titulaire_exploitant_and_more.py new file mode 100644 index 00000000..ed020998 --- /dev/null +++ b/mesads/app/migrations/0080_remove_adsuser_siret_empty_for_titulaire_exploitant_and_more.py @@ -0,0 +1,55 @@ +# Generated by Django 4.1.9 on 2024-03-11 15:34 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("app", "0079_fix_adsuser_salarie"), + ] + + operations = [ + migrations.RemoveConstraint( + model_name="adsuser", + name="siret_empty_for_titulaire_exploitant", + ), + migrations.AddConstraint( + model_name="adsuser", + constraint=models.CheckConstraint( + check=models.Q( + ("deleted_at__isnull", False), + models.Q(("siret", ""), ("status", "titulaire_exploitant")), + models.Q(("status", "titulaire_exploitant"), _negated=True), + _connector="OR", + ), + name="siret_empty_for_titulaire_exploitant", + violation_error_message="Le SIRET du conducteur ne peut pas être renseigné pour le titulaire de l'ADS.", + ), + ), + migrations.AddConstraint( + model_name="adsuser", + constraint=models.CheckConstraint( + check=models.Q( + ("deleted_at__isnull", False), + models.Q(("siret", ""), ("status", "legal_representative")), + models.Q(("status", "legal_representative"), _negated=True), + _connector="OR", + ), + name="siret_empty_for_legal_representative", + violation_error_message="Le SIRET du conducteur ne peut pas être renseigné pour le représentant légal de la société.", + ), + ), + migrations.AddConstraint( + model_name="adsuser", + constraint=models.CheckConstraint( + check=models.Q( + ("deleted_at__isnull", False), + models.Q(("siret", ""), ("status", "salarie")), + models.Q(("status", "salarie"), _negated=True), + _connector="OR", + ), + name="siret_empty_for_salarie", + violation_error_message="Le SIRET du conducteur ne peut pas être renseigné pour un salarié.", + ), + ), + ] diff --git a/mesads/app/models.py b/mesads/app/models.py index b25984af..21278929 100644 --- a/mesads/app/models.py +++ b/mesads/app/models.py @@ -407,26 +407,6 @@ class Meta: name="attribution_date_null_for_new_ads", violation_error_message="La date d'attribution ne peut être renseignée que pour les ADS créées avant le 1er octobre 2014.", ), - # Check used_by_owner: - # - For new ADS, used_by_owner should always be null - # - For old ADS, used_by_owner can be set or not - # - For ADS with an unknown creation date, used_by_owner should be null - models.CheckConstraint( - check=( - Q( - ads_creation_date__isnull=False, - ads_creation_date__gte=date(2014, 10, 1), - used_by_owner__isnull=True, - ) - | Q( - ads_creation_date__isnull=False, - ads_creation_date__lt=date(2014, 10, 1), - ) - | Q(ads_creation_date__isnull=True, used_by_owner__isnull=True) - ), - name="used_by_owner_null_for_new_ads", - violation_error_message="Le champ 'ADS exploitée par son titulaire' ne peut être renseigné que pour les ADS créées avant le 1er octobre 2014.", - ), # Check attribution_type: # - For new ADS, attribution_type should always be empty # - For old ADS, attribution_type can be set or not @@ -476,26 +456,6 @@ class Meta: name="attribution_reason_empty_for_new_ads", violation_error_message="Le champ 'Raison d'attribution' ne peut être renseigné que pour les ADS créées avant le 1er octobre 2014.", ), - # Check owner_license_number - # - For new ADS, owner_license_number can be set or not - # - Otherwise, owner_license_number can only be set if used_by_owner is true - models.CheckConstraint( - check=( - Q( - ads_creation_date__isnull=False, - ads_creation_date__gte=date(2014, 10, 1), - ) - | Q(used_by_owner__isnull=True, owner_license_number="") - | Q( - used_by_owner__isnull=False, - used_by_owner=False, - owner_license_number="", - ) - | Q(used_by_owner__isnull=False, used_by_owner=True) - ), - name="owner_license_number_empty_if_not_used_by_owner", - violation_error_message="Le champ 'Numéro de la carte professionnelle du titulaire' doit être vide si l'ADS n'est pas exploitée par son titulaire.", - ), # Check renewal date nullable: # - For new ADS, renew date can be set or not # - For old ADS, renew date must always be empty @@ -537,6 +497,19 @@ def save(self, *args, **kwargs): raise ValidationError( "Il n'est pas possible d'apporter des modifications sur les ADS d'une administration verrouillée." ) + if self.ads_creation_date and self.ads_creation_date >= date(2014, 10, 1): + existing_users = ADSUser.objects.filter(ads=self) + if existing_users.count() > 1: + raise ValidationError( + "Un seul exploitant peut être déclaré pour une ADS créée après le 1er octobre 2014." + ) + if ( + existing_users.exists() + and existing_users.first().status != "titulaire_exploitant" + ): + raise ValidationError( + "Le conducteur doit nécessairement être le titulaire de l'ADS (personne physique) pour une ADS créée après le 1er octobre 2014." + ) return super().save(*args, **kwargs) def delete(self, *args, **kwargs): @@ -552,7 +525,7 @@ def unique_error_message(self, model_class, unique_check): """Constraints can have a custom violation error message set with the parameter `violation_error_message`. However, it appears that this is not possible for UniqueConstraint, which, for backward compatibility - reasons (django 4.1), ignore this parameter and instead call + reasons (django 4.1), ignores this parameter and instead calls unique_error_message. See https://github.com/django/django/blob/69069a443a906dd4060a8047e683657d40b4c383/django/db/models/constraints.py#L356 @@ -692,13 +665,6 @@ def unique_error_message(self, model_class, unique_check): ), ) - owner_license_number = models.CharField( - max_length=64, - blank=True, - null=False, - verbose_name="Numéro de la carte professionnelle du titulaire", - ) - owner_phone = models.CharField( max_length=128, blank=True, @@ -718,10 +684,6 @@ def unique_error_message(self, model_class, unique_check): verbose_name="Email du titulaire de l'ADS", ) - used_by_owner = models.BooleanField( - blank=True, null=True, verbose_name="ADS exploitée par son titulaire ?" - ) - def get_legal_filename(instance, filename): now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") @@ -791,10 +753,104 @@ class ADSUser( class Meta: verbose_name = "Exploitant de l'ADS" verbose_name_plural = "Exploitants de l'ADS" + constraints = [ + # there can be only one titulaire_exploitant for a given ADS + models.UniqueConstraint( + fields=("ads", "status"), + condition=Q(status="titulaire_exploitant", deleted_at__isnull=True), + name="only_one_titulaire_exploitant", + violation_error_message="Il ne peut y avoir qu'un seul titulaire par ADS.", + ), + # name should be empty if status = 'titulaire_exploitant', because the value is expected to be provided in ADS.owner_name + models.CheckConstraint( + check=( + Q( + deleted_at__isnull=False, + ) + | Q( + status="titulaire_exploitant", + name="", + ) + | ~Q( + status="titulaire_exploitant", + ) + ), + name="name_empty_for_titulaire_exploitant", + violation_error_message="Le nom du conducteur ne peut être renseigné que s'il ne s'agit pas du titulaire de l'ADS.", + ), + # SIRET should be empty if status = 'titulaire_exploitant', because the value is expected to be provided in ADS.owner_siret + models.CheckConstraint( + check=( + Q( + deleted_at__isnull=False, + ) + | Q( + status="titulaire_exploitant", + siret="", + ) + | ~Q( + status="titulaire_exploitant", + ) + ), + name="siret_empty_for_titulaire_exploitant", + violation_error_message="Le SIRET du conducteur ne peut pas être renseigné pour le titulaire de l'ADS.", + ), + # SIRET should be empty if status = 'legal_representative', because the value is expected to be provided in ADS.owner_siret + models.CheckConstraint( + check=( + Q( + deleted_at__isnull=False, + ) + | Q( + status="legal_representative", + siret="", + ) + | ~Q( + status="legal_representative", + ) + ), + name="siret_empty_for_legal_representative", + violation_error_message="Le SIRET du conducteur ne peut pas être renseigné pour le représentant légal de la société.", + ), + # SIRET should be empty if status = 'salarie', because employees don't have a SIRET number + models.CheckConstraint( + check=( + Q( + deleted_at__isnull=False, + ) + | Q( + status="salarie", + siret="", + ) + | ~Q( + status="salarie", + ) + ), + name="siret_empty_for_salarie", + violation_error_message="Le SIRET du conducteur ne peut pas être renseigné pour un salarié.", + ), + ] def __str__(self): return f"{self.name}" + def save(self, *args, **kwargs): + if ( + not self.deleted_at + and self.ads.ads_creation_date + and self.ads.ads_creation_date >= date(2014, 10, 1) + ): + existing_users = ADSUser.objects.filter(ads=self.ads).exclude(pk=self.pk) + if existing_users.exists(): + raise ValidationError( + "Un seul exploitant peut être déclaré pour une ADS créée après le 1er octobre 2014." + ) + if self.status != "titulaire_exploitant": + raise ValidationError( + "Le conducteur doit nécessairement être le titulaire de l'ADS (personne physique) pour une ADS créée après le 1er octobre 2014." + ) + return super().save(*args, **kwargs) + ads = models.ForeignKey(ADS, on_delete=models.CASCADE) SMART_VALIDATION_WATCHED_FIELDS = { @@ -802,11 +858,26 @@ def __str__(self): } ADS_USER_STATUS = [ - ("titulaire_exploitant", "Titulaire exploitant"), - ("cooperateur", "Locataire coopérateur"), - ("locataire_gerant", "Locataire gérant"), - ("salarie", "Salarié"), - ("autre", "Autre"), + ( + "titulaire_exploitant", + "Le titulaire de l'ADS (personne physique)", + ), + ( + "legal_representative", + "Le représentant légal de la société titulaire de l'ADS (gérant ou président non salarié)", + ), + ( + "salarie", + "Salarié du titulaire de l'ADS", + ), + ( + "cooperateur", + "Le locataire-coopérateur de l'ADS", + ), + ( + "locataire_gerant", + "Le locataire-gérant de l'ADS", + ), ] status = models.CharField( diff --git a/mesads/app/test_reversion_diff.py b/mesads/app/test_reversion_diff.py index af9f4b16..7bc4250f 100644 --- a/mesads/app/test_reversion_diff.py +++ b/mesads/app/test_reversion_diff.py @@ -167,7 +167,7 @@ def test_render_fields(self): ModelHistory(ADSUser()).render_field( ADSUser, "status", "titulaire_exploitant" ), - "Titulaire exploitant", + "Le titulaire de l'ADS (personne physique)", ) self.assertEqual( diff --git a/mesads/app/test_views.py b/mesads/app/test_views.py index 2c64cf77..febd210c 100644 --- a/mesads/app/test_views.py +++ b/mesads/app/test_views.py @@ -1012,7 +1012,7 @@ def test_create_with_ads_user(self): "adsuser_set-INITIAL_FORMS": 0, "adsuser_set-MIN_NUM_FORMS": 0, "adsuser_set-MAX_NUM_FORMS": 10, - "adsuser_set-0-status": "autre", + "adsuser_set-0-status": "cooperateur", "adsuser_set-0-name": "Paul", "adsuser_set-0-siret": "12312312312312", "adslegalfile_set-TOTAL_FORMS": 10, @@ -1029,7 +1029,7 @@ def test_create_with_ads_user(self): self.assertEqual(ADSUser.objects.count(), 1) new_ads_user = ADSUser.objects.get() - self.assertEqual(new_ads_user.status, "autre") + self.assertEqual(new_ads_user.status, "cooperateur") self.assertEqual(new_ads_user.name, "Paul") self.assertEqual(new_ads_user.siret, "12312312312312") diff --git a/mesads/app/views.py b/mesads/app/views.py index 33e64dcc..ec773d68 100644 --- a/mesads/app/views.py +++ b/mesads/app/views.py @@ -417,29 +417,33 @@ def get_success_url(self): def get_context_data(self, **kwargs): ctx = super().get_context_data(**kwargs) ctx["ads_manager"] = ADSManager.objects.get(id=self.kwargs["manager_id"]) + ctx["ads_users_formset"] = self.ads_users_formset + ctx["ads_legal_files_formset"] = self.ads_legal_files_formset + return ctx + + def get_object(self, queryset=None): + ads = get_object_or_404(ADS, id=self.kwargs["ads_id"]) if self.request.POST and self.request.POST.get( ADSUserFormSet().management_form["TOTAL_FORMS"].html_name ): - ctx["ads_users_formset"] = ADSUserFormSet( - self.request.POST, instance=self.object - ) + self.ads_users_formset = ADSUserFormSet(self.request.POST, instance=ads) else: - ctx["ads_users_formset"] = ADSUserFormSet(instance=self.object) + self.ads_users_formset = ADSUserFormSet(instance=ads) + # Always display at least a form + if not ads.adsuser_set.count(): + self.ads_users_formset.extra = 1 if self.request.POST and self.request.POST.get( ADSLegalFileFormSet().management_form["TOTAL_FORMS"].html_name ): - ctx["ads_legal_files_formset"] = ADSLegalFileFormSet( - self.request.POST, self.request.FILES, instance=self.object + self.ads_legal_files_formset = ADSLegalFileFormSet( + self.request.POST, self.request.FILES, instance=ads ) else: - ctx["ads_legal_files_formset"] = ADSLegalFileFormSet(instance=self.object) + self.ads_legal_files_formset = ADSLegalFileFormSet(instance=ads) - return ctx - - def get_object(self, queryset=None): - return get_object_or_404(ADS, id=self.kwargs["ads_id"]) + return ads def form_invalid(self, form): messages.error( @@ -449,44 +453,47 @@ def form_invalid(self, form): return super().form_invalid(form) def form_valid(self, form): - ctx = self.get_context_data() - ads_users_formset = ctx["ads_users_formset"] - ads_legal_files_formset = ctx["ads_legal_files_formset"] - - html_name_ads_users_formset = ads_users_formset.management_form[ + html_name_ads_users_formset = self.ads_users_formset.management_form[ "TOTAL_FORMS" ].html_name if ( self.request.POST.get(html_name_ads_users_formset) is not None - and not ads_users_formset.is_valid() + and not self.ads_users_formset.is_valid() ): - return super().form_invalid(form) + return self.form_invalid(form) - html_name_ads_legal_files_formset = ads_legal_files_formset.management_form[ - "TOTAL_FORMS" - ].html_name + html_name_ads_legal_files_formset = ( + self.ads_legal_files_formset.management_form["TOTAL_FORMS"].html_name + ) if ( self.request.POST.get(html_name_ads_legal_files_formset) is not None - and not ads_legal_files_formset.is_valid() + and not self.ads_legal_files_formset.is_valid() ): - return super().form_invalid(form) + return self.form_invalid(form) - resp = super().form_valid(form) - - if self.request.POST.get(html_name_ads_users_formset): - ads_users_formset.instance = self.object - ads_users_formset.save() - else: + if not self.request.POST.get(html_name_ads_users_formset): ADSUser.objects.filter(ads=self.object).delete() - - if self.request.POST.get(html_name_ads_legal_files_formset): - ads_legal_files_formset.instance = self.object - ads_legal_files_formset.save() else: + try: + with transaction.atomic(): + self.ads_users_formset.save() + except IntegrityError: + errmsg = [ + c + for c in ADSUser._meta.constraints + if c.name == "only_one_titulaire_exploitant" + ][0].violation_error_message + self.ads_users_formset.non_form_errors().append(errmsg) + return self.form_invalid(form) + + if not self.request.POST.get(html_name_ads_legal_files_formset): ADSLegalFile.objects.filter(ads=self.object).delete() + else: + self.ads_legal_files_formset.instance = self.object + self.ads_legal_files_formset.save() + resp = super().form_valid(form) messages.success(self.request, "Les modifications ont été enregistrées.") - return resp @@ -537,8 +544,27 @@ def get_success_url(self): class ADSCreateView(ADSView, CreateView): def dispatch(self, request, manager_id): """If the ADSManager has the flag no_ads_declared to True, it is - imposisble to create ADS for it.""" + impossible to create ADS for it.""" get_object_or_404(ADSManager, id=manager_id, no_ads_declared=False) + + html_name_ads_users_formset = ( + ADSUserFormSet().management_form["TOTAL_FORMS"].html_name + ) + if self.request.POST.get(html_name_ads_users_formset): + self.ads_users_formset = ADSUserFormSet(self.request.POST) + else: + self.ads_users_formset = ADSUserFormSet() + self.ads_users_formset.extra = 1 + + html_name_ads_legal_files_formset = ( + ADSLegalFileFormSet().management_form["TOTAL_FORMS"].html_name + ) + if self.request.POST.get(html_name_ads_legal_files_formset): + self.ads_legal_files_formset = ADSLegalFileFormSet( + self.request.POST, self.request.FILES + ) + else: + self.ads_legal_files_formset = ADSLegalFileFormSet() return super().dispatch(request, manager_id) def get_object(self, queryset=None): @@ -595,7 +621,6 @@ def prefecture_export_ads(request, ads_manager_administrator): "Véhicule électrique ou hybride ?", "Nom du titulaire", "SIRET titulaire", - "ADS exploitée par le titulaire ?", "Statuts des exploitants (un par ligne)", "Noms des exploitants (un par ligne)", "SIRET des exploitants (un par ligne)", @@ -646,7 +671,6 @@ def display_date(value): display_bool(ads.eco_vehicle), ads.owner_name, ads.owner_siret, - display_bool(ads.used_by_owner), "\n".join( [ f"{idx + 1}. {dict(ADSUser.status.field.choices).get(status, '')}" diff --git a/mesads/static/style.css b/mesads/static/style.css index a11c97ee..bf46b14c 100644 --- a/mesads/static/style.css +++ b/mesads/static/style.css @@ -126,7 +126,7 @@ h1 { .mesads-fieldset { border: 1px solid #ddd; - padding-top: 2rem; + padding-top: 1rem; padding-bottom: 2rem; } diff --git a/mesads/templates/webpack/pages/ads_register/ads.html b/mesads/templates/webpack/pages/ads_register/ads.html index 9219d3fa..a5f258e0 100644 --- a/mesads/templates/webpack/pages/ads_register/ads.html +++ b/mesads/templates/webpack/pages/ads_register/ads.html @@ -133,7 +133,6 @@

-