[16.0][IMP] cooperator: Don't send attachment if no remaining shares #3
Conversation
There was a problem hiding this comment.
the code seems correct, with some nice refactoring. thanks for this!
currently, all computation of share quantities are done directly on the partner. please note that res.partner.share_ids lists all share.line records linked to the partner, across all companies, which makes this code not multi-company-compatible. this would be a good chance to fix this. instead of using res.partner, use the explicit cooperative.membership linked to the company_id of the operation.request (using res.partner.get_cooperative_membership()).
cooperator/models/res_partner.py
Outdated
| @@ -283,3 +285,21 @@ def create_cooperative_membership(self, company_id): | |||
| } | |||
| ) | |||
| return result | |||
|
|
|||
| def get_share_quantities(self): | |||
There was a problem hiding this comment.
i think that this should better be defined on cooperative.membership, as this is different per company.
There was a problem hiding this comment.
I did this, but I couldn't really get around keeping a wrapper method in res.partner. This method was/is called from operation.request, which has a partner_id and a company_id, but not a cooperative_membership_id.
I could do something like self.partner_id.with_company(self.company_id).cooperative_membership_id.get_share_quantities(), but that seemed utterly absurd to me. Instead I do self.partner_id.get_share_quantities(self.company_id), which quietly does the latter for me.
huguesdk
force-pushed
the
16.0-mig-cooperator
branch
from
5f81e5a
to
291d05e
Compare
huguesdk
force-pushed
the
16.0-cooperator-no-shares-no-certificate
branch
from
b12ff69
to
1091e84
Compare
| def test_mail_template_share_transfer_all_shares(self): | ||
| """Test that executing a share transfer wherein all shares are | ||
| transferred sends a message with no certificate in attachment. | ||
| """ | ||
| self._test_mail_template_share_transfer_all_shares( | ||
| self.create_dummy_cooperator(), self.get_dummy_subscription_requests_vals() | ||
| ) | ||
|
|
There was a problem hiding this comment.
could you please add a test for a company cooperator?
huguesdk
force-pushed
the
16.0-mig-cooperator
branch
from
291d05e
to
98286eb
Compare
Signed-off-by: Carmen Bianca BAKKER <[email protected]>
Signed-off-by: Carmen Bianca BAKKER <[email protected]>
huguesdk
force-pushed
the
16.0-cooperator-no-shares-no-certificate
branch
from
0a4e1b5
to
4391d32
Compare
huguesdk
force-pushed
the
16.0-mig-cooperator
branch
from
98286eb
to
772320b
Compare
huguesdk
force-pushed
the
16.0-cooperator-no-shares-no-certificate
branch
from
4391d32
to
a01a374
Compare
Signed-off-by: Carmen Bianca BAKKER <[email protected]>
huguesdk
force-pushed
the
16.0-cooperator-no-shares-no-certificate
branch
from
a01a374
to
fd98a75
Compare
Ported from OCA#98
Internal task: https://gestion.coopiteasy.be/web#id=11178&action=479&model=project.task&view_type=form&menu_id=536