[9.0] Merge syleam printers module into base_report_to_printer #60

Garamotte · 2016-10-23T16:51:12Z

I'm currently adding additional features of Syleam's printers module into base_report_to_printer :

Add printing.server model : Management of multiple cups servers
Add printing.job model : Cups jobs tracking into Odoo
Add some methods on printing.printer model : cancell_all_jobs, enable, disable, etc.
Update cron jobs
Add migration script to create a printing.server record from configuration for existing databases

After this PR is merged, I plan to adapt our ZPL II labels printing module to work with base_report_to_printer.

Garamotte

This PR is ready to be reviewed.

There is a remaining question (inline comment), everything else should be OK.

Garamotte · 2016-10-30T07:36:40Z

base_report_to_printer/models/printing_printer.py

+        options = self.print_options(
+            report=report, format=format, copies=copies)
+        # TODO : Check if this works in all cases
+        mime_type = 'application/octet-stream'


Last thing to check carefully : I didn't found any way to get the mime type of a variable's contents in python.

Does this type work for all file types and all printer drivers ?
If it doesn't, does someone know a way to find the mime type ?

Garamotte · 2016-10-31T10:02:26Z

I removed the rewrite of the print_document method, because printing with writeRequestData needs the last fixes commited in pycups, which are not released yet (the current version has a bug with binary data). We'll have to wait for the pycups 1.9.74 release for this to work.
I kept the commit on a separate branch, to propose another PR when pycups' fix is released.

yvaucher

Nice workI! I only have few cosmetic code changes to propose but not blocking.

yvaucher · 2016-11-01T16:42:32Z

base_report_to_printer/models/printing_server.py

+        help='List of printers available on this server.')
+
+    @api.multi
+    def _openConnection(self, raise_on_error=False):


I understand that you used camel case as you open a Connection however you get the instance which is connection thus could we keep snake case here?

You're right, I'll change that.

yvaucher · 2016-11-01T16:47:34Z

base_report_to_printer/models/printing_server.py

+                        'aborted',
+                        'completed',
+                    )),
+                    ('active', '=', True),


The active check is not necessary as applied by default

Right, I don't remember why it was put here.

yvaucher · 2016-11-01T16:53:20Z

base_report_to_printer/models/printing_server.py

+
+            # Retrieve known uncompleted jobs data to update them
+            if which == 'not-completed':
+                min_jobs = job_obj.search([


as you search with a limit=1 variable should be singular.
Plus what does min means? is it to select the oldest unfinished job ?

Yes, min means that we search for the oldest : order='jobid'.
This is used in CUPS' getJob method arguments as a filter (first_job_id=min_job.jobid), there is no need to retrieve the full list, but only jobs not known as completed in Odoo's database.

yvaucher · 2016-11-01T16:53:52Z

base_report_to_printer/models/printing_server.py

+                    ('active', '=', True),
+                ], limit=1, order='jobid')
+                if min_jobs:
+                    min_job = min_jobs[0]


No need for that with a proper variable name and the limit=1

This is a remaining part of the old API, I'll fix that.

* Set api.multi for action called as `object` on view * Merge syleam printers module into base_report_to_printer (#60) * [IMP] Updated unit tests * [FIX] Fixed renamed attributes * [FIX] Remove deleted fields * [IMP] Add printing.server and printing.job models * [IMP] Allow to cancel all jobs, enable, and disable printers * [IMP] Split the cups part of print_document into a new print_file method * [IMP] Updated cron job to run the action_update_jobs method * [ADD] Add a migration script to create a printing server from configuration * [MIG] Migrate base_report_to_printer to v10.0 Removed deprecated methods on printing.printer (replaced by methods on printing.server) * [IMP] Add wkhtmltopdf in travis configuration file * [FIX] base_report_to_printer: Fix Update Job Cron * Fix API issue with Update Job Cron ** Forward Port from 9.0 * [FIX] Fixed the res.users view The string attribute should not be used as a selector, because it is translatable. * [FIX] Fixed the print_document method of report The new API migration was made to @api.multi because of the "cr, uid, ids" signature, but "ids" was the ids of the records to print here, not the report's ids. Also, the new API version of "get_pdf" get directly the ids of the records to print in the standard module, not a recordset. * [FIX] UI is now (un)blocked only when using qweb-pdf reports in standard addons

* Set api.multi for action called as `object` on view * Merge syleam printers module into base_report_to_printer (OCA#60) * [IMP] Updated unit tests * [FIX] Fixed renamed attributes * [FIX] Remove deleted fields * [IMP] Add printing.server and printing.job models * [IMP] Allow to cancel all jobs, enable, and disable printers * [IMP] Split the cups part of print_document into a new print_file method * [IMP] Updated cron job to run the action_update_jobs method * [ADD] Add a migration script to create a printing server from configuration * [MIG] Migrate base_report_to_printer to v10.0 Removed deprecated methods on printing.printer (replaced by methods on printing.server) * [IMP] Add wkhtmltopdf in travis configuration file * [FIX] base_report_to_printer: Fix Update Job Cron * Fix API issue with Update Job Cron ** Forward Port from 9.0 * [FIX] Fixed the res.users view The string attribute should not be used as a selector, because it is translatable. * [FIX] Fixed the print_document method of report The new API migration was made to @api.multi because of the "cr, uid, ids" signature, but "ids" was the ids of the records to print here, not the report's ids. Also, the new API version of "get_pdf" get directly the ids of the records to print in the standard module, not a recordset. * [FIX] UI is now (un)blocked only when using qweb-pdf reports in standard addons

In version 9.0.2.0.0, added in OCA#60, the cron was modified. However, since it is a noupdate=1 record, preinstalled instances started failing there. This fixes it. @Tecnativa TT18838

* Set api.multi for action called as `object` on view * Merge syleam printers module into base_report_to_printer (OCA#60) * [IMP] Updated unit tests * [FIX] Fixed renamed attributes * [FIX] Remove deleted fields * [IMP] Add printing.server and printing.job models * [IMP] Allow to cancel all jobs, enable, and disable printers * [IMP] Split the cups part of print_document into a new print_file method * [IMP] Updated cron job to run the action_update_jobs method * [ADD] Add a migration script to create a printing server from configuration * [MIG] Migrate base_report_to_printer to v10.0 Removed deprecated methods on printing.printer (replaced by methods on printing.server) * [IMP] Add wkhtmltopdf in travis configuration file * [FIX] base_report_to_printer: Fix Update Job Cron * Fix API issue with Update Job Cron ** Forward Port from 9.0 * [FIX] Fixed the res.users view The string attribute should not be used as a selector, because it is translatable. * [FIX] Fixed the print_document method of report The new API migration was made to @api.multi because of the "cr, uid, ids" signature, but "ids" was the ids of the records to print here, not the report's ids. Also, the new API version of "get_pdf" get directly the ids of the records to print in the standard module, not a recordset. * [FIX] UI is now (un)blocked only when using qweb-pdf reports in standard addons

Garamotte force-pushed the 9.0-merge-syleam-printers branch 10 times, most recently from 115884a to 3760fc0 Compare October 23, 2016 21:52

lasley added the work in progress label Oct 25, 2016

lasley added this to the 9.0 milestone Oct 25, 2016

Garamotte force-pushed the 9.0-merge-syleam-printers branch 7 times, most recently from 3b33529 to 76078aa Compare October 30, 2016 07:30

Garamotte commented Oct 30, 2016

View reviewed changes

Garamotte changed the title ~~[9.0][WIP] Merge syleam printers module into base_report_to_printer~~ [9.0] Merge syleam printers module into base_report_to_printer Oct 30, 2016

Garamotte force-pushed the 9.0-merge-syleam-printers branch 4 times, most recently from a98d810 to c5d2c38 Compare October 31, 2016 09:59

Garamotte force-pushed the 9.0-merge-syleam-printers branch from c5d2c38 to 72dea46 Compare October 31, 2016 13:37

yvaucher approved these changes Nov 1, 2016

View reviewed changes

yvaucher added needs review and removed work in progress labels Nov 1, 2016

Garamotte mentioned this pull request Nov 24, 2016

[10.0][MIG] base_report_to_printer #68

Merged

yajo mentioned this pull request Dec 23, 2019

[FIX] base_report_to_printer: Migrate noupdate record #195

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[9.0] Merge syleam printers module into base_report_to_printer #60

[9.0] Merge syleam printers module into base_report_to_printer #60

Garamotte commented Oct 23, 2016 •

edited

Loading

Garamotte left a comment

Garamotte Oct 30, 2016

Garamotte commented Oct 31, 2016

yvaucher left a comment

yvaucher Nov 1, 2016

Garamotte Nov 1, 2016

yvaucher Nov 1, 2016

Garamotte Nov 1, 2016

yvaucher Nov 1, 2016

Garamotte Nov 1, 2016

yvaucher Nov 1, 2016

Garamotte Nov 1, 2016

[9.0] Merge syleam printers module into base_report_to_printer #60

[9.0] Merge syleam printers module into base_report_to_printer #60

Conversation

Garamotte commented Oct 23, 2016 • edited Loading

Garamotte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Garamotte commented Oct 31, 2016

yvaucher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Garamotte commented Oct 23, 2016 •

edited

Loading