[16.0][IMP] choose between brand or company values in layout

[Kev-Roche]

Inspired of #180 and could replace it.
Here a way to select and use brand fields on reports and avoid the missing field error if a module add a new field on res.company

[OCA-git-bot]

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

[bosd]

Functional test! :+1
Thanks for this fix!

[bosd]

Actually some fields are still not overridden.

'report_header'
'company_details'
'report_layout_id'

To elaborate.
The error is gone. A report is generated. But it is rather a mixture of the report set on the company level and the brand level.

The colors are a mixture of both settings.
The tag line/header is not filled with the "brand" one.
Background image not working.

The adress still show the company adress. Instead of the adress of the Brand.

[bosd]

Oopsie part of this bug is actually fixed in #195

[metaminux]

Hi @bosd

Can you mark this as resolved to allow merging again ?

@OCA/brand-maintainers Can someone approve this PR ? Please...

[bosd]

Hi @bosd

Can you mark this as resolved to allow merging again ?

@OCA/brand-maintainers Can someone approve this PR ? Please...

Yeah, I could do that. But it is actually not resolved. So maybe better fix the functionality before merging.

[sbejaoui]

Apologies for the delayed review; I’ve been quite busy lately.

I understand the issue we’re trying to address here, but I have some concerns about the user experience.

IMO, we should continue adding fields to the brand model and let it evolve in line with how odoo evolves the company model. The purpose of this repository is to make the layout branded, and this solution may prevent us from noticing new fields added in each version that could break the layout.

For instance, in v16, the field is_company_detail_empty was introduced. With the proposed solution, this field would return True if it’s empty in the company object, even if the user filled it at the brand level. This issue would go unnoticed, and users might not understand why the brand details are not visible.

To summarize, I suggest keeping things as they are. During each migration, we can easily identify the layout changes introduced by odoo as they will inevitably breaks the layout, allowing us to address them appropriately.

[Kev-Roche]

Apologies for the delayed review; I’ve been quite busy lately.

I understand the issue we’re trying to address here, but I have some concerns about the user experience.

IMO, we should continue adding fields to the brand model and let it evolve in line with how odoo evolves the company model. The purpose of this repository is to make the layout branded, and this solution may prevent us from noticing new fields added in each version that could break the layout.

For instance, in v16, the field is_company_detail_empty was introduced. With the proposed solution, this field would return True if it’s empty in the company object, even if the user filled it at the brand level. This issue would go unnoticed, and users might not understand why the brand details are not visible.

To summarize, I suggest keeping things as they are. During each migration, we can easily identify the layout changes introduced by odoo as they will inevitably breaks the layout, allowing us to address them appropriately.

Thank you for taking the time to review this. I appreciate your feedback and would like to address your concerns.

• Conflict with evolving layouts:
  The issue isn’t limited to migrations. Whenever new fields are added to the company model, via custom or OCA modules, they can conflict with the layout if not anticipated. For example, in the Cooperative module, added fields could create conflicts or unexpected behaviors. The current approach requires manual adjustments for each new field, whereas the present PR minimizes such conflicts and ensures smoother integration.

• Ease of maintenance:
  My approach simplifies maintenance by centralizing field handling at the brand level. This avoids repetitive manual adjustments and ensures consistency across layouts, even when new fields are introduced.

To summarize, this PR proactively addresses conflicts during both migrations and modules installation, reducing bugs and maintenance effort while providing a more robust and user-friendly experience.

[bosd]

@Kev-Roche Can you (or ocabot) please rebase, So we can test it against the latest code on a fresh runboat?

[bosd]

@OCA-git-bot I know we're not friends... But here goes nothing..

/ocabot rebase

[OCA-git-bot]

Sorry @bosd you are not allowed to rebase.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

[Kev-Roche]

@Kev-Roche Can you (or ocabot) please rebase, So we can test it against the latest code on a fresh runboat?

Done !

[bosd]

Thanks, To keep you guys no longer waiting. here is my initial response.
But might need more time for thorough testing.

This following fields are still not made brand specific.

'company_details'
'report_layout_id'

Later i will test more.

Thinking out loud. Would it be possible to just inherit the whole company model with all of its fields?

[bosd]

Functional test on local system in combination with account brand.
It's not working :(

The only branded item changed in the report was the logo.
Other fields stayed the same.

Steps to reproduce. install account_brand set brand policy to optional
Create a brand.
Usability improvement suggestion: Assign a default layout, as Empty is not allowed.

Details

Configure branded document.

For completeness my test company settings:

Details

Create a unbranded invoice:

branded invoice:

Result:
🟢 Logo, Layout and document colors changed. Fonts
🔴 Header and footer not added.
🔴 Company details incorrect

untested: paperformat

[bosd]

Suggested change

	"layout_background_image",
	"layout_background_image",
	"company_details",

This is fixes one of the not overriden fields.

[metaminux]

Hi @bosd, @sbejaoui and @Kev-Roche

This PR is a replacement for #180 which was opened to fix #167 and #179.
The latter is about using this module in a multi-company setup.

Odoo native behavior is about configuring a report layout for each company.
This module is about using brands to define report layouts, but I don't think every piece of information should be overridden...

To add some context, one of my customer is a corporate group that is manufacturing and selling products mainly in France, UK, USA and Canada. Those products are sold under 4 different brands. So, there are 4 brands used in 5 companies.
They want to have a common style for each brand in all companies.
In those conditions, we just want to use the "style" fields from brands and keep the others from each companies. That's why we choose to override only a subset of fields.
For instance, we do want the footer to be different and related to the actual company for a same brand. Addresses should also refer to companies, not brands...

Now, I'm aware this subset could seem arbitrary, as already explained in comments of the original PR (see #180 (comment) and further comments)
The original PR was replacing "at rendering time" the company fields by the brand ones, so every field overridden had to be defined in brands and there were no way to use information from actual company.
This one is way "smarter" (thanks to @Kev-Roche) because if no value is set at brand level, we use the company value.
Maybe this is a bit disturbing at first sight and should be made more visible to end users... but I think this could allow to use brand report layout in multi-company environment and avoid conflicts with other modules.

I hope this comment will help going forward on that PR and stay open to all your thoughts and advice.

[bosd]

@metaminux Aha,
I see we have different use cases.
Thanks for the feedback on your use case.

We are using it for manufacturers / subcontractors. They have one facility and produce for different brands / entity.
They want to keep their setup simple and flat. Because a multi company setup would create intercompany (warehouse) transactions.
Instead we use the module to sent out branded invoices. (accounting is not handled by odoo in most cases).
Each brand will have their own logo and company details, header footer and such.

This one is way "smarter" (thanks to @Kev-Roche) because if no value is set at brand level, we use the company value.

Yes, That would allow usage for both of our uses cases.

Maybe this is a bit disturbing at first sight and should be made more visible to end users... but I think this could allow to use brand report layout in multi-company environment and avoid conflicts with other modules.

It would be a good addition to update the docs with this new functionality.

    def _get_company_overriden_fields(self):
        return {

Here the subset of fields to override is strict.
As discussed, this could lead to issues on maintainability.
Would it be an option to just check on all available field?
IMO, it would make it easier to migration as well.

https://github.com/OCA/brand/pull/180/files#diff-af7160d79a4a7e74dfb1e70cd2d4f512bd7b5c5e2ffe73c4e05fb182c77bee2dR109-R121

In it's current form, with only the minor subset included. It is changing the behaviour of the module.
E.g. breaking our use case, which we had deployed at a handfull of clients for couple of years on different versions.

[christian-ramos-tecnativa]

Hello @Kev-Roche, as @bosd said, IMO retrieving the company values dynamically should cover every use case. Could you apply the change to test it? Thanks in advance

[github-actions]

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

[metaminux]

Hi everyone,

To avoid this PR being closed...

I'll try to add missing fields to _get_company_overriden_fields so that @bosd use case explained earlier is OK
We'll override the method in our customer specific module to fit our use case.

Regards,

[github-actions]

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

[bosd]

To avoid this PR being closed...

1 day left for auto closing. If i did not reply today :)

[marcos-mendez]

Automated Review -- Tests Failed

1. Root Cause

The test failures in web_editor JS tests are caused by a change in how company data is passed to report templates. Specifically, the company variable now refers to a CompanyBrandAdapter object instead of a raw res.company record, which affects how fields are accessed and rendered in the report template, particularly in contexts involving branding overrides.

2. Suggested Fix

In brand_external_report_layout/models/res_company.py, the CompanyBrandAdapter class does not inherit from models.Model, so it cannot be used directly in Odoo's templating system (t-value assignment in XML). The adapter should be replaced with a proper Odoo model or a dictionary-based approach that can be correctly interpreted in Jinja2 templates.

Specifically, modify res_company.py to avoid using CompanyBrandAdapter as a direct replacement for res.company.

# Replace the adapter class with a method that returns a dict or a proper model instance
# Or refactor to use a proxy model or a simple dict-based approach

3. Additional Code Issues

• Missing __init__.py in brand_external_report_layout/models: While the __init__.py is updated, the res_company.py file is added without ensuring it's properly imported or that its class is used correctly in the context of the report rendering logic.
• Inheritance of CompanyBrandAdapter: The class inherits from object and does not use models.Model or any Odoo ORM features, which may lead to unexpected behavior when used in templates or ORM contexts.

4. Test Improvements

Add tests in brand_external_report_layout/tests using SavepointCase or TransactionCase to cover:

• Report rendering with brand override: Ensure that when a brand is assigned to a company and report layout is overridden, the correct fields are rendered.
• Field inheritance behavior: Verify that CompanyBrandAdapter correctly merges company and brand fields.
• Template context handling: Test that the company context variable in the report template behaves as expected when overridden by a brand.

Example test case pattern:

def test_report_template_with_brand_override(self):
    # Create a brand with a custom layout
    brand = self.env['res.brand'].create({
        'name': 'Test Brand',
        'external_report_layout_id': layout.id,
        'logo': base64.b64encode(b"fake_logo"),
    })
    # Assign brand to a company
    company = self.env['res.company'].create({
        'name': 'Test Company',
        'brand_id': brand.id,
    })
    # Render report and assert fields are correctly overridden

Use @tag('post_install') or @tag('standard') to categorize tests appropriately in OCA-style test suites.

---

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 Passed

All tests for brand_external_report_layout passed successfully on Odoo 16.0.
Environment: Minikube + K8s Job + oca-ci/py3.10-odoo16.0

---

Test Coverage Suggestions

Coverage Gaps

The new CompanyBrandAdapter class and its usage in res_company.py introduces logic that is not directly tested:

• The CompanyBrandAdapter.__init__ method dynamically sets attributes based on field presence and inheritance logic.
• The _get_company_brand_adapter method is used in the report template, but its behavior under various brand/company field combinations is not tested.
• The _get_company_overriden_fields method returns a hardcoded list; no test ensures this list is consistent with actual report layout fields.

Suggested Test Cases

def test_company_brand_adapter_field_override(self):
    """Test that CompanyBrandAdapter correctly overrides company fields with brand values."""

def test_company_brand_adapter_default_fields(self):
    """Test that CompanyBrandAdapter falls back to company values when brand fields are empty."""

def test_get_company_overriden_fields_consistency(self):
    """Test that _get_company_overriden_fields returns expected fields for report layout."""

Codecov Risk

• CompanyBrandAdapter.__init__ and ResCompany._get_company_brand_adapter are new code paths that could reduce coverage if not tested.
• The new res_company.py model file adds 24 lines, which should be covered by tests to maintain 80%+ threshold.

✅ Note: Existing tests likely cover the report rendering path, but not the core adapter logic. Adding tests for the adapter ensures robustness of the new feature.

---

⚠️ PR Aging Alert: CRITICAL

This PR by @Kev-Roche has been waiting for 651 days — that is over 21 months without being merged or closed.

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

---

Reciprocal Review Request

Hi everyone! Thanks for your contribution to OCA. I reviewed and approved this PR. If any of you have a moment, I would really appreciate a review on 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 faster. Thank you!

---

Automated review by OCA Neural Reviewer + qwen3-coder:30b