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

[FIX] sale: avoid crash when posting move linked to multiple SO #110560

Conversation

madprog
Copy link
Contributor

@madprog madprog commented Jan 20, 2023

Because of the M2M relation, an account.move.line can be linked to several sale.order.lines, making the cardinality of sale_line_ids superior to 1.
Even if it is not possible to obtain this result with standard code, a custom module could create this situation and posting the invoice would result in a "ValueError: Expected singleton" exception.

This commit prevents this exception by checking whether the line is linked to at least one downpayment SO line in the method action_post. The test also checks that the methods button_draft and button_cancel
behave properly (spoiler alert: they will fail too in 16.0).

Closes #77195
Fixes #77028

Because of the M2M relation, an account.move.line can be linked to
several sale.order.lines, making the cardinality of sale_line_ids
superior to 1.
Even if it is not possible to obtain this result with standard code,
a custom module could create this situation and posting the invoice
would result in a "ValueError: Expected singleton" exception.

This commit prevents this exception by checking whether the line is
linked to at least one downpayment SO line in the method action_post.
The test also checks that the methods button_draft and button_cancel
behave properly (spoiler alert: they will fail too in 16.0).
@madprog madprog requested a review from arj-odoo January 20, 2023 13:32
@robodoo
Copy link
Contributor

robodoo commented Jan 20, 2023

@C3POdoo C3POdoo requested a review from a team January 20, 2023 13:39
@C3POdoo C3POdoo added the RD research & development, internal work label Jan 20, 2023
@arj-odoo
Copy link
Contributor

@robodoo delegate+

@madprog
Copy link
Contributor Author

madprog commented Jan 20, 2023

@Feyensv Thanks for the approval!
I see that you have linked other issues and PRs that reported the same problem, and one of them proposed a fix that was introducing and using a method named _is_downpayment that was doing the same as if I was using an all instead of any.
Do you think that an all would be better? I am not 100% sure of the meaning of having some linked lines that are down payments and some that aren't.

@Feyensv
Copy link
Contributor

Feyensv commented Jan 20, 2023

Well, since there is no standard flow to have multiple linked lines, even less downpayment ones, I have no idea if we want an all or an any 🤔.
Furthermore, the modified flow (hooked in the action_post of the AM) is an old flow I don't know much about, so for me right now, both solutions are equivalent.

@madprog
Copy link
Contributor Author

madprog commented Jan 20, 2023

@robodoo r+

robodoo pushed a commit that referenced this pull request Jan 20, 2023
Because of the M2M relation, an account.move.line can be linked to
several sale.order.lines, making the cardinality of sale_line_ids
superior to 1.
Even if it is not possible to obtain this result with standard code,
a custom module could create this situation and posting the invoice
would result in a "ValueError: Expected singleton" exception.

This commit prevents this exception by checking whether the line is
linked to at least one downpayment SO line in the method action_post.
The test also checks that the methods button_draft and button_cancel
behave properly (spoiler alert: they will fail too in 16.0).

closes #110560

Signed-off-by: Paul Morelle (pmo) <pmo@odoo.com>
@robodoo robodoo temporarily deployed to merge January 20, 2023 18:21 Inactive
@robodoo robodoo closed this Jan 20, 2023
@fw-bot
Copy link
Contributor

fw-bot commented Jan 24, 2023

@fw-bot
Copy link
Contributor

fw-bot commented Jan 25, 2023

@madprog this pull request has forward-port PRs awaiting action (not merged or closed):
#110633

1 similar comment
@fw-bot
Copy link
Contributor

fw-bot commented Jan 26, 2023

@madprog this pull request has forward-port PRs awaiting action (not merged or closed):
#110633

OSevangelist pushed a commit to OSevangelist/odoo that referenced this pull request Jan 26, 2023
Because of the M2M relation, an account.move.line can be linked to
several sale.order.lines, making the cardinality of sale_line_ids
superior to 1.
Even if it is not possible to obtain this result with standard code,
a custom module could create this situation and posting the invoice
would result in a "ValueError: Expected singleton" exception.

This commit prevents this exception by checking whether the line is
linked to at least one downpayment SO line in the method action_post.
The test also checks that the methods button_draft and button_cancel
behave properly (spoiler alert: they will fail too in 16.0).

closes odoo#110560

Signed-off-by: Paul Morelle (pmo) <pmo@odoo.com>
@fw-bot
Copy link
Contributor

fw-bot commented Jan 27, 2023

@madprog this pull request has forward-port PRs awaiting action (not merged or closed):
#110633

3 similar comments
@fw-bot
Copy link
Contributor

fw-bot commented Jan 28, 2023

@madprog this pull request has forward-port PRs awaiting action (not merged or closed):
#110633

@fw-bot
Copy link
Contributor

fw-bot commented Jan 29, 2023

@madprog this pull request has forward-port PRs awaiting action (not merged or closed):
#110633

@fw-bot
Copy link
Contributor

fw-bot commented Jan 30, 2023

@madprog this pull request has forward-port PRs awaiting action (not merged or closed):
#110633

@madprog madprog deleted the 14.0-avoid_crash_multiple_so_per_invoice-pmo branch January 31, 2023 08:19
@fw-bot
Copy link
Contributor

fw-bot commented Feb 1, 2023

@madprog this pull request has forward-port PRs awaiting action (not merged or closed):
#111366

jdoutreloux pushed a commit to acsone/odoo that referenced this pull request Jul 5, 2023
Because of the M2M relation, an account.move.line can be linked to
several sale.order.lines, making the cardinality of sale_line_ids
superior to 1.
Even if it is not possible to obtain this result with standard code,
a custom module could create this situation and posting the invoice
would result in a "ValueError: Expected singleton" exception.

This commit prevents this exception by checking whether the line is
linked to at least one downpayment SO line in the method action_post.
The test also checks that the methods button_draft and button_cancel
behave properly (spoiler alert: they will fail too in 16.0).

closes odoo#110560

Signed-off-by: Paul Morelle (pmo) <pmo@odoo.com>
jdoutreloux pushed a commit to acsone/odoo that referenced this pull request Aug 10, 2023
Because of the M2M relation, an account.move.line can be linked to
several sale.order.lines, making the cardinality of sale_line_ids
superior to 1.
Even if it is not possible to obtain this result with standard code,
a custom module could create this situation and posting the invoice
would result in a "ValueError: Expected singleton" exception.

This commit prevents this exception by checking whether the line is
linked to at least one downpayment SO line in the method action_post.
The test also checks that the methods button_draft and button_cancel
behave properly (spoiler alert: they will fail too in 16.0).

closes odoo#110560

Signed-off-by: Paul Morelle (pmo) <pmo@odoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants