diff --git a/Furious/Repository/Storage.py b/Furious/Repository/Storage.py index 8fff29a..2e173a2 100644 --- a/Furious/Repository/Storage.py +++ b/Furious/Repository/Storage.py @@ -118,6 +118,11 @@ class Storage: """Return one subscription group by stable ID.""" return Storage._UserSubsStorage().group(unique) + @staticmethod + def moveSubscriptionGroups(groupIds, position: str) -> bool: + """Move selected subscription groups in their persisted display order.""" + return Storage._UserSubsStorage().moveGroups(groupIds, position) + @staticmethod def upsertSubscriptionGroup(group: SubscriptionGroup): """Persist one subscription group through the shared repository.""" diff --git a/Furious/Repository/Subscriptions.py b/Furious/Repository/Subscriptions.py index cb9ae0f..5878414 100644 --- a/Furious/Repository/Subscriptions.py +++ b/Furious/Repository/Subscriptions.py @@ -219,6 +219,41 @@ class UserSubs(Mixins.CleanupOnExit, StorageBackend): else None ) + def moveGroups(self, groupIds, position: str) -> bool: + """Move selected groups one position while preserving relative order.""" + selected = {str(groupId) for groupId in groupIds} + items = list(self._data.items()) + selected.intersection_update(unique for unique, _value in items) + + if not selected or position not in ('up', 'down'): + return False + + originalOrder = list(items) + + def isSelected(item): + """Return whether the group belongs to the selected identity set.""" + return item[0] in selected + + if position == 'up': + for index in range(1, len(items)): + if isSelected(items[index]) and not isSelected(items[index - 1]): + items[index - 1], items[index] = items[index], items[index - 1] + else: + for index in range(len(items) - 2, -1, -1): + if isSelected(items[index]) and not isSelected(items[index + 1]): + items[index], items[index + 1] = items[index + 1], items[index] + + if items == originalOrder: + return False + + self._data.clear() + self._data.update(items) + + for order, value in enumerate(self._data.values()): + value['sortOrder'] = order + + return True + def upsertGroup(self, group: SubscriptionGroup): """Insert or replace one group without changing its identity.""" self.upsertGroups((group,)) diff --git a/Furious/Widget/SubscriptionTableView.py b/Furious/Widget/SubscriptionTableView.py index 715d62a..cbd0a09 100644 --- a/Furious/Widget/SubscriptionTableView.py +++ b/Furious/Widget/SubscriptionTableView.py @@ -41,7 +41,6 @@ from Furious.Qt import ( AppQHeaderView, AppQMenu, AppQMessageBox, - AppQSeparator, AppQTableView, ) from Furious.Qt import gettext as _ @@ -580,23 +579,36 @@ class SubscriptionTableView(Mixins.QTranslatable, AppQTableView): self.setDropIndicatorShown(False) self.setDefaultDropAction(QtCore.Qt.DropAction.IgnoreAction) - contextMenuActions = [ - AppQAction( - _('Move Up'), - callback=lambda: self.moveSelectedGroup(-1), + self.moveUpActionRef = AppQAction( + _('Move Up'), + callback=lambda: self.moveSelectedGroups('up'), + shortcut=QtCore.QKeyCombination( + QtCore.Qt.KeyboardModifier.ControlModifier, + QtCore.Qt.Key.Key_Up, ), - AppQAction( - _('Move Down'), - callback=lambda: self.moveSelectedGroup(1), + parent=self, + ) + self.moveDownActionRef = AppQAction( + _('Move Down'), + callback=lambda: self.moveSelectedGroups('down'), + shortcut=QtCore.QKeyCombination( + QtCore.Qt.KeyboardModifier.ControlModifier, + QtCore.Qt.Key.Key_Down, ), - AppQSeparator(), - AppQAction( - _('Delete'), - callback=lambda: self.deleteSelectedItem(), - ), - ] + parent=self, + ) + + self.contextMenu = AppQMenu( + self.moveUpActionRef, + self.moveDownActionRef, + parent=self, + ) + + for action in (self.moveUpActionRef, self.moveDownActionRef): + action.setShortcutContext(QtCore.Qt.ShortcutContext.WidgetShortcut) + + self.addAction(action) - self.contextMenu = AppQMenu(*contextMenuActions) self.setContextMenuPolicy(QtCore.Qt.ContextMenuPolicy.CustomContextMenu) # Signals @@ -619,6 +631,39 @@ class SubscriptionTableView(Mixins.QTranslatable, AppQTableView): return tuple(keys[row] for row in self.selectedIndex if row < len(keys)) + def _currentUnique(self): + """Return the stable subscription ID represented by the current row.""" + row = self.currentIndex().row() + keys = tuple(Storage.UserSubs()) + + return keys[row] if 0 <= row < len(keys) else None + + def _restoreGroupSelection(self, selectedUniques, currentUnique): + """Restore logical selection, current item, and keyboard focus by ID.""" + rows = {unique: row for row, unique in enumerate(Storage.UserSubs())} + selection = self.selectionModel() + selection.clearSelection() + flags = ( + QtCore.QItemSelectionModel.SelectionFlag.Select + | QtCore.QItemSelectionModel.SelectionFlag.Rows + ) + + for unique in selectedUniques: + row = rows.get(unique) + + if row is not None: + selection.select(self.sourceModel.index(row, 0), flags) + + currentRow = rows.get(currentUnique) + + if currentRow is not None: + selection.setCurrentIndex( + self.sourceModel.index(currentRow, 0), + QtCore.QItemSelectionModel.SelectionFlag.NoUpdate, + ) + + self.setFocus() + @QtCore.Slot(QtCore.QPoint) def handleCustomContextMenuRequested(self, point): """Handle custom context menu requested.""" @@ -687,36 +732,24 @@ class SubscriptionTableView(Mixins.QTranslatable, AppQTableView): # Show the MessageBox asynchronously mbox.open() - def moveSelectedGroup(self, offset: int): - """Move one group while preserving its stable ID and timer.""" - indexes = self.selectedIndex + def moveSelectedGroups(self, position: str): + """Move selected groups while preserving identity, selection, and timers.""" + selectedUniques = self.selectedUniques - if len(indexes) != 1 or offset not in (-1, 1): + if not selectedUniques or position not in ('up', 'down'): return - source = indexes[0] - target = source + offset - - items = list(Storage.UserSubs().items()) - - if target < 0 or target >= len(items): - return + currentUnique = self._currentUnique() self.sourceModel.layoutAboutToBeChanged.emit() - item = items.pop(source) - - items.insert(target, item) - - Storage.UserSubs().clear() - Storage.UserSubs().update(items) - - for order, (unique, value) in enumerate(items): - value['sortOrder'] = order + changed = Storage.moveSubscriptionGroups(selectedUniques, position) self.sourceModel.layoutChanged.emit() - self.selectRow(target) - self.groupsChanged.emit() + self._restoreGroupSelection(selectedUniques, currentUnique) + + if changed: + self.groupsChanged.emit() @QtCore.Slot(object) def refreshSubscriptionState(self, uniques): diff --git a/tests/test_qt_interactions.py b/tests/test_qt_interactions.py index 293a2e0..391dcfc 100644 --- a/tests/test_qt_interactions.py +++ b/tests/test_qt_interactions.py @@ -28,6 +28,7 @@ from Furious.Repository import Storage, SubscriptionGroup from Furious.Qt import AppQDialog, AppQSwitch, gettext from Furious.Widget.RoutingSelector import RoutingSelector from Furious.Widget.ServerTableView import ServerTableView +from Furious.Widget.SubscriptionTableView import SubscriptionTableView from Furious.Window.HomePage import HomePage from Furious.Window.SettingsPage import ( _SystemProxySettingsCard, @@ -591,6 +592,188 @@ class ServerTableQtInteractionTest(unittest.TestCase): self._destroyTable(table) +class SubscriptionTableQtInteractionTest(unittest.TestCase): + """Protect subscription ordering through real selection and shortcuts.""" + + @classmethod + def setUpClass(cls): + """Create the process-wide headless QApplication.""" + application() + + def tearDown(self): + """Release the subscription repository and deferred table objects.""" + Storage._UserSubsStorage.cache_clear() + collectAtBoundary() + + @staticmethod + def _table(parent=None): + """Build one visible table with deterministic stable subscription IDs.""" + for order, unique in enumerate(('A', 'B', 'C', 'D', 'E')): + Storage.upsertSubscriptionGroup( + SubscriptionGroup( + id=unique, + remark=unique, + sortOrder=order, + ) + ) + + table = SubscriptionTableView(parent=parent) + table.resize(900, 360) + table.show() + table.activateWindow() + processQtEvents() + + return table + + def _clickRow(self, table, row, modifiers=QtCore.Qt.NoModifier): + """Select one subscription row through the real viewport mouse path.""" + index = table.sourceModel.index(row, 0) + rectangle = table.visualRect(index) + + self.assertTrue(index.isValid()) + self.assertFalse(rectangle.isEmpty()) + + QTest.mouseClick( + table.viewport(), + QtCore.Qt.MouseButton.LeftButton, + modifiers, + rectangle.center(), + ) + processQtEvents() + + @staticmethod + def _destroyTable(table): + """Release the persistent table, menu, and owned actions through Qt.""" + table.close() + table.deleteLater() + + def testContextMenuHasOnlyWidgetScopedMoveShortcuts(self): + """Expose only table-scoped Ctrl+Up and Ctrl+Down move commands.""" + with isolatedSettings(): + table = self._table() + + try: + actions = tuple( + action + for action in table.contextMenu.actions() + if not action.isSeparator() + ) + + self.assertEqual( + [action.textEnglish for action in actions], + ['Move Up', 'Move Down'], + ) + self.assertNotIn('Delete', [action.textEnglish for action in actions]) + + for action, key in ( + (table.moveUpActionRef, QtCore.Qt.Key.Key_Up), + (table.moveDownActionRef, QtCore.Qt.Key.Key_Down), + ): + self.assertEqual( + action.shortcut(), + QtGui.QKeySequence( + QtCore.QKeyCombination( + QtCore.Qt.KeyboardModifier.ControlModifier, + key, + ) + ), + ) + self.assertEqual( + action.shortcutContext(), + QtCore.Qt.ShortcutContext.WidgetShortcut, + ) + self.assertIn(action, table.actions()) + self.assertIs(action.parent(), table) + finally: + self._destroyTable(table) + + def testMultiMoveShortcutsPreserveIdentityCurrentItemAndFocus(self): + """Keep noncontiguous selection ready across repeated keyboard moves.""" + with isolatedSettings(): + table = self._table() + + try: + self._clickRow(table, 1) + self._clickRow( + table, + 3, + QtCore.Qt.KeyboardModifier.ControlModifier, + ) + + selected = {'B', 'D'} + current = 'D' + + for expected in ( + ('B', 'A', 'D', 'C', 'E'), + ('B', 'D', 'A', 'C', 'E'), + ): + QTest.keyClick( + table, + QtCore.Qt.Key.Key_Up, + QtCore.Qt.KeyboardModifier.ControlModifier, + ) + processQtEvents() + + self.assertEqual(tuple(Storage.UserSubs()), expected) + self.assertEqual(set(table.selectedUniques), selected) + self.assertEqual(table._currentUnique(), current) + self.assertTrue(table.hasFocus()) + + QTest.keyClick( + table, + QtCore.Qt.Key.Key_Down, + QtCore.Qt.KeyboardModifier.ControlModifier, + ) + processQtEvents() + + self.assertEqual( + tuple(Storage.UserSubs()), + ('A', 'B', 'D', 'C', 'E'), + ) + self.assertEqual(set(table.selectedUniques), selected) + self.assertEqual(table._currentUnique(), current) + self.assertTrue(table.hasFocus()) + finally: + self._destroyTable(table) + + def testMoveShortcutDoesNotFireFromSiblingEditor(self): + """Keep subscription movement inactive while a sibling editor has focus.""" + with isolatedSettings(): + window = QWidget() + layout = QVBoxLayout(window) + editor = QLineEdit(window) + table = self._table(parent=window) + + layout.addWidget(editor) + layout.addWidget(table) + window.resize(900, 500) + window.show() + window.activateWindow() + processQtEvents() + + try: + self._clickRow(table, 1) + editor.setFocus() + + self.assertTrue(waitFor(lambda: application().focusWidget() is editor)) + + before = tuple(Storage.UserSubs()) + + QTest.keyClick( + editor, + QtCore.Qt.Key.Key_Down, + QtCore.Qt.KeyboardModifier.ControlModifier, + ) + processQtEvents() + + self.assertEqual(tuple(Storage.UserSubs()), before) + self.assertIs(application().focusWidget(), editor) + finally: + self._destroyTable(table) + window.close() + window.deleteLater() + + class SharedSettingsQtWorkflowTest(unittest.TestCase): """Exercise real Home/Settings controls around one shared controller.""" diff --git a/tests/test_repository_contracts.py b/tests/test_repository_contracts.py index a47687d..a7c2ae7 100644 --- a/tests/test_repository_contracts.py +++ b/tests/test_repository_contracts.py @@ -175,6 +175,31 @@ class RepositoryContractTest(unittest.TestCase): ['A', 'Hidden', 'B', 'C', 'D'], ) + def testSubscriptionMultiMovesPreserveSelectedAndUnselectedOrder(self): + """Move subscription selections as stable blocks in repository order.""" + with isolatedSettings(): + repository = UserSubs() + + for order, unique in enumerate(('A', 'B', 'C', 'D', 'E')): + repository.upsertGroup( + SubscriptionGroup( + id=unique, + remark=unique, + sortOrder=order, + ) + ) + + self.assertTrue(repository.moveGroups(('B', 'D'), 'up')) + self.assertEqual(tuple(repository.data()), ('B', 'A', 'D', 'C', 'E')) + self.assertEqual( + [value['sortOrder'] for value in repository.data().values()], + list(range(5)), + ) + + self.assertTrue(repository.moveGroups(('B', 'D'), 'down')) + self.assertEqual(tuple(repository.data()), ('A', 'B', 'C', 'D', 'E')) + self.assertFalse(repository.moveGroups(('A',), 'up')) + def testMovingBetweenSubscriptionGroupsDetachesSyncOwnership(self): """Keep no-op ownership but make cross-group moves locally managed.""" with isolatedSettings():