[12.0][FIX] cooperator_eater: don't send mail if validation isn't succesfull#571
Conversation
mihien
changed the title
mihien
force-pushed
the
12.0-fix-no_mail_subscription_worker_with_parent
branch
from
42a7e09 to
f921a34
Compare
| # the subscription and writing the partner as worker in db | ||
| partner = self.partner_id | ||
| if partner: | ||
| setattr(partner, "eater", "worker_eater") |
There was a problem hiding this comment.
Why not using partner.eater = "worker_eater" ?
There was a problem hiding this comment.
If I understood/remember everything correctly, the problem was that partner.eater = "worker_eater" makes the changes directly in the db (and maybe trigger onchange behaviours, but I'm not sure anymore if that's not the case with setattr as well), which we don't want to happen before validating the subscription request. Using setattr willonly change the value in the current object and will raise an error if the new value is impossible, without doing any actual change in db
| setattr(partner, "eater", "worker_eater") | ||
|
|
||
| invoice = super().validate_subscription_request()[0] | ||
| partner = invoice.partner_id |
There was a problem hiding this comment.
Don’t remove this line. If self.partner_id is None then the invoice partner is another one (a found one or a newly created one).
Look here.
There was a problem hiding this comment.
Oops yes that's my bad, I don't think that one was intended
| # ensure partner can be changed to be a worker before both validating | ||
| # the subscription and writing the partner as worker in db | ||
| partner = self.partner_id | ||
| if partner: | ||
| setattr(partner, "eater", "worker_eater") | ||
|
|
||
| invoice = super().validate_subscription_request()[0] | ||
| partner = invoice.partner_id | ||
|
|
||
| vals = self.get_eater_vals(partner, self.share_product_id) | ||
| partner.write(vals) |
There was a problem hiding this comment.
I think that this is not correct. We should look more deeply into the parent method.
self.partner_id is used only if not None.
As far as I know, the issue depends on the share product and the partner. The issue happen only if self.partner_id is not null and if the return of the method get_eater_vals() is "worker_eater" and self.partner_id.parent_eater_id is not null.
I would suggest something like that:
| # ensure partner can be changed to be a worker before both validating | |
| # the subscription and writing the partner as worker in db | |
| partner = self.partner_id | |
| if partner: | |
| setattr(partner, "eater", "worker_eater") | |
| invoice = super().validate_subscription_request()[0] | |
| partner = invoice.partner_id | |
| vals = self.get_eater_vals(partner, self.share_product_id) | |
| partner.write(vals) | |
| # if existing partner, ensure eater settings can be changed | |
| # before validating the subscription request. | |
| if self.partner_id: | |
| vals = self.get_eater_vals(partner, self.share_product_id) | |
| self.partner_id.write(vals) | |
| invoice = super().validate_subscription_request()[0] | |
| partner = invoice.partner_id | |
| if partner != self.partner_id: | |
| vals = self.get_eater_vals(partner, self.share_product_id) | |
| partner.write(vals) |
mihien
force-pushed
the
12.0-fix-no_mail_subscription_worker_with_parent
branch
from
f921a34 to
f1db7db
Compare
remytms
left a comment
remytms
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the changes.
|
/ocabot merge patch |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at 6f40b71. Thanks a lot for contributing to beescoop. ❤️ |
github-grap-bot
deleted the
12.0-fix-no_mail_subscription_worker_with_parent
branch
3 participants
15101