Project

General

Profile

Bug #67843

Le prefill sur les listes à choix multiple doit accepter la valeur None

Added by Thomas Noël about 1 year ago. Updated about 1 year ago.

Status:
Fermé
Priority:
Normal
Assignee:
Frédéric Péters
Target version:
-
Start date:
01 August 2022
Due date:
% Done:

0%

Estimated time:
Patch proposed:
Yes
Planning:
No

Description

Actuellement sur le calcul d'un prefill sur un ItemsFields, depuis #66995 on fait une vérification sur le type reçu :

        value, explicit_lock = super().get_prefill_value(user=user, force_string=False)
        if not isinstance(value, (str, tuple, list)) or not all(isinstance(x, (int, str)) for x in value):
            get_publisher().record_error(
                _('Invalid value for items prefill on field "%s"') % self.label, 
                formdef=getattr(self, 'formdef', None),
            ...

Or value peut être None, notamment pour le cas où prefill est {'type': 'none'}

Il ne font donc pas logguer si value is None

Files

0001-fields-allow-none-prefill-in-items-fields-67843.patch (2.51 KB) 0001-fields-allow-none-prefill-in-items-fields-67843.patch Thomas Noël, 01 August 2022 12:28 PM
form-reservation-halle-toiles.wcs (2.5 KB) form-reservation-halle-toiles.wcs Nicolas Roche, 01 August 2022 03:25 PM
0001-fields-allow-none-prefill-in-items-fields-67843.patch (3.07 KB) 0001-fields-allow-none-prefill-in-items-fields-67843.patch Thomas Noël, 01 August 2022 04:17 PM
0001-fields-use-accessor-to-get-prefill-configuration-678.patch (7.1 KB) 0001-fields-use-accessor-to-get-prefill-configuration-678.patch Frédéric Péters, 09 August 2022 06:54 AM
0002-fields-allow-none-prefill-in-items-fields-67843.patch (2.5 KB) 0002-fields-allow-none-prefill-in-items-fields-67843.patch Frédéric Péters, 09 August 2022 06:54 AM

Associated revisions

Revision dd79662c (diff)
Added by Frédéric Péters about 1 year ago

fields: use accessor to get prefill configuration (#67843)

This allows an uniform handling of the unexpected {'type': 'none'}
situation.

Revision 4050ddd8 (diff)
Added by Thomas Noël about 1 year ago

fields: allow none prefill in items fields (#67843)

History

#1

Updated by Thomas Noël about 1 year ago

  • Assignee set to Thomas Noël
#3

Updated by Thomas Noël about 1 year ago

#5

Updated by Nicolas Roche about 1 year ago

  • Status changed from Solution proposée to Solution validée

Je suis embêté pour valider ici, parce que oui ça paraît être la bonne correction,
cependant le test ne couvre pas les 2 types de traces relevées dans les tickets liés, parce qu'ici on sort avant d'entrer dans get_prefill_value.
Sur le test :

        # check with a "none" explicit prefill, or a None value
        for none_prefill_value in [
            {'type': 'none'},

dans le code, on sort avant d'atteindre la correction, sur :

> /home/nroche/src/wcs/wcs/forms/root.py(421)apply_field_prefills()
417                  if field.prefill:

J'imagine que je chipote pour rien, auquel cas je valide.

#6

Updated by Thomas Noël about 1 year ago

  • Status changed from Solution validée to En cours

Effectivement, ça ne va pas... Le test ne plante pas avec prefill={'type': 'none'}.

#7

Updated by Nicolas Roche about 1 year ago

J'arrive à reproduire avec ce petit formulaire (ici d'un export du formulaire d'un des 2 tickets lié : #67651)

#8

Updated by Thomas Noël about 1 year ago

Voici une idée de patch que je pense bonne... mais un test manque qui passerait par reset_locked_data dans wcs/forms/root.py avec un ItemsField configuré avec un prefill={'type': 'none'}

Mystère trop profond pour moi, je ne sais pas écrire de test sur le sujet... :/

#10

Updated by Frédéric Péters about 1 year ago

  • Assignee set to Frédéric Péters
  • Patch proposed changed from Yes to No
#11

Updated by Frédéric Péters about 1 year ago

J'ai repris le commit mais j'y ajouté avant un autre, qui ajoute une méthode get_prefill_configuration, qui assure que tout le temps le dictionnaire "prefill" sera vide pour signifier qu'il n'y a pas de préremplissage, jamais {"type": "none"}.

Rebasé le commit là-dessus, c'était lui enlever la partie touchant à wcs/forms/root.py. J'ai aussi modifié le test pour passer sur le cas prefill dictionnaire vide (même si ça n'apporte pas grand chose) + y corriger le commentaire "it will use foo,bar as selected in the first part of this test" qui ne correspondait pas à ce qui était attendu/vérifié.

#12

Updated by Thomas Noël about 1 year ago

  • Status changed from Solution proposée to Solution validée

Clairement plus solide.

#13

Updated by Frédéric Péters about 1 year ago

  • Status changed from Solution validée to Résolu (à déployer)
commit 4050ddd838366ff23baa4fbb1acdc76efd7207e3
Author: Thomas Noël <tnoel@entrouvert.com>
Date:   Mon Aug 1 12:28:23 2022 +0200

    fields: allow none prefill in items fields (#67843)

commit dd79662c1b0e23191a8e0a17a55c5e045ec316ec
Author: Frédéric Péters <fpeters@entrouvert.com>
Date:   Mon Aug 8 18:57:30 2022 +0200

    fields: use accessor to get prefill configuration (#67843)

    This allows an uniform handling of the unexpected {'type': 'none'}
    situation.
#14

Updated by Transition automatique about 1 year ago

  • Status changed from Résolu (à déployer) to Solution déployée
#15

Updated by Transition automatique 12 months ago

Automatic expiration

Also available in: Atom PDF