From 3eb0c70acbc7105c6bb740fb4cd1b7beb60f599c Mon Sep 17 00:00:00 2001 From: Jairo Llopis Date: Mon, 9 Apr 2018 09:53:41 +0100 Subject: [PATCH] [IMP] base_report_to_printer: Fix XMLRPC calls and tests - When added the patch to be able to configure report copies by report object, the expected signature changed to expect a report object. That can't work through the XMLRPC interface, so we revert it to what it was before: expecting the report name. - Some tests that were producing warnings are muted now. - The tests that had been changed went back to normal too. - Current implementation didn't produce the expected results when actually forcing to print 1 copy. - Added a demo report to test, since searching the database for the 1st unkonwn report found is not very deterministic and can lead to problems, like those addressed in #122 and #123. - Finally, this update requires a database upgrade, so I pushed correctly the manifest version too. --- base_report_to_printer/__manifest__.py | 11 ++++++---- base_report_to_printer/demo/report.xml | 20 +++++++++++++++++++ .../models/printing_printer.py | 12 +++++++---- base_report_to_printer/models/report.py | 8 ++++++-- .../tests/test_printing_job.py | 4 ++++ .../tests/test_printing_printer.py | 20 ++++++++++++++----- .../tests/test_printing_printer_wizard.py | 4 ++++ .../tests/test_printing_server.py | 5 +++++ base_report_to_printer/tests/test_report.py | 2 +- 9 files changed, 70 insertions(+), 16 deletions(-) create mode 100644 base_report_to_printer/demo/report.xml diff --git a/base_report_to_printer/__manifest__.py b/base_report_to_printer/__manifest__.py index 0049e43..206fd22 100644 --- a/base_report_to_printer/__manifest__.py +++ b/base_report_to_printer/__manifest__.py @@ -8,11 +8,11 @@ { 'name': "Report to printer", - 'version': '10.0.1.0.4', + 'version': '10.0.2.0.0', 'category': 'Generic Modules/Base', - 'author': "Agile Business Group & Domsense, Pegueroles SCP, NaN," - "LasLabs, Odoo Community Association (OCA)", - 'website': 'http://www.agilebg.com', + 'author': "Agile Business Group & Domsense, Pegueroles SCP, NaN, " + "LasLabs, Tecnativa, Odoo Community Association (OCA)", + 'website': 'https://github.com/OCA/report-print-send', 'license': 'AGPL-3', "depends": ['report'], 'data': [ @@ -27,6 +27,9 @@ 'views/ir_actions_report_xml_view.xml', 'wizards/printing_printer_update_wizard_view.xml', ], + 'demo': [ + 'demo/report.xml', + ], 'installable': True, 'application': True, 'external_dependencies': { diff --git a/base_report_to_printer/demo/report.xml b/base_report_to_printer/demo/report.xml new file mode 100644 index 0000000..6999a6d --- /dev/null +++ b/base_report_to_printer/demo/report.xml @@ -0,0 +1,20 @@ + + + + + + + + + + diff --git a/base_report_to_printer/models/printing_printer.py b/base_report_to_printer/models/printing_printer.py index bbbee97..0dc935d 100644 --- a/base_report_to_printer/models/printing_printer.py +++ b/base_report_to_printer/models/printing_printer.py @@ -73,6 +73,7 @@ class PrintingPrinter(models.Model): } return vals + # TODO Rename param report to report_name, to make behavior obvious @api.multi def print_options(self, report=None, format=None, copies=1): """ Hook to set print options """ @@ -83,8 +84,9 @@ class PrintingPrinter(models.Model): options['copies'] = str(copies) return options + # TODO Rename param report to report_name, to make behavior obvious @api.multi - def print_document(self, report, content, format, copies=1): + def print_document(self, report, content, format, copies=None): """ Print a file Format could be pdf, qweb-pdf, raw, ... @@ -96,17 +98,19 @@ class PrintingPrinter(models.Model): os.write(fd, content) finally: os.close(fd) - if copies == 1: + report_obj = self.env["report"]._get_report_from_name(report) + if copies is None: # If number of copies is not indicated by argument, check context # or report definition copies = ( self.env.context.get('report_copies') or - (report and report.report_copies) or - copies + report_obj.report_copies or + 1 ) return self.print_file( file_name, report=report, copies=copies, format=format) + # TODO Rename param report to report_name, to make behavior obvious @api.multi def print_file(self, file_name, report=None, copies=1, format=None): """ Print a file """ diff --git a/base_report_to_printer/models/report.py b/base_report_to_printer/models/report.py index 28ca298..1c21396 100644 --- a/base_report_to_printer/models/report.py +++ b/base_report_to_printer/models/report.py @@ -20,7 +20,11 @@ class Report(models.Model): raise exceptions.Warning( _('No printer configured to print this report.') ) - return printer.print_document(report, document, report.report_type) + return printer.print_document( + report_name, + document, + report.report_type, + ) @api.multi def _can_print_report(self, behaviour, printer, document): @@ -51,6 +55,6 @@ class Report(models.Model): can_print_report = self._can_print_report(behaviour, printer, document) if can_print_report: - printer.print_document(report, document, report.report_type) + printer.print_document(report_name, document, report.report_type) return document diff --git a/base_report_to_printer/tests/test_printing_job.py b/base_report_to_printer/tests/test_printing_job.py index 6a82422..fc14e78 100644 --- a/base_report_to_printer/tests/test_printing_job.py +++ b/base_report_to_printer/tests/test_printing_job.py @@ -6,6 +6,9 @@ import mock from odoo import fields from odoo.tests.common import TransactionCase +from odoo.tools import mute_logger + +from ..models import printing_server model = 'odoo.addons.base_report_to_printer.models.printing_server' @@ -45,6 +48,7 @@ class TestPrintingJob(TransactionCase): values['printer_id'] = printer.id return self.env['printing.job'].create(values) + @mute_logger(printing_server.__name__) @mock.patch('%s.cups' % model) def test_cancel_job_error(self, cups): """ It should catch any exception from CUPS and update status """ diff --git a/base_report_to_printer/tests/test_printing_printer.py b/base_report_to_printer/tests/test_printing_printer.py index c79d24e..fc1e9f2 100644 --- a/base_report_to_printer/tests/test_printing_printer.py +++ b/base_report_to_printer/tests/test_printing_printer.py @@ -8,6 +8,9 @@ import mock from odoo.exceptions import UserError from odoo.tests.common import TransactionCase +from odoo.tools import mute_logger + +from ..models import printing_server model = 'odoo.addons.base_report_to_printer.models.printing_printer' @@ -32,9 +35,7 @@ class TestPrintingPrinter(TransactionCase): 'location': 'Location', 'uri': 'URI', } - self.report = self.env['ir.actions.report.xml'].search([ - ('report_type', '=', 'qweb-pdf'), - ], limit=1) + self.report = self.env.ref("base_report_to_printer.action_report_1") def new_record(self): return self.Model.create(self.printer_vals) @@ -59,13 +60,18 @@ class TestPrintingPrinter(TransactionCase): with mock.patch('%s.mkstemp' % model) as mkstemp: mkstemp.return_value = fd, file_name printer = self.new_record() - printer.print_document(self.report, 'content to print', 'pdf') + printer.print_document( + self.report.report_name, + 'content to print', + 'pdf', + ) cups.Connection().printFile.assert_called_once_with( printer.system_name, file_name, file_name, options={}) + @mute_logger(printing_server.__name__) @mock.patch('%s.cups' % server_model) def test_print_report_error(self, cups): """ It should print a report through CUPS """ @@ -76,7 +82,10 @@ class TestPrintingPrinter(TransactionCase): printer = self.new_record() with self.assertRaises(UserError): printer.print_document( - self.report, 'content to print', 'pdf') + self.report.report_name, + 'content to print', + 'pdf', + ) @mock.patch('%s.cups' % server_model) def test_print_file(self, cups): @@ -90,6 +99,7 @@ class TestPrintingPrinter(TransactionCase): file_name, options={}) + @mute_logger(printing_server.__name__) @mock.patch('%s.cups' % server_model) def test_print_file_error(self, cups): """ It should print a file through CUPS """ diff --git a/base_report_to_printer/tests/test_printing_printer_wizard.py b/base_report_to_printer/tests/test_printing_printer_wizard.py index 437e221..0567786 100644 --- a/base_report_to_printer/tests/test_printing_printer_wizard.py +++ b/base_report_to_printer/tests/test_printing_printer_wizard.py @@ -6,6 +6,9 @@ import mock from odoo.tests.common import TransactionCase from odoo.exceptions import UserError +from odoo.tools import mute_logger + +from ..models import printing_server model = 'odoo.addons.base_report_to_printer.models.printing_server' @@ -55,6 +58,7 @@ class TestPrintingPrinterWizard(TransactionCase): self.Model.action_ok() cups.Connection().getPrinters.assert_called_once_with() + @mute_logger(printing_server.__name__) @mock.patch('%s.cups' % model) def test_action_ok_raises_warning_on_error(self, cups): """ It should raise Warning on any error """ diff --git a/base_report_to_printer/tests/test_printing_server.py b/base_report_to_printer/tests/test_printing_server.py index 1bb8489..bfa977e 100644 --- a/base_report_to_printer/tests/test_printing_server.py +++ b/base_report_to_printer/tests/test_printing_server.py @@ -6,6 +6,9 @@ import mock from odoo import fields from odoo.tests.common import TransactionCase +from odoo.tools import mute_logger + +from ..models import printing_server model = 'odoo.addons.base_report_to_printer.models.printing_server' @@ -45,6 +48,7 @@ class TestPrintingServer(TransactionCase): values['printer_id'] = printer.id return self.env['printing.job'].create(values) + @mute_logger(printing_server.__name__) @mock.patch('%s.cups' % model) def test_update_printers_error(self, cups): """ It should catch any exception from CUPS and update status """ @@ -140,6 +144,7 @@ class TestPrintingServer(TransactionCase): ], ) + @mute_logger(printing_server.__name__) @mock.patch('%s.cups' % model) def test_update_jobs_error(self, cups): """ It should catch any exception from CUPS and update status """ diff --git a/base_report_to_printer/tests/test_report.py b/base_report_to_printer/tests/test_report.py index f57afbc..2a62d30 100644 --- a/base_report_to_printer/tests/test_report.py +++ b/base_report_to_printer/tests/test_report.py @@ -104,7 +104,7 @@ class TestReport(common.HttpCase): document = self.Model.get_pdf( self.partners.ids, self.report.report_name) print_document.assert_called_once_with( - self.report, document, self.report.report_type) + self.report.report_name, document, self.report.report_type) def test_print_document_not_printable(self): """ It should print the report, regardless of the defined behaviour """