-
-
Notifications
You must be signed in to change notification settings - Fork 796
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.
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
[14.0][ADD] report_xlsx_pdf: print directly to PDF from xlsx #637
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
add in manifest |
f680c05
to
94b7689
Compare
why this error in runboat? ++ oca_list_external_dependencies deb
however for me it is ok |
|
||
For testing it is also necessary ``xlrd`` Python module installed:: | ||
|
||
$ pip3 install xlrd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add libreoffice installed as a requirement as well (only on Linux systems for now).
report_xlsx_pdf/models/ir_report.py
Outdated
_inherit = "ir.actions.report" | ||
|
||
report_xlsx_to_pdf = fields.Boolean( | ||
string="Report xlsx to pdf", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string="Report xlsx to pdf", | |
string="Export to pdf format", |
report_xlsx_pdf/__manifest__.py
Outdated
"name": "Extend report xlsx to export directly to pdf", | ||
"summary": "Extend report xlsx to export directly to pdf", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"name": "Extend report xlsx to export directly to pdf", | |
"summary": "Extend report xlsx to export directly to pdf", | |
"name": "Export xlsx report to pdf", | |
"summary": "Extend report_xlsx to export directly to pdf", |
@@ -0,0 +1 @@ | |||
This module extend xlsx report to convert in pdf using libreoffice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This module extend xlsx report to convert in pdf using libreoffice | |
This module extend ``report_xlsx`` to convert xlsx to pdf using libreoffice. |
94b7689
to
634cd7e
Compare
I'm at loss here. It's looking for the wrong file, the correct link is:
not
but I'm not sure how to fix it. @sbidoul (sorry to bother you) I might be completely off track, tho. I've never looked at those scripts before. |
@TheMule71 |
OK, but tests are still failing. |
@TheMule71 yes that's what we need to do. There is an open issue for that in oca/oca-ci (OCA/oca-ci#26). Would you like to do a PR for that ? |
Now it is green and ready to review 😄 |
Thanks!
Looks like @etobella beat me to that. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks!
merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Readme is not complete.
Tests are for report_xlsx, can you test the transformation?
@@ -0,0 +1,10 @@ | |||
Make sure you have ``xlsxwriter`` Python module installed:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comes from report_xlsx, so it is probably not needed here. The only thing needed is libreoffice in this case, as xlsxwriter comes from report_xlsx
active_model="res.partner" | ||
) | ||
self.report_name = "report_xlsx.partner_xlsx" | ||
self.report_xlsx_to_pdf = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this. It is never used... You never tested that is is transformed. This test is for report_xlsx standard. Can you test the code from this module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.report_xlsx_to_pdf = True
is the new parameter for this app
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But you are editing self, in this case, the TestClass, it does not make sense. You should do self.xlsx_report.report_xlsx_to_pdf = True
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andreampiovesana can you attend this comment? Thanks!
for new merge of PR #628 |
634cd7e
to
ea5a63a
Compare
merge? |
@andreampiovesana have @etobella 's remarks been addressed? |
self.report_xlsx_to_pdf = True
is the specific parameter
right?
Il giorno sab 27 ago 2022 alle ore 22:13 Enric Tobella <
***@***.***> ha scritto:
… ***@***.**** requested changes on this pull request.
Readme is not complete.
Tests are for report_xlsx, can you test the transformation?
------------------------------
In report_xlsx_pdf/readme/USAGE.rst
<#637 (comment)>:
> @@ -0,0 +1,31 @@
+An example of XLSX report for partners on a module called `module_name`:
This usage comes from report_xlsx it is probably not needed, isn't it?
The real usage is showing how the flag is used, Can you change it?
------------------------------
In report_xlsx_pdf/readme/INSTALL.rst
<#637 (comment)>:
> @@ -0,0 +1,10 @@
+Make sure you have ``xlsxwriter`` Python module installed::
This comes from report_xlsx, so it is probably not needed here. The only
thing needed is libreoffice in this case, as xlsxwriter comes from
report_xlsx
------------------------------
In report_xlsx_pdf/tests/test_report.py
<#637 (comment)>:
> +
+try:
+ from xlrd import open_workbook
+except ImportError:
+ _logger.debug("Can not import xlrd`.")
+
+
+class TestReport(common.TransactionCase):
+ def setUp(self):
+ super(TestReport, self).setUp()
+ report_object = self.env["ir.actions.report"]
+ self.xlsx_report = self.env["report.report_xlsx.abstract"].with_context(
+ active_model="res.partner"
+ )
+ self.report_name = "report_xlsx.partner_xlsx"
+ self.report_xlsx_to_pdf = True
I don't understand this. It is never used... You never tested that is is
transformed. This test is for report_xlsx standard. Can you test the code
from this module?
—
Reply to this email directly, view it on GitHub
<#637 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADKOJA4KGUMTTVUYVAO24CTV3JZGVANCNFSM54IHGR5A>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
@OCA/reporting-engine-maintainers we use this app in production and this fix many situations where print in qweb-pdf take too much time to export, specially with report with hundreds of pages and we use the same technology of py3o. |
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. |
ping @etobella |
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. |
Reopening, as the necessary change is minimal. Can you review it @TheMule71 @andreampiovesana |
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor changes required, after that, it can be merged :)
active_model="res.partner" | ||
) | ||
self.report_name = "report_xlsx.partner_xlsx" | ||
self.report_xlsx_to_pdf = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andreampiovesana can you attend this comment? Thanks!
{ | ||
"name": "Export xlsx report to pdf", | ||
"summary": "Extend report_xlsx to export directly to pdf", | ||
"author": "Openindustry.it," "Odoo Community Association (OCA)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"author": "Openindustry.it," "Odoo Community Association (OCA)", | |
"author": "Openindustry.it, Odoo Community Association (OCA)", |
Extend report xlsx to export directly to pdf.