Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[12.0][IMP] - manage different report layouts by brand #16

Merged
merged 5 commits into from
Feb 9, 2020
Merged

[12.0][IMP] - manage different report layouts by brand #16

merged 5 commits into from
Feb 9, 2020

Conversation

sbejaoui
Copy link
Contributor

@sbejaoui sbejaoui commented Nov 15, 2019 •
edited
Loading

Fixes #758

Manage different report layouts by brand:

1

2

3

4

@sbejaoui sbejaoui force-pushed the 12.0-brand_external_report_layout-sbj branch from 789e8be to ad21b3f Compare November 15, 2019 12:10
@sbejaoui sbejaoui added this to the 12.0 milestone Nov 15, 2019
@sbejaoui sbejaoui force-pushed the 12.0-brand_external_report_layout-sbj branch 2 times, most recently from a7d1cb8 to f7ab106 Compare November 15, 2019 12:41
@sbejaoui
Copy link
Contributor Author

FYI @gva-acsone

@sbejaoui sbejaoui force-pushed the 12.0-brand_external_report_layout-sbj branch from 8d56c54 to a887006 Compare November 15, 2019 17:07
@sbejaoui sbejaoui mentioned this pull request Nov 15, 2019
Closed
sbidoul
sbidoul reviewed Nov 15, 2019
View reviewed changes
Copy link
Member

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good a few comments.

brand_external_report_layout/models/res_brand.py Outdated Show resolved Hide resolved
brand_external_report_layout/models/res_brand.py Outdated Show resolved Hide resolved
brand_external_report_layout/models/res_brand.py Outdated Show resolved Hide resolved
brand_external_report_layout/views/res_brand.xml Show resolved Hide resolved
brand_external_report_layout/views/report_template.xml
<xpath expr="//t[@t-if='not company']" position="after">
<t t-if="'brand_id' in o.fields_get() and o.brand_id and o.brand_id.external_report_layout_id"
t-call="{{o.brand_id.external_report_layout_id.key}}">
<t t-set="company" t-value="o.brand_id.sudo()"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So yes, this is a little bit of a hack, but I think it's worth it for the sake of code simplicity.
Knowing that reports normally don't use this field and rely on the record company_id field, this should not be an issue in practice, as explained in the README.

brand_external_report_layout/readme/CREDITS.rst Outdated Show resolved Hide resolved
brand_external_report_layout/views/report_template.xml Show resolved Hide resolved
@sbejaoui sbejaoui mentioned this pull request Feb 7, 2020
Closed
dreispt
dreispt approved these changes Feb 7, 2020
View reviewed changes
Copy link
Sponsor Member

@dreispt dreispt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried and tested.

@dreispt
Copy link
Sponsor Member

dreispt commented Feb 7, 2020

I understand that @sbidoul is also OK to merge.

@sbidoul
Copy link
Member

sbidoul commented Feb 7, 2020

Yes.

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 12.0-ocabot-merge-pr-16-by-sbidoul-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Feb 7, 2020
@OCA-git-bot
Merge PR #16 into 12.0
960538c 
Signed-off-by sbidoul
@OCA-git-bot
Copy link
Contributor

@sbidoul your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-16-by-sbidoul-bump-patch.

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.

@sbejaoui sbejaoui force-pushed the 12.0-brand_external_report_layout-sbj branch from 595bed6 to 0a8b112 Compare February 9, 2020 16:57
@sbejaoui
Copy link
Contributor Author

sbejaoui commented Feb 9, 2020

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 12.0-ocabot-merge-pr-16-by-sbejaoui-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Feb 9, 2020
@OCA-git-bot
Merge PR #16 into 12.0
ec06a8b 
Signed-off-by sbejaoui
@OCA-git-bot OCA-git-bot merged commit 0a8b112 into OCA:12.0 Feb 9, 2020
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 9e1eb1f. Thanks a lot for contributing to OCA. ❤️

@sbidoul sbidoul deleted the 12.0-brand_external_report_layout-sbj branch February 18, 2020 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dreispt dreispt dreispt approved these changes

@max3903 max3903 Awaiting requested review from max3903

@osi-scampbell osi-scampbell Awaiting requested review from osi-scampbell

@sbidoul sbidoul Awaiting requested review from sbidoul

Assignees
No one assigned
Labels
merged 🎉 needs review
Projects
None yet
Milestone
12.0
Development

Successfully merging this pull request may close these issues.

[IMP] partner_brand
4 participants
@sbejaoui @dreispt @sbidoul @OCA-git-bot