zds-site icon indicating copy to clipboard operation
zds-site copied to clipboard

N'affiche *que* les tutoriels en bêta lorsque le filtre est actif

Open philippemilink opened this issue 2 years ago • 5 comments

Avec MariaDB, les chaînes de caractères qui sont censées être nulles, peuvent être NULL ou une chaîne de caractères vide. Cet commit ajoute une expression de recherche (field lookup) qui permet de traiter facilement ces deux cas.

Cela amenait des contenus pas en bêta à s'afficher en bêta sur la liste des contenus d'un utilisateur. Par exemple https://beta.zestedesavoir.com/tutoriels/voir/SpaceFox/?filter=beta ne doit pas contenir Introduction à l'astronomie. Le problème semble être spécifique à MariaDB (tout du moins il ne se produit pas avec SQLite), mais je n'ai pas réussi à le reproduire, même avec MariaDB en local.

Contrôle qualité

S'assurer que les contenus s'affichent correctement sur les listes de contenus d'un utilisateur, en jouant avec les différents filtres.

Les tests semblent suffisant pour couvrir ces cas (des tests m'ont permis de corriger un bug dans IsEmpty). Le patch est déployé sur la bêta.

philippemilink avatar May 07 '22 20:05 philippemilink

On autorise certains champs à être soit vide soit null : https://github.com/zestedesavoir/zds-site/blob/fdac9572b172dbbb0a8e9fc2046827695f393e81/zds/tutorialv2/models/database.py#L91

J'ai appris très récemment (aujourd'hui), que ce n'est pas recommandé. Il faudrait peut-être songer à changer ça à l'avenir.

J'ai appris ça en jettant un oeil aux règles que vérifie l'outil Code Review Doctor. La règle en question : avoid null string field.

Arnaud-D avatar May 07 '22 20:05 Arnaud-D

Coverage Status

Coverage increased (+0.002%) to 87.527% when pulling f07add43a7fb7ec58f17e177116e1dd101a87b43 on philippemilink:fix-list-beta into ad855f7d1eb5b23679004c65cb5d9cef8470fc42 on zestedesavoir:dev.

coveralls avatar May 07 '22 20:05 coveralls

On autorise certains champs à être soit vide soit null :

https://github.com/zestedesavoir/zds-site/blob/fdac9572b172dbbb0a8e9fc2046827695f393e81/zds/tutorialv2/models/database.py#L91

J'ai appris très récemment (aujourd'hui), que ce n'est pas recommandé. Il faudrait peut-être songer à changer ça à l'avenir.

J'ai appris ça en jettant un oeil aux règles que vérifie l'outil Code Review Doctor. La règle en question : avoid null string field.

Ça m'étonne, dans la doc il est dit que null est au niveau BDD et blank est au niveau validation des formulaires : https://docs.djangoproject.com/fr/3.2/ref/models/fields/#blank.

philippemilink avatar May 07 '22 20:05 philippemilink

J'ai pas été très clair : ce qui n'est pas recommandé, c'est d'avoir des CharField nullables (cf https://docs.djangoproject.com/en/3.2/ref/models/fields/#django.db.models.Field.null), parce que ça donne deux valeurs pour vide : "" et None, ce qui sème la confusion.

Le fait que "blank" soit à True ne doit pas changer grand chose dans notre cas, parce qu'il ne me semble pas qu'on ait des formulaires avec ça, si ?

Arnaud-D avatar May 07 '22 21:05 Arnaud-D

(je tente de faire une PR un peu plus propre qui traite directement le problème des champs NULL-able)

philippemilink avatar May 08 '22 15:05 philippemilink