rework existing code in select_product_from_bc to simplify, fix a bug and meet…
rework existing code in select_product_from_bc to simplify, fix a bug and meet requirements of story 4820 + do previous qty preselection after user clic on Editer btn
-
-
Developer
Ta simplification de la fonction a complétement fait disparaitre la logique qui avait été codée en cas de scan d'un produit au poids (l.130 à 140 dans l'ancienne version du fichier)
Tu précises :
I think code implementing such logic should be triggered on btns clicks as well so we don't need it here
Mais du coup ce n'est implémenté nulle part ailleurs !
-
DeveloperEdited by Damien Moulard
Pour préciser, la logique était qu'en cas de bip d'un produit au poids, le produit est automatiquement édité avec le poids scanné
-
Master
Grosse bourde en effet.
-
Master
Compte tenu de ça je pense que le mieux est d'abandonner ma proposition.
-
-
-
Developer
in case article was already processed, edit line was opened but product was not removed from processed ones
Oui, cela dit il y avait un commentaire dans le code, l.152 dans l'ancienne version, qui indiquait que ce comportement était voulu, il faudrait donc le conserver...
which caused article to be added more than once after a user click on the cross
... et plutôt traiter cet effet de bord.
-
Master
Je pensais que le maintien du produit à droite était un bug (suite au spotage du vrai bug du clic sur la croix), je n'avais pas compris que c'était souhaité, malgré le commentaire. Du coup là soit tu sais ce qu'il faut faire soit il faut retravailler la spec.
-
Master
Ok pour essayer de corriger le bug seul.
-
-
125 product_id = e.product_id[0]; 126 return false; 115 127 } 116 128 }); 117 129 118 // Does the product come from processed ? 119 if (foundProduct.data == null) { 120 $.each(list_processed, function(i, e) { 121 if (e.product_id[0] == scannedProduct.data[barcodes['keys']['id']]) { 122 foundProduct.data = JSON.parse(JSON.stringify(e)); 123 foundProduct.data.product_qty = null; // Set qty to null from product already scanned 124 foundProduct.place = 'processed'; 125 } 126 }); 130 if(product_id !== null) { 131 $('#'+product_id+' > td > .toProcess_line_valid').click(); -
Developer
Un futur changement dans la structure HTML aurait pour cause que cette partie de code ne fonctionnerait plus si on oubliait de rapporter la modification ici.
La probabilité que ça arrive est certes faible, mais c'est une potentielle source d'erreur, d'autant plus si de nouvelles personnes viennent à travailler sur ce module !
-
Developer
De plus, pourquoi lancer la validation directe du produit ? L'idée était de l'envoyer à l'édition plutôt, non ?
-
Master
En effet. Et oui, la validation directe, ce n'est pas du tout ce qui est attendu...
-
-
119 if (foundProduct.data == null) { 120 $.each(list_processed, function(i, e) { 121 if (e.product_id[0] == scannedProduct.data[barcodes['keys']['id']]) { 122 foundProduct.data = JSON.parse(JSON.stringify(e)); 123 foundProduct.data.product_qty = null; // Set qty to null from product already scanned 124 foundProduct.place = 'processed'; 125 } 126 }); 130 if(product_id !== null) { 131 $('#'+product_id+' > td > .toProcess_line_valid').click(); 132 return; 127 133 } 128 134 129 if (foundProduct.data !== null) { 130 if (foundProduct.data.product_uom[0] == 21) { //if qty is in weight 131 if (scannedProduct.rule === 'weight') { -
138 setupPopUpBtnStyle(scannedProduct); 139 } 140 } 141 142 if (scannedProduct.rule !== 'price_to_weight') { 143 if (foundProduct.data.product_uom[0] != 21) { 144 setLineEdition(foundProduct.data); 145 } 146 147 if (foundProduct.place === 'to_process') { 148 let row = table_to_process.row($('#'+foundProduct.data.product_id[0])); 135 var product_qty = null; 149 136 150 remove_from_toProcess(row, foundProduct.data); 151 } 152 // Don't remove product from processed list -
DeveloperEdited by Damien Moulard
Ici le commentaire qui justifiait que ne pas enlever le produit du tableau de droite était un comportement désiré (en revanche je suis d'accord qu'un commentaire plus précis précisant pourquoi aurait été judicieux !)
-
-
-
Developer
Peux-tu préciser pourquoi tu as voulu autant modifier cette fonction ? Est-ce que c'était dans le but de simplifier le code ?
Je ne suis pas contre en soi, mais je vois comme soucis que cette simplification a créé des effets de bord alors que le précédent code n'était pas dysfonctionnel (hormis le bug de la croix)
-
Master
A la base je comprenais pas pourquoi un truc qui était spécifié dans un ticket avait l'air d'être déjà codé, mais avec quelques petites différences quand même (dans les messages d'alerte par exemple). Ensuite j'ai essayé de comprendre le code mais j'ai buté sur la partie "uom = 21" (qui m'avait l'air très intriquée avec le reste) que je n'arrivais pas à connecter avec la spec qui n'aborde pas le sujet du comportement attendu en cas de poids dans le code barre. Le bug de la croix m'a ensuite conforté dans l'idée que ce code n'était pas utilisé. Finalement je me suis dit que je voyais comment faire pour implémenter la spec et donc j'y suis allé. Mauvaise idée !
-
-
closed
Toggle commit list
