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

Incorrect computation of cooperator_type ? #345

Closed
victor-champonnois opened this issue Jul 13, 2022 · 11 comments
Closed

Incorrect computation of cooperator_type ? #345

victor-champonnois opened this issue Jul 13, 2022 · 11 comments

Comments

@victor-champonnois
Copy link
Member

In cooperator, the field cooperator_type is computed in the following manner:

    @api.multi
    @api.depends(
        "share_ids",
        "share_ids.share_product_id",
        "share_ids.share_product_id.default_code",
        "share_ids.share_number",
    )
    def _compute_cooperator_type(self):
        for partner in self:
            share_type = ""
            for line in partner.share_ids:
                if line.share_number > 0:
                    share_type = line.share_product_id.default_code
                    break
            partner.cooperator_type = share_type

This will set cooperator_type to the first share_type in the share_line table. In the case when a cooperator has several share types, the first one will arbitrarily be chosen.
It is technically possible for a cooperator to have several share types (no constraints exists), so this could be an issue.

@robinkeunen
Copy link
Member

Until we find a foolproof way of doing this (needs to check the analysts), fetching the share created last could be a good heuristic.

@enricostano
Copy link
Contributor

Hi @robinkeunen and @victor-champonnois !

We're migrating right now a quite big cooperative (6k members) and we face this specific issue since it is a multi stakeholder coopderative and the same member could pertain to different membership "categories" at the same time.

Translated to EMC... ehm, sorry... Cooperator language, it means that a single member can subscribe multiple share types at the same time.

This leads to the issue @victor-champonnois is pointing out. Actually, the whole Cooperators "list" should be re-designed to cover this use case IMHO.

What do you think?

@enricostano
Copy link
Contributor

@robinkeunen
Copy link
Member

Hi @enricostano

I agree we could do better with the cooperator list. One way of doing so is to have something similar to what we did in investor-wallet-platform to allow multiple memberships. We could also get inspiration from oca/vertical-association.

In both case, the modules add a "membership" model separate from the partner.

@robinkeunen
Copy link
Member

Audit cooperator_type

cooperator

The field is defined on res.partner here :

cooperator_type = fields.Selection(
selection="_get_share_type",
compute=_compute_cooperator_type,
string="Cooperator Type",
store=True,
)

def _compute_cooperator_type(self):
for partner in self:
share_type = ""
for line in partner.share_ids:
if line.share_number > 0:
share_type = line.share_product_id.default_code
break
partner.cooperator_type = share_type

Issues:

  • can't handle several share types of different types (takes the first share type)
  • no unicity on default_code Bug with cooperator_type #318
  • no enough doc on how default code is used
  • bad variable naming

How should we handle multiple share types ?

  • forbid several share types ? cf existing unmix_share_type parameter (only through operation requests)
  • sequence on share types allows to set a hierarchy of share type ?
  • allow to force share type manually ?
  • last created share ?

cooperator_website

Prevents creating several shares of different types for a partner through the online form.

beesdoo_easy_my_coop

Will be split into

  • cooperator_worker_information
  • cooperator_eater_configuration

cf beescoop/Obeesdoo#427

The field definition is overridden in this way

    cooperator_type = fields.Selection(
        [
            ("share_a", "Share A"),
            ("share_b", "Share B"),
            ("share_c", "Share C"),
        ],
        store=True,
        compute=None,
    )

source

From the share type, also computes

  • partner.is_worker
  • partner.can_shop
  • max number of eaters

Issues:

  • breaks if other share type are defined
  • hardcoded but no use of share string litterals in code ("share_a")

Also see #318

@robinkeunen
Copy link
Member

Décision :

=> ajouter un champs séquence / hierarchie (pas de ir.sequence) sur les types de part
=> requis si le produit est une part
=> attention initialisation existant vs nouvelle base

@enricostano
Copy link
Contributor

@robinkeunen would be great to have a small chat about this changes, would you mind a call next week? Thanks!

@robinkeunen
Copy link
Member

Hey @enricostano, sure. I'm available monday-tuesday-wednesday mornings.

@robinkeunen robinkeunen changed the title 12.0 - Incorrect computation of cooperator_type ? Incorrect computation of cooperator_type ? Sep 6, 2022
@robinkeunen
Copy link
Member

robinkeunen commented Sep 6, 2022

@enricostano I need to have this conversation soon, I'll be working on this this week or the next.

@enricostano
Copy link
Contributor

@robinkeunen sorry, I missed your comment. Would you mind a call talking about this? Thanks!

@victor-champonnois
Copy link
Member Author

closed in favor of OCA/cooperative#18

Repository owner moved this from Todo to Done in Moving Easy My Coop to OCA (Komunigi) Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants