Simplification des champs customer, allow_shopping et can_shop

[victor-champonnois]

Simplification des champs customer, allow_shopping et can_shop

Les champs customer, allow_shopping et can_shop semblent vouloir indiquer la même chose. Cette issue propose une désambiguïsation de ces termes, afin de proposer une simplification.

note : cette analyse à été faite à partir de la branche split_beesdoo_emc

customer

• définit sur un produit
• Le champs customer est aussi définit sur les partners à partir du champs customer de la part qu'il souscrit : (voir ici et ici)

allow_shopping

• définit sur un produit
  

  Obeesdoo/cooperator_worker/models/product_template.py

  Lines 14 to 17 in ce3db9c

  	allow_shopping = fields.Boolean(
  	string="Allow owner to shop?",
  	help="Owner of this type of share are allowed to shop.",
  	)

Le champs n'est impliqué dans aucun calcul. Quelle est l'utilité de ce champs ? A quel moment est il enforcé pour interdire d'acheter ?

can_shop

Obeesdoo/beesdoo_shift/models/cooperative_status.py

Line 103 in 1848e7f

can_shop = fields.Boolean(compute="_compute_can_shop", store=True)

• champs dynamique calculés selon le statut des shifts du worker.

Proposition

• Les champs customer et allow_shopping semblent servir à la même chose.
• Utiliser le champs customer plutôt que allow_shopping ?

Problèmes potentiels

• customer est utilisé pour définir l'apparition de l'onglet Eater. Si on utilise la logique de allow_shopping pour le champs customer, est-ce que cela pose problème.

• Il faut s'assurer que la logique des eater puisse fonctionner même si cooperator n'est pas installé (et que customer n'est pas définit via la part)

• customer est utilisé pour masquer l'onglet eater. Un coopérateur qui a une part qui ne permet pas de consommer, ne peut donc pas avoir de eaters ? (cela semble logique)

• note : une donnée de démo a customer=True et allow_shopping=False.

Avantages

• simplification du code (plus de champs allow_shopping)
• can_shop est un champs dynamique (déterminé par le statut des shifts) alors que allow_shopping est lié à la part. Le renommage de allow_shopping en customer rend cette différence plus claire.

[robinkeunen]

@polchampion @remytms

[polchampion]

J'ajouterais une donnée : si je ne m'abuse, en v14 le champ customer sur les res.partner devient un champ calculé.

[polchampion]

• Pour customer : étant donné que le champ existe sur les res.partner en standard, il faut faire un choix de créer les coopérateurices customer = True ou customer = False donc c'est assez natuel d'avoir introduit ce champ sur les parts.

• Pour allow_shopping : Dans un scénario où les coops part A et B ont le droit de faire leurs courses mais les coops part C et D, non. Si les modules des shifts sont installés, osef parce que c'est la logique des shifts et le can_shop qui font la loi. Mais si les modules des shifts ne sont pas installés, alors ce champ, s'il était transféré aux partner, pourrait indiquer si une personne est éligible ou pas.

[polchampion]

Mon avis :

• Je ne vois pas de vraie raison fonctionnelle qui justifie la présence du champ customer sur les parts. Je ne connais aucune structure où une part ne donne pas droit à être customer et où cela peut avoir un impact (les personnes ne gagnent rien à être customer = True). Si les coopérateurices sont créé·es par défaut customer = True, alors on peut retirer ce champ des parts.

• Si on voulait faire des améliorations, on pourrait étendre la fonctionnalité can_shop des shifts et l'introduire dans Cooperators : si les shifts ne sont pas installés, can_shop = allow_shopping ; si les shifts sont installés, can_shop est calculé comme actuellement. Dans ce cas on garderait allow_shopping. Je ne le renommerais pas car c'est une logique parallèle à allow_working sur les parts. J'imagine que allow_working est utilisé dans les shifts, pour rendre is_worker = True sur les coopérateurices.

• Le champ customer ne devrait pas être conditionné ou utilisé pour conditionner de nouvelles choses, s'il est effectivement devenu un champ calculé dans les versions ultérieures. Pour l'onglet mangeurs, ça demandera peut-être des adaptations lors de la migration en v16. En fait, l'affichage de l'onglet mangeurs devrait dépendre du champ max_nb_eater_allowed.

• Si allow_shopping ne sert à rien (c'est à dire en l'état), et qu'on ne fait pas l'amélioration, on peut supprimer ce champ.

[polchampion]

@robinkeunen @victor-champonnois ma review ci-dessus

[victor-champonnois]

@polchampion

J'ajouterais une donnée : si je ne m'abuse, en v14 le champ customer sur les res.partner devient un champ calculé.

Je n'arrive pas à trouver l'info, j'ai l'impression que le champs customer "disparaît" de res.partner en 14. Tu as une ref ?

Si on voulait faire des améliorations, on pourrait étendre la fonctionnalité can_shop des shifts et l'introduire dans Cooperators : si les shifts ne sont pas installés, can_shop = allow_shopping ; si les shifts sont installés, can_shop est calculé comme actuellement.

Je ne suis pas fan de cette idée, car la logique de allow_shopping est spécifique aux supermarchés, cooperator n'a pas de raison d'être lié à des logiques d'achats à mon avis.

Pour allow_shopping : Dans un scénario où les coops part A et B ont le droit de faire leurs courses mais les coops part C et D, non. Si les modules des shifts sont installés, osef parce que c'est la logique des shifts et le can_shop qui font la loi. Mais si les modules des shifts ne sont pas installés, alors ce champ, s'il était transféré aux partner, pourrait indiquer si une personne est éligible ou pas.

OK, bon point. Mais est-ce que ce dernier cas est déjà arrivé ? Et ne devrait-il pas être pris en charge dans un module à part, qui l'enforcerai dans le POS ? Conserver le champs allow_shopping dans l'éventualité où il pourrait servir ne me semble pas être une bonne raison. Je suis donc d'accord avec toi qu'on peut le supprimer s'il ne sert à rien.

Le champ customer ne devrait pas être conditionné ou utilisé pour conditionner de nouvelles choses, s'il est effectivement devenu un champ calculé dans les versions ultérieures. Pour l'onglet mangeurs, ça demandera peut-être des adaptations lors de la migration en v16. En fait, l'affichage de l'onglet mangeurs devrait dépendre du champ max_nb_eater_allowed.

OK, c'est bon à savoir

[polchampion]

@victor-champonnois https://www.odoo.com/fr_FR/forum/aide-1/where-are-is-customer-is-vendor-checkboxes-in-odoo-13-contacts-157566

D'accord avec toi sur les autres points.

[victor-champonnois]

Ah OK, en 14 customer n'est plus définit dans Odoo base, mais il y a is_customer dans OCA partner-manual-rank , calculé à partir du champs customer_rank définit dans Odoo account.

[polchampion]

Je ne suis pas fan de cette idée, car la logique de allow_shopping est spécifique aux supermarchés, cooperator n'a pas de raison d'être lié à des logiques d'achats à mon avis.

@victor-champonnois En testant cette PR, j'ai l'impression qu'il y a déjà un lien entre can_shop et cooperator : can_shop est cochée quand allow_working est coché sur le type de parts.

Problème : Quand une personne devient coopératrice, tant que son régime de travail n'est pas défini, can_shop est vrai.

To be : quand les shifts sont installés, can_shop devrait seulement dépendre du statut.
Test : Si les shifts sont installés, si pas de statut défini, can_shop = False

Cette amélioration sort peut-être du cadre de Komunigi.

[victor-champonnois]

Si on voulait faire des améliorations, on pourrait étendre la fonctionnalité can_shop des shifts et l'introduire dans Cooperators : si les shifts ne sont pas installés, can_shop = allow_shopping ; si les shifts sont installés, can_shop est calculé comme actuellement.

@victor-champonnois En testant cette PR, j'ai l'impression qu'il y a déjà un lien entre can_shop et cooperator : can_shop est cochée quand allow_working est coché sur le type de parts.

Oui exact, en fait je pensait que tu parlais d'intégrer allow_working dans le module cooperator, maintenant je comprends que tu propose de l'intégrer dans feu beesdoo_easy_my_coop. allow_working bien est impliqué dans le calcul de can_shop (cf

Obeesdoo/cooperator_worker/models/res_partner.py

Line 84 in dbe4389

def _compute_can_shop(self):

)

[victor-champonnois]

Récap de la discussion:

le champs customer change entre v12 et v14.

https://www.odoo.com/fr_FR/forum/aide-1/where-are-is-customer-is-vendor-checkboxes-in-odoo-13-contacts-157566

en 14 customer n'est plus définit dans Odoo base, mais il y a is_customer dans OCA partner-manual-rank , calculé à partir du champs customer_rank définit dans Odoo account.

Il faudra donc rediscuter de cette simplification lors du portage en 14.

allow_shopping ne sert pas à rien

Il est impliqué dans le calcul de can_shop. On ne peut donc pas le supprimer actuellement, et comme vu plus haut, on ne peut pas simplement le remplacer par customer sans faire une analyse du nouveau champs customer en 14.

proposition d'amélioration de @polchampion

Si on voulait faire des améliorations, on pourrait étendre la fonctionnalité can_shop des shifts et l'introduire dans cooperator_worker : si les shifts ne sont pas installés, can_shop = allow_shopping ; si les shifts sont installés, can_shop est calculé comme actuellement.