Development #48264
ne pas modifier le mail sur un user.mark_as_deleted()
0%
Description
C'est con et ça va casser des trucs derrières (notification de w.c.s. sur démarche éventuellement en cours ou combo sur une facture).
La bonne façon de faire c'est d'ignorer les comptes en cours de suppression quand on cherche des doublons de mail via .exclude(deleted__isnull=False).
Fichiers
Révisions associées
Historique
Mis à jour par Paul Marillonnet il y a plus de 3 ans
C'est-à-dire que ça nécessite de revoir les contraintes d'unicité, c'est bien ça ? Est-ce qu'on tolère la création d'un nouveau compte pour cette même adresse ou bien tant pis on attend que l'objet soit effectivement supprimé en base ?
Mis à jour par Paul Marillonnet il y a plus de 3 ans
Paul Marillonnet a écrit :
C'est-à-dire que ça nécessite de revoir les contraintes d'unicité, c'est bien ça ? Est-ce qu'on tolère la création d'un nouveau compte pour cette même adresse ou bien tant pis on attend que l'objet soit effectivement supprimé en base ?
Si première des deux options j'imagine qu'il faut aussi cadrer l'api, i.e. que l'appelant sache qui parmi plusieurs usagers correspondant à un email est celui actif.
Mis à jour par Benjamin Dauvergne il y a plus de 3 ans
Paul Marillonnet a écrit :
C'est-à-dire que ça nécessite de revoir les contraintes d'unicité, c'est bien ça ? Est-ce qu'on tolère la création d'un nouveau compte pour cette même adresse ou bien tant pis on attend que l'objet soit effectivement supprimé en base ?
On a pas de contrainte d'unicité sur le mail justement, si jamais on en ajouté oui il faudrait les revoir; en attendant d'en avoir je proposais de faire le tour des endroits ou cette contrainte est vérifiée manuellement pour exclure les comptes supprimés.
Si première des deux options j'imagine qu'il faut aussi cadrer l'api, i.e. que l'appelant sache qui parmi plusieurs usagers correspondant à un email est celui actif.
Dans l'API de toute façon il faudrait de base exclure les comptes supprimés, ils sont invisibles.
Mis à jour par Paul Marillonnet il y a plus de 3 ans
Benjamin Dauvergne a écrit :
On a pas de contrainte d'unicité sur le mail justement, si jamais on en ajouté oui il faudrait les revoir; en attendant d'en avoir je proposais de faire le tour des endroits ou cette contrainte est vérifiée manuellement pour exclure les comptes supprimés.
C’est chose faite dans la branche. J’en ai profité pour renforcer un peu le code de vérification de cette unicité dans l'api d'enregistrement.
J'ai ajouté les tests côté api, mais je dois encore en écrire deux ou trois côté /accounts/ et /manage/ pour m'assurer qu’on n’a pas de régression.
Dans l'API de toute façon il faudrait de base exclure les comptes supprimés, ils sont invisibles.
Oui ok, c’est fait.
Mis à jour par Paul Marillonnet il y a plus de 3 ans
- Statut changé de Nouveau à En cours
- Assigné à mis à Paul Marillonnet
Mis à jour par Paul Marillonnet il y a plus de 3 ans
- Fichier 0001-misc-do-not-modify-email-when-marking-users-as-delet.patch 0001-misc-do-not-modify-email-when-marking-users-as-delet.patch ajouté
- Statut changé de En cours à Solution proposée
- Patch proposed changé de Non à Oui
Voilà, je pense avoir fait le tour.
Comme dit plus haut ici, j’ai retouché un peu le code de vérification de l'unicité dans l'api d’inscription, parce que ça m’avait l’air un peu léger en l’état.
Ça m’a l’air pas trop indigeste en un seul patch, mais bien sûr je peux découper en plusieurs patches si la relecture l’exige.
Mis à jour par Benjamin Dauvergne il y a plus de 3 ans
- Statut changé de Solution proposée à Solution validée
Juste au passage j'aurai modifié les messages d'erreur dans l'API qui utilisent "You", il n'y a pas de You quand on utilise l'API, il faudrait un truc plus neutre 'Account already exists' par exemple.
Mis à jour par Paul Marillonnet il y a plus de 3 ans
Benjamin Dauvergne a écrit :
Juste au passage j'aurai modifié les messages d'erreur dans l'API qui utilisent "You", il n'y a pas de You quand on utilise l'API, il faudrait un truc plus neutre 'Account already exists' par exemple.
Ok oui c’est vrai. Modifié et rebasé la branche. Je laisse Jenkins tourner à nouveau, et pousserai ensuite si pas de surprise.
Mis à jour par Paul Marillonnet il y a plus de 3 ans
- Statut changé de Solution validée à Résolu (à déployer)
commit 8c3902b2c2430b94ce3587754ae7730f1a535dcc
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Thu Nov 5 15:18:31 2020 +0100
misc: do not modify email when marking users as deleted (#48264)
Mis à jour par Frédéric Péters il y a plus de 3 ans
- Statut changé de Résolu (à déployer) à Solution déployée
misc: do not modify email when marking users as deleted (#48264)