[15.0][MIG] report_qweb_pdf_watermark - #680
Conversation
bosd
commented
Nov 3, 2022
- Standard migration
- Updated instructions
- Added Global Company Watermark
When no background configuration is present on the report definition, don't try to browse the report model especially because this breaks some upstream reports (see odoo/odoo#23389)
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: reporting-engine-11.0/reporting-engine-11.0-report_qweb_pdf_watermark Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-11-0/reporting-engine-11-0-report_qweb_pdf_watermark/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: reporting-engine-12.0/reporting-engine-12.0-report_qweb_pdf_watermark Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-12-0/reporting-engine-12-0-report_qweb_pdf_watermark/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: reporting-engine-12.0/reporting-engine-12.0-report_qweb_pdf_watermark Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-12-0/reporting-engine-12-0-report_qweb_pdf_watermark/
external_dependencies
b4c6518 to
2d81120
Compare
[MIG] report_qweb_pdf_watermark: Migration to 15.0
2d81120 to
0f64e92
Compare
|
@kittiu Can you please review? |
|
@grzana12 Can you please review? |
|
@CasVissers-360ERP Can you review? |
|
This PR has the |
| _inherit = "ir.actions.report" | ||
|
|
||
| pdf_watermark = fields.Binary("Watermark") | ||
| use_company_watermark = fields.Boolean( |
There was a problem hiding this comment.
A migration script is needed for this case
There was a problem hiding this comment.
@etobella Could you explain why an migration script would be needed?
The field pdf_watermark still exsists. See line 43
14.0...bosd:reporting-engine:15.0-mig-report_qweb_pdf_watermark#diff-66dceb063022cd58404dbbbb4649ffec677cbc89be10c385ad5714231929a220R34
The other field are a new addition, and should not cause a problem when migrating, right?
There was a problem hiding this comment.
If you migrate you will need to access all reports with watermarks in order to set the boolean to true. That is the migration needed...
There was a problem hiding this comment.
Oh, now I think I understand.
By toggling the boolean you want to migrate the exsisting watermarks on the reports to the new use_company_watermark function.
I don’t think we want a migration script for that 😄
The use_company_watermark is an additional new function. 🎉
The current functionality of the pdf_watermark which can be defined on a per report basis is kept as is.
In the current situation after migrating to V15 the watermark on these reports stay unchanged.
So the result on V15 will look exactly the same as on V14. This is what we want. 👍
For example if we would like to automatically migrate exsisting reports to the new use_company_watermark function things will get complicated and error prone.
Example:
If this module is installed on a V14 database one could have the situation that there is a different watermark specified for a SO report then a Warehouse out report.
E.g.
An SO report could have set an pdf_watermark.
mywatermark1.pdf
The Warehouse delivery report could have mywatermark2.pdf
Migrating existing pdf watermarks to the new use_global_watermark function is becoming complicated very quickly and could lead to unwanted results when done by a migration script.
In above situation the script must decide to either set mywatermark1.pdf or mywatermark2.pdf as the global company watermark.
If there are even more then 3 watermark pdf’sor custom regexes used things get unnecessary complicated.
In that scenario, it is much easier for an human to change to the global company watermark function trough the UI for those who want to use it.
There was a problem hiding this comment.
@etobella Do you agree with with above explanation of the way of working?
Can we merge this one?
There was a problem hiding this comment.
Ouch, I see, sorry for the misunderstanding
There was a problem hiding this comment.
NP, Thanks for the review/merge!!
|
/ocabot migration report_qweb_pdf_watermark |
|
Sorry @bosd you are not allowed to mark the addon tobe migrated. 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 |
|
/ocabot migration report_qweb_pdf_watermark |
|
/ocabot merge nobump |
|
What a great day to merge this nice PR. Let's do it! |
|
Congratulations, your PR was merged at 68c6841. Thanks a lot for contributing to OCA. ❤️ |