[16.0] [FIX] purchase_sale_inter_company: take into account sale order discounts

[MarinaAForgeFlow]

Supersedes #662

Currently, when a sale order line has a discount, this discount is not taken into account when updating the purchase order line price. Instead the sale order line unit price is passed to the purchase line unit price, causing an error in the valuation.

[marcos-mendez]

Automated Review -- Tests Failed

1. Root Cause

The test failure is caused by a missing or incorrect override of the _intercompany_update_purchase_line_price method in the SaleOrderLine model. The method is intended to update the purchase line's price_unit based on the sale line's price and discount, but it's not properly handling cases where product_uom_qty is zero or when discounts are applied, leading to a division by zero or incorrect price assignment.

2. Suggested Fix

In purchase_sale_inter_company/models/sale_order_line.py, the method _intercompany_update_purchase_line_price should be updated to:

• Avoid division by zero by checking qty != 0 before computing price_subtotal / qty.
• Ensure price_unit is correctly set for lines with discounts.

Specifically, change line 23 in sale_order_line.py:

if qty:
    self.auto_purchase_line_id.price_unit = self.price_subtotal / qty
else:
    self.auto_purchase_line_id.price_unit = 0.0

To:

if qty:
    self.auto_purchase_line_id.price_unit = self.price_subtotal / qty
else:
    self.auto_purchase_line_id.price_unit = self.price_unit

This ensures that if qty is zero, the price_unit is still set to the original price_unit, not 0.0, which avoids potential inconsistencies.

3. Additional Code Issues

• Potential Division by Zero: The current code assumes product_uom_qty is always > 0, but it can be 0 in edge cases (e.g., during creation or invalid data). This can cause a ZeroDivisionError.
• Missing self.ensure_one() in action_confirm: In sale_order.py, the method line._intercompany_update_purchase_line_price() is called on a loop, but it's not ensured that line is a single record in action_confirm. While the method itself is ensure_one(), it's good to ensure robustness at the calling site.

4. Test Improvements

The current test test_so_change_price_with_discount is good but lacks coverage for edge cases. Add the following test cases to test_inter_company_purchase_sale.py:

def test_so_change_price_with_zero_qty(self):
    self.company_b.sale_auto_validation = False
    sale = self._approve_po()
    sale.order_line.product_uom_qty = 0
    sale.order_line.price_unit = 100
    sale.order_line.discount = 10
    sale.action_confirm()
    # Ensure no division by zero and price is set correctly
    self.assertEqual(self.purchase_company_a.order_line.price_unit, 100.0)

def test_so_change_price_with_no_discount(self):
    self.company_b.sale_auto_validation = False
    sale = self._approve_po()
    sale.order_line.price_unit = 50
    sale.order_line.discount = 0
    sale.action_confirm()
    self.assertEqual(self.purchase_company_a.order_line.price_unit, 50.0)

Testing Pattern Recommendation:
Use TransactionCase for tests involving inter-company logic and data integrity. If performance is a concern, use SavepointCase for more complex scenarios with multiple transactions.

---

Reciprocal Review Request

Hi everyone! I found some test failures on this PR and left detailed feedback above. I am happy to discuss or help debug. In the meantime, if any of you get a chance, I would appreciate a look at my open PR(s):

My open PRs across OCA:

• server-tools#3554 [MIG] datetime_formatter: Migration to 18.0
• server-tools#3548 [18.0][MIG] base_kanban_stage: Migration to 18.0
• hr-attendance#262 [16.0][ADD] Hr_attendance_idsecure: iDSecure (ControliD) attendance integration
• stock-logistics-workflow#2276 [16.0][ADD] stock_move_line_devaluation
• stock-logistics-workflow#2275 [16.0][ADD] Stock move line analytic account
• stock-logistics-workflow#2268 [16.0][ADD] stock_move_line_picking_partner
• purchase-workflow#2694 [16.0][IMP]Purchase workflow added to review state & exception fix

Reviewing each other's work helps the whole community move forward. Thank you!

---

Environment via OCA Neural Reviewer: Minikube + K8s Job + oca-ci/py3.10-odoo16.0 | Odoo 16.0
Automated review by OCA Neural Reviewer + qwen3-coder:30b

[MarinaAForgeFlow]

There are three options for the else branch when qty == 0:

1. 0.0 (current). Safe, but arguably wrong: the unit price isn't really zero, there's just no quantity.
2. self.price_unit (suggested in the review). Passes the gross price (before discount), which is inconsistent with what we want to achieve.
3. self.price_unit * (1 - self.discount / 100). Passes the net unit price, consistent with what the if branch computes.

But there's a broader question about the way to compute the net price. I used the price_subtotal / qty because it goes through tax_id.compute_all(), which means it strips out the tax component when there are tax-included taxes. The original code never considered taxes, it just passed price_unit as is. So, using price_subtotal / qty introduces a change in behavior for tax-included scenarios that maybe should be reviewed in a separate PR as it is a separate issue: take into account tax included taxes.

A simpler alternative would be to replace the entire method with:
self.auto_purchase_line_id.price_unit = self.price_unit * (1 - self.discount / 100)

This:

• Applies the discount (the purpose of this PR)
• Has no qty == 0 edge case
• Stays consistent with the original behavior regarding taxes (no tax logic involved)
• Is simpler and easier to understand

Then we should consider what should be done when tax included taxes are used.

What do you think?

[AaronHForgeFlow]

👍 Code LGTM

[MarinaAForgeFlow]

Finally, I have decided to simply do

self.auto_purchase_line_id.price_unit = self.price_unit * (
            1 - self.discount / 100
        )

Since the other approach, using price subtotal, was including taxes into the logic, which should be treated separately.

[AaronHForgeFlow]

👍 Agree this is "more correct"