[16.0][MIG] analytic brand

[pcastelovigo]

using oca-port

[marielejeune]

Please see change commit that was applied in v18.
I think this is relevant in v16 too, as Odoo changed analytic accounts to analytic distributions.
4b05ed0

[pcastelovigo]

Please see change commit that was applied in v18. I think this is relevant in v16 too, as Odoo changed analytic accounts to analytic distributions. 4b05ed0

yes, it seems it need to be adapted to distributions,
i'll try to do this week, thanks

[pcastelovigo]

@marielejeune i think its ready

[marielejeune]

Please follow migration guidelines: https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-16.0

All the commit history is squashed, and the migration commit doesn't contain anything.

[marielejeune]

LGTM.

Note that I'm currently facing some issues/difficulties using this module in v18.
The reason is that the analytic distribution is defined on the brand, while other composants use the analytic distribution models.

I think I will modify the module to add the brand dimension inside analytic distribution models too. I think this is something that could be useful for v16 too.

[pcastelovigo]

LGTM.

Note that I'm currently facing some issues/difficulties using this module in v18. The reason is that the analytic distribution is defined on the brand, while other composants use the analytic distribution models.

I think I will modify the module to add the brand dimension inside analytic distribution models too. I think this is something that could be useful for v16 too.

I expected it to add automatically the analytic tag to account.moves using that brand but it does not work that way, it was also in the precedent versions.
I had code to add them with used with account_brand.. I'll check your work in the future

[marielejeune]

Hi @pcastelovigo,
Please check at my refactoring to use account analytic distribution models, ready here: #286

[WesleyOliveira98]

LGTM

[OCA-git-bot]

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

[sbejaoui]

/ocabot merge nobump

[OCA-git-bot]

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-283-by-sbejaoui-bump-nobump, awaiting test results.

[OCA-git-bot]

@sbejaoui your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-283-by-sbejaoui-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

[OCA-git-bot]

@sbejaoui your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-283-by-sbejaoui-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

[marcos-mendez]

Automated Review -- Tests Failed

1. Root Cause

The test failure is caused by a missing or incorrect assignment of an analytic distribution to a brand in the analytic_brand module. This leads to a failure when the system tries to retrieve or use the default analytic account associated with a brand, particularly in contexts where analytic accounting is enabled.

2. Suggested Fix

In the analytic_brand module, ensure that when a brand is created or updated, the analytic_distribution field is properly assigned. Specifically, check the brand.py file (likely in models/brand.py) around line where analytic_distribution is handled. The assignment should be done during brand creation or update to ensure it's available for use in analytic accounting contexts.

3. Additional Code Issues

• Missing validation: There is no validation to ensure that the analytic_distribution is valid or correctly formatted when assigned to a brand. This could lead to runtime errors if invalid data is passed.
• Potential race condition: If multiple processes attempt to set the analytic_distribution simultaneously, there may be a race condition that results in incorrect data being saved.

4. Test Improvements

Add tests to cover the following scenarios using TransactionCase or SavepointCase:

• Test brand creation with analytic distribution: Ensure that when a brand is created with an analytic distribution, it is correctly stored and retrievable.
• Test brand update with analytic distribution: Verify that updating a brand's analytic distribution works correctly and persists the change.
• Test usage in analytic accounting context: Ensure that the analytic distribution is correctly used as a default value in contexts where analytic accounting is active.
• Test invalid analytic distribution: Validate that invalid or malformed analytic distributions are handled gracefully (e.g., by raising appropriate exceptions or logging warnings).

These tests should be tagged with @tag('analytic_brand') to ensure they are run as part of the module's test suite.

---

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

[rvalyi]

/ocabot merge nobump

[OCA-git-bot]

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-283-by-rvalyi-bump-nobump, awaiting test results.

[OCA-git-bot]

@rvalyi your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-283-by-rvalyi-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

[OCA-git-bot]

@rvalyi your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-283-by-rvalyi-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

[marcos-mendez]

Automated Review -- Tests Failed

1. Root cause of the test failure

The test failure is caused by a missing Python dependency openupgradelib, which is declared in the module's manifest under external_dependencies but not installed in the test environment. This prevents Odoo from loading the analytic_brand module.

2. Suggested fix

Ensure that openupgradelib is installed in the test environment. If using a CI or test runner like odoo-test, add openupgradelib to the list of required Python packages. Alternatively, if the migration script is not essential for basic functionality, consider removing openupgradelib from external_dependencies and moving the migration logic to a separate, optional upgrade script.

3. Additional code issues

• Missing field definition in res_brand.py: The model inherits from analytic.mixin but does not explicitly define the analytic_distribution field. This may lead to unexpected behavior or missing fields in views.
  • File: analytic_brand/models/res_brand.py
  • Suggestion: Add explicit field definition for analytic_distribution or ensure analytic.mixin properly defines it.

4. Test improvements

To better cover this module:

• Add a TransactionCase test to verify that analytic_distribution is correctly set on a brand and used in analytic accounting contexts.
• Add a SavepointCase test to validate the migration script (pre-migrate.py) logic, especially the ALTER TABLE and UPDATE SQL queries.
• Add a test case that checks field visibility or default values in views when analytic.mixin is used.
• Use tagged tests for migration-related logic to ensure they are only run when needed.

Example test snippet:

def test_analytic_distribution_default(self):
    brand = self.env['res.brand'].create({'name': 'Test Brand'})
    self.assertEqual(brand.analytic_distribution, {})

This follows OCA patterns for testing extension hooks and field behavior.

---

⏰ PR Aging Alert

This PR by @pcastelovigo has been open for 112 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:

• 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

[pcastelovigo]

CI fix in #296