Project

General

Profile

Bug #36037

crash après le retrait d'une source de données dans un champ liste

Added by Paul Marillonnet 4 days ago. Updated 4 days ago.

Status:
En cours
Priority:
Normal
Target version:
-
Start date:
11 Sep 2019
Due date:
% Done:

0%

Patch proposed:
No
Planning:
No

Description

Dans l'ordre, je :
- crée un formulaire
- ajoute un champ liste, avec une source de données (en l'occurrence l'URL d'un JSON)
- modifie le champ liste, retire la source de champs liste
- retourne sur la page principale backoffice du formulaire → Crash

0001-WIP-do-not-crash-on-unknown-data-source-type-36037.patch View (2.69 KB) Paul Marillonnet, 11 Sep 2019 02:52 PM

0001-WIP-do-not-crash-on-unknown-data-source-type-36037.patch View (1.24 KB) Paul Marillonnet, 11 Sep 2019 03:20 PM

0001-data_sources-do-not-crash-on-unknown-source-type-360.patch View (2.17 KB) Paul Marillonnet, 11 Sep 2019 04:59 PM

History

#1 Updated by Paul Marillonnet 4 days ago

  • Assignee set to Paul Marillonnet

#2 Updated by Paul Marillonnet 4 days ago

A priori on peut s'occuper de gérer les types de sources inconnus.
Alternativement, on pourrait aussi faire en sorte qu'une source de données retirée d'un champ liste ne laisse pas de reliquat comme c'est le cas actuellement.

Qu'en disent les dévs w.c.s. ?

#3 Updated by Paul Marillonnet 4 days ago

La trace, comme demandée de vive voix au bureau :

[2019-09-11 14:37:41] exception caught
Exception:
  type = '<type 'exceptions.AttributeError'>', value = ''NoneType' object has no attribute 'startswith''

Stack trace (most recent call first):
  File "/home/paul/src/wcs/wcs/data_sources.py", line 245, in get_object
   243         named_data_source.data_source = data_source
   244         return named_data_source
>  245     if ds_type.startswith('carddef:'):
   246         named_data_source = NamedDataSource()
   247         named_data_source.data_source = data_source

  locals: 
     data_source = {'value': '{{ passerelle_url }}csvdatasource/liste-des-ecoles-primaires-triees/query/type/?type={% if form_var_niveau_ecole == "Toute petite section" or form_var_niveau_ecole == "Petite section" or form_var_niveau_ecole == "Moyenne section" or form_var_niveau_ecole == "Grande section" %}maternelle{% elif form_var_niveau_ecole == "CP" or form_var_niveau_ecole == "CE1" or form_var_niveau_ecole == "CE2" or form_var_niveau_ecole == "CM1" or form_var_niveau_ecole == "CM2" or form_var_niveau_ecole == "ULIS" %}\xc3\xa9l\xc3\xa9mentaire{% endif %}'}
     ds_type = None

  File "/home/paul/src/wcs/wcs/fields.py", line 1264, in perform_more_widget_changes
  1262 
  1263     def perform_more_widget_changes(self, form, kwargs, edit=True):
> 1264         data_source = data_sources.get_object(self.data_source)
  1265 
  1266         if data_source and data_source.type == 'jsonp':

  locals: 
     edit = True
     form = <qommon.form.Form object at 0x7f5adc83df10>
     kwargs = {'required': True}
     self = <ItemField 18 '\xc3\x89cole'>

[...]

#4 Updated by Frédéric Péters 4 days ago

Le gros pâté du patch modifie cette ligne :

    if data_source.get('type') and data_source.get('type').startswith('carddef:'):

Qui assure déjà que le second terme ne sera pas évalué si le premier est None.

Pâté à dégager.

#5 Updated by Paul Marillonnet 4 days ago

En fait la partie problématique est le bloc d'en dessous

if data_source.get('type') not in ('json', 'jsonp', 'formula'):

lorsque data_source.get('type') est évalué à None.

Je pensais, plutôt que d'inclure un if data_source.get('type') and … (voire une truc plus crade genre un ajout de None dans le tuple des not in (…)), faire remonter cette condition pour la cerner dans les deux blocs consécutifs. D'où le gros paté. Je me plante ?

#6 Updated by Paul Marillonnet 4 days ago

(Et ceci, sous l'hypothèse que la lisibilité finale du code une fois patché est plus importante que la lisibilité (et la taille) du patch. Et non pas l'inverse. Je me plante aussi ici ?)

#7 Updated by Frédéric Péters 4 days ago

Je pensais, plutôt que d'inclure un if data_source.get('type') and … (voire une truc plus crade genre un ajout de None dans le tuple des not in (…)), faire remonter cette condition pour la cerner dans les deux blocs consécutifs. D'où le gros paté. Je me plante ?

Je ne vois pas pourquoi tu continues à parler d'une partie du code qui n'a aucune raison de changer.

#8 Updated by Paul Marillonnet 4 days ago

Ok, je n'aimais pas la forme non factorisée des tests sur data_source.get('type'), mais c'est pas non plus affreux.
Je vais écrire des tests

#9 Updated by Paul Marillonnet 4 days ago

#10 Updated by Frédéric Péters 4 days ago

datasource = {'value': "https://whatever.com/json"}

Ok via ce test je vois donc ce qu'il y avait là, mais du coup ça me semble plutôt devoir se corriger ailleurs, plutôt qu'avoir à surveiller quantité d'endroits sur des données malformées, genre :

@@ -90,7 +90,7 @@ class DataSourceSelectionWidget(CompositeWidget):
             value = self.get(name)
             if value:
                 values[name] = value
-        if values.get('type', '') == 'none':
+        if not values.get('type') or values.get('type') == 'none':
             values = None
         self.value = values or None

#11 Updated by Paul Marillonnet 4 days ago

  • Status changed from Solution proposée to En cours
  • Patch proposed changed from Yes to No

Ok nickel, c'est mieux comme ça, merci. Je vais donc écrire le test qui va avec, dans tests/test_widgets.py.

Also available in: Atom PDF