[16.0][MIG] contract_brand

[pcastelovigo]

Depends on #283

Backports #284

thank you @marielejeune

[marcos-mendez]

Automated Review -- Tests Failed

1. Root Cause

The test failure occurs because the test_contract_analytic_distribution_onchange_brand method has an incomplete implementation — it only asserts False without a proper assertion on the expected behavior of the analytic_distribution field when the brand is updated.

2. Suggested Fix

In contract_brand/tests/test_contract.py, replace the incomplete assertion:

self.assertFalse(

with a proper assertion that checks whether the analytic_distribution is correctly propagated from the brand to the contract or invoice. For example:

self.assertEqual(self.contract.analytic_distribution, {self.analytic_account.id: 100.0})

This should happen after setting self.brand_id.analytic_distribution.

3. Additional Code Issues

• The test_contract_create_branded_move test assumes that recurring_create_invoice() returns a move with a brand_id field, but this depends on the module's integration with the invoice creation logic. If brand_id is not automatically set on the invoice, the test may fail unless there is explicit code in the contract module that propagates the brand to invoices.
• The setUpClass method creates an analytic plan and account, but it does not verify that these are used in the tests. Ensure that these objects are actually used in the test logic to avoid dead code.

4. Test Improvements

To better cover the changed code, add the following test cases:

• Test brand propagation on invoice creation: Ensure that when a contract with a brand is invoiced, the invoice inherits the brand.
• Test brand change effect on existing contracts: Create a contract, set a brand, then change the brand and verify that the change propagates correctly.
• Test analytic_distribution inheritance: Verify that when a brand has analytic_distribution set, it is correctly applied to the contract or related invoices.

Use SavepointCase for tests that modify data to ensure isolation, and tag tests with @tag('post_install') if they depend on installation logic. Reference OCA testing patterns for best practices.

---

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

[marcos-mendez]

Automated Review -- Tests Failed

1. Root Cause

The test failure occurs because the contract_brand module depends on analytic_brand, but this dependency is not installed in the test environment, causing a MissingError when trying to access fields from the analytic_brand module during registry loading.

2. Suggested Fix

Ensure that the analytic_brand module is included in the test dependencies or mocked properly. In contract_brand/__manifest__.py, verify that "analytic_brand" is present under depends. If not, add it. Alternatively, in the test file (test_contract.py), mock the analytic_brand behavior using patch or ensure the module is installed in the test environment.

3. Additional Code Issues

• Missing analytic_brand in manifest: The manifest file contract_brand/__manifest__.py includes "analytic_brand" in depends, which is correct, but the error log implies this module may not be available during test setup. Double-check if analytic_brand is properly installed in the test DB.
• Incomplete test case: The test method test_contract_analytic_distribution_onchange_brand is cut off and does not complete its assertion. It should assert the expected behavior of analytic_distribution after setting the brand's analytic_distribution.

4. Test Improvements

• Add test case for invoice creation with brand: The existing test test_contract_create_branded_move is good, but add a test case that ensures that when no brand is set, no brand is propagated to the invoice.
• Test analytic distribution logic: Complete the test_contract_analytic_distribution_onchange_brand method to properly assert the result of the analytic_distribution logic when a brand with analytic_distribution is assigned.
• Use SavepointCase or TransactionCase: For better test isolation, especially when dealing with inheritance and field modifications, prefer using odoo.tests.common.SavepointCase or TransactionCase instead of TransactionCase if needed for more complex setups.

Example:

def test_contract_analytic_distribution_onchange_brand(self):
    self.brand_id.analytic_distribution = {self.analytic_account.id: 100.0}
    self.contract.brand_id = self.brand_id
    self.contract._compute_analytic_distribution()
    self.assertEqual(self.contract.analytic_distribution, {self.analytic_account.id: 100.0})

This ensures that the logic in _compute_analytic_distribution works correctly when a brand with an analytic distribution is assigned to a contract line.

---

⏰ PR Aging Alert

This PR by @pcastelovigo has been open for 94 days (3 months).

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:

• 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