Conversation
This mixin replaces the core `discount` field of `account.move.line`, which has no upper bound, and `write()` redirects any write of `discount` into `discount1`. `CHECK (discountN <= 100.0)` therefore rejects values that core Odoo itself produces: `account_edi_ubl_cii` infers the discount percentage from the imported amounts, uncapped and with rounding disabled, so a credit note whose `price_subtotal` has the opposite sign of `quantity * price_unit` yields exactly 200%. Installing this module on a database holding such a line aborts, since `post_init_hook` copies `discount` into `discount1` after the constraints have been created. Removing the constraints is safe: the computation already handles values over 100%, as `discount1 = 200` gives a factor of -1 and an aggregated discount of 200. The bound was one-sided anyway, since negative discounts stay allowed and are covered by `test_01_discounts`.
StefanRijnhart
left a comment
Member
There was a problem hiding this comment.
Maybe replace them with ORM constraints? This will allow us to keep the feature, as well as an easy override option if not desired. Such a change would also be compatible with the installation on existing databases that have one or more existing lines with a discount > 100.
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
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.
Problem
triple.discount.mixinredefines the corediscountfield ofaccount.move.line, andwrite()redirects any write ofdiscountintodiscount1. Corediscounthas no upper bound (account/models/account_move_line.py#L371-L375), soCHECK (discountN <= 100.0)rejects values that Odoo itself produces and stores.account_edi_ubl_ciiinfers the discount percentage from the imported amounts, uncapped:account_edi_ubl_cii/models/account_edi_common.py#L842-L847account_edi_ubl_cii/models/account_edi_ubl.py#L3061-L3067account_edi_ubl_cii/models/account_edi_ubl.py#L3543-L3547and it does so with rounding deliberately disabled, so the raw value is stored as-is:
_disable_discount_precisionforces theDiscountprecision to 13 digits (account/models/decimal_precision.py#L6-L13).A vendor credit note whose
price_subtotalhas the opposite sign ofquantity * price_unittherefore yields exactly 200%:Installing this module on a database that already holds such a line aborts the whole upgrade, because
post_init_hookcopiesdiscountintodiscount1after the constraints have been created:The constraints cannot prevent the situation either — they are created while
discount1is still NULL, so they always apply successfully and only fail later, on data that was already valid under core Odoo.Reproduce
account_edi_ubl_ciiinstalled, import a Peppol vendor bill or credit note whose line amounts imply a discount above 100% (or simplyUPDATE account_move_line SET discount = 200 WHERE id = <any line>).account_invoice_triple_discount.Fix
Remove the three
CHECK (discountN <= 100.0)constraints.This is safe:
discount1 = 200gives a factor of-1and an aggregateddiscountof 200.test_01_discounts, which setsdiscount1 = -5.No migration script is needed: the constraints are reflected as
ir.model.constraintxml_ids, soir.model.data._process_end()drops them on upgrade.Added
test_08_discount_over_100as a regression test.Note
19.0carries the identical constraints (ported tomodels.Constraint) and needs the same fix. Happy to open the forward-port once this is agreed.