[16.0][ADD] sale_order_general_discount_fixed: add module#7
Open
tendil wants to merge 1 commit into
Open
Conversation
WalkthroughAdds a new Odoo addon that provides a sale-order-level monetary fixed discount, distributes it proportionally to eligible order lines with rounding and validation, wires view insertion, test coverage, packaging, and documentation. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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_general_discount_fixed/models/sale_order.py`:
- Around line 158-164: The `@api.constrains` decorator currently uses unsupported
dotted field names; remove "order_line.price_unit",
"order_line.product_uom_qty", and "order_line.display_type" from the
`@api.constrains` call and leave only "general_discount_fixed" and "order_line";
then, inside the constraint method (the method decorated by `@api.constrains` in
this file), iterate over self.order_line and access each line's price_unit,
product_uom_qty and display_type directly to perform the validation/raises so
the constraint triggers correctly when lines change.
In `@sale_order_general_discount_fixed/readme/DESCRIPTION.md`:
- Around line 1-2: Replace the awkward phrase "allow set discount defined as a
fixed monetary amount in the sale order currency, rather than a percentage, on
the sale order level." with a clear, grammatical sentence such as "allows
setting a discount as a fixed monetary amount in the sale order currency instead
of a percentage at the sale order level." Locate and update the module
description line containing the exact string "allow set discount defined" to use
this improved wording.
In `@sale_order_general_discount_fixed/readme/USAGE.md`:
- Line 7: The README note is outdated: update the sentence in USAGE.md to
reflect that recalculation happens automatically when order lines change because
the hooks in sale_order_general_discount_fixed/models/sale_order_line.py
(create, write, unlink) call _sync_general_discount_fixed() for orders with
general_discount_fixed; remove the instruction to manually re-enter/re-save and
replace it with a brief note that line additions/removals trigger automatic
recalculation via those hooks.
In
`@sale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.py`:
- Line 53: Replace ambiguous lambda parameter names like "l" with a more
descriptive name (e.g., "line" or "ln") in all occurrences where
order.order_line.filtered(lambda l: ...) is used in the test module; update each
lambda expression (the one shown as order.order_line.filtered(lambda l: not
l.display_type) and the other similar filters) so the parameter is renamed
consistently to "line" (or "ln") to improve readability and avoid confusion.
🪄 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: 8ded14df-c059-4448-8a7e-4d01669f9c4d
⛔ Files ignored due to path filters (1)
sale_order_general_discount_fixed/static/description/icon.pngis excluded by!**/*.png
📒 Files selected for processing (14)
sale_order_general_discount_fixed/README.rstsale_order_general_discount_fixed/__init__.pysale_order_general_discount_fixed/__manifest__.pysale_order_general_discount_fixed/models/__init__.pysale_order_general_discount_fixed/models/sale_order.pysale_order_general_discount_fixed/models/sale_order_line.pysale_order_general_discount_fixed/readme/CONTEXT.mdsale_order_general_discount_fixed/readme/CONTRIBUTORS.mdsale_order_general_discount_fixed/readme/DESCRIPTION.mdsale_order_general_discount_fixed/readme/USAGE.mdsale_order_general_discount_fixed/static/description/index.htmlsale_order_general_discount_fixed/tests/__init__.pysale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.pysale_order_general_discount_fixed/views/sale_order_views.xml
tendil
force-pushed
the
16.0-t5379-sale_order_general_fixed_discount-add_module
branch
from
411ce73 to
1352911
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
sale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.py (1)
84-85:⚠️ Potential issue | 🟡 MinorRename the remaining
lambda lparameters.These occurrences still trip Ruff/Flake8
E741, so the file will stay red until they are renamed consistently toline/ln.Also applies to: 105-107, 140-145, 187-190, 218-220, 253-255
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.py` around lines 84 - 85, The test file uses single-letter lambda parameters (e.g., "lambda l") which triggers Ruff/Flake8 E741; locate the sorting/filtering expressions such as the line that sets "lines = order.order_line.filtered(lambda ln: not ln.display_type).sorted(key=lambda l: l.price_unit)" and replace all "lambda l" occurrences with a descriptive name ("lambda line" or "lambda ln") consistently (also update the other similar lambdas referenced in the review: the key functions and filters around the same areas) so every lambda parameter is renamed to "line" or "ln" to satisfy the linter.
🤖 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_general_discount_fixed/models/sale_order_line.py`:
- Around line 7-16: SYNC_TRIGGER_FIELDS is missing line-discount triggers so
manual edits to line discounts don't sync order-level general_discount_fixed;
add "discount_fixed" and "discount" to the SYNC_TRIGGER_FIELDS frozenset (the
constant named SYNC_TRIGGER_FIELDS in sale_order_line.py) so changes to those
fields trigger redistribution; you don't need to alter redistribution logic
because SKIP_SYNC_CONTEXT_KEY is already used to prevent recursion.
In
`@sale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.py`:
- Around line 241-287: Add two tests to exercise the sale.order.line
create/write resync hooks: one that saves an order with general_discount_fixed
set, then calls order.order_line.create(...) to add a new normal line (product
and qty) and asserts the discount_fixed redistribution and order amounts update
correctly; and another that saves an order with multiple lines and then calls
order.order_line.filtered(...).write({'product_uom_qty': new_qty}) or
write({'price_unit': new_price}) to trigger the write resync logic and assert
discounts and amounts are recomputed as expected. Target the resync behavior
implemented in sale_order_general_discount_fixed/models/sale_order_line.py (the
sale.order.line.create and sale.order.line.write hooks) and mirror patterns used
in existing tests (e.g., test_remove_line_and_retrigger_distribution) for
assertions.
In `@sale_order_general_discount_fixed/views/sale_order_views.xml`:
- Around line 7-8: The inserted field element for general_discount_fixed (the
<xpath expr="//field[`@name`='tax_totals']" position="before"> insertion that
contains <field name="general_discount_fixed" />) must include the subtotal
footer CSS class so it aligns with the totals block; update the field tag to add
class="oe_subtotal_footer" (i.e. ensure the inserted field element for
general_discount_fixed has class="oe_subtotal_footer" so it renders in the
subtotal footer layout).
---
Duplicate comments:
In
`@sale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.py`:
- Around line 84-85: The test file uses single-letter lambda parameters (e.g.,
"lambda l") which triggers Ruff/Flake8 E741; locate the sorting/filtering
expressions such as the line that sets "lines = order.order_line.filtered(lambda
ln: not ln.display_type).sorted(key=lambda l: l.price_unit)" and replace all
"lambda l" occurrences with a descriptive name ("lambda line" or "lambda ln")
consistently (also update the other similar lambdas referenced in the review:
the key functions and filters around the same areas) so every lambda parameter
is renamed to "line" or "ln" to satisfy the linter.
🪄 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: 24b14094-6194-4b3b-b08d-d99faea77444
⛔ Files ignored due to path filters (1)
sale_order_general_discount_fixed/static/description/icon.pngis excluded by!**/*.png
📒 Files selected for processing (16)
sale_order_general_discount_fixed/README.rstsale_order_general_discount_fixed/__init__.pysale_order_general_discount_fixed/__manifest__.pysale_order_general_discount_fixed/models/__init__.pysale_order_general_discount_fixed/models/sale_order.pysale_order_general_discount_fixed/models/sale_order_line.pysale_order_general_discount_fixed/readme/CONTEXT.mdsale_order_general_discount_fixed/readme/CONTRIBUTORS.mdsale_order_general_discount_fixed/readme/DESCRIPTION.mdsale_order_general_discount_fixed/readme/USAGE.mdsale_order_general_discount_fixed/static/description/index.htmlsale_order_general_discount_fixed/tests/__init__.pysale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.pysale_order_general_discount_fixed/views/sale_order_views.xmlsetup/sale_order_general_discount_fixed/odoo/addons/sale_order_general_discount_fixedsetup/sale_order_general_discount_fixed/setup.py
tendil
force-pushed
the
16.0-t5379-sale_order_general_fixed_discount-add_module
branch
from
1352911 to
a79f389
Compare
tendil
force-pushed
the
16.0-t5379-sale_order_general_fixed_discount-add_module
branch
from
a79f389 to
1f855e5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
sale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.py (1)
84-86: 🧹 Nitpick | 🔵 TrivialRemaining ambiguous variable names in lambda expressions.
Several lambda expressions still use
lwhich static analysis flags as ambiguous (can be confused with1). Line 53 and 259 were updated to useln, but the following lines still usel: 85, 107, 141, 144, 188, 220, 254.♻️ Proposed fix
- lines = order.order_line.filtered(lambda ln: not ln.display_type).sorted( - key=lambda l: l.price_unit + lines = order.order_line.filtered(lambda ln: not ln.display_type).sorted( + key=lambda ln: ln.price_unit )Apply similar changes to lines 107, 141, 144, 188, 220, and 254.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.py` around lines 84 - 86, Replace ambiguous lambda parameter name "l" with "ln" in all test lambdas that operate on order lines—specifically change occurrences like sorted(key=lambda l: l.price_unit) and any filtered(lambda l: ...) or sorted(key=lambda l: ...) patterns to use lambda ln: ... so they match the existing usage (e.g., order.order_line.filtered(lambda ln: not ln.display_type).sorted(key=lambda ln: ln.price_unit)) and update the same style for the other occurrences referenced in the review.
🤖 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_general_discount_fixed/models/sale_order.py`:
- Around line 111-135: In _get_general_discount_fixed_line_values, round the
computed percentage assigned to "discount" to the "Discount" precision before
returning values: fetch precision via
self.env["decimal.precision"].precision_get("Discount"), compute the percentage
from per_unit_discount and line.price_unit as before, then apply float_round
with precision_digits set to that precision and store the rounded result in
values["discount"] while leaving values["discount_fixed"] as per_unit_discount.
---
Duplicate comments:
In
`@sale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.py`:
- Around line 84-86: Replace ambiguous lambda parameter name "l" with "ln" in
all test lambdas that operate on order lines—specifically change occurrences
like sorted(key=lambda l: l.price_unit) and any filtered(lambda l: ...) or
sorted(key=lambda l: ...) patterns to use lambda ln: ... so they match the
existing usage (e.g., order.order_line.filtered(lambda ln: not
ln.display_type).sorted(key=lambda ln: ln.price_unit)) and update the same style
for the other occurrences referenced in the review.
🪄 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: 62a28884-2a35-43c8-9e4b-b2edf822f046
⛔ Files ignored due to path filters (1)
sale_order_general_discount_fixed/static/description/icon.pngis excluded by!**/*.png
📒 Files selected for processing (16)
sale_order_general_discount_fixed/README.rstsale_order_general_discount_fixed/__init__.pysale_order_general_discount_fixed/__manifest__.pysale_order_general_discount_fixed/models/__init__.pysale_order_general_discount_fixed/models/sale_order.pysale_order_general_discount_fixed/models/sale_order_line.pysale_order_general_discount_fixed/readme/CONTEXT.mdsale_order_general_discount_fixed/readme/CONTRIBUTORS.mdsale_order_general_discount_fixed/readme/DESCRIPTION.mdsale_order_general_discount_fixed/readme/USAGE.mdsale_order_general_discount_fixed/static/description/index.htmlsale_order_general_discount_fixed/tests/__init__.pysale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.pysale_order_general_discount_fixed/views/sale_order_views.xmlsetup/sale_order_general_discount_fixed/odoo/addons/sale_order_general_discount_fixedsetup/sale_order_general_discount_fixed/setup.py
Comment on lines
+111
to
+135
| def _get_general_discount_fixed_line_values( | ||
| self, | ||
| line: models.Model, | ||
| line_discount: float, | ||
| ) -> tuple[float, dict[str, float]]: | ||
| """Prepare line values for a proportional fixed-discount share. | ||
|
|
||
| :param line: Sale order line to update. | ||
| :param line_discount: Total fixed discount allocated to the line. | ||
| :return: Tuple of actual applied line discount and write values. | ||
| """ | ||
| self.ensure_one() | ||
|
|
||
| per_unit_discount = line._fields["discount_fixed"].convert_to_cache( | ||
| line_discount / line.product_uom_qty, | ||
| line, | ||
| ) | ||
| actual_line_discount = self.currency_id.round( | ||
| per_unit_discount * line.product_uom_qty | ||
| ) | ||
| values = { | ||
| "discount_fixed": per_unit_discount, | ||
| "discount": (per_unit_discount / line.price_unit) * 100, | ||
| } | ||
| return actual_line_discount, values |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how sale_fixed_discount handles discount precision and if it validates on write
ast-grep --pattern $'@api.constrains($$$"discount"$$$)
def $_($$$) {
$$$
}'Repository: cetmix/oca-sale-workflow
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Search for Discount precision usage in sale_fixed_discount
rg -n "Discount" --type py -C3Repository: cetmix/oca-sale-workflow
Length of output: 34058
🏁 Script executed:
cat -n sale_fixed_discount/models/sale_order_line.py | head -50Repository: cetmix/oca-sale-workflow
Length of output: 2395
🏁 Script executed:
rg -n "_get_discount_from_fixed_discount|_check_discounts" --type py -B2 -A15Repository: cetmix/oca-sale-workflow
Length of output: 4875
🏁 Script executed:
fd -i "sale.*order.*line" --type f -name "*.py" | head -5Repository: cetmix/oca-sale-workflow
Length of output: 298
🏁 Script executed:
rg -n "discount.*fields\.Float" --type py -B2 -A2 | head -40Repository: cetmix/oca-sale-workflow
Length of output: 2799
🏁 Script executed:
cat -n sale_order_general_discount_fixed/models/sale_order.py | sed -n '111,140p'Repository: cetmix/oca-sale-workflow
Length of output: 1420
🏁 Script executed:
rg -n "discount" sale_order_general_discount_fixed/models/sale_order.py -B1 -A1Repository: cetmix/oca-sale-workflow
Length of output: 7676
🏁 Script executed:
cat -n sale_fixed_discount/models/sale_order_line.py | sed -n '85,96p'Repository: cetmix/oca-sale-workflow
Length of output: 513
🏁 Script executed:
rg -n "float_round|float_compare" sale_order_general_discount_fixed/models/sale_order.py | head -10Repository: cetmix/oca-sale-workflow
Length of output: 573
🏁 Script executed:
rg -n "convert_to_cache|discount.*Float.*digits" sale_order_general_discount_fixed/ -B1 -A1Repository: cetmix/oca-sale-workflow
Length of output: 372
🏁 Script executed:
rg -n "discount_fixed|discount.*=" sale_order_general_discount_fixed/tests/ -B2 -A2 | head -50Repository: cetmix/oca-sale-workflow
Length of output: 5093
🏁 Script executed:
rg -n "discount.*fields\.Float|discount.*fields\.Monetary" sale_fixed_discount/ sale_order_general_discount_fixed/ -B1 -A3Repository: cetmix/oca-sale-workflow
Length of output: 888
🏁 Script executed:
rg -n "class SaleOrderLine" sale_fixed_discount/models/sale_order_line.py -A20 | head -30Repository: cetmix/oca-sale-workflow
Length of output: 913
🏁 Script executed:
rg -n "class.*SaleOrderLine" sale_order_general_discount_fixed/models/ -A30 | grep -E "discount|class"Repository: cetmix/oca-sale-workflow
Length of output: 3204
🏁 Script executed:
rg -n "^ discount = fields\." sale_fixed_discount/ -B1 -A3Repository: cetmix/oca-sale-workflow
Length of output: 50
🏁 Script executed:
python3 << 'EOF'
# Check if Odoo field definitions with digits auto-round on assignment
import re
# From the output we saw:
# sale_fixed_discount has discount_fixed with digits="Product Price"
# The constraint uses float_round on the calculated value
# The onchange uses simple assignment: self.discount = self._get_discount_from_fixed_discount()
# So even without explicit rounding, Odoo's field system rounds to the specified precision
# The key is: does discount field have digits="Discount"?
# We need to check the core sale.order.line model or inherited versions
print("Analysis: Odoo fields with 'digits' parameter automatically round values to that precision")
print("when they are assigned or written to the database.")
print("")
print("The question is: does sale.order.line.discount field have digits='Discount'?")
print("If yes, then auto-rounding occurs and review concern is incorrect.")
print("If no, then the review concern is valid.")
EOFRepository: cetmix/oca-sale-workflow
Length of output: 395
🏁 Script executed:
rg -n "discount.*=.*fields\.Float\(" -B2 -A4 | grep -E "(discount =|digits)" | head -20Repository: cetmix/oca-sale-workflow
Length of output: 1228
🏁 Script executed:
git ls-files | grep -E "sale.*order.*line\.py$" | head -5Repository: cetmix/oca-sale-workflow
Length of output: 325
🏁 Script executed:
rg -n "class SaleOrderLine" --type py -A50 | grep -E "discount.*Float|_inherit" | head -20Repository: cetmix/oca-sale-workflow
Length of output: 1824
🏁 Script executed:
python3 << 'EOF'
# The key question: when you write {"discount": value} to a sale.order.line,
# does it automatically round to "Discount" precision?
# This depends on whether the discount field is defined with digits="Discount"
# From the searches:
# 1. sale_fixed_discount constraint explicitly rounds calculated discount to "Discount" precision
# 2. sale_fixed_discount onchange just assigns: self.discount = self._get_discount_from_fixed_discount()
# without explicit rounding
# 3. Many custom discount fields DO have digits="Discount"
# 4. But we haven't confirmed the base sale.order.line.discount field
# The behavior suggests: the base discount field probably does NOT have digits="Discount"
# Otherwise why would the constraint explicitly round the calculated value?
# Let's think about the actual flow:
# 1. _get_general_discount_fixed_line_values returns unrounded discount percentage
# 2. This gets written to line via _apply_general_discount_fixed_values
# 3. If discount field has no digits="Discount", it won't be rounded
# 4. Then when _check_discounts constraint runs, it compares:
# - stored discount (unrounded)
# - calculated discount (explicitly rounded to "Discount" precision)
# 5. These might differ due to precision, causing ValidationError
print("Critical question: Does sale.order.line.discount field have digits='Discount'?")
print("If NO -> Review concern is VALID")
print("If YES -> Review concern is INCORRECT (auto-rounding handles it)")
EOFRepository: cetmix/oca-sale-workflow
Length of output: 244
🏁 Script executed:
# Search for where discount field is first defined (likely in base Odoo or an addon)
rg -n "discount.*=.*fields\.Float" addons/sale/models/ 2>/dev/null || echo "File not in repo, checking inherited"Repository: cetmix/oca-sale-workflow
Length of output: 104
🏁 Script executed:
# Check if there's a sale_order_line.py in the repo that defines discount
fd "sale_order_line.py" --type fRepository: cetmix/oca-sale-workflow
Length of output: 2552
🏁 Script executed:
# Search for discount field definition with digits in the codebase
rg -n "discount.*=.*fields\.Float.*digits.*Discount" --type pyRepository: cetmix/oca-sale-workflow
Length of output: 324
🏁 Script executed:
# Check what precision "Discount" typically is in Odoo
rg -n "decimal.*precision.*Discount|Discount.*decimal" --type py | head -10Repository: cetmix/oca-sale-workflow
Length of output: 322
🏁 Script executed:
# Look at the actual test to see what precision is expected
cat -n sale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.py | sed -n '45,120p'Repository: cetmix/oca-sale-workflow
Length of output: 2979
🏁 Script executed:
# Check how float_compare actually works - does it round both values?
rg -n "float_compare" sale_fixed_discount/ -B2 -A2Repository: cetmix/oca-sale-workflow
Length of output: 867
🏁 Script executed:
# Most importantly, check if the tests actually verify discount field values or just the amounts
cat -n sale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.py | sed -n '50,70p'Repository: cetmix/oca-sale-workflow
Length of output: 829
🏁 Script executed:
# Verify: Does the base sale.order.line.discount field have digits="Discount"?
# Check by looking at how discount is used in constraints or onchanges in this repo
rg -n "line\.discount\|\.discount =" sale_fixed_discount/models/sale_order_line.py -B1 -A1Repository: cetmix/oca-sale-workflow
Length of output: 50
🏁 Script executed:
# Check if base sale.order.line has discount field and its configuration
# by looking at the imports and field references in the tested code
cat -n sale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.py | sed -n '1,30p'Repository: cetmix/oca-sale-workflow
Length of output: 1240
🏁 Script executed:
# Trace what happens in _apply_general_discount_fixed_values - does it modify values?
rg -n "_apply_general_discount_fixed_values" sale_order_general_discount_fixed/models/ -A20 | head -40Repository: cetmix/oca-sale-workflow
Length of output: 3732
🏁 Script executed:
# Final check: Are tests actually verifying discount field value, not just amounts?
rg -n "\.discount" sale_order_general_discount_fixed/tests/test_sale_order_general_discount_fixed.pyRepository: cetmix/oca-sale-workflow
Length of output: 338
🏁 Script executed:
# Final verification: Check if base discount field definition exists in this repo
# and what precision it uses
rg -n "discount = fields\.Float" sale_fixed_discount/ -B2 -A4Repository: cetmix/oca-sale-workflow
Length of output: 50
🏁 Script executed:
# Check if the constraint actually triggers in practice by looking at test for exceptions
rg -n "ValidationError\|_check_discounts" sale_order_general_discount_fixed/tests/ -B3 -A3Repository: cetmix/oca-sale-workflow
Length of output: 50
🏁 Script executed:
# One more critical check: How does float_compare work with precision?
# Does it round both values before comparing?
web_search
Odoo float_compare precision_digits rounding behaviorRepository: cetmix/oca-sale-workflow
Length of output: 161
🏁 Script executed:
# Check sale_fixed_discount/__manifest__.py to see dependencies
cat sale_fixed_discount/__manifest__.pyRepository: cetmix/oca-sale-workflow
Length of output: 805
🏁 Script executed:
# Check sale_order_general_discount_fixed/__manifest__.py to see dependencies
cat sale_order_general_discount_fixed/__manifest__.pyRepository: cetmix/oca-sale-workflow
Length of output: 552
🏁 Script executed:
rg -n "def write|def create" sale_fixed_discount/models/sale_order_line.py -A3Repository: cetmix/oca-sale-workflow
Length of output: 50
Round discount percentage to "Discount" precision for consistency with the sale_fixed_discount constraint.
The calculated discount percentage at line 133 is not explicitly rounded to "Discount" precision, though the sale_fixed_discount constraint validates both discount_fixed and discount at that precision. While the constraint's use of float_compare will round both values before comparison (masking the mismatch), explicitly rounding the percentage to match the constraint's expectation improves code clarity and follows the pattern used in related calculations.
Suggested fix:
precision = self.env["decimal.precision"].precision_get("Discount")
values = {
"discount_fixed": per_unit_discount,
"discount": float_round(
(per_unit_discount / line.price_unit) * 100,
precision_digits=precision,
),
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sale_order_general_discount_fixed/models/sale_order.py` around lines 111 -
135, In _get_general_discount_fixed_line_values, round the computed percentage
assigned to "discount" to the "Discount" precision before returning values:
fetch precision via self.env["decimal.precision"].precision_get("Discount"),
compute the percentage from per_unit_discount and line.price_unit as before,
then apply float_round with precision_digits set to that precision and store the
rounded result in values["discount"] while leaving values["discount_fixed"] as
per_unit_discount.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task: 5379
Summary by CodeRabbit
New Features
Tests
Documentation