[16.0][ADD] sale_order_line_discount_switch: add module#10
Conversation
WalkthroughThis PR introduces a new Odoo module Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sale_order_line_discount_switch/models/sale_order_line.py`:
- Line 80: Replace the confusing conditional "if not line.discount_mode ==
'fixed':" with the clearer inequality operator by changing the condition to use
"!="; locate the check on line.discount_mode in the sale order line logic (the
conditional that branches on discount_mode) and update it to "if
line.discount_mode != 'fixed':" so readability and static-analysis warnings are
resolved.
- Around line 26-27: Summary: The fallback for rounding is incorrect because
when prec is 0 the current expression yields 1.0 instead of using the intended
fallback. Fix: change the logic around precision_get and rounding (the lines
using precision_get("Product Price") storing to prec and the subsequent rounding
calculation) to explicitly treat None (or missing) as the fallback case; e.g.
call precision_get into prec and then set rounding = 10 ** -prec when prec is
not None, otherwise use the fallback 0.0001. Ensure you only treat None/missing
as the fallback so a valid prec == 0 produces the correct 10**-0 result (1.0) if
that is intended, or if you want 0.0001 for zero decimals, explicitly check prec
== 0 and choose the fallback accordingly.
- Around line 82-84: Replace the hardcoded tolerance 1e-10 with the project's
decimal precision: retrieve the "Product Price" precision via
self.env['decimal.precision'].precision_get('Product Price') and pass it to
float_is_zero using the precision_digits argument (e.g.,
precision_digits=product_price_digits) for both checks of line.price_unit and
line.discount; update the call(s) at the float_is_zero(...) site(s) in
sale_order_line.py so they use that retrieved precision for consistency with
other uses in the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8d9581b7-ba69-4c8b-91a8-4ec369253105
⛔ Files ignored due to path filters (1)
sale_order_line_discount_switch/static/description/icon.pngis excluded by!**/*.png
📒 Files selected for processing (19)
README.mdsale_order_line_discount_switch/README.rstsale_order_line_discount_switch/__init__.pysale_order_line_discount_switch/__manifest__.pysale_order_line_discount_switch/hooks.pysale_order_line_discount_switch/models/__init__.pysale_order_line_discount_switch/models/sale_order_line.pysale_order_line_discount_switch/readme/CONFIGURE.rstsale_order_line_discount_switch/readme/CONTEXT.rstsale_order_line_discount_switch/readme/CONTRIBUTORS.rstsale_order_line_discount_switch/readme/DESCRIPTION.rstsale_order_line_discount_switch/readme/USAGE.rstsale_order_line_discount_switch/static/description/index.htmlsale_order_line_discount_switch/tests/__init__.pysale_order_line_discount_switch/tests/test_sale_order_line_discount_switch.pysale_order_line_discount_switch/views/sale_order_views.xmlsetup/_metapackage/setup.pysetup/sale_order_line_discount_switch/odoo/addons/sale_order_line_discount_switchsetup/sale_order_line_discount_switch/setup.py
| prec = self.env["decimal.precision"].precision_get("Product Price") | ||
| rounding = 10**-prec if prec else 0.0001 |
There was a problem hiding this comment.
Bug: Incorrect fallback when precision is zero.
When prec is 0, the expression 10**-prec evaluates to 10**0 = 1.0, not 0.0001. The condition if prec else only triggers when prec is falsy (i.e., None or 0), but precision_get returns 0 for zero decimal places, making the else branch unreachable for that case.
🐛 Proposed fix
- rounding = 10**-prec if prec else 0.0001
+ rounding = 10**-prec if prec is not None else 0.0001Or more defensively, handle zero precision explicitly:
- rounding = 10**-prec if prec else 0.0001
+ rounding = 10**-prec if prec and prec > 0 else 0.0001🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sale_order_line_discount_switch/models/sale_order_line.py` around lines 26 -
27, Summary: The fallback for rounding is incorrect because when prec is 0 the
current expression yields 1.0 instead of using the intended fallback. Fix:
change the logic around precision_get and rounding (the lines using
precision_get("Product Price") storing to prec and the subsequent rounding
calculation) to explicitly treat None (or missing) as the fallback case; e.g.
call precision_get into prec and then set rounding = 10 ** -prec when prec is
not None, otherwise use the fallback 0.0001. Ensure you only treat None/missing
as the fallback so a valid prec == 0 produces the correct 10**-0 result (1.0) if
that is intended, or if you want 0.0001 for zero decimals, explicitly check prec
== 0 and choose the fallback accordingly.
| ) | ||
| res = super().write(vals) | ||
| for line in lines_to_fill_fixed: | ||
| if not line.discount_mode == "fixed": |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use != instead of not ... == for clarity.
Static analysis correctly flags this as less readable.
♻️ Proposed fix
- if not line.discount_mode == "fixed":
+ if line.discount_mode != "fixed":📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if not line.discount_mode == "fixed": | |
| if line.discount_mode != "fixed": |
🧰 Tools
🪛 Ruff (0.15.10)
[warning] 80-80: Use line.discount_mode != "fixed" instead of not line.discount_mode == "fixed"
Replace with != operator
(SIM201)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sale_order_line_discount_switch/models/sale_order_line.py` at line 80,
Replace the confusing conditional "if not line.discount_mode == 'fixed':" with
the clearer inequality operator by changing the condition to use "!="; locate
the check on line.discount_mode in the sale order line logic (the conditional
that branches on discount_mode) and update it to "if line.discount_mode !=
'fixed':" so readability and static-analysis warnings are resolved.
| if float_is_zero( | ||
| line.price_unit, precision_rounding=1e-10 | ||
| ) or float_is_zero(line.discount, precision_rounding=1e-10): |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using consistent precision handling.
The hardcoded 1e-10 differs from the decimal precision approach used elsewhere in this file (lines 26-27, 48, 86). For consistency, consider using the "Product Price" precision or at minimum documenting why a different tolerance is needed here.
♻️ Proposed fix for consistency
+ prec = self.env["decimal.precision"].precision_get("Product Price")
+ rounding = 10**-prec if prec and prec > 0 else 0.0001
for line in lines_to_fill_fixed:
if line.discount_mode != "fixed":
continue
- if float_is_zero(
- line.price_unit, precision_rounding=1e-10
- ) or float_is_zero(line.discount, precision_rounding=1e-10):
+ if float_is_zero(
+ line.price_unit, precision_rounding=rounding
+ ) or float_is_zero(line.discount, precision_rounding=rounding):
continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sale_order_line_discount_switch/models/sale_order_line.py` around lines 82 -
84, Replace the hardcoded tolerance 1e-10 with the project's decimal precision:
retrieve the "Product Price" precision via
self.env['decimal.precision'].precision_get('Product Price') and pass it to
float_is_zero using the precision_digits argument (e.g.,
precision_digits=product_price_digits) for both checks of line.price_unit and
line.discount; update the call(s) at the float_is_zero(...) site(s) in
sale_order_line.py so they use that retrieved precision for consistency with
other uses in the file.
| @@ -0,0 +1 @@ | |||
| * Cetmix | |||
| @@ -0,0 +1,7 @@ | |||
| Teams that use fixed monetary discounts on some order lines and standard percentage | |||
There was a problem hiding this comment.
the file formats in the readme folder should be .md, not .rst
| _inherit = "sale.order.line" | ||
|
|
||
| discount_mode = fields.Selection( | ||
| selection=[ |
There was a problem hiding this comment.
use the same approach here as you did there - https://arc.net/l/quote/pehcjdwq
| ("fixed", "Fixed amount"), | ||
| ], | ||
| string="Discount type", | ||
| default="percent", |
There was a problem hiding this comment.
and here - https://arc.net/l/quote/jeoprceu
Summary by CodeRabbit
New Features
Documentation
Tests