Projet

Général

Profil

Development #42428

Prise en compte du flag "is_active" communiqué par authentic

Ajouté par Frédéric Péters il y a presque 4 ans. Mis à jour il y a plus de 3 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Version cible:
-
Début:
04 mai 2020
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Non
Planning:
Non

Description

On l'a dans les infos transmises lors du provisionning, on devrait l'enregistrer. (pour on verra)


Fichiers


Demandes liées

Lié à Publik - Development #26907: Cycle de vie des comptesFermé24 novembre 2020

Actions

Révisions associées

Révision b145351e (diff)
Ajouté par Benjamin Dauvergne il y a plus de 3 ans

users: add column to store is_active (#42428)

Révision 3a2b8d45 (diff)
Ajouté par Frédéric Péters il y a plus de 3 ans

tests: test is_active user column migration (#42428)

Révision 4ef65f99 (diff)
Ajouté par Benjamin Dauvergne il y a plus de 3 ans

hobo_notify: set is_active attribute (#42428)

Révision 248b0c28 (diff)
Ajouté par Benjamin Dauvergne il y a plus de 3 ans

roles: don't include inactive members emails (#42428)

Révision 612bfb1e (diff)
Ajouté par Benjamin Dauvergne il y a plus de 3 ans

wf: do not notify inactive users (#42428)

Révision f2e85509 (diff)
Ajouté par Benjamin Dauvergne il y a plus de 3 ans

sessions: unlog inactive users (#42428)

Révision 83e341c9 (diff)
Ajouté par Frédéric Péters il y a plus de 3 ans

misc: ignore email attribute for disabled users (#42428)

Historique

#1

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

#2

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

  • Assigné à mis à Benjamin Dauvergne
#3

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

0001: ajout du nouvel attribut
0002: utilisation au provisionning et au deprovisionning
0003: ignorer les emails des utilisateurs inactifs
0004: ne pas notifier les utilisateurs inactifs
0005: logout implicite des utilisateurs inactifs (comme dans Django)

#4

Mis à jour par Frédéric Péters il y a plus de 3 ans

0001: ajout du nouvel attribut

Il manque l'incrément à SQL_LEVEL.

0002: utilisation au provisionning et au deprovisionning

Poser le flag, oui, mais non, pour la suppression il y a #42393.

De manière générale, attendre que l'autre soit poussé, ou s'accorder et avoir une méthode sur user plutôt que multiplier les tests directs sur x.is_active. (dans 0003 0004 0005).

#5

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Frédéric Péters a écrit :

0001: ajout du nouvel attribut

Il manque l'incrément à SQL_LEVEL.

Fait sur la branche

0002: utilisation au provisionning et au deprovisionning

Poser le flag, oui, mais non, pour la suppression il y a #42393.

Ok...

De manière générale, attendre que l'autre soit poussé, ou s'accorder et avoir une méthode sur user plutôt que multiplier les tests directs sur x.is_active. (dans 0003 0004 0005).

Mais ça m'irait que pour les autres comportement on se base sur le is_active introduit ici, en posant is_active dans set_deleted, comme ça on ne multiplie pas les conditions, un compte est actif ou pas, supprimé ou pas, mais c'est le fait d'être actif qui détermine son usage concernant les notifications ou autre. On trouvera peut-être un jour un cas ou c'est le statut supprimé qui importe mais là je n'en vois pas.

Je veux bien reprendre le patch 0003 de #42393 ici pour le baser sur is_active et revoir les patchs 0003 et 0004 en conséquence, en me basant plutôt sur les modifications à get_users_with...; pour le 0005 je ne vois pas d'équivalent sur l'autre ticket et je serai pour garder l'équivalence avec Django en se basant sur is_active pour la déconnexion immédiat d'un compte.

En m'inspirant de #42393 je propose aussi d'ajouter un affichage en BO du statut inactif.

#6

Mis à jour par Frédéric Péters il y a plus de 3 ans

  • Statut changé de Solution proposée à En cours
  • Assigné à changé de Benjamin Dauvergne à Frédéric Péters
  • Patch proposed changé de Oui à Non

Ça se mélange trop à #42393 (sur les comportements, sur la migration, etc.), je vais terminer celui-là, puis je prendrai celui-ci, il ne me semble pas avoir vu d'urgence.

#7

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Frédéric Péters a écrit :

Ça se mélange trop à #42393 (sur les comportements, sur la migration, etc.), je vais terminer celui-là, puis je prendrai celui-ci, il ne me semble pas avoir vu d'urgence.

L'urgence c'est Thomas qui l'a signalé: les comptes supprimés ont leur mail qui change mais sont aussi signalés inactifs, ce mail étant invalide ça bloque l'envoie des mails aux rôle dont l'utilisateur est membre dans w.c.s.. Il y a eu au moins un cas la semaine dernière.

#8

Mis à jour par Frédéric Péters il y a plus de 3 ans

Ok je n'ai pas vu ce message de Thomas.

#9

Mis à jour par Frédéric Péters il y a plus de 3 ans

  • Statut changé de En cours à Solution proposée

J'ai repris et poussé dans wip/42428-is-active-flag,

  • rebase du côté SQL par rapport au numéro de migration et à la présence désormais d'un deleted_timestamp
  • adaptation à la migration pour faire un SET is_active = FALSE quand deleted_timestamp est posé
  • test de la migration (nouveau commit)
  • modification sur l'action de notif pour également prendre en compte le _submitter (pas juste les rôles)
  • modif à get_submitter_email pour ignorer l'utilisateur associé s'il a is_active à False (nouveau commit)
#10

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

  • Statut changé de Solution proposée à Solution validée

Frédéric Péters a écrit :

  • modif à get_submitter_email pour ignorer l'utilisateur associé s'il a is_active à False (nouveau commit)

Je ne suis pas certain de la logique de ce dernier, mais soit on verra bien si ça pose problème à l'usage; ça voudrait dire qu'un utilisateur qui supprime son compte ne recevra plus de notification pour une demande en cours....

#11

Mis à jour par Frédéric Péters il y a plus de 3 ans

Je ne suis pas certain de la logique de ce dernier, mais soit on verra bien si ça pose problème à l'usage; ça voudrait dire qu'un utilisateur qui supprime son compte ne recevra plus de notification pour une demande en cours....

Sauf si l'adresse a été explicitement donnée dans un champ de la demande.

Pour moi ça suit la logique de ton patch de ne plus envoyer de notif.

#12

Mis à jour par Frédéric Péters il y a plus de 3 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit 83e341c9d929ddb4604a0d9dfaa7de97811c056f
Author: Frédéric Péters <fpeters@entrouvert.com>
Date:   Mon Jul 27 14:40:30 2020 +0200

    misc: ignore email attribute for disabled users (#42428)

commit f2e85509974e0a4612db039a4d613a165edc4e89
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Tue Jul 7 16:09:30 2020 +0200

    sessions: unlog inactive users (#42428)

commit 612bfb1ea18283f30b462530b0bb5168716b6773
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Tue Jul 7 16:09:11 2020 +0200

    wf: do not notify inactive users (#42428)

commit 248b0c28017121ee6e33de36a82c99709d35bdc6
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Tue Jul 7 12:02:35 2020 +0200

    roles: don't include inactive members emails (#42428)

commit 4ef65f99473dcb01af5d559787ffd0018dfb5288
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Tue Jul 7 12:00:47 2020 +0200

    hobo_notify: set is_active attribute (#42428)

commit 3a2b8d45e4249268ba0b0647047778201d46f31b
Author: Frédéric Péters <fpeters@entrouvert.com>
Date:   Mon Jul 27 14:47:19 2020 +0200

    tests: test is_active user column migration (#42428)

commit b145351e2d255adc80e095ecd4ac94007dd57b17
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Tue Jul 7 11:59:34 2020 +0200

    users: add column to store is_active (#42428)
#13

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
#14

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Frédéric Péters a écrit :

Sauf si l'adresse a été explicitement donnée dans un champ de la demande.

Oui si le formulaire est pensé pour un usage doubl en mode connecté / non-connecté, et contient donc un champ email, ça passe.

Pour moi ça suit la logique de ton patch de ne plus envoyer de notif.

Oui mais ça ne concernait que les rôles, contenant généralement uniquement des agents il me semble, ici ça va concerner un utilisateur attaché à un formulaire, souvent un usager, et donc pour un formulaire pensé uniquement pour un usage connecté mais ou par hasard la personne supprimerait son compte (jusqu'à ce qu'on décide de bloquer ça), on avait décidé il me semble que du point de vu de l'usager le traitement se terminerait normalement dans w.c.s. autant que faire se peut, d'où le fait qu'on ne supprime pas les usagers dans w.c.s. par exemple.

Il y a un truc qui me gêne ici, qui ne me gênait pas pour les mails/notifications basés sur les rôles.

#15

Mis à jour par Frédéric Péters il y a plus de 3 ans

Toujours confusion is_active et compte supprimé, ce ticket concerne is_active, qui pris indépendamment peut uniquement être posé par un agent depuis le backoffice, et oui communément sans doute concernerait d'autres agents. Dans la pratique surtout j'ai l'impression que personne n'utilise vraiment ça. Et on n'a jamais eu de discussion sur la signification que cette fonctionnalité aurait.

Reste qu'au moment de la suppression, authentic fait is_active=False et email += #random, et là, on ne voudrait pas qu'un email se trouve envoyé à email#random, il me semblait même que c'est de là que venait "l'urgence" de ce ticket.

Formats disponibles : Atom PDF