Produits Entr'ouvert » w.c.s.

Rapide commentaire perso, je préférerais la partie "suppression de la fonctionnalité d'archivage" dans un commit séparé.

Et faut plutôt faire le taf dans un afterjob (comme c'était le cas pour l'archivage).

Pour la valeur de "anonymised", plutôt que le time.localtime(), je prendrais le datetime.datetime.now() (ça nécessite derrière une modif du côté SQL).

Moi j'aime bien que ça soit un timestamp en SQL (time.localtime()), comme pour receipt_time et les dates des évolutions. J'AI RIEN DIT.

C'est OK pour que je pousse le patch 0001_remove_archive_feature.patch puis le 0003_anonymise_forms.patch sur le dépôt ?

Tu parlais d'un truc de PostgreSQL de config de timezone ou un truc du même acabit, il en est quoi ?

Je pensais que le timestamp avec avec gestion des timzone était obligatoire pour l'utilisation de datetime mais ce n'est pas le cas. J'ai choisi un timestamp sans timezone pour rester cohérent avec les autres timestamp de w.c.s. Maintenant si tu penses que ça peut être utile de rajouter la timezone je peux changer le type du champ anonymized.

Oui, je serais pour, ici, enregistrer l'information du fuseau horaire (et si je lis bien la doc de postgresql, il n'y a rien à faire, le datetime.now() retourne une valeur dans la timezone du serveur, et postgresql la convertira automatiquement pour stockage en utc).

J'allais dire que c'était ok mais je me demande si la suppression de l'archivage peut se faire sans être compensée par l'ajout d'une fonctionnalité de suppression de données (d'un certain âge?) (et aussi une possibilité d'export des données ?).

Aussi l'anonymisation se fait sur tous les formulaires traités, il faudrait peut-être permettre de spécifier un âge minimum quand même. (?) Ou bien ce cas de figure se gère via une nouvelle action de workflow (déclenchée sur timeout, blabla). (?)

Bref, est-ce que pousser ça maintenant oblige à avoir dans la foulée des développements particuliers ?

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

J'allais dire que c'était ok mais je me demande si la suppression de l'archivage peut se faire sans être compensée par l'ajout d'une fonctionnalité de suppression de données (d'un certain âge?) (et aussi une possibilité d'export des données ?).

Ouaip. Je dirais : on supprime l'archivage uniquement en mode SQL et seulement parce qu'il ne fonctionne pas encore. On le remet quand il remarchera...

Aussi l'anonymisation se fait sur tous les formulaires traités, il faudrait peut-être permettre de spécifier un âge minimum quand même. (?) Ou bien ce cas de figure se gère via une nouvelle action de workflow (déclenchée sur timeout, blabla). (?)

• lors de l'anonymisation, permettre de choisir parmi les statuts terminaux (tous cochés par défaut)
• ajouter un champ "demande terminée depuis plus de "x" jours", avec x par défaut à 30 (on n'anonymise par défaut que les demandes dont le dernier traitement a eu lieu il y a plus de 30 jours).

Est ce que le dernier patch (0001_anonymisation_keep_archiving.patch) vous semble OK pour être pousser dans le git ?

Jérôme Schneider a écrit :

Est ce que le dernier patch (0001_anonymisation_keep_archiving.patch) vous semble OK pour être pousser dans le git ?

Tel quel non : il manque encore la gestion de l'affichage des données anonymisées dans les listings. A reflechir un peu.

+        r += htmltext('<li><a href="anonymise">%s</a></li>') % _('Anonymise handled forms')

Mineur : juste "Anonymise forms".

+    def anonymise(self):
+        all_forms = self.formdef.data_class().select()
+        for formdata in all_forms:
+            if not formdata.anonymised:
+                formdata.anonymise()
+        if get_request().form.get('job'):
+            return self.anonymise_processing()
[...]

Majeur : c'est la méthode qui affiche le formulaire, c'est à mon avis une erreur (reste d'un ancien patch) d'avoir la procédure d'anonymisation en haut de celle-ci.

+        form.add(CheckboxesWidget, 'endpoints', title=_('Status of the requests to anonymise'),

Mineur : s/requests/forms/

+            def anonymise(self, job=None):
+                all_forms = self.formdef.data_class().select()
+                all_forms = [x for x in all_forms if x.status in self.status_ids]
+                all_forms = [x for x in all_forms if (
+                    x.evolution and x.evolution[-1].time < self.before_date) or (
+                    x.receipt_time < self.before_date)]
+                for formdata in all_forms:
+                    if not formdata.anonymised:
+                        formdata.anonymise()

Mineur : en terme de style, si le plan est de réduire et réduire la liste, le test "if not formdata.anonymised" devrait être faire de la même manière. En style alternatif, les différentes conditions peuvent être ajoutées dans la boucle, ainsi :

for formdata in self.formdef.data_class().select():
    if formdata.anonymised:
        continue
    if formdata.status not in self.status_ids:
        continue
    if (formdata.evolution and ...):
        continue
    if not formdata.anonymised:
        continue
    formdata.anonymise()

Comme tu veux.

En troisième voie il pourrait aussi y avoir l'utilisation de l'index sur les statuts, pour ne pas devoir passer par un select() général, mais ça demande plus de changements dans le code et comme on n'a pas d'exigence de rapidité ici, ce n'est pas nécessaire.

@@ -633,6 +635,7 @@ class FileField(WidgetField):
     key = 'file'
     description = N_('File Upload')

+    anonymise = False
     widget_class = FileWithPreviewWidget

Majeur : les fichiers attachés doivent selon moi être anonymisés.

+    def anonymise(self):
+        for field in self.formdef.fields:
+            if field.anonymise:
+                self.data[field.id] = None

Majeur : il peut y avoir une variante _display (il faut tester field.store_display_value) et celle-ci devrait être anonymisée aussi (même si avec les champs existants aujourd'hui, ceux-là devraient tous avoir le anonymise à False).

Majeur : d'ailleurs, ItemsField, il devrait avoir son anonymise à False, non ?

+                    r += htmltext('<td class="cell-anonymised">%s</td>') % anonymised

Mineur : je ferais remonter dans le <tr> un attribut data-anonymised="true" (ou une classe), qui pourrait par la suite servir à leur appliquer une apparence différente.

Thomas Noël a écrit :

Mineur : je ferais remonter dans le <tr> un attribut data-anonymised="true" (ou une classe), qui pourrait par la suite servir à leur appliquer une apparence différente.

... et je viens de me rendre compte que ma technique est idiote car elle donne un data-anonymised="true" ... partie à revoir.

Ouaip, faut faire la substitution à l'intérieur du htmltext() si tu veux faire ça ainsi...

data += ' data-anonymised="true"' ; j'aurais juste mis un =; mais c'est un détail, pour moi c'est ok ainsi.

En préparant les traductions j'ai noté qu'il y avait une chaine "Requests ended before", j'ai changé ça pour utiliser "Forms" et non "Requests".