QmlDesigner: Move code from list editor dialog to model

And add tests to the code.

Change-Id: I9fb183729c716a50bbab861d207a212ff704ee7b
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>

src/plugins/qmldesigner/components/listmodeleditor/listmodeleditordialog.cpp

@@ -114,40 +114,12 @@ void ListModelEditorDialog::openColumnDialog()
 
 void ListModelEditorDialog::removeRows()
 {
-    const QList<QModelIndex> indices = m_tableView->selectionModel()->selectedRows();
-    std::vector<int> rows;
-    rows.reserve(indices.size());
-
-    for (QModelIndex index : indices)
-        rows.push_back(index.row());
-
-    std::sort(rows.begin(), rows.end());
-
-    rows.erase(std::unique(rows.begin(), rows.end()), rows.end());
-
-    std::reverse(rows.begin(), rows.end());
-
-    for (int row : rows)
-        m_model->removeRow(row);
+    m_model->removeRows(m_tableView->selectionModel()->selectedRows());
 }
 
 void ListModelEditorDialog::removeColumns()
 {
-    const QList<QModelIndex> indices = m_tableView->selectionModel()->selectedColumns();
-    std::vector<int> columns;
-    columns.reserve(indices.size());
-
-    for (QModelIndex index : indices)
-        columns.push_back(index.column());
-
-    std::sort(columns.begin(), columns.end());
-
-    columns.erase(std::unique(columns.begin(), columns.end()), columns.end());
-
-    std::reverse(columns.begin(), columns.end());
-
-    for (int row : columns)
-        m_model->removeColumn(row);
+    m_model->removeColumns(m_tableView->selectionModel()->selectedColumns());
 }
 
 void ListModelEditorDialog::changeHeader(int column)

src/plugins/qmldesigner/components/listmodeleditor/listmodeleditormodel.cpp

@@ -260,6 +260,26 @@ void ListModelEditorModel::removeColumn(int column)
     }
 }
 
+void ListModelEditorModel::removeColumns(const QList<QModelIndex> &indices)
+{
+    std::vector<int> columns = filterColumns(indices);
+
+    std::reverse(columns.begin(), columns.end());
+
+    for (int column : columns)
+        removeColumn(column);
+}
+
+void ListModelEditorModel::removeRows(const QList<QModelIndex> &indices)
+{
+    std::vector<int> rows = filterRows(indices);
+
+    std::reverse(rows.begin(), rows.end());
+
+    for (int row : rows)
+        removeRow(row);
+}
+
 void ListModelEditorModel::removeRow(int row)
 {
     QList<QStandardItem *> rowItems = QStandardItemModel::takeRow(row);
@@ -299,4 +319,38 @@ void ListModelEditorModel::renameColumn(int oldColumn, const QString &newColumnN
     setHorizontalHeaderLabels(convertToStringList(m_propertyNames));
 }
 
+std::vector<int> ListModelEditorModel::filterColumns(const QList<QModelIndex> &indices)
+{
+    std::vector<int> columns;
+    columns.reserve(indices.size());
+
+    for (QModelIndex index : indices) {
+        if (index.column() >= 0)
+            columns.push_back(index.column());
+    }
+
+    std::sort(columns.begin(), columns.end());
+
+    columns.erase(std::unique(columns.begin(), columns.end()), columns.end());
+
+    return columns;
+}
+
+std::vector<int> ListModelEditorModel::filterRows(const QList<QModelIndex> &indices)
+{
+    std::vector<int> rows;
+    rows.reserve(indices.size());
+
+    for (QModelIndex index : indices) {
+        if (index.row() >= 0)
+            rows.push_back(index.row());
+    }
+
+    std::sort(rows.begin(), rows.end());
+
+    rows.erase(std::unique(rows.begin(), rows.end()), rows.end());
+
+    return rows;
+}
+
 } // namespace QmlDesigner

src/plugins/qmldesigner/components/listmodeleditor/listmodeleditormodel.h

@@ -33,6 +33,8 @@ namespace QmlDesigner {
 
 class ListModelEditorModel : public QStandardItemModel
 {
+    using QStandardItemModel::removeColumns;
+    using QStandardItemModel::removeRows;
 
 public:
     void setListModel(ModelNode node)
@@ -48,11 +50,16 @@ public:
 
     bool setValue(int row, int column, QVariant value, Qt::ItemDataRole role = Qt::EditRole);
 
-    void removeColumn(int column);
-    void removeRow(int row);
+    void removeColumns(const QList<QModelIndex> &indices);
+    void removeRows(const QList<QModelIndex> &indices);
     void renameColumn(int column, const QString &newColumnName);
 
+    static std::vector<int> filterColumns(const QList<QModelIndex> &indices);
+    static std::vector<int> filterRows(const QList<QModelIndex> &indices);
+
 private:
+    void removeRow(int row);
+    void removeColumn(int column);
     void populateModel();
     void createItems(const QList<ModelNode> &listElementNodes);
     void appendItems(const ModelNode &listElementNode);

tests/unit/unittest/listmodeleditor-test.cpp

@@ -39,6 +39,7 @@ namespace {
 
 using QmlDesigner::AbstractProperty;
 using QmlDesigner::AbstractView;
+using QmlDesigner::ListModelEditorModel;
 using QmlDesigner::ModelNode;
 
 MATCHER_P2(HasItem,
@@ -172,6 +173,8 @@ public:
         return properties;
     }
 
+    QModelIndex index(int row, int column) const { return model.index(row, column); }
+
 protected:
     std::unique_ptr<QmlDesigner::Model> designerModel{QmlDesigner::Model::create("QtQuick.Item", 1, 1)};
     NiceMock<MockListModelEditorView> mockView;
@@ -427,7 +430,7 @@ TEST_F(ListModelEditor, RemoveColumnRemovesDisplayValues)
 {
     model.setListModel(listModelNode);
 
-    model.removeColumn(2);
+    model.removeColumns({index(0, 2)});
 
     ASSERT_THAT(displayValues(),
                 ElementsAre(ElementsAre(IsInvalid(), "foo", 42),
@@ -442,14 +445,14 @@ TEST_F(ListModelEditor, RemoveColumnRemovesProperties)
     EXPECT_CALL(mockView, propertiesRemoved(ElementsAre(IsAbstractProperty(element2, "image"))));
     EXPECT_CALL(mockView, propertiesRemoved(ElementsAre(IsAbstractProperty(element3, "image"))));
 
-    model.removeColumn(0);
+    model.removeColumns({index(0, 0)});
 }
 
 TEST_F(ListModelEditor, RemoveColumnRemovesPropertyName)
 {
     model.setListModel(listModelNode);
 
-    model.removeColumn(1);
+    model.removeColumns({index(0, 1)});
 
     ASSERT_THAT(model.propertyNames(), ElementsAre("image", "value", "value2"));
 }
@@ -458,7 +461,7 @@ TEST_F(ListModelEditor, RemoveRowRemovesDisplayValues)
 {
     model.setListModel(listModelNode);
 
-    model.removeRow(1);
+    model.removeRows({index(1, 0)});
 
     ASSERT_THAT(displayValues(),
                 ElementsAre(ElementsAre(IsInvalid(), "foo", 1, 42),
@@ -471,7 +474,7 @@ TEST_F(ListModelEditor, RemoveRowRemovesElementInListModel)
 
     EXPECT_CALL(mockView, nodeRemoved(Eq(element2), _, _));
 
-    model.removeRow(1);
+    model.removeRows({index(1, 0)});
 }
 
 TEST_F(ListModelEditor, ConvertStringFloatToFloat)
@@ -721,7 +724,7 @@ TEST_F(ListModelEditor, RemoveColumnAfterRenameColumn)
     model.setListModel(listModelNode);
     model.renameColumn(1, "mood");
 
-    model.removeColumn(1);
+    model.removeColumns({index(0, 1)});
 
     ASSERT_THAT(properties(),
                 ElementsAre(UnorderedElementsAre(IsVariantProperty("value", 1),
@@ -909,30 +912,7 @@ TEST_F(ListModelEditor, RemoveLastRow)
     model.addColumn("mood");
     model.addRow();
 
-    model.removeRow(0);
-
-    ASSERT_THAT(displayValues(), IsEmpty());
-}
-
-TEST_F(ListModelEditor, RemoveLastColumn)
-{
-    model.setListModel(emptyListModelNode);
-    model.addColumn("mood");
-    model.addRow();
-
-    model.removeColumn(0);
-
-    ASSERT_THAT(displayValues(), ElementsAre(IsEmpty()));
-}
-
-TEST_F(ListModelEditor, RemoveLastEmptyColumn)
-{
-    model.setListModel(emptyListModelNode);
-    model.addColumn("mood");
-    model.addRow();
-    model.removeRow(0);
-
-    model.removeColumn(0);
+    model.removeRows({index(0, 0)});
 
     ASSERT_THAT(displayValues(), IsEmpty());
 }
@@ -942,11 +922,115 @@ TEST_F(ListModelEditor, RemoveLastEmptyRow)
     model.setListModel(emptyListModelNode);
     model.addColumn("mood");
     model.addRow();
-    model.removeColumn(0);
+    model.removeColumns({index(0, 0)});
 
-    model.removeRow(0);
+    model.removeRows({index(0, 0)});
+
+    ASSERT_THAT(displayValues(), ElementsAre(IsEmpty()));
+}
+
+TEST_F(ListModelEditor, RemoveLastColumn)
+{
+    model.setListModel(emptyListModelNode);
+    model.addColumn("mood");
+    model.addRow();
+
+    model.removeColumns({index(0, 0)});
+
+    ASSERT_THAT(displayValues(), ElementsAre(IsEmpty()));
+}
+
+TEST_F(ListModelEditor, RemoveLastEmptyColumn)
+{
+    model.setListModel(emptyListModelNode);
+    model.addColumn("mood");
+    model.addRow();
+    model.removeRows({index(0, 0)});
+
+    model.removeColumns({index(0, 0)});
 
     ASSERT_THAT(displayValues(), IsEmpty());
 }
 
+TEST_F(ListModelEditor, RemoveColumns)
+{
+    model.setListModel(listModelNode);
+    model.removeColumns({index(0, 1), index(0, 3), index(1, 1), index(0, 4)});
+
+    ASSERT_THAT(properties(),
+                ElementsAre(UnorderedElementsAre(IsVariantProperty("value", 1)),
+                            UnorderedElementsAre(IsVariantProperty("image", "pic.png"),
+                                                 IsVariantProperty("value", 4)),
+                            UnorderedElementsAre(IsVariantProperty("image", "pic.png"),
+                                                 IsVariantProperty("value", 111))));
+}
+
+TEST_F(ListModelEditor, RemoveRows)
+{
+    model.setListModel(listModelNode);
+
+    model.removeRows({index(1, 0), index(2, 0), index(3, 0), index(2, 0)});
+
+    ASSERT_THAT(properties(),
+                ElementsAre(UnorderedElementsAre(IsVariantProperty("name", "foo"),
+                                                 IsVariantProperty("value", 1),
+                                                 IsVariantProperty("value2", 42))));
+}
+
+TEST_F(ListModelEditor, FilterColumns)
+{
+    model.setListModel(listModelNode);
+    QList<QModelIndex> indices = {index(0, 0), index(1, 1), index(0, 2), index(0, 1)};
+
+    auto columns = ListModelEditorModel::filterColumns(indices);
+
+    ASSERT_THAT(columns, ElementsAre(0, 1, 2));
+}
+
+TEST_F(ListModelEditor, FilterColumnsInvalidColumns)
+{
+    QList<QModelIndex> indices = {index(0, 0), index(1, 1), index(0, 2), index(0, 1)};
+
+    auto columns = ListModelEditorModel::filterColumns(indices);
+
+    ASSERT_THAT(columns, IsEmpty());
+}
+
+TEST_F(ListModelEditor, FilterColumnsEmptyInput)
+{
+    QList<QModelIndex> indices;
+
+    auto columns = ListModelEditorModel::filterColumns(indices);
+
+    ASSERT_THAT(columns, IsEmpty());
+}
+
+TEST_F(ListModelEditor, FilterRows)
+{
+    model.setListModel(listModelNode);
+    QList<QModelIndex> indices = {index(0, 0), index(1, 1), index(2, 2), index(0, 1)};
+
+    auto rows = ListModelEditorModel::filterRows(indices);
+
+    ASSERT_THAT(rows, ElementsAre(0, 1, 2));
+}
+
+TEST_F(ListModelEditor, FilterRowsInvalidColumns)
+{
+    QList<QModelIndex> indices = {index(0, 0), index(1, 1), index(2, 2), index(0, 1)};
+
+    auto rows = ListModelEditorModel::filterRows(indices);
+
+    ASSERT_THAT(rows, IsEmpty());
+}
+
+TEST_F(ListModelEditor, FilterRowsEmptyInput)
+{
+    QList<QModelIndex> indices;
+
+    auto rows = ListModelEditorModel::filterRows(indices);
+
+    ASSERT_THAT(rows, IsEmpty());
+}
+
 } // namespace