Merge lp:~ahayzen/ubuntu-ui-extras/fix-colorModel-no-update into lp:ubuntu-ui-extras/staging

Proposed by Andrew Hayzen
Status: Merged
Approved by: Jonas G. Drange
Approved revision: 135
Merged at revision: 134
Proposed branch: lp:~ahayzen/ubuntu-ui-extras/fix-colorModel-no-update
Merge into: lp:ubuntu-ui-extras/staging
Diff against target: 99 lines (+46/-8)
3 files modified
modules/Ubuntu/Components/Extras/Printers/printer/printerjob.cpp (+6/-7)
modules/Ubuntu/Components/Extras/Printers/structs.h (+3/-1)
tests/unittests/Printers/tst_printerjob.cpp (+37/-0)
To merge this branch: bzr merge lp:~ahayzen/ubuntu-ui-extras/fix-colorModel-no-update
Reviewer Review Type Date Requested Status
system-apps-ci-bot continuous-integration Approve
Jonas G. Drange (community) Approve
Review via email: mp+321681@code.launchpad.net

Commit message

* Fix changing of printer not loading defaults and add a test
* Improve colorModel struct to != when name or colorModelType are different

Description of the change

* Fix changing of printer not loading defaults and add a test
* Improve colorModel struct to != when name or colorModelType are different

To post a comment you must log in.
Revision history for this message
Jonas G. Drange (jonas-drange) wrote :

Could you clarify “loaded job” on L15?

review: Needs Information
135. By Andrew Hayzen

* Update comments

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

Updated the comment :-)

Revision history for this message
Jonas G. Drange (jonas-drange) wrote :

This makes good sense now, and thanks for updating the comment. Also, +1 for the tests!

review: Approve
Revision history for this message
system-apps-ci-bot (system-apps-ci-bot) wrote :

PASSED: Continuous integration, rev:134
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-ui-extras-staging-ci/8/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build/2380
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-0-fetch/2380
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2193
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2193/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=zesty/2193
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=zesty/2193/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2193
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2193/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=zesty/2193
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=zesty/2193/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/2193
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/2193/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=zesty/2193
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=zesty/2193/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-ui-extras-staging-ci/8/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'modules/Ubuntu/Components/Extras/Printers/printer/printerjob.cpp'
--- modules/Ubuntu/Components/Extras/Printers/printer/printerjob.cpp 2017-03-16 15:47:11 +0000
+++ modules/Ubuntu/Components/Extras/Printers/printer/printerjob.cpp 2017-04-03 11:59:25 +0000
@@ -215,13 +215,9 @@
215 return;215 return;
216 }216 }
217217
218 if (jobId() > 0) {218 // Only load the defaults if this isn't a job in the queue (one which has a
219 loadAttributes(219 // jobId) as this is when user is making a job and has changed the printer
220 m_backend->printerGetJobAttributes(220 if (jobId() <= 0) {
221 printerName(), jobId()
222 )
223 );
224 } else {
225 setColorModel(m_printer->supportedColorModels().indexOf(m_printer->defaultColorModel()));221 setColorModel(m_printer->supportedColorModels().indexOf(m_printer->defaultColorModel()));
226 setDuplexMode(m_printer->supportedDuplexModes().indexOf(m_printer->defaultDuplexMode()));222 setDuplexMode(m_printer->supportedDuplexModes().indexOf(m_printer->defaultDuplexMode()));
227 setQuality(m_printer->supportedPrintQualities().indexOf(m_printer->defaultPrintQuality()));223 setQuality(m_printer->supportedPrintQualities().indexOf(m_printer->defaultPrintQuality()));
@@ -401,6 +397,9 @@
401 Q_EMIT printerNameChanged();397 Q_EMIT printerNameChanged();
402 }398 }
403399
400 // When the printer changes ensure we update any defaults
401 loadDefaults();
402
404 Q_EMIT printerChanged();403 Q_EMIT printerChanged();
405 }404 }
406}405}
407406
=== modified file 'modules/Ubuntu/Components/Extras/Printers/structs.h'
--- modules/Ubuntu/Components/Extras/Printers/structs.h 2017-03-16 15:34:02 +0000
+++ modules/Ubuntu/Components/Extras/Printers/structs.h 2017-04-03 11:59:25 +0000
@@ -36,13 +36,15 @@
3636
37 bool operator==(const ColorModel& a) const37 bool operator==(const ColorModel& a) const
38 {38 {
39 return (name == a.name && originalOption == a.originalOption);39 return (name == a.name && originalOption == a.originalOption
40 && text == a.text && colorType == a.colorType);
40 }41 }
41 bool operator!=(const ColorModel& a) const42 bool operator!=(const ColorModel& a) const
42 {43 {
43 return !(*this == a);44 return !(*this == a);
44 }45 }
45 void operator=(const ColorModel &m) {46 void operator=(const ColorModel &m) {
47 colorType = m.colorType;
46 name = m.name;48 name = m.name;
47 text = m.text;49 text = m.text;
48 originalOption = m.originalOption;50 originalOption = m.originalOption;
4951
=== modified file 'tests/unittests/Printers/tst_printerjob.cpp'
--- tests/unittests/Printers/tst_printerjob.cpp 2017-03-14 14:47:54 +0000
+++ tests/unittests/Printers/tst_printerjob.cpp 2017-04-03 11:59:25 +0000
@@ -304,6 +304,43 @@
304 QCOMPARE(m_instance->title(), title);304 QCOMPARE(m_instance->title(), title);
305 }305 }
306306
307 void testSetPrinterLoadDefaults()
308 {
309 ColorModel a;
310 a.colorType = PrinterEnum::ColorModelType::GrayType;
311 a.text = "Gray";
312
313 ColorModel b;
314 b.colorType = PrinterEnum::ColorModelType::ColorType;
315 b.text = "RGB";
316 QList<ColorModel> models({a, b});
317
318
319 MockPrinterBackend *backend1 = new MockPrinterBackend("printer1");
320 backend1->printerOptions["printer1"].insert("SupportedColorModels", QVariant::fromValue(models));
321 backend1->printerOptions["printer1"].insert("DefaultColorModel", QVariant::fromValue(a));
322 QSharedPointer<Printer> printer1 = QSharedPointer<Printer>(new Printer(backend1));
323
324 MockPrinterBackend *backend2 = new MockPrinterBackend("printer2");
325 backend2->printerOptions["printer2"].insert("SupportedColorModels", QVariant::fromValue(models));
326 backend2->printerOptions["printer2"].insert("DefaultColorModel", QVariant::fromValue(b));
327 QSharedPointer<Printer> printer2 = QSharedPointer<Printer>(new Printer(backend2));
328
329 // Base case
330 m_instance->setPrinter(printer1);
331 qDebug() << m_instance->colorModel() << models.indexOf(a);
332 QCOMPARE(m_instance->colorModel(), models.indexOf(a));
333 QCOMPARE(m_instance->getColorModel().text, a.text);
334 QCOMPARE(m_instance->colorModelType(), a.colorType);
335
336 // Change to a different printer and check the default updates
337 m_instance->setPrinter(printer2);
338 qDebug() << m_instance->colorModel() << models.indexOf(b);
339 QCOMPARE(m_instance->colorModel(), models.indexOf(b));
340 QCOMPARE(m_instance->getColorModel().text, b.text);
341 QCOMPARE(m_instance->colorModelType(), b.colorType);
342 }
343
307 void testState()344 void testState()
308 {345 {
309 QCOMPARE(m_instance->state(), PrinterEnum::JobState::Pending);346 QCOMPARE(m_instance->state(), PrinterEnum::JobState::Pending);

Subscribers

People subscribed via source and target branches