Skip to content

[FIX] sale_fixed_discount: fix fixed discount on API-created order lines#3258

Open
sefirosweb wants to merge 7 commits intoOCA:16.0from
sefirosweb:16.0
Open

[FIX] sale_fixed_discount: fix fixed discount on API-created order lines#3258
sefirosweb wants to merge 7 commits intoOCA:16.0from
sefirosweb:16.0

Conversation

@sefirosweb
Copy link
Copy Markdown

This fix when you want to create order via API and you want use discount_fixed instead "discount" rate

Without that the sales_order_line is created with discount_fixed but discount is 0

image

Copy link
Copy Markdown
Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sefirosweb Could you fix this?

Moreover, adding a test case for that flow should be great.

@sefirosweb
Copy link
Copy Markdown
Author

@rousseldenis I saw a bit bug when is creating the records i fixed them

@rousseldenis
Copy link
Copy Markdown
Contributor

/ocabot rebase

sefirosweb and others added 5 commits May 30, 2025 09:56
This fix when you want to create order via API and you want use discount_fixed instead "discount" rate

Without that the sales_order_line is created with discount_fixed  but discount is 0
Co-authored-by: Denis Roussel (ACSONE) <rousseldenis@users.noreply.github.com>
Co-authored-by: Denis Roussel (ACSONE) <rousseldenis@users.noreply.github.com>
@OCA-git-bot
Copy link
Copy Markdown
Contributor

Congratulations, PR rebased to 16.0.

@rousseldenis
Copy link
Copy Markdown
Contributor

@sefirosweb Could you improve your commits messages like [IMP] sale_fixed_discount: (...) and fix pre-commit ?

@rousseldenis
Copy link
Copy Markdown
Contributor

@sefirosweb Do you plan to finish this ?

@sefirosweb
Copy link
Copy Markdown
Author

OK!

@sefirosweb sefirosweb changed the title Fix issue when order is created by api [FIX] sale_fixed_discount: fix fixed discount on API-created order lines Feb 2, 2026
Copy link
Copy Markdown

@alexey-pelykh alexey-pelykh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @sefirosweb, the core idea of overriding _compute_discount is the right approach here.

A few things still need attention before this can be merged:

  1. Remove _onchange_discount_fixed -- as @rousseldenis mentioned in the earlier inline comment, once _compute_discount is in place the onchange is redundant. The compute method fires in both UI and API contexts, so the onchange should be dropped entirely.

  2. Test indentation -- the body of test_05_discount_fixed_when_created_by_api is indented with 12 spaces instead of the expected 8 (4 for class + 4 for method). Please align with the rest of the test file.

  3. Missing newline at end of file -- the test file is missing a trailing newline, which will also trip pre-commit.

  4. Reuse existing fixtures -- the new test creates its own res.partner and product.product when self.partner and self.product (or self.product2) from setUpClass could be reused. Keeps tests consistent and leaner.

  5. Commit messages -- @rousseldenis already flagged this: commits should follow OCA format, e.g. [FIX] sale_fixed_discount: .... The intermediate commits should be squashed into a single clean commit.

Points 1-4 are likely what's causing the CI failures. Should be straightforward to fix up.

@rousseldenis
Copy link
Copy Markdown
Contributor

Thanks for the fix @sefirosweb, the core idea of overriding _compute_discount is the right approach here.

A few things still need attention before this can be merged:

  1. Remove _onchange_discount_fixed -- as @rousseldenis mentioned in the earlier inline comment, once _compute_discount is in place the onchange is redundant. The compute method fires in both UI and API contexts, so the onchange should be dropped entirely.
  2. Test indentation -- the body of test_05_discount_fixed_when_created_by_api is indented with 12 spaces instead of the expected 8 (4 for class + 4 for method). Please align with the rest of the test file.
  3. Missing newline at end of file -- the test file is missing a trailing newline, which will also trip pre-commit.
  4. Reuse existing fixtures -- the new test creates its own res.partner and product.product when self.partner and self.product (or self.product2) from setUpClass could be reused. Keeps tests consistent and leaner.
  5. Commit messages -- @rousseldenis already flagged this: commits should follow OCA format, e.g. [FIX] sale_fixed_discount: .... The intermediate commits should be squashed into a single clean commit.

Points 1-4 are likely what's causing the CI failures. Should be straightforward to fix up.

@alexey-pelykh Comments for specific changes should be done in PR's code line level. Please avoid such reviews. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants