Make Importer's Select all check box more intuitive (#192)

spine_items/importer/mvcmodels/mappings_model.py

@@ -161,7 +161,7 @@ def _make_select_all_tables_item():
         Returns:
             SourceTableItem: 'select all' item
         """
-        return SourceTableItem("Select all", checked=False, real=False, select_all=True)
+        return SourceTableItem("Select all", checked=True, real=False, select_all=True)
 
     def columnCount(self, parent=QModelIndex()):
         if not parent.isValid():
@@ -507,7 +507,7 @@ def _set_table_list_data(self, table_item, index, value, role):
         if role == Qt.ItemDataRole.CheckStateRole and table_item.checkable:
             checked = value == Qt.CheckState.Checked.value
             if row == 0:
-                self._set_multiple_checked_undoable(checked, *range(1, len(self._mappings)))
+                self._set_multiple_checked_undoable(checked, *range(len(self._mappings)))
             else:
                 self._undo_stack.push(SetTableChecked(table_item.name, self, checked, row))
             return True
@@ -562,7 +562,7 @@ def update_table_item(self, row, data_dict, add_empty_row=False, remove_empty_ro
         if add_empty_row:
             table_item.real = True
             table_item.checkable = True
-            table_item.checked = True
+            table_item.checked = self._mappings[0].checked
             table_item.empty = False
             table_item.in_source = True
             default_flattened_mappings = FlattenedMappings(self._create_default_mapping())
@@ -628,16 +628,17 @@ def set_table_checked(self, checked, *rows):
                 min_row = min(row, min_row)
                 max_row = max(row, max_row)
             self._mappings[row].checked = checked
-        if min_row is None:
-            return
-        top_left = self.index(min_row, 0)
-        bottom_right = self.index(max_row, 0)
-        self.dataChanged.emit(top_left, bottom_right, [Qt.ItemDataRole.CheckStateRole])
+        if min_row is not None:
+            top_left = self.index(min_row, 0)
+            bottom_right = self.index(max_row, 0)
+            self.dataChanged.emit(top_left, bottom_right, [Qt.ItemDataRole.CheckStateRole])
         self._update_all_checked()
 
     def _update_all_checked(self):
         """Updates the checked state of 'Select All' table item if needed."""
-        checkables = (m for m in self._mappings[1:] if m.checkable)
+        checkables = tuple(m for m in self._mappings[1:] if m.checkable)
+        if not checkables:
+            return
         all_checked = all(m.checked for m in checkables)
         all_checked_item = self._mappings[0]
         if all_checked_item.checked != all_checked:
@@ -658,7 +659,9 @@ def append_new_table_with_mapping(self, table_name, root_mapping):
         flattened_mappings = FlattenedMappings(root_mapping)
         list_item = MappingListItem("Mapping 1")
         list_item.set_flattened_mappings(flattened_mappings)
-        table_item = SourceTableItem(table_name, checked=True, in_source=True, in_specification=has_root_mapping)
+        table_item = SourceTableItem(
+            table_name, checked=self._mappings[0].checked, in_source=True, in_specification=has_root_mapping
+        )
         table_item.append_to_mapping_list(list_item)
         self.beginInsertRows(QModelIndex(), len(self._mappings), len(self._mappings))
         self._mappings.append(table_item)

tests/importer/mvcmodels/test_mappings_model.py

@@ -141,7 +141,7 @@ def test_empty_model_has_select_all_source_table_item(self):
         index = self._model.index(0, 0)
         self.assertEqual(index.data(), "Select all")
         self.assertIsNone(index.data(Qt.ItemDataRole.ForegroundRole))
-        self.assertFalse(index.data(Qt.ItemDataRole.CheckStateRole).value)
+        self.assertEqual(index.data(Qt.ItemDataRole.CheckStateRole), Qt.CheckState.Checked)
         self.assertIsNone(index.data(Qt.ItemDataRole.FontRole))
         self.assertIsNone(index.data(Qt.ItemDataRole.ToolTipRole))
         flags = self._model.flags(index)
@@ -170,7 +170,7 @@ def test_turn_empty_row_into_non_real_table(self):
         index = self._model.index(1, 0)
         self.assertEqual(index.data(), "my shiny table (new)")
         self.assertIsNone(index.data(Qt.ItemDataRole.ForegroundRole))
-        self.assertTrue(index.data(Qt.ItemDataRole.CheckStateRole).value)
+        self.assertEqual(index.data(Qt.ItemDataRole.CheckStateRole), Qt.CheckState.Checked)
         self.assertIsNone(index.data(Qt.ItemDataRole.FontRole))
         self.assertEqual(
             index.data(Qt.ItemDataRole.ToolTipRole), "Table's mappings haven't been saved with the specification yet."
@@ -186,9 +186,24 @@ def test_turn_empty_row_into_non_real_table(self):
         index = self._model.index(2, 0)
         self._assert_empty_row(index)
 
+    def test_when_empty_row_turns_into_non_real_table_its_check_status_equals_select_all(self):
+        self._model.add_empty_row()
+        index = self._model.index(0, 0)
+        self.assertTrue(self._model.setData(index, Qt.CheckState.Unchecked.value, Qt.ItemDataRole.CheckStateRole))
+        empty_row_index = self._model.index(1, 0)
+        with signal_waiter(self._model.dataChanged, timeout=1.0) as waiter:
+            self._model.setData(empty_row_index, "my shiny table")
+            waiter.wait()
+        self.assertEqual(self._model.rowCount(), 3)
+        index = self._model.index(1, 0)
+        self.assertEqual(index.data(), "my shiny table (new)")
+        self.assertEqual(index.data(Qt.ItemDataRole.CheckStateRole), Qt.CheckState.Unchecked)
+        index = self._model.index(2, 0)
+        self._assert_empty_row(index)
+
     def _assert_empty_row(self, index):
         self.assertEqual(index.data(), "<rename this to add table>")
-        self.assertFalse(index.data(Qt.ItemDataRole.CheckStateRole).value)
+        self.assertEqual(index.data(Qt.ItemDataRole.CheckStateRole), Qt.CheckState.Unchecked)
         self.assertIsNone(index.data(Qt.ItemDataRole.ForegroundRole))
         self.assertTrue(index.data(Qt.ItemDataRole.FontRole).italic())
         flags = self._model.flags(index)