[FIX][16.0] sale_commission_product_criteria: Negative commissions on refund invoices

[Shide]

When a refund invoice happens on a Commission type of "product" (Product Criteria), amount in refund invoices are always positive and Settlement Report shows like positive commissions.

With this fix, refund types gaves negative commissions like https://github.com/OCA/commission/blob/16.0/account_commission/models/account_move.py#L217-L219 on model "account.invoice.line.agent".

PR Series

• sale_commission_product_criteria: [FIX][16.0] sale_commission_product_criteria: Negative commissions on refund invoices #656
• sale_commission_product_criteria_domain: [FIX][16.0] sale_commission_product_criteria_domain: Negative commissions on refund invoices #657

---

MT-12373 @moduon @pedrobaeza @ilyasProgrammer @rafaelbn @yajo @Gelojr @fcvalgar please review if you want 😄

[OCA-git-bot]

Hi @ilyasProgrammer,
some modules you are maintaining are being modified, check this out!

[yajo]

Code review. Missing video/screenshots.

[marcos-mendez]

Automated Review -- Tests Failed

1. Root Cause

The test failure occurs because inv_line.move_type is accessed on a record that has not yet been saved to the database (<NewId>), causing a CacheMiss error when trying to access journal_id from the invoice line. This typically happens when the code attempts to compute values or trigger logic before the record is fully initialized or committed.

2. Suggested Fix

In sale_commission_product_criteria/models/account_move.py, at line 28, the check if inv_line.move_type and "refund" in inv_line.move_type: assumes inv_line.move_type is available. However, in test scenarios, especially during invoice creation via wizard, the move line may not be fully initialized.

Fix: Add a guard to check if inv_line.move_type is set and not a NewId before performing the string check:

# Before:
if inv_line.move_type and "refund" in inv_line.move_type:

# After:
if inv_line.move_type and not isinstance(inv_line.move_type, models.NewId) and "refund" in inv_line.move_type:

This ensures that the code only evaluates the condition when move_type is a valid string, not a temporary NewId.

3. Additional Code Issues

• Potential issue with line.amount = -line.amount: If line is not properly initialized or if line.amount is not computed before this assignment, it could lead to incorrect commission computation or errors. Ensure that line.amount is valid before negating it.

4. Test Improvements

To better cover this logic, add tests in test_sale_commission_product_criteria.py using SavepointCase or TransactionCase with the following scenarios:

• Test refund invoice lines: Create a sale order, confirm it, create a refund invoice, and assert that commission amounts are correctly negated.
• Test with different move types: Ensure that the logic correctly identifies refund types like "out_refund" and "in_refund" and handles them appropriately.
• Test with NewId records: Add a unit test to simulate and verify behavior with unsaved records to prevent regressions like this one.

Use tagged tests (e.g., @tag('post_install')) for tests that involve invoice creation and commission computation to ensure they run in proper transactional context.

Example test snippet:

def test_refund_invoice_commission_negated(self):
    # Create a sale order and invoice
    so = self.env['sale.order'].create({...})
    so.action_confirm()
    invoice = self._invoice_sale_order(so)
    # Create a refund from the invoice
    refund_wizard = self.env['account.move.reversal'].with_context(active_model='account.move', active_ids=invoice.ids).create({})
    refund = refund_wizard.reverse_moves()
    # Assert that commission amounts are negated for refund lines
    ...

---

⏰ This PR has been open for 51 days.

Every ignored PR is a contributor who might not come back. Review time matters. (OCA Aging Report)

---

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#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