refactor(device_manager): share one SortTableItem with an optional sort key

bec_widgets/applications/views/device_manager_view/device_manager_dialogs/upload_redis_dialog.py

@@ -12,6 +12,9 @@ from qtpy import QtCore, QtGui, QtWidgets
 
 from bec_widgets.utils.colors import get_accent_colors
 from bec_widgets.utils.error_popups import SafeSlot
+from bec_widgets.widgets.control.device_manager.components.device_table.device_table import (
+    SortTableItem,
+)
 from bec_widgets.widgets.control.device_manager.components.ophyd_validation import (
     ConfigStatus,
     ConnectionStatus,
@@ -95,38 +98,9 @@ class DeviceStatusItem(QtWidgets.QWidget):
         self._update_display()
 
 
-class SortTableItem(QtWidgets.QTableWidgetItem):
-    """Custom TableWidgetItem with hidden __column_data attribute for sorting."""
-
-    def __lt__(self, other: QtWidgets.QTableWidgetItem) -> bool:
-        """Override less-than operator for sorting."""
-        if not isinstance(other, QtWidgets.QTableWidgetItem):
-            return NotImplemented
-        self_data = self.data(QtCore.Qt.ItemDataRole.UserRole)
-        other_data = other.data(QtCore.Qt.ItemDataRole.UserRole)
-        if self_data is not None and other_data is not None:
-            self_data: DeviceStatusItem
-            other_data: DeviceStatusItem
-            if self_data.config_status != other_data.config_status:
-                return self_data.config_status < other_data.config_status
-            else:
-                return self_data.connection_status < other_data.connection_status
-        return super().__lt__(other)
-
-    def __gt__(self, other: QtWidgets.QTableWidgetItem) -> bool:
-        """Override less-than operator for sorting."""
-        if not isinstance(other, QtWidgets.QTableWidgetItem):
-            return NotImplemented
-        self_data = self.data(QtCore.Qt.ItemDataRole.UserRole)
-        other_data = other.data(QtCore.Qt.ItemDataRole.UserRole)
-        if self_data is not None and other_data is not None:
-            self_data: DeviceStatusItem
-            other_data: DeviceStatusItem
-            if self_data.config_status != other_data.config_status:
-                return self_data.config_status > other_data.config_status
-            else:
-                return self_data.connection_status > other_data.connection_status
-        return super().__gt__(other)
+def _status_sort_key(item: DeviceStatusItem) -> tuple[ConfigStatus, ConnectionStatus]:
+    """Sort device status items by config status first, then by connection status."""
+    return item.config_status, item.connection_status
 
 
 class ValidationSection(QtWidgets.QGroupBox):
@@ -180,7 +154,7 @@ class ValidationSection(QtWidgets.QGroupBox):
         else:
             row_position = self.table.rowCount()
             self.table.insertRow(row_position)
-            sort_item = SortTableItem(device_name)
+            sort_item = SortTableItem(device_name, sort_key=_status_sort_key)
             sort_item.setText("")
             self.table.setItem(row_position, 0, sort_item)
             device_item = DeviceStatusItem(device_config, config_status, connection_status)

bec_widgets/widgets/control/device_manager/components/device_table/device_table.py

@@ -145,26 +145,48 @@ class CheckBoxDelegate(QtWidgets.QStyledItemDelegate):
 
 
 class SortTableItem(QtWidgets.QTableWidgetItem):
-    """Custom TableWidgetItem with hidden __column_data attribute for sorting."""
+    """
+    Table item that sorts by the value stored under the UserRole data role.
+
+    Items without UserRole data fall back to the QTableWidgetItem comparison.
+
+    Args:
+        *args: Positional arguments forwarded to QTableWidgetItem.
+        sort_key (Callable[[Any], Any] | None): Optional callable that maps the UserRole data
+            to the value that is compared. If None, the UserRole data is compared directly.
+        **kwargs: Keyword arguments forwarded to QTableWidgetItem.
+    """
+
+    def __init__(self, *args, sort_key: Callable[[Any], Any] | None = None, **kwargs) -> None:
+        super().__init__(*args, **kwargs)
+        self._sort_key = sort_key
+
+    def _sort_values(self, other: QtWidgets.QTableWidgetItem) -> tuple[Any, Any] | None:
+        """Return the values to compare, or None if either item has no UserRole data."""
+        self_data = self.data(QtCore.Qt.ItemDataRole.UserRole)
+        other_data = other.data(QtCore.Qt.ItemDataRole.UserRole)
+        if self_data is None or other_data is None:
+            return None
+        if self._sort_key is None:
+            return self_data, other_data
+        return self._sort_key(self_data), self._sort_key(other_data)
 
     def __lt__(self, other: QtWidgets.QTableWidgetItem) -> bool:
         """Override less-than operator for sorting."""
         if not isinstance(other, QtWidgets.QTableWidgetItem):
             return NotImplemented
-        self_data = self.data(QtCore.Qt.ItemDataRole.UserRole)
-        other_data = other.data(QtCore.Qt.ItemDataRole.UserRole)
-        if self_data is not None and other_data is not None:
-            return self_data < other_data
+        values = self._sort_values(other)
+        if values is not None:
+            return values[0] < values[1]
         return super().__lt__(other)
 
     def __gt__(self, other: QtWidgets.QTableWidgetItem) -> bool:
-        """Override less-than operator for sorting."""
+        """Override greater-than operator for sorting."""
         if not isinstance(other, QtWidgets.QTableWidgetItem):
             return NotImplemented
-        self_data = self.data(QtCore.Qt.ItemDataRole.UserRole)
-        other_data = other.data(QtCore.Qt.ItemDataRole.UserRole)
-        if self_data is not None and other_data is not None:
-            return self_data > other_data
+        values = self._sort_values(other)
+        if values is not None:
+            return values[0] > values[1]
         return super().__gt__(other)
 
 

tests/unit_tests/test_device_manager_components.py

@@ -37,6 +37,9 @@ from bec_widgets.widgets.control.device_manager.components.device_config_templat
     ReadoutPriorityComboBox,
     _try_literal_eval,
 )
+from bec_widgets.widgets.control.device_manager.components.device_table.device_table import (
+    SortTableItem,
+)
 from bec_widgets.widgets.control.device_manager.components.device_table.device_table_row import (
     DeviceTableRow,
 )
@@ -295,6 +298,42 @@ class TestDeviceTableRow:
         )
 
 
+class TestSortTableItem:
+    """Test class for SortTableItem."""
+
+    @staticmethod
+    def _item(data=None, text: str = "", sort_key=None) -> SortTableItem:
+        item = SortTableItem(text, sort_key=sort_key)
+        if data is not None:
+            item.setData(QtCore.Qt.ItemDataRole.UserRole, data)
+        return item
+
+    def test_compares_user_role_data(self):
+        """Without a sort key, the UserRole data is compared directly."""
+        low, high = self._item(1, text="b"), self._item(2, text="a")
+        assert low < high
+        assert high > low
+        assert not high < low
+
+    def test_sort_key(self):
+        """A sort key maps the UserRole data before comparing."""
+
+        def key(data: dict) -> tuple[int, int]:
+            return data["first"], data["second"]
+
+        a = self._item({"first": 0, "second": 2}, sort_key=key)
+        b = self._item({"first": 1, "second": 0}, sort_key=key)
+        c = self._item({"first": 0, "second": 1}, sort_key=key)
+        assert a < b and b > a
+        assert c < a and a > c
+
+    def test_non_item_comparison(self):
+        """Comparing with a non-item returns NotImplemented."""
+        item = self._item(1)
+        assert item.__lt__(1) is NotImplemented
+        assert item.__gt__(1) is NotImplemented
+
+
 class TestDeviceTable:
     """Test class for DeviceTable component."""
 

tests/unit_tests/test_device_manager_view.py

@@ -372,6 +372,22 @@ class TestDeviceManagerViewDialogs:
         section.update_summary("2 devices validated, 1 failed.")
         assert section.summary_label.text() == "2 devices validated, 1 failed."
 
+    def test_validation_section_sorts_by_status(self, device_config: dict, qtbot):
+        """Rows sort by config status first, then by connection status."""
+        section = ValidationSection(title="Validation Results")
+        qtbot.addWidget(section)
+        for name, config_status, connection_status in (
+            ("dev_a", 1, 0),
+            ("dev_b", 0, 2),
+            ("dev_c", 0, 1),
+        ):
+            config = device_config.copy()
+            config["name"] = name
+            section.add_device(config, config_status, connection_status)
+
+        names = [section.table.cellWidget(row, 0).device_name for row in range(3)]
+        assert names == ["dev_c", "dev_b", "dev_a"]
+
     @pytest.fixture
     def device_configs_valid(self, device_config: dict):
         """Fixture for multiple device configurations."""