Merge lp:~ahayzen/ubuntu-settings-components/model-update-printer-printerjob into lp:~phablet-team/ubuntu-settings-components/printer-components

[Andrew Hayzen]

* Add updateFrom to Printer and PrinterJob
* Call updateFrom in PrinterModel and JobModel when printers are the same
* Emit dataChanged signal in models when updateFrom returns a change
* Ensure sorting is done by name last

* Add updateFrom to Printer and PrinterJob
* Call updateFrom in PrinterModel and JobModel when printers are the same
* Emit dataChanged signal in models when updateFrom returns a change
* Ensure sorting is done by name last

[Jonas G. Drange (jonas-drange)]

two minor comments

review: Needs Fixing

[lp:~ahayzen/ubuntu-settings-components/model-update-printer-printerjob]

230. By Andrew Hayzen on 2017-02-03

* Add deepCompare to Printer and PrinterJob
* Change updateFrom to only copy values not compare
* Remove toVector as it isn't needed in dataChanged

[Jonas G. Drange (jonas-drange)]

awesomsauce

review: Approve

=== modified file 'plugins/Ubuntu/Settings/Printers/backend/backend_cups.cpp'
--- plugins/Ubuntu/Settings/Printers/backend/backend_cups.cpp	2017-02-03 12:04:46 +0000
+++ plugins/Ubuntu/Settings/Printers/backend/backend_cups.cpp	2017-02-03 14:43:53 +0000
@@ -245,6 +245,8 @@
     Q_FOREACH(auto job, jobs) {
         auto newJob = QSharedPointer<PrinterJob>(new PrinterJob(name, this, job->id));
 
+        // TODO: needs to extract other properties like copies/duplex etc
+
         newJob->setState(static_cast<PrinterEnum::JobState>(job->state));
         newJob->setTitle(QString::fromLocal8Bit(job->title));
 

=== modified file 'plugins/Ubuntu/Settings/Printers/models/jobmodel.cpp'
--- plugins/Ubuntu/Settings/Printers/models/jobmodel.cpp	2017-02-02 17:22:55 +0000
+++ plugins/Ubuntu/Settings/Printers/models/jobmodel.cpp	2017-02-03 14:43:53 +0000
@@ -68,9 +68,16 @@
         bool exists = false;
 
         Q_FOREACH(QSharedPointer<PrinterJob> p, newJobs) {
-            // TODO: update status here
             if (p->jobId() == m_jobs.at(i)->jobId()) {
                 exists = true;
+
+                // Ensure the other properties of the job are up to date
+                if (!m_jobs.at(i)->deepCompare(p)) {
+                    m_jobs.at(i)->updateFrom(p);
+
+                    Q_EMIT dataChanged(index(i), index(i));
+                }
+
                 break;
             }
         }

=== modified file 'plugins/Ubuntu/Settings/Printers/models/printermodel.cpp'
--- plugins/Ubuntu/Settings/Printers/models/printermodel.cpp	2017-02-03 12:04:46 +0000
+++ plugins/Ubuntu/Settings/Printers/models/printermodel.cpp	2017-02-03 14:43:53 +0000
@@ -80,6 +80,14 @@
         Q_FOREACH(Printer *p, newPrinters) {
             if (p->name() == m_printers.at(i)->name()) {
                 exists = true;
+
+                // Ensure the other properties of the Printer are up to date
+                if (!m_printers.at(i)->deepCompare(p)) {
+                    m_printers.at(i)->updateFrom(p);
+
+                    Q_EMIT dataChanged(index(i), index(i));
+                }
+
                 break;
             }
         }
@@ -460,7 +468,15 @@
             int leftPdf = sourceModel()->data(left, PrinterModel::IsPdfRole).toInt();
             int rightPdf = sourceModel()->data(right, PrinterModel::IsPdfRole).toInt();
 
-            return leftPdf > rightPdf;
+            // If Pdf is also same then sort by name
+            if (leftPdf == rightPdf) {
+                QString leftName = sourceModel()->data(left, PrinterModel::NameRole).toString();
+                QString rightName = sourceModel()->data(right, PrinterModel::NameRole).toString();
+
+                return leftName > rightName;
+            } else {
+                return leftPdf > rightPdf;
+            }
         } else {
             return leftData.toInt() < rightData.toInt();
         }

=== modified file 'plugins/Ubuntu/Settings/Printers/printer/printer.cpp'
--- plugins/Ubuntu/Settings/Printers/printer/printer.cpp	2017-01-23 19:41:11 +0000
+++ plugins/Ubuntu/Settings/Printers/printer/printer.cpp	2017-02-03 14:43:53 +0000
@@ -185,9 +185,6 @@
     QStringList vals({colorModel.name});
     QString reply = m_backend->printerAddOption(name(), "ColorModel", vals);
     Q_UNUSED(reply);
-
-    loadColorModel();
-    Q_EMIT defaultColorModelChanged();
 }
 
 void Printer::setAccessControl(const PrinterEnum::AccessControl &accessControl)
@@ -198,9 +195,6 @@
 void Printer::setDescription(const QString &description)
 {
     QString answer = m_backend->printerSetInfo(name(), description);
-
-    m_backend->refresh();
-    Q_EMIT descriptionChanged();
 }
 
 void Printer::setDefaultDuplexMode(const PrinterEnum::DuplexMode &duplexMode)
@@ -216,9 +210,6 @@
 
     QStringList vals({Utils::duplexModeToPpdChoice(duplexMode)});
     QString reply = m_backend->printerAddOption(name(), "Duplex", vals);
-
-    m_backend->refresh();
-    Q_EMIT defaultDuplexModeChanged();
 }
 
 void Printer::setEnabled(const bool enabled)
@@ -249,7 +240,6 @@
 
     QStringList vals({quality.name});
     QString reply = m_backend->printerAddOption(name(), quality.originalOption, vals);
-    loadPrintQualities();
 }
 
 void Printer::setDefaultPageSize(const QPageSize &pageSize)
@@ -289,3 +279,40 @@
 {
 
 }
+
+void Printer::deepCompare(Printer *other) const
+{
+    bool changed = false;
+
+    changed |= defaultColorModel() != other->defaultColorModel();
+    changed |= defaultPrintQuality() != other->defaultPrintQuality();
+    changed |= description() != other->description();
+    changed |= defaultDuplexMode() != other->defaultDuplexMode();
+    changed |= defaultPageSize() != other->defaultPageSize();
+    changed |= state() != other->state();
+
+    // TODO: accessControl
+    // TODO: enabled
+    // TODO: errorPolicy
+
+    return changed;
+}
+
+void Printer::updateFrom(Printer* newPrinter)
+{
+    m_backend->refresh();
+
+    loadColorModel();
+    loadPrintQualities();
+
+    Q_EMIT descriptionChanged();
+    Q_EMIT defaultColorModelChanged();
+    Q_EMIT defaultDuplexModeChanged();
+    Q_EMIT defaultPageSizeChanged();
+    Q_EMIT defaultPrintQualityChanged();
+    Q_EMIT stateChanged();
+
+    // TODO: accessControl
+    // TODO: enabled
+    // TODO: errorPolicy
+}

=== modified file 'plugins/Ubuntu/Settings/Printers/printer/printer.h'
--- plugins/Ubuntu/Settings/Printers/printer/printer.h	2017-01-22 14:21:11 +0000
+++ plugins/Ubuntu/Settings/Printers/printer/printer.h	2017-02-03 14:43:53 +0000
@@ -71,6 +71,7 @@
     void setDefaultPrintQuality(const PrintQuality &quality);
     void setDefaultPageSize(const QPageSize &pageSize);
 
+    bool deepCompare(Printer *other) const;
 public Q_SLOTS:
     // Add user that is either denied or allowed printer. See AccessControl.
     void addUser(const QString &username);
@@ -85,6 +86,7 @@
     // Requests ink levels for printer.
     void requestInkLevels(const QString &name);
 
+    void updateFrom(Printer *newPrinter);
 Q_SIGNALS:
     void nameChanged();
     void enabledChanged();
@@ -92,6 +94,7 @@
     void defaultPageSizeChanged();
     void defaultDuplexModeChanged();
     void defaultColorModelChanged();
+    void defaultPrintQualityChanged();
     void qualityChanged();
     void accessControlChanged();
     void errorPolicyChanged();

=== modified file 'plugins/Ubuntu/Settings/Printers/printer/printerjob.cpp'
--- plugins/Ubuntu/Settings/Printers/printer/printerjob.cpp	2017-02-02 16:00:55 +0000
+++ plugins/Ubuntu/Settings/Printers/printer/printerjob.cpp	2017-02-03 14:43:53 +0000
@@ -349,3 +349,39 @@
 {
     return m_title;
 }
+
+bool PrinterJob::deepCompare(QSharedPointer<PrinterJob> other) const
+{
+    // jobId and printerName will be the same
+
+    bool changed = false;
+
+    changed |= collate() != newPrinterJob->collate();
+    changed |= colorModel() != newPrinterJob->colorModel();
+    changed |= copies() != newPrinterJob->copies();
+    changed |= duplexMode() != newPrinterJob->duplexMode();
+    changed |= landscape() != newPrinterJob->landscape();
+    changed |= printRange() != newPrinterJob->printRange();
+    changed |= printRangeMode() != newPrinterJob->printRangeMode();
+    changed |= quality() != newPrinterJob->quality();
+    changed |= reverse() != newPrinterJob->reverse();
+    changed |= state() != newPrinterJob->state();
+    changed |= title() != newPrinterJob->title();
+
+    return changed;
+}
+
+void PrinterJob::updateFrom(QSharedPointer<PrinterJob> newPrinterJob)
+{
+    setCollate(newPrinterJob->collate());
+    setColorModel(newPrinterJob->colorModel());
+    setCopies(newPrinterJob->copies());
+    setDuplexMode(newPrinterJob->duplexMode());
+    setLandscape(newPrinterJob->landscape());
+    setPrintRange(newPrinterJob->printRange());
+    setPrintRangeMode(newPrinterJob->printRangeMode());
+    setQuality(newPrinterJob->quality());
+    setReverse(newPrinterJob->reverse());
+    setState(newPrinterJob->state());
+    setTitle(newPrinterJob->title());
+}

=== modified file 'plugins/Ubuntu/Settings/Printers/printer/printerjob.h'
--- plugins/Ubuntu/Settings/Printers/printer/printerjob.h	2017-02-02 16:00:55 +0000
+++ plugins/Ubuntu/Settings/Printers/printer/printerjob.h	2017-02-03 14:43:53 +0000
@@ -75,6 +75,8 @@
     bool reverse() const;
     PrinterEnum::JobState state() const;
     QString title() const;
+
+    bool deepCompare(QSharedPointer<PrinterJob> other) const;
 public Q_SLOTS:
     PrinterEnum::DuplexMode getDuplexMode() const;
     ColorModel getColorModel() const;
@@ -92,6 +94,8 @@
     void setQuality(const int quality);
     void setReverse(const bool reverse);
     void setTitle(const QString &title);
+
+    void updateFrom(QSharedPointer<PrinterJob> newPrinterJob);
 private Q_SLOTS:
     void loadDefaults();
     void setIsTwoSided(const bool isTwoSided);

=== modified file 'plugins/Ubuntu/Settings/Printers/structs.h'
--- plugins/Ubuntu/Settings/Printers/structs.h	2017-01-26 21:47:06 +0000
+++ plugins/Ubuntu/Settings/Printers/structs.h	2017-02-03 14:43:53 +0000
@@ -70,6 +70,10 @@
     {
         return (name == a.name && originalOption == a.originalOption);
     }
+    bool operator !=(const PrintQuality& a) const
+    {
+        return !(operator ==(a));
+    }
 };
 
 struct PrinterDriver