Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problème déconsigne d'écocup #577

Open
Juknum opened this issue Mar 3, 2023 · 18 comments
Open

Problème déconsigne d'écocup #577

Juknum opened this issue Mar 3, 2023 · 18 comments
Assignees
Labels
s: counters severity: critical The issue causes data loss, crashes or makes the system unresponsive, etc

Comments

@Juknum
Copy link
Member

Juknum commented Mar 3, 2023

Il est possible de déconsigner plus d'écocup que ce que l'utilisateur à déjà consigné

@Juknum Juknum added severity: critical The issue causes data loss, crashes or makes the system unresponsive, etc s: counters labels Mar 3, 2023
@klmp200
Copy link
Contributor

klmp200 commented Mar 4, 2023

@klmp200 klmp200 closed this as completed Mar 4, 2023
@Juknum Juknum reopened this Mar 4, 2023
@Juknum
Copy link
Member Author

Juknum commented Mar 4, 2023

C'est clairement pas une feature quand cette limite n'est pas respectée. On a réussi à déconsigner 50 ecocups d'un coup.

@klmp200
Copy link
Contributor

klmp200 commented Mar 4, 2023

Par hasard, ils n'auraient psa crée plusieurs produits d'ecocup qui correspondent plus à ceux suivis par la feature ?
il faut plus de détails, quels produits sont affectés comment c'est utilisé ? Sinon on ne peut pas investiger.

@imperosol
Copy link
Contributor

C'est clairement pas une feature quand cette limite n'est pas respectée. On a réussi à déconsigner 50 ecocups d'un coup.

Pour le coup, je suis vraiment curieux sur ça.
Soit c'est une incompréhension et il n'y a pas de problème, soit c'est très grave, mais dans les deux cas, il faut vraiment beaucoup plus de détails que ça.

Qui a remarqué ce détail ? Quel jour ? Quelle heure ? Où ? Quel code le produit avait ?

@klmp200
Copy link
Contributor

klmp200 commented Mar 5, 2023

Je veux bien m'intéresser au problème mais comme expliqué, il me faut plus de détails.
Sachant que je n'ai accès à aucune donnée de production, je ne peux pas y vérifier mes hypothèses et j'ai aucune raison de penser que la fonctionnalitée originale ne fonctionne plus sachant qu'ellle fonctionnait très bien avant et rien n'a été modifié de ce côté.

@Juknum
Copy link
Member Author

Juknum commented Mar 6, 2023

Capture d’écran 2023-03-06 à 14 24 32

Capture d’écran 2023-03-06 à 14 25 29

Capture d’écran 2023-03-06 à 14 27 57

Capture d’écran 2023-03-06 à 14 30 06

@klmp200
Copy link
Contributor

klmp200 commented Mar 6, 2023

La feature fonctionne par rapport à l'ID du produit, pas par rapport à ses charactéristiques.
Il existe 1 produit de consigne et 1 produit de déconsigne qui sont définis dans la configuration du logiciel.
Je ne peux pas déterminer avec les informations que tu me fournis si le site est bien configuré par rapport aux produits que tu me présente.
Au vu de tous les changements qui ont eu lieu au niveau des consignes/déconsignes, il n'est pas à exclure que les gestionnaires de comptoires aient recrée les consignes/déconsignes à partir de nouveaux produits qui ne seraient donc pas traqués par la feature. Il se peut aussi qu'ils aient crées plusieurs produits de consignes qui ne sont également pas suivis.

Je n'ai pas ces informations dans ces screenshots.

@Juknum
Copy link
Member Author

Juknum commented Mar 6, 2023

Capture d’écran 2023-03-06 à 14 51 21

Capture d’écran 2023-03-06 à 14 52 33

Capture d’écran 2023-03-06 à 14 53 27

Quand tu dis ID, c'est l'ID de l'article dans la base de donnée ? ce serait hardcodé du coup ? Car je n'ai aucun moyen de te dire si l'ID de la déconsigne actuelle correspond à celle qui serait enregistrée.

Si c'est hardcodé, dans ce cas il faudrait ajouter une catégorie "Déconsigne" pour ça fonctionne avec n'importe quel article, nouveau ou ancien.

@klmp200
Copy link
Contributor

klmp200 commented Mar 6, 2023

Tu retrouve l'id du produit dans l'url
C'est hardcodé dans la config
C'est bel et bien ce que je soupçonait, la fonctionnalité va très bien, c'est l'entrée utilisateur qui ne va pas.

Le problème d'une catégorie « Déconsigne » c'est qu'il faut lier la déconsigne à un produit. Si on veut faire ça de manière dynamique ça nécessite une interface utilisateur assez compliquée.

Le plus simple c'est de faire des paires de produit « consigne/déconsigne » dans la config.

Conclusion, comme je l'ai indiqué au début, ce n'est pas un bug et ça fonctionne parfaitement, c'est un problème humain ;)

@Juknum
Copy link
Member Author

Juknum commented Mar 6, 2023

@TheoDurr tu pourrais checker la config du site steuplé avec les ids :

  • 1151 - déconsigne écocup
  • 2730 - déconsigne pichet

@TheoDurr
Copy link
Member

TheoDurr commented Mar 7, 2023

On a bien une config pour la consigne/déconsigne écocup.

sith3/sith/settings.py

Lines 468 to 470 in b7f20fe

SITH_ECOCUP_CONS = 1152
SITH_ECOCUP_DECO = 1151

Il y a même un seuil maximal configuré entre la différence consigne/déconsigne :

sith3/sith/settings.py

Lines 472 to 473 in b7f20fe

# The limit is the maximum difference between cons and deco possible for a customer
SITH_ECOCUP_LIMIT = 3

Donc cela me semble être particulier comme comportement :/

En ce qui concerne le pichet par contre ouais c'est pas du tout dans la config.

Edit : Là j'ai mentionné le fichier de config de base, mais la config ne diffère pas en production, après vérification.

@imperosol
Copy link
Contributor

Ok, j'ai un peu enquêté, et en effet, il s'agit d'un exploit assez énervé dû à un enchainement de fonctions assez mal codées et très peu maintenables.

Plus précisément, le bug date du 15 août 2017 et a été introduit... par Sli.
Ca tient au fait très simple que si on déconsigne plusieurs gobelets en même temps (13XDCONS), il y a un problème de calcul dans la fonction qui regarde si on a le droit de déconsigner, puisque celle-ci considèrera toujours qu'on en déconsigne un seul à la fois.

C'est un bug assez énervé, mais ça reste "pas trop grave" dans la mesure où il est resté là 6 ans sans qu'on le voie et que seuls les barmen y ont accès (et que ceux-ci sont censés savoir qu'on ne peut pas déconsigner plus de trois ecocups).
C'est à résoudre, bien sûr, mais on n'est pas sur une urgence absolue.

Pour ce qui est du temps que ça peut prendre à réparer, c'est dur à dire. Ca peut prendre une heure comme ça peut prendre des semaines. Tout ce qui touche aux clics tient dans une seule vue monstrueuse avec 1000 fois trop de responsabilités ; donc il ne faut pas négliger la possibilité que la résolution du bug oblige à modifier plus de choses dans le plat de spaghettis.

@klmp200
Copy link
Contributor

klmp200 commented Mar 8, 2023

J'aurais du mettre des tests 😩

@TheoDurr
Copy link
Member

TheoDurr commented Mar 8, 2023

On sait quoi faire maintenant du coup ! Ecrire le petit test en priorité pour s'assurer que ça fonctionne et corriger tout ça ;).

On fera sûrement un déploiement dans le mois pour fixer ça. On attendra pas avril

@imperosol
Copy link
Contributor

imperosol commented Mar 8, 2023

Je serais pas si optimiste. Même pour des bugs mineurs, c'est dur de savoir combien de temps ça prendra, parce qu'on peut avoir d'autres obligations en dehors du site ou que quelque chose d'autre arrive peu après qui prend la priorité. Pour ce bug là, comme je l'ai dit, c'est encore plus incertain, parce que ça se trouve dans une partie spaghetti.

@imperosol
Copy link
Contributor

On fera sûrement un déploiement dans le mois pour fixer ça. On attendra pas avril

Finalement, on a attendu août :)

@Juknum
Copy link
Member Author

Juknum commented Aug 16, 2023

C'est sûr que dire ça sa fait avancer le schmilblick :)

@Juknum Juknum modified the milestones: Qualifications Mai 2023, Qualifications Octobre 2023 Sep 6, 2023
@Juknum
Copy link
Member Author

Juknum commented Jan 19, 2024

On a trouvé ça : si la personne a déjà déconsignée >= 3 ecocup elle peut plus en deconsigner davantage mais rien n'empeche quelqu'un de deconsigner + de 3 ecocup d'un coup.

Du coup pour contourner la limite on peut reconsigner des ecocups (jusqu'à ce que l'utilisateur soit au dessus de -3) et déconsigner derrière (autant qu'on veut : le bug)

@imperosol imperosol removed this from the Qualifications 2023/10 milestone Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s: counters severity: critical The issue causes data loss, crashes or makes the system unresponsive, etc
Projects
Status: Todo
Development

No branches or pull requests

4 participants