[6131][ADD] purchase_deposit_company_amount by kanda999 · Pull Request #147 · qrtl/axls-custom

kanda999 · 2026-05-16T10:32:13Z

AungKoKoLin1997 · 2026-05-20T03:35:21Z

@kanda999 Please fix the pre-commit issue.

…eposit flow The rate override is wanted for the purchase-deposit flow only. Gate company_amount on the presence of a deposit line on the move, so it applies to the deposit vendor bill and to the final invoice that carries the deposit offset, and nowhere else. - add _is_company_amount_allowed() and expose it as company_amount_allowed - guard _apply_company_amount_override() and _get_gross_unit_price() - reject a value set outside the deposit flow with a constraint, and make the column read-only there - declare purchase_stock, which _get_gross_unit_price and the tests already rely on Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

kanda999 · 2026-08-14T05:30:34Z

In line with the policy change, the scope of the rate limit (company_amount) has been restricted to advance payment flows (10453f6).

Background

The method for fixing inventory valuation ([6131][ADD] purchase_deposit_separate_valuation #148) is no longer necessary, and the rate limit in this PR will take precedence. → [6131][ADD] purchase_deposit_separate_valuation #148 has been closed
[5661][ADD] purchase_deposit_preserve_amount #130 purchase_deposit_preserve_amount, the predecessor to this PR, could not be used concurrently because it overwrites the balance of the same journal line twice. → [5661][ADD] purchase_deposit_preserve_amount #130 has also been closed and removed from staging

Changes

We added _is_company_amount_allowed() to enable the limit value only when a transfer (account.move) contains an advance payment line. This applies to the following two cases, both of which can be determined by checking whether an advance payment line exists in the move:

Vendor invoices with prepayments (containing a positive prepayment line)
Final invoices (containing a negative prepayment offset line)—where the product line absorbs the rate difference

For all other vendor invoices, Odoo’s standard amount_currency × currency_rate conversion applies as-is.

Add the same guard to _apply_company_amount_override() / _get_gross_unit_price()
If a value is entered in a line that is not subject to this rule, reject it using @api.constrains (do not silently ignore it)
Columns in the tree are also read-only for non-targeted lines (add company_amount_allowed)
Added purchase_stock to depends — both _get_gross_unit_price and the picking_ids in the test actually depended on this
Version 16.0.2.2.0 → 16.0.3.0.0 (due to behavior restrictions)

Tests

One existing test and three new tests—a total of four—pass.

purchase_deposit_currency: 0 failed, 0 error(s) of 4 tests

test_company_amount_not_allowed_without_deposit — For invoices without a prepayment, company_amount_allowed is False; entering a value results in a ValidationError
test_standard_conversion_untouched_without_deposit — Invoices without advance payments retain the standard conversion (USD 100 → ¥16,000)
test_company_amount_allowed_on_deposit_bill — Product lines in advance payment invoices and final invoices are within the allowed range

Going Forward

If a generic fixed-price invoice (a standard supplier invoice without a deposit) becomes necessary, we plan to split the module into three layers: a generic portion independent of purchase_deposit (fixed-price field + required balance), the purchase_stock bridge, and the deposit glue. I have also noted this in DESCRIPTION.md.

…erride Rename from purchase_deposit_currency. The module's subject is the company-currency amount override, which the old name did not name. The module is unreleased, so no migration is involved. Apply the balance override from account.move.line._sync_invoice instead of from an onchange plus line create/write plus move create/write. That is where Odoo derives balance from amount_currency for invoice lines, and it leaves an externally written balance alone, so the override stops racing the standard computation. It also settles before the payment-term sync, so the payable is rebuilt from the overridden balances rather than the rate-converted ones. Derive deposit_company_amount from posted bill lines instead of snapshotting it in action_post. Resetting or reversing a deposit bill used to leave the purchase order handing a stale amount to the final bill. Move the scope constraint to account.move and depend on the fields it reads. Hung off account.move.line.company_amount alone it only fired when that field was written, so dropping the deposit line from a bill that already carried an override went unchecked. Keep company_amount for user input only and compute the deposit rate difference at sync time, retiring the deposit_amount_adjusted flag that existed to tell automatic values from manual ones.

…nt style Rewrite DESCRIPTION.md in the repository's usual form: a short statement of what the module does, with the rationale under Background. Move the worked example and the field's usage notes to USAGE.md, where they belong in the OCA readme structure.

…n a rate change Editing the bill date moves currency_rate, so the standard invoice sync re-derives every line's balance from the new rate, payable included, while the overridden lines are put straight back where they were. Odoo would normally rebuild the payable from needed_terms, but that sync only fires when the needed values change, and pinning a balance leaves them identical. The payable kept its rate-converted value and the bill could not be saved: The move (...) is not balanced. The total of debits equals 3,900 and the total of credits equals 4,500. Absorb the leftover imbalance into the payment-term line(s) after applying the overrides, which enforces the invariant that sync would have enforced. Also split the scope constraint across account.move and account.move.line. Odoo 16 ignores dotted paths in @api.constrains and warns that they are "not a field name", so the three line_ids.* triggers were inert and a value written straight onto a line was never checked.

…ice with standard The override reproduced standard's formula with an abs() and an explicit in_refund sign flip. Since price_subtotal is balance * currency_rate on a bill and its negation on a refund -- the same negation standard then applies -- the two cancel, and the formula is simply standard's with the overridden balance substituted in. Dropping them removes a branch and fixes the sign on a negative-quantity line, where abs() over a signed quantity diverged from standard. Guard zero quantity the way standard does, with the unit-of-measure rounding rather than a truthiness check, so a quantity below the UoM precision falls back instead of producing an enormous unit price. Document that the hook acts on the goods line and never on the deposit line, which purchase_deposit requires to be a service and which therefore carries no valuation layer, and correct the manifest note that attributed _get_gross_unit_price to purchase_stock rather than stock_account.

… deposit lines The deposit is the one amount known outside Odoo, so it is the only line worth typing on. A goods line sharing the bill could previously be priced by hand, which was a second and silent way to restate the cost of goods on top of the rate difference it already absorbs. Split the gate in two while doing so. Deciding who may enter a value and deciding whether stock valuation must follow the pinned balances are different questions, and they were sharing one method: the goods line must keep qualifying for the second, since it is the line whose balance absorbs the deposit's rate difference and therefore the one the valuation layer has to track. Reusing the narrowed gate there would have silently dropped the valuation adjustment. The rule is now a property of the line alone, where before it had to inspect the move's other lines, so the constraint on account.move is no longer needed and a single one on account.move.line covers it.

…ate change Add the counterpart to the deposit-bill test and record why only one of them needs the rebalance. The payment-term sync acts on changes to the needed totals, so the gap is exactly where the override makes the total blind to the rate: on the deposit bill the single non-payable line is pinned to the amount paid, leaving the total identical across a rate change, while the final bill's goods line is rate-based plus the deposit's rate difference and so still moves.

…the deposit bill The offset line on the final bill is the same deposit purchase order line as the deposit bill's, so it passed every test the rule applied and was editable. Its value is read back from the posted deposit bill, so typing over it quietly untied the two. Require a positive quantity, which is what distinguishes the deposit bill's own line from the negative offset that nets it off. Add account.move.is_deposit for the view. The column used to appear on every vendor bill and merely turn read-only; it is now hidden outside deposit bills, which is where it can actually be used. The field is not needed for the rule itself -- a deposit line with a positive quantity already implies it -- so the check stays a property of the line alone.

Assert that the goods line on the final bill is refused by the editing rule yet accepted by the deposit-flow one. The two look interchangeable and are one line apart, but collapsing them zeroes the stock valuation adjustment: the goods line is where the deposit's rate difference lands, so valuation must follow its balance even though nobody may type there. Nothing else in the suite objects to the merge, which is what makes it worth stating.

It duplicated the scan account.move._get_deposit_offset_lines already does, so the valuation gate now calls that instead and the helper goes away. Keying it on the move rather than on the line also states the condition more accurately: what matters is that the bill nets off a deposit, which is what pins the goods line's balance. It cannot be account.move.is_deposit, which is about the deposit bill: the goods line sits on the final bill, where is_deposit is False. The suite pins that -- swapping the gate zeroes the valuation adjustment.

The Register Deposit wizard raises one deposit line per order and one bill from it, so a deposit bill holds exactly one line. Hiding the column off deposit bills therefore says everything the per-line read-only state said, and the technical field and its compute go away with it. Enforcement is unaffected: the constraint on account.move.line is what refuses a value, and the suite still passes with the column rule taken out entirely, so the view was only ever the convenience half.

…is_deposit alone The constraint layered four conditions where one does the job. is_deposit looks for a deposit line with a positive quantity, so the final bill is already out -- its offset reuses the same purchase order line with a negative quantity -- and the quantity, display type and move type tests were all redundant behind that. _is_company_amount_allowed loses its last caller and goes. What remains is two concepts on account.move, disjoint and both needed: is_deposit for who may enter a value, and _get_deposit_offset_lines for whose balances are pinned and therefore whose stock valuation must follow.

…y the suite Every test used a single-goods-line order, so absorbing_lines had length one and control went straight to the remainder branch on the first pass. The weighting and the rounding sweep -- the only real arithmetic in the module, and what keeps the move balanced -- had no coverage at all. Add a three-line order whose numbers expose both. USD 20 + 30 + 50 with 3801 paid gives a difference of -999, weighted into -199.8, -299.7 and -499.5: rounded on their own those come to -1000, a yen more than there is to give, so the sweep has to hand the last line -499. Two lines cannot show this, as their fractional shares are complementary and independent rounding happens to add up. Both halves are pinned -- an equal split puts the first line at 2867 instead of 3000, and dropping the sweep puts the last at 7500 instead of 7501. Drop the equal-split fallback for a zero total weight, which no order can reach: goods worth nothing give a percentage deposit of nothing and so no difference to spread. It only ever guarded the division, which an early return does more plainly, and the payment-term rebalance still keeps the move balanced. Fold the flow into helpers rather than repeating it in each test, name the line lookups instead of respelling the same filters, and merge the two plain-bill tests, which set up the same bill to assert one thing each. Eight tests become seven with more covered.

AungKoKoLin1997 · 2026-08-18T08:22:39Z

@kanda999 @nobuQuartile This PR is ready to review.

nobuQuartile · 2026-08-24T03:33:36Z

@AungKoKoLin1997
What do you think of these issues?

Critical
Deposit-offset target sign is hardcoded, so credit notes/reversals of the final bill are corrupted — models/account_move.py:82 — the manual branch derives direction from the line (sign = -1 if line.amount_currency < 0 else 1, line 75) but the offset branch writes targets[line] = -deposit_amount unconditionally. _apply_company_amount_overrides (line 147) explicitly includes in_refund, and a reversal of the final bill keeps the offset line intact: _reverse_moves only sign-flips entry/cogs lines, so the copied offset line keeps quantity < 0 (still matched by _get_deposit_offset_lines) while direction_sign flips to -1 for in_refund (get_outbound_types excludes it), making its rate-based balance +4800. The override pins it to -3900, so delta = 4800 - (-3900) = 8700 is spread into the goods lines (−16000 → −7300) and the deposit asset account is credited a second time instead of reversed. Fix by reusing the same sign derivation: sign * abs(deposit_amount). No test covers in_refund, which is why this is invisible today.

Major
Enforcement is bill-level while the field help and README promise line-level — models/account_move_line.py:34-36 — _check_company_amount_allowed only rejects lines whose move_id.is_deposit is False, and column_invisible is per-column, not per-cell. On a deposit bill a user can type a Company Currency Amount on any added product line and _get_company_amount_targets (line 73) will honour it, contradicting "Only available on the deposit line of a deposit bill". Add and line.purchase_line_id.is_deposit and line.quantity > 0 to the constraint.

Override ignores field protection, unlike the core sync it wraps — models/account_move.py:153-155 — core _sync_invoice guards every balance write with not self.env.is_protected(self._fields['balance'], line) plus a changed('balance') check precisely so an explicitly-supplied balance is never clobbered (account.move.line.create installs env.protecting(_get_protected_vals(...))). This module writes unconditionally, so a caller that creates or writes a product-line balance on a deposit-related bill (EDI/bill import, line_ids writes during reversal, data migration) has it silently replaced by rate_based + share. The docstring's claim that a manual value is never overwritten only holds for company_amount, not for balance.

AungKoKoLin1997 · 2026-08-24T06:10:25Z

@nobuQuartile For critical point, I think it is valid and fix it.

First major point: It is over engineering and I believe we don't need that because the logic is already cover by the compute method of is_deposit fileld.

Second major point: I don't think the suggestion is not needed because _rebalance_payment_term_lines is only called when there is deposit related line in bill.

…n from the line The deposit-offset target was pinned to a fixed negative, so it only ever described a vendor bill. _reverse_moves sign-flips neither the quantity nor purchase_line_id, so the reversal of a final bill arrives as an offset line too, only pointing the other way: crediting the deposit account again instead of reinstating it. Odoo's own check_amount_currency_balance_sign catches the mismatch, so crediting a foreign-currency final bill on a deposit order fails outright rather than posting bad figures. Derive the direction from the line's amount_currency, as the manual branch already does. move_type is not usable here -- it flips the moment the reversal is created, while amount_currency is still mid-sync -- and amount_currency is the value the constraint compares the balance against, which also keeps the override safe to re-run. Cover the credit note and the reverse-and-rebill that usually follows it. The suite also needed the post_install tag: sale_stock loads after this module, so at at_install time res_company has no security_lead field, its default cannot apply, and the NOT NULL column made setUpClass fail before any test ran.

AungKoKoLin1997 · 2026-08-24T06:23:45Z

I think we should sort out this logic since it will effect other module test cases.

axls-custom/purchase_order_no_owner/models/purchase_order.py

Lines 11 to 22 in b24821b

    
           no_owner = fields.Boolean() 
        
           def button_confirm(self): 
        
               for record in self: 
        
                   if not record.no_owner and not record.owner_id: 
        
                       raise UserError( 
        
                           _( 
        
                               "Please select the owner. if you don't want to select any owner for" 
        
                               " this order, you can set No Owner field as True." 
        
                           ) 
        
                       ) 
        
               return super(PurchaseOrder, self).button_confirm()

nobuQuartile

Partial review from me

nobuQuartile · 2026-08-25T08:24:01Z

@AungKoKoLin1997
This review is from odoo-review.
I confirmed this issue occurs in the Test environment.
By the way, you are doing well.
We're almost there, let's keep going!

Critical

Rewrites balance on posted journal entries — models/account_move.py:157-168

_apply_company_amount_overrides() has no state guard. account.move.write() enters _sync_dynamic_lines() → line_ids._sync_invoice() regardless of state (addons/account/models/account_move.py:2360, same:2213-2215), so any write against a posted bill — editing the reference or due date, button_draft, a write from another module — recomputes the targets and runs line.balance = target.

There are two realistic ways the targets end up differing from what was posted:

A currency rate added or corrected for the bill's date. _get_rate_based_balance() depends on currency_rate, which comes from the rate table, so a rate landing on that date after the bill was posted changes delta and leaves the goods lines' targets out of step with the posted balances. This repo has currency_rate_update_mizuho feeding rates in by cron, so posting a bill dated today before the cron runs is enough to trigger it.

A deposit bill reset to draft, corrected and re-posted — specifically when deposit_company_amount ends up at a different non-zero value (resetting it to zero is safe, since the targets then come out empty).

Either outcome is bad:

If the entry is reconciled, balance is a reconciliation-protected field (addons/account/models/account_move_line.py:2815 → same:1261-1266), so a UserError ("You cannot do this modification on a reconciled journal entry") blocks an otherwise unrelated operation. Inside a lock period, _check_fiscalyear_lock_date() / _check_tax_lock_date() fail the same way.
If it is not reconciled, the posted entry's amounts are silently rewritten while the stock.valuation.layer records and price-difference journal items created at posting time keep the old values.
Suggested fix: in _sync_invoice, capture each affected move's state before the yield (i.e. before the write lands) and skip the moves that were already posted. Guarding simply on move.state != "draft" would also skip during write({'state': 'posted'}), but that should be harmless for vendor bills: _post requires invoice_date (addons/account/models/account_move.py:3564-3568), so the rate cannot move at posting time and the balances pinned in draft carry through unchanged. Worth confirming with a test.

Major

Clearing company_amount does not restore the standard conversion — models/account_move.py:45-94, models/account_move_line.py:13-24

Both the field's help text and readme/USAGE.md state that leaving the field empty keeps the standard conversion, but it does not come back. Clearing the value empties product_lines.filtered("company_amount"), so targets is {} and _apply_company_amount_overrides() bails out at if not targets: continue. Core's _sync_invoice will not re-derive the balance either, because none of amount_currency, currency_rate or move_type changed (addons/account/models/account_move_line.py:1436-1443). The result is that the balance pinned at 3900 — and the payable line as adjusted by _rebalance_payment_term_lines() — stay in place, and the user can post that.

Suggested fix: give every eligible line an explicit target of _get_rate_based_balance() when its company_amount is empty, i.e. express "no override" as "back to the rate-converted value" rather than as absence from targets. A single test stating the documented behaviour would have caught this.

AungKoKoLin1997 · 2026-08-25T09:02:37Z

@nobuQuartile I understand the scenario you showed me.
But what I am wondering is this can happen in real operation.
After deposit bill and vendor bill are posted, user change the deposit bill company amount by doing reset to draft.
I understand they can make mistake in deposit bill and update but that can only happen before creating final vendor bill or before posting final vendor bill.

nobuQuartile · 2026-08-27T08:00:19Z

Not in scope: `company_amount` accepted on non-deposit lines

_check_company_amount_allowed and _get_company_amount_targets gate on the
move (allow_company_amount), not on the line, so on a deposit bill the
override would also be honoured on a product line other than the deposit
product.

In practice this does not occur: every deposit bill raised by the Register
Deposit wizard carries the deposit product line only — we have no case of a
deposit bill containing any other product. Reaching the state requires manually
adding a line to the deposit bill, which is not part of the flow this module
supports. I have checked that the client does not add other lines for deposit invoices in Copy.

Treating it as out of scope for this PR.

nobuQuartile

LGTM: Functional review and code review

yostashiro

Partial review and it's taking a lot of time.

Can we shorten/remove comments and docstrings as they seem to be overly verbose and sometimes incorrect.

I think it's a good candidate for OCA. Is there something holding you back?

yostashiro · 2026-09-01T05:33:33Z

+    "name": "Purchase Deposit Company Amount",
+    "summary": "Book purchase deposits at the company-currency amount actually paid",
+    "version": "16.0.1.0.0",
+    "author": "Quartile Limited",


Suggested change

"author": "Quartile Limited",

"author": "Quartile",

yostashiro · 2026-09-01T06:01:07Z

+    # purchase_stock: holds the price-difference logic that consumes
+    # ``_get_gross_unit_price`` (which is itself defined in stock_account) and
+    # writes the stock valuation adjustment the override has to reach.
+    "depends": ["purchase_deposit", "purchase_stock"],


Suggested change

"depends": ["purchase_deposit", "purchase_stock"],

"depends": ["purchase_stock", "purchase_deposit"],

yostashiro · 2026-09-01T06:10:19Z

@@ -0,0 +1,15 @@
+This module does the following:


Suggested change

This module does the following:

This module books a foreign-currency purchase deposit at the company-currency amount

actually paid and carries that figure through to the final bill, so the deposit account

clears to zero and the exchange-rate difference lands in the inventory cost of the goods

instead of being stranded on the balance sheet.

In detail:

yostashiro · 2026-09-01T06:53:02Z

+            if move.move_type not in ("in_invoice", "in_refund"):
+                continue
+            if move.state == "posted":


Suggested change

if move.move_type not in ("in_invoice", "in_refund"):

continue

if move.state == "posted":

if move.is_purchase_document() or move.state == "posted":

AungKoKoLin1997 · 2026-09-01T09:22:34Z

Followed up on suggestion points and created a OCA PR.
OCA/purchase-workflow#3161

kanda999 added 2 commits May 16, 2026 10:30

purchase_deposit_currency/

c0dcd07

upd purchase_deposit_currency

34c4467

AungKoKoLin1997 reviewed May 21, 2026

View reviewed changes

Comment thread purchase_deposit_currency/__manifest__.py Outdated

Comment thread purchase_deposit_currency/models/account_move.py Outdated

upd

ac2a819

This was referenced Aug 14, 2026

[5661][ADD] purchase_deposit_preserve_amount #130

Closed

[6131][ADD] purchase_deposit_separate_valuation #148

Closed

AungKoKoLin1997 added 3 commits August 17, 2026 06:48

yostashiro changed the title ~~[6131][ADD] purchase_deposit_currency~~ [6131][ADD] purchase_deposit_company_amount Aug 18, 2026

AungKoKoLin1997 added 9 commits August 18, 2026 06:19

nobuQuartile reviewed Aug 25, 2026

View reviewed changes

Comment thread purchase_deposit_company_amount/models/account_move_line.py

Comment thread purchase_deposit_company_amount/models/account_move.py

Comment thread purchase_deposit_company_amount/models/account_move.py

Comment thread purchase_deposit_company_amount/models/account_move.py

AungKoKoLin1997 added 2 commits August 25, 2026 10:27

fixup

68b0b1a

fixup

13ac996

nobuQuartile reviewed Aug 27, 2026

View reviewed changes

Comment thread purchase_deposit_company_amount/models/account_move_line.py

nobuQuartile approved these changes Aug 27, 2026

View reviewed changes

yostashiro reviewed Sep 1, 2026

View reviewed changes

AungKoKoLin1997 closed this Sep 1, 2026

	"depends": ["purchase_deposit", "purchase_stock"],
	"depends": ["purchase_stock", "purchase_deposit"],

-This module does the following:
+This module books a foreign-currency purchase deposit at the company-currency amount
+actually paid and carries that figure through to the final bill, so the deposit account
+clears to zero and the exchange-rate difference lands in the inventory cost of the goods
+instead of being stranded on the balance sheet.
+In detail:

Uh oh!

Conversation

kanda999 commented May 16, 2026 • edited by nobuQuartile Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AungKoKoLin1997 commented May 20, 2026

Uh oh!

Uh oh!

Uh oh!

kanda999 commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Changes

Tests

Going Forward

Uh oh!

AungKoKoLin1997 commented Aug 18, 2026

Uh oh!

nobuQuartile commented Aug 24, 2026

Uh oh!

AungKoKoLin1997 commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AungKoKoLin1997 commented Aug 24, 2026

Uh oh!

nobuQuartile left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nobuQuartile commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AungKoKoLin1997 commented Aug 25, 2026

Uh oh!

Uh oh!

nobuQuartile commented Aug 27, 2026

Not in scope: company_amount accepted on non-deposit lines

Uh oh!

nobuQuartile left a comment

Choose a reason for hiding this comment

Uh oh!

yostashiro left a comment

Choose a reason for hiding this comment

Uh oh!

yostashiro Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

yostashiro Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

yostashiro Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

yostashiro Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

AungKoKoLin1997 commented Sep 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kanda999 commented May 16, 2026 •

edited by nobuQuartile

Loading

kanda999 commented Aug 14, 2026 •

edited

Loading

AungKoKoLin1997 commented Aug 24, 2026 •

edited

Loading

nobuQuartile commented Aug 25, 2026 •

edited

Loading

Not in scope: `company_amount` accepted on non-deposit lines