Merge lp:~ahayzen/ubuntu-settings-components/defaults-emit-to-qml-printermodel-getter-and-multi-remove-fix into lp:~phablet-team/ubuntu-settings-components/printer-components

[Andrew Hayzen]

* Add getter method to PrinterModel
* Fix issue in update when removing multiple rows (also added regression test)
* Add refresh command to PrinterInfo
* Add ability for setting defaults to emit signals, so when a default is set it remains set on the QML side

* Add getter method to PrinterModel
* Fix issue in update when removing multiple rows (also added regression test)
* Add refresh command to PrinterInfo
* Add ability for setting defaults to emit signals, so when a default is set it remains set on the QML side

[lp:~ahayzen/ubuntu-settings-components/defaults-emit-to-qml-printermodel-getter-and-multi-remove-fix]

208. By Andrew Hayzen on 2017-01-19

* Remove count() as that is done in the other branch

[Konrad Zapałowicz (kzapalowicz)]

LGTM however it seems like the getter could be const. Is that possible?

review: Needs Information (code)

[lp:~ahayzen/ubuntu-settings-components/defaults-emit-to-qml-printermodel-getter-and-multi-remove-fix]

209. By Andrew Hayzen on 2017-01-19

* Change PrinterModel::get and PrinterFilter::get to const

210. By Andrew Hayzen on 2017-01-19

* Make PrinterInfoImpl constructor use refresh to create it's QPrinterInfo, so we don't have duplicate code

[Andrew Hayzen (ahayzen)]

Nice catch, I've changed PrinterModel::get and PrinterFilter::get to be consts :-)

[Jonas G. Drange (jonas-drange)]

Looks good. Nitpick: I would make refresh() a slot that you connect e.g. descriptionChanged to. Makes the setters less knowledgeable.

review: Approve

[Konrad Zapałowicz (kzapalowicz)]

Ack

review: Approve (code)

[lp:~ahayzen/ubuntu-settings-components/defaults-emit-to-qml-printermodel-getter-and-multi-remove-fix]

211. By Andrew Hayzen on 2017-01-19

* Merge of upstream

=== modified file 'plugins/Ubuntu/Settings/Printers/models/printermodel.cpp'
--- plugins/Ubuntu/Settings/Printers/models/printermodel.cpp	2017-01-19 11:16:13 +0000
+++ plugins/Ubuntu/Settings/Printers/models/printermodel.cpp	2017-01-19 14:20:00 +0000
@@ -89,6 +89,8 @@
             beginRemoveRows(QModelIndex(), i, i);
             d->printers.removeAt(i);
             endRemoveRows();
+
+            i--;  // as we have removed an item decrement
         }
     }
 
@@ -305,16 +307,18 @@
     return names;
 }
 
-QSharedPointer<Printer> PrinterModel::get(const int index)
+QVariantMap PrinterModel::get(const int row) const
 {
-    Q_D(const PrinterModel);
+    QHashIterator<int, QByteArray> iterator(roleNames());
+    QVariantMap result;
+    QModelIndex modelIndex = index(row, 0);
 
-    if (index > -1 && index < d->printers.count()) {
-        return d->printers.at(index);
-    } else {
-        qWarning() << "Invalid index:" << index;
-        return QSharedPointer<Printer>(Q_NULLPTR);
+    while (iterator.hasNext()) {
+        iterator.next();
+        result[iterator.value()] = modelIndex.data(iterator.key());
     }
+
+    return result;
 }
 
 QSharedPointer<Printer> PrinterModel::getPrinterFromName(const QString &name)
@@ -333,6 +337,20 @@
 
 }
 
+QVariantMap PrinterFilter::get(const int row) const
+{
+    QHashIterator<int, QByteArray> iterator(roleNames());
+    QVariantMap result;
+    QModelIndex modelIndex = index(row, 0);
+
+    while (iterator.hasNext()) {
+        iterator.next();
+        result[iterator.value()] = modelIndex.data(iterator.key());
+    }
+
+    return result;
+}
+
 void PrinterFilter::onSourceModelChanged()
 {
     connect((PrinterModel*) sourceModel(),

=== modified file 'plugins/Ubuntu/Settings/Printers/models/printermodel.h'
--- plugins/Ubuntu/Settings/Printers/models/printermodel.h	2017-01-19 11:16:13 +0000
+++ plugins/Ubuntu/Settings/Printers/models/printermodel.h	2017-01-19 14:20:00 +0000
@@ -75,7 +75,7 @@
 
     QSharedPointer<Printer> getPrinterFromName(const QString &name);
 
-    Q_INVOKABLE QSharedPointer<Printer> get(const int index);
+    Q_INVOKABLE QVariantMap get(const int row) const;
 private:
     QScopedPointer<PrinterModelPrivate> const d_ptr;
     QTimer m_update_timer;
@@ -95,8 +95,12 @@
 public:
     explicit PrinterFilter(QObject *parent = Q_NULLPTR);
     ~PrinterFilter();
+
+    Q_INVOKABLE QVariantMap get(const int row) const;
+
     void filterOnState(const PrinterEnum::State &state);
     void filterOnRecent(const bool recent);
+
     int count() const;
 protected:
     virtual bool filterAcceptsRow(

=== modified file 'plugins/Ubuntu/Settings/Printers/printer/printer.cpp'
--- plugins/Ubuntu/Settings/Printers/printer/printer.cpp	2017-01-18 16:25:02 +0000
+++ plugins/Ubuntu/Settings/Printers/printer/printer.cpp	2017-01-19 14:20:00 +0000
@@ -206,6 +206,9 @@
     QStringList vals({Utils::colorModelToPpdColorModel(colorModel)});
     QString reply = d->cups->printerAddOption(name(), "ColorModel", vals);
     Q_UNUSED(reply);
+
+    d->loadColorModel();
+    Q_EMIT defaultColorModelChanged();
 }
 
 void Printer::setAccessControl(const PrinterEnum::AccessControl &accessControl)
@@ -217,6 +220,9 @@
 {
     Q_D(const Printer);
     QString answer = d->cups->printerSetInfo(d->info->printerName(), description);
+
+    d->info->refresh();
+    Q_EMIT descriptionChanged();
 }
 
 void Printer::setDefaultDuplexMode(const PrinterEnum::DuplexMode &duplexMode)
@@ -234,6 +240,9 @@
 
     QStringList vals({Utils::duplexModeToPpdChoice(duplexMode)});
     QString reply = d->cups->printerAddOption(name(), "Duplex", vals);
+
+    d->info->refresh();
+    Q_EMIT defaultDuplexModeChanged();
 }
 
 void Printer::setEnabled(const bool enabled)
@@ -276,6 +285,9 @@
 
     QStringList vals({pageSize.key()});
     QString reply = d->cups->printerAddOption(name(), "PageSize", vals);
+
+    d->info->refresh();
+    Q_EMIT defaultPageSizeChanged();
 }
 
 void Printer::addUser(const QString &username)

=== modified file 'plugins/Ubuntu/Settings/Printers/printer/printer.h'
--- plugins/Ubuntu/Settings/Printers/printer/printer.h	2017-01-18 16:25:02 +0000
+++ plugins/Ubuntu/Settings/Printers/printer/printer.h	2017-01-19 14:20:00 +0000
@@ -47,7 +47,7 @@
     Q_PROPERTY(QString name READ name NOTIFY nameChanged)
     Q_PROPERTY(PrinterEnum::Quality quality READ quality WRITE setQuality NOTIFY qualityChanged)
     Q_PROPERTY(QString description READ description WRITE setDescription NOTIFY descriptionChanged)
-    Q_PROPERTY(QPageSize defaultPageSize READ defaultPageSize WRITE setDefaultPageSize NOTIFY pageSizeChanged)
+    Q_PROPERTY(QPageSize defaultPageSize READ defaultPageSize WRITE setDefaultPageSize NOTIFY defaultPageSizeChanged)
     Q_PROPERTY(QList<QPageSize> supportedPageSizes READ supportedPageSizes CONSTANT)
     Q_PROPERTY(PrinterEnum::AccessControl accessControl READ accessControl WRITE setAccessControl NOTIFY accessControlChanged)
     Q_PROPERTY(PrinterEnum::ErrorPolicy errorPolicy READ errorPolicy WRITE setErrorPolicy NOTIFY errorPolicyChanged)
@@ -108,7 +108,7 @@
     void nameChanged();
     void enabledChanged();
     void descriptionChanged();
-    void pageSizeChanged();
+    void defaultPageSizeChanged();
     void defaultDuplexModeChanged();
     void defaultColorModelChanged();
     void qualityChanged();

=== modified file 'plugins/Ubuntu/Settings/Printers/printer/printerinfo.h'
--- plugins/Ubuntu/Settings/Printers/printer/printerinfo.h	2017-01-18 16:18:40 +0000
+++ plugins/Ubuntu/Settings/Printers/printer/printerinfo.h	2017-01-19 14:20:00 +0000
@@ -54,6 +54,7 @@
     virtual PrinterInfo* printerInfo(const QString &printerName) = 0;
     virtual QString defaultPrinterName() = 0;
 
+    virtual void refresh() = 0;
 protected:
     const QString m_printerName;
 

=== modified file 'plugins/Ubuntu/Settings/Printers/printer/printerinfo_impl.cpp'
--- plugins/Ubuntu/Settings/Printers/printer/printerinfo_impl.cpp	2017-01-18 16:25:02 +0000
+++ plugins/Ubuntu/Settings/Printers/printer/printerinfo_impl.cpp	2017-01-19 14:20:00 +0000
@@ -23,11 +23,7 @@
 
 PrinterInfoImpl::PrinterInfoImpl(const QString &name) : PrinterInfo(name)
 {
-    if (m_printerName.isEmpty()) {
-        m_info = QPrinterInfo();
-    } else {
-        m_info = QPrinterInfo::printerInfo(m_printerName);
-    }
+    refresh();
 }
 
 PrinterInfoImpl::PrinterInfoImpl(QPrinterInfo info)
@@ -140,3 +136,12 @@
 {
     return QPrinterInfo::defaultPrinterName();
 }
+
+void PrinterInfoImpl::refresh()
+{
+    if (m_printerName.isEmpty()) {
+        m_info = QPrinterInfo();
+    } else {
+        m_info = QPrinterInfo::printerInfo(m_printerName);
+    }
+}

=== modified file 'plugins/Ubuntu/Settings/Printers/printer/printerinfo_impl.h'
--- plugins/Ubuntu/Settings/Printers/printer/printerinfo_impl.h	2017-01-18 16:18:40 +0000
+++ plugins/Ubuntu/Settings/Printers/printer/printerinfo_impl.h	2017-01-19 14:20:00 +0000
@@ -51,6 +51,8 @@
     virtual PrinterInfo* printerInfo(const QString &printerName) override;
     virtual QString defaultPrinterName() override;
 
+    virtual void refresh() override;
+
 private:
     QPrinterInfo m_info;
 };

=== modified file 'tests/unittests/Printers/mockprinterinfo.h'
--- tests/unittests/Printers/mockprinterinfo.h	2017-01-18 16:18:40 +0000
+++ tests/unittests/Printers/mockprinterinfo.h	2017-01-19 14:20:00 +0000
@@ -129,6 +129,11 @@
         return m_defaultPrinterName;
     }
 
+    virtual void refresh() override
+    {
+
+    }
+
     QString m_description = QString::null;
     QString m_location = QString::null;
     QString m_makeAndModel = QString::null;

=== modified file 'tests/unittests/Printers/tst_printermodel.cpp'
--- tests/unittests/Printers/tst_printermodel.cpp	2017-01-17 18:49:15 +0000
+++ tests/unittests/Printers/tst_printermodel.cpp	2017-01-19 14:20:00 +0000
@@ -147,6 +147,39 @@
         QCOMPARE(args.at(1).toInt(), 1);
         QCOMPARE(args.at(2).toInt(), 1);
     }
+    void testUpdateRemoveMulti()
+    {
+        // Setup four printers
+        PrinterInfo* printerA = new MockPrinterInfo("a-printer");
+        PrinterInfo* printerB = new MockPrinterInfo("b-printer");
+        PrinterInfo* printerC = new MockPrinterInfo("c-printer");
+        PrinterInfo* printerD = new MockPrinterInfo("d-printer");
+        ((MockPrinterInfo*) m_mock_info)->m_availablePrinters << printerA << printerB << printerC << printerD;
+
+        QTRY_COMPARE_WITH_TIMEOUT(m_model->count(), 4, 500);
+
+        // Setup spy and remove middle two printers
+        QSignalSpy countSpy(m_model, SIGNAL(countChanged()));
+        QSignalSpy removeSpy(m_model, SIGNAL(rowsRemoved(const QModelIndex&, int, int)));
+        ((MockPrinterInfo*) m_mock_info)->m_availablePrinters.removeAt(2);
+        ((MockPrinterInfo*) m_mock_info)->m_availablePrinters.removeAt(1);
+
+        // Wait until one count signal has been fired
+        // (to ensure this is only one iteration of update)
+        QTRY_COMPARE_WITH_TIMEOUT(countSpy.count(), 1, 500);
+        QCOMPARE(m_model->count(), 2);
+        QCOMPARE(removeSpy.count(), 2);
+
+        // Check items were removed from the middle
+        QList<QVariant> args;
+        args = removeSpy.at(0);
+        QCOMPARE(args.at(1).toInt(), 1);
+        QCOMPARE(args.at(2).toInt(), 1);
+
+        args = removeSpy.at(1);
+        QCOMPARE(args.at(1).toInt(), 1);
+        QCOMPARE(args.at(2).toInt(), 1);
+    }
 private:
     CupsFacade *m_mock_cups;
     PrinterInfo *m_mock_info;