Projet

Général

Profil

Bug #15163

source de données: ne pas en permettre la suppression ou la modification du slug si elle est en cours d'utilisation

Ajouté par Thomas Noël il y a environ 7 ans. Mis à jour il y a plus de 4 ans.

Statut:
Fermé
Priorité:
Normal
Version cible:
-
Début:
27 février 2017
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Parce que sinon ça casse tout, par exemple l'affichage d'un formulaire en backoffice.

C'est assez fastidieux, il faut parcourir l'ensemble des formdef, y compris ceux dans les workflows (dans les formulaires de workflow, mais aussi les champs de traitement, etc).

Dans un premier temps, on peut aussi travailler à faire en sorte que la suppression d'une source de données ne fasse pas tout planter.


Fichiers


Demandes liées

Lié à w.c.s. - Development #36865: avoir un get_formdefs_of_all_kinds()Fermé11 octobre 2019

Actions
Dupliqué par w.c.s. - Development #24629: Empêcher la suppression/modification de slug d'une source de données en cours d'utilisationRejeté19 juin 2018

Actions

Révisions associées

Révision 215209fb (diff)
Ajouté par Nicolas Roche (absent jusqu'au 3 avril) il y a plus de 4 ans

admin: protect datasources in use from deletion or slug change (#15163)

Historique

#1

Mis à jour par Thomas Noël il y a environ 7 ans

Exemple d'une trace quand un data_source n'existe plus (parce que supprimé ou slug modifié):

Exception:
  type = '<type 'exceptions.KeyError'>', value = ''

Stack trace (most recent call first):
  File "/usr/lib/python2.7/dist-packages/wcs/data_sources.py", line 259, in get_by_slug
   257         if objects:
   258             return objects[0]
>  259         raise KeyError()
   260
   261     @classmethod

  locals:
     x = <NamedDataSource 'rsu_genre' id:10>
     objects = []
     slug = 'voiesnanterrev0'
     cls = <class 'wcs.data_sources.NamedDataSource'>

  File "/usr/lib/python2.7/dist-packages/wcs/data_sources.py", line 193, in get_real
   191     if ds_type in ('json', 'jsonp', 'formula'):
   192         return data_source
>  193     return NamedDataSource.get_by_slug(ds_type).data_source
   194
   195

  locals:
     data_source = {'type': 'voiesnanterrev0'}
     ds_type = 'voiesnanterrev0'

  File "/usr/lib/python2.7/dist-packages/wcs/fields.py", line 1066, in perform_more_widget_changes
  1064
  1065     def perform_more_widget_changes(self, form, kwargs, edit = True):
> 1066         real_data_source = data_sources.get_real(self.data_source)
  1067         if real_data_source and real_data_source.get('type') == 'jsonp':
  1068             kwargs['url'] = real_data_source.get('value')

  locals:
     edit = True
     self = <wcs.fields.ItemField object at 0x7f4d82597050>
     form = <qommon.form.Form object at 0x7f4d82516f50>
     kwargs = {'required': True}

  File "/usr/lib/python2.7/dist-packages/wcs/fields.py", line 347, in add_to_form
   345             if hasattr(self, k):
   346                 kwargs[k] = getattr(self, k)
>  347         self.perform_more_widget_changes(form, kwargs)
   348         if self.hint and self.hint.startswith('<'):
   349             hint = htmltext(self.hint)

  locals:
     self = <wcs.fields.ItemField object at 0x7f4d82597050>
     form = <qommon.form.Form object at 0x7f4d82516f50>
     value = None
     kwargs = {'required': True}

  File "/usr/lib/python2.7/dist-packages/wcs/admin/forms.py", line 786, in get_preview
   784             field.id = i
   785             if hasattr(field, str('add_to_form')):
>  786                 field.add_to_form(form)
   787             else:
   788                 if field.key == 'page':

  locals:
     i = 19
     field = <wcs.fields.ItemField object at 0x7f4d82597050>
     self = <wcs.admin.forms.FormDefPage object at 0x7f4d82345e50>
     on_page = 4
     form = <qommon.form.Form object at 0x7f4d82516f50>

  File "/usr/lib/python2.7/dist-packages/wcs/admin/forms.py", line 507, in _q_index
   505         r += htmltext('<h3 class="clear">%s <span class="change">(<a href="fields/">%s</a>)</span></h3>') % (
   506                         _('Fields'), _('edit'))
>  507         r += self.get_preview()
   508         r += htmltext('</div>')
   509         return r.getvalue()

  locals:
     online_status = 'Activ\xc3\xa9e'
     self = <wcs.admin.forms.FormDefPage object at 0x7f4d82345e50>
     add_option_line = <function add_option_line at 0x7f4d8248a848>
     role_id = '65b6fbf28e6e48baa5a3bcdd5bd9e301'
     pristine_workflow = <Workflow 'Cr\xc3\xa9ation fiche adulte (BO)' id:2>
     r = <TemplateIO object at 0x7f4d8641c5f0>
     wf_role_id = '_receiver'
     wf_role_label = 'Destinataire'
     role = <Role 'Agent' id:65b6fbf28e6e48baa5a3bcdd5bd9e301>
     warning_class = ''
     role_label = 'Agent'
#2

Mis à jour par Frédéric Péters il y a environ 7 ans

Pour faire ça, et d'autres trucs similaires, ce sera sans doute utile de commencer par avoir un get_formdefs_of_all_kinds(), qui prendrait les formdefs mais aussi ceux qui peuvent se trouver à divers endroits des workflows, comme profil utilisateur, etc.

wcs/admin/settings.py:class UserFieldsFormDef(FormDef):
wcs/wf/form.py:class WorkflowFormFieldsFormDef(FormDef):
wcs/workflows.py:class WorkflowVariablesFieldsFormDef(FormDef):
wcs/workflows.py:class WorkflowBackofficeFieldsFormDef(FormDef):
#3

Mis à jour par Frédéric Péters il y a presque 6 ans

  • Dupliqué par Development #24629: Empêcher la suppression/modification de slug d'une source de données en cours d'utilisation ajouté
#5

Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a plus de 4 ans

  • Assigné à mis à Nicolas Roche (absent jusqu'au 3 avril)
#6

Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a plus de 4 ans

#7

Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a plus de 4 ans

Premier jet, c'est très moche.
J'ai découpé le patch en deux pour décorréler :
  • d'une part le refus d'exécuter les actions,
  • et d'autre part le masquage de ces actions dans l'UI.

(Mais en fait le découpage est surtout là pour tenter de faciliter les tests des relecteurs.
J'intuite que je n'aurais pas du faire ce découpage qui introduit un recoupement, mais ça me donnera l'occasion de faire mieux au coup suivant)

Vos lumières sont plus que les bienvenues (ça touche pas mal de trucs nouveaux pour moi).

#8

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

Nicolas Roche a écrit :

Premier jet, c'est très moche.

Ça m'a l'air très correct.

J'ai découpé le patch en deux pour décorréler :
  • d'une part le refus d'exécuter les actions,
  • et d'autre part le masquage de ces actions dans l'UI.

(Mais en fait le découpage est surtout là pour tenter de faciliter les tests des relecteurs.
J'intuite que je n'aurais pas du faire ce découpage qui introduit un recoupement, mais ça me donnera l'occasion de faire mieux au coup suivant)

Vos lumières sont plus que les bienvenues (ça touche pas mal de trucs nouveaux pour moi).

1. j'aurai mis la méthode is_using_datasource dans datasource (c'est plus spécifique à datasource qu'à formdef, si on doit faire la même chose pour wscall ou un autre objet un jour on ne va pas tout mettre dans FormDef),
2. dans is_using_datasource tu ne parcours pas les champs issus du workflow, tu peux regarder/importer le commit "ease access to widget fields" sur le ticket #33186 pour cela,
3. plutôt que de cacher le bouton je trouve plus correct de le désactiver et de mettre la raison en title sur le bouton, mais je ne sais pas si on a une classe bouton désactivé pour les liens (de toute façon utiliser des liens pour faire des boutons dans un formulaire n'est pas une très une bonne pratique, déjà supprimer un truc via GET ce n'est pas cool)

#9

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

4. (pour un autre ticket) si les différents FormDef se dotaient d'un get_admin_url() et d'un nom correct on pourrait modifier usage_in_formdefs() pour parcourir aussi tous les FormDefs, c'est utilisé dans ce template :

wcs/templates/wcs/backoffice/data-sources.html:{% with formdefs=view.usage_in_formdefs %}
wcs/templates/wcs/backoffice/data-sources.html-  {% if formdefs %}
wcs/templates/wcs/backoffice/data-sources.html-  <h3>{% trans "Usage in forms" %}</h3>
wcs/templates/wcs/backoffice/data-sources.html-  <ul class="biglist">
wcs/templates/wcs/backoffice/data-sources.html-  {% for formdef in formdefs %}
wcs/templates/wcs/backoffice/data-sources.html-    <li><a href="{{ formdef.get_admin_url }}">{{ formdef.name }}</a></li>
wcs/templates/wcs/backoffice/data-sources.html-  {% endfor %}
wcs/templates/wcs/backoffice/data-sources.html-  </ul>
wcs/templates/wcs/backoffice/data-sources.html-  {% endif %}
wcs/templates/wcs/backoffice/data-sources.html-{% endwith %}

#10

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

Je préférerais éviter les erreurs 404, simplement dans la page, plutôt que le formulaire de confirmation de la suppression, avoir un message "non non pas possible". (et on a une classe disabled qui pourrait être mise sur le lien mais elle va un peu trop fort empêcher le clic, donc je laisserais comme il est).

Pour la modification du slug, c'est déjà en paramètre avancé avec un commentaire comme quoi il faut faire attention, à changer ça, ce serait plutôt afficher ce champ uniquement quand la source de données n'est pas utilisée et retirer le message disant qu'il faut faire attention.

#11

Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a plus de 4 ans

Merci pour vos remarques :

1. j'ai déplacé la fonction :
wcs/data_sources.py::NamedDataSource::is_used_in_formdef()
et j'ai rajouté un test (dans test_datasource.py).

2. Benjamin, tu me pointes le commit "ease access to widget fields"
sur le ticket #33186 afin que je n'accède pas par mégarde à un item
qui ne serait pas un widget et qui possèderait quand même un
attribut 'data_source'.

3. Compris : pas besoin des 404, juste les anticiper.

#12

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

        for field in (f for f in formdef.fields or [] if isinstance(f, WidgetField)):

Question forme, ce sera beaucoup plus lisible en n'imbriquant pas tant.

        for field in formdef.fields or []:
            if not isinstance(field, WidgetField):
                continue

(et vu le contenu de la boucle, il n'y a même pas à faire ce test sur WidgetField).

#13

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

Nicolas Roche a écrit :

2. Benjamin, tu me pointes le commit "ease access to widget fields"
sur le ticket #33186 afin que je n'accède pas par mégarde à un item
qui ne serait pas un widget et qui possèderait quand même un
attribut 'data_source'.

Non je pointais que ton code (et le code d'avant non plus) ne parcourait pas les champs correspondant à des variables de traitement, mais en fait si via WorkflowBackofficeFieldsFormDef donc oublie, tu n'en as pas besoin.

#15

Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a plus de 4 ans

(on n'a pas "formularies" dans notre vocabulaire), mais passons, mettons plutôt : "This datasource is still used, it cannot be deleted."

(modification du message d'erreur)

#16

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

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

Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a plus de 4 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit 215209fb98bdd4ade58b624a32c23e19d8911b0c
Author: Nicolas ROCHE <nroche@entrouvert.com>
Date:   Thu Oct 10 18:01:34 2019 +0200

    admin: protect datasources in use from deletion or slug change (#15163)

#18

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

  • Statut changé de Résolu (à déployer) à Solution déployée

Formats disponibles : Atom PDF