Index: doc/changelog.md ================================================================== --- doc/changelog.md +++ doc/changelog.md @@ -1,9 +1,16 @@ # Changelog # Next version (in development) +# New features + +- Move cards around via drag&drop + - Drop onto other pages to move cards to those pages + - Drop cards between pages to move those cards onto a new page at that location + - Re-order cards within a page + # Fixed issues - Fixed crash when trying to load a document that contains a hidden printing that does not have an available replacement printing. - Restored reporting if hidden printings were migrated to available ones during document loading Index: mtg_proxy_printer/document_controller/compact_document.py ================================================================== --- mtg_proxy_printer/document_controller/compact_document.py +++ mtg_proxy_printer/document_controller/compact_document.py @@ -19,11 +19,11 @@ from mtg_proxy_printer.model.document import Document from mtg_proxy_printer.units_and_sizes import PageType from ._interface import DocumentAction, IllegalStateError, ActionList, Self from .page_actions import ActionRemovePage -from .move_cards import ActionMoveCards +from .move_cards import ActionMoveCardsBetweenPages from mtg_proxy_printer.logger import get_logger logger = get_logger(__name__) @@ -81,11 +81,11 @@ page_to_draw_from = document.pages[last_index] if page_to_draw_from.page_type() is not page_type: last_index -= 1 continue cards_to_take = min(len(page_to_draw_from), cards_to_add) - action = ActionMoveCards(last_index, range(cards_to_take), current_index) + action = ActionMoveCardsBetweenPages(last_index, range(cards_to_take), current_index) self.actions.append(action.apply(document)) cards_to_add -= cards_to_take logger.debug(f"Moved {cards_to_take} from page {last_index} to page {current_index}. " f"Free slots in target: {maximum_cards_per_page-len(current_page)}") if not page_to_draw_from: Index: mtg_proxy_printer/document_controller/edit_document_settings.py ================================================================== --- mtg_proxy_printer/document_controller/edit_document_settings.py +++ mtg_proxy_printer/document_controller/edit_document_settings.py @@ -20,11 +20,11 @@ from PySide6.QtCore import QObject from ._interface import DocumentAction, ActionList, Self, split_iterable from .card_actions import ActionRemoveCards -from .move_cards import ActionMoveCards +from .move_cards import ActionMoveCardsBetweenPages from .page_actions import ActionNewPage from mtg_proxy_printer.logger import get_logger from mtg_proxy_printer.units_and_sizes import PageType from mtg_proxy_printer.model.page_layout import PageLayoutSettings @@ -110,11 +110,11 @@ page_capacity = document.page_layout.compute_page_card_capacity(partition.page_type) # TODO: The algorithm currently isn't very optimized. This loop should insert new pages, # if the excess exceeds some threshold. for page_index, page in enumerate(partition.pages[:-1], start=start_index): if (page_length := len(page)) > page_capacity: - action = ActionMoveCards(page_index, range(page_capacity, page_length), page_index+1, 0) + action = ActionMoveCardsBetweenPages(page_index, range(page_capacity, page_length), page_index + 1, 0) self.reflow_actions.append(action.apply(document)) last_page = partition.pages[-1] if (page_length := len(last_page)) > page_capacity: excess = (c.card for c in last_page[page_capacity:]) excess = split_iterable(excess, page_capacity) Index: mtg_proxy_printer/document_controller/move_cards.py ================================================================== --- mtg_proxy_printer/document_controller/move_cards.py +++ mtg_proxy_printer/document_controller/move_cards.py @@ -10,39 +10,41 @@ # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU General Public License for more details. # # You should have received a copy of the GNU General Public License # along with this program. If not, see . - +import enum from collections.abc import Sequence import functools import typing from PySide6.QtCore import QModelIndex, QObject from mtg_proxy_printer.natsort import to_list_of_ranges from ._interface import DocumentAction, IllegalStateError, Self from mtg_proxy_printer.logger import get_logger +from .page_actions import ActionNewPage if typing.TYPE_CHECKING: from mtg_proxy_printer.model.document_page import Page from mtg_proxy_printer.model.document import Document logger = get_logger(__name__) del get_logger __all__ = [ - "ActionMoveCards", + "ActionMoveCardsBetweenPages", + "ActionMoveCardsWithinPage", ] -class ActionMoveCards(DocumentAction): +class ActionMoveCardsBetweenPages(DocumentAction): """ Moves a sequence of cards from a source page to a target page. By default, cards are appended. Values of consecutive card ranges are inclusive. """ - COMPARISON_ATTRIBUTES = ["source_page", "target_page", "card_ranges_to_move", "target_row"] + COMPARISON_ATTRIBUTES = ["source_page", "target_page", "card_ranges_to_move", "target_row", "insert_page_action"] def __init__( self, source: int, cards_to_move: Sequence[int], target_page: int, target_row: int = None, parent: QObject = None): """ @@ -49,18 +51,27 @@ :param source: The source page, as integer page number (0-indexed) :param cards_to_move: The cards to move, as indices into the source Page. May be in any order. (0-indexed) :param target_page: The target page, as integer page number. (0-indexed) :param target_row: If given, the cards_to_move are inserted at that array index (0-indexed). Existing cards in the target page at that index are pushed back. + None means "append". -1 means "0, but insert new page at target_page" """ super().__init__(parent) - self.source_page = source + if target_row == -1: + target_row = None + self.insert_page_action = ActionNewPage(target_page, parent=self) + else: + self.insert_page_action = None + # When inserting a new page before the source page, add one to compensate + self.source_page = source + (target_page < source and self.insert_page_action is not None) self.target_page = target_page self.target_row = target_row self.card_ranges_to_move = to_list_of_ranges(cards_to_move) def apply(self, document: "Document") -> Self: + if self.insert_page_action is not None: + self.insert_page_action.apply(document) source_page = document.pages[self.source_page] target_page = document.pages[self.target_page] source_page_type = source_page.page_type() target_page_type = target_page.page_type() if not target_page.accepts_card(source_page_type): @@ -68,16 +79,16 @@ f"Can not move card requesting page type {source_page_type} " f"onto a page with type {target_page_type}" ) source_index = document.index(self.source_page, 0) target_index = document.index(self.target_page, 0) + destination_row = len(target_page) if self.target_row is None else self.target_row - target_row = len(target_page) if self.target_row is None else self.target_row for source_row_first, source_row_last in reversed(self.card_ranges_to_move): self._move_cards_to_target_page( document, source_index, source_page, source_row_first, source_row_last, target_index, - target_page, target_row + target_page, destination_row ) if source_page.page_type() != source_page_type: document.page_type_changed.emit(source_index) if target_page.page_type() != target_page_type: document.page_type_changed.emit(target_index) @@ -110,11 +121,12 @@ source_row_last = source_row_first + target_row_last - target_row_first self._move_cards_to_target_page( document, source_index, source_page, source_row_first, source_row_last, target_index, target_page, target_row_first ) - + if self.insert_page_action is not None: + self.insert_page_action.undo(document) if source_page.page_type() != source_page_type: document.page_type_changed.emit(source_index) if target_page.page_type() != target_page_type: document.page_type_changed.emit(target_index) return super().undo(document) @@ -129,5 +141,118 @@ count = self._total_moved_cards() return self.tr( "Move %n card(s) from page {source_page} to {target_page}", "Undo/redo tooltip text", count ).format(source_page=source_page, target_page=target_page) + + +class CardMove(typing.NamedTuple): + first: int + last: int + target_row: int + moved_cards_count: int + + +class ActionMoveCardsWithinPage(DocumentAction): + """Move a subset of cards on a page to another position within the same page.""" + + def __init__( + self, page: int, cards_to_move: Sequence[int], + target_row: int | None, parent: QObject = None): + """ + :param page: The page with cards, as integer page number (0-indexed) + :param cards_to_move: The cards to move, as indices into the source Page. May be in any order. (0-indexed) + :param target_row: The cards_to_move are inserted before that array index (0-indexed). + """ + super().__init__(parent) + self.page = page + self.card_ranges_to_move = to_list_of_ranges(cards_to_move) + self.target_row = target_row + self.card_moves: list[CardMove] = [] + + def _total_moved_cards(self) -> int: + return sum(last-first+1 for first, last in self.card_ranges_to_move) + + def _get_card_move_ranges_without_zero_moves_at_ends(self, target_row: int, cards_on_page: int): + card_ranges = self.card_ranges_to_move.copy() + # Shortcut two special cases: + # If the first row is selected and the target is before the first row, skip that move and move the target back + # so that further moves put cards after the first block + if target_row == (card_range := card_ranges[0])[0] == 0: + target_row += card_range[1] - card_range[0] + 1 + del card_ranges[0] + if not card_ranges: + return [], target_row + # If the last row is selected and the target is after the last row, skip that move and move the target forward + # so that further moves put cards before the last block + if target_row == (card_range := card_ranges[-1])[1] + 1 == cards_on_page: + target_row -= card_range[1] - card_range[0] + 1 + del card_ranges[-1] + return card_ranges, target_row + + def _compute_card_moves(self, document: "Document", page_index: QModelIndex): + result: list[CardMove] = [] + card_ranges, target_row = self._get_card_move_ranges_without_zero_moves_at_ends( + self._get_target_row(document, page_index), document.rowCount(page_index)) + if not card_ranges: + return result + + source_offset = 0 + for first, last in card_ranges: + moved_cards = last-first+1 + if first <= target_row <= last+1: + # This batch of cards is currently at the correct location already, so no need to do anything further + continue + if last < target_row: + # While processing batches before the target_row, moving cards to the back will move the next ranges + # by that many cards to the front + first -= source_offset + last -= source_offset + source_offset += moved_cards + result.append(CardMove(first, last, target_row, moved_cards)) + if first > target_row: + # When moving cards to the front, the target row moves back that many cards to keep the order stable + target_row += moved_cards + return result + + def apply(self, document: "Document") -> Self: + super().apply(document) + page_index = document.index(self.page, 0) + page: Page = page_index.internalPointer() + self.card_moves = self._compute_card_moves(document, page_index) + for first, last, target_row, moved_cards_count in self.card_moves: # type: int, int, int, int + document.beginMoveRows(page_index, first, last, page_index, target_row) + moving_cards = page[first:last+1] + del page[first:last+1] + # If cards were removed before the target row, the target shifts moved_cards_count slots to the front. + target_row -= (last < target_row) * moved_cards_count + page[target_row:target_row] = moving_cards + document.endMoveRows() + return self + + def _get_target_row(self, document: "Document", page_index: QModelIndex): + return self.target_row if isinstance(self.target_row, int) else document.rowCount(page_index) + + def undo(self, document: "Document") -> Self: + super().undo(document) + page_index = document.index(self.page, 0) + page: Page = page_index.internalPointer() + card_moves = list(reversed(self.card_moves)) + for target_row, _, first, moved_cards_count in card_moves: # type: int, int, int, int + first -= (first > target_row) * moved_cards_count + last = first + moved_cards_count + document.beginMoveRows(page_index, first, last-1, page_index, target_row + (first < target_row) * moved_cards_count) + moving_cards = page[first:last] + del page[first:last] + # If cards were removed before the target row, the target shifts moved_cards_count slots to the front. + page[target_row:target_row] = moving_cards + document.endMoveRows() + return self + + @functools.cached_property + def as_str(self): + page = self.page+1 + count = self._total_moved_cards() + return self.tr( + "Reorder %n card(s) on page {page}", + "Undo/redo tooltip text", count + ).format(page=page) Index: mtg_proxy_printer/model/document.py ================================================================== --- mtg_proxy_printer/model/document.py +++ mtg_proxy_printer/model/document.py @@ -12,24 +12,29 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -import collections +from collections import deque, Counter from collections.abc import Generator, Iterable +import json +import typing import enum import itertools import math from pathlib import Path -import sys -from typing import Any, Counter +from typing import Any, Literal from PySide6.QtCore import QAbstractItemModel, QModelIndex, Qt, Slot, Signal, \ QPersistentModelIndex, QMimeData from mtg_proxy_printer.async_tasks.base import AsyncTask from mtg_proxy_printer.async_tasks.image_downloader import SingleDownloadTask, SingleActions +from mtg_proxy_printer.document_controller import DocumentAction +from mtg_proxy_printer.document_controller.replace_card import ActionReplaceCard +from mtg_proxy_printer.document_controller.save_document import ActionSaveDocument +from mtg_proxy_printer.document_controller.move_cards import ActionMoveCardsBetweenPages, ActionMoveCardsWithinPage from mtg_proxy_printer.document_controller.move_page import ActionMovePage from mtg_proxy_printer.model.imagedb_files import ImageKey from mtg_proxy_printer.natsort import to_list_of_ranges from mtg_proxy_printer.document_controller.edit_custom_card import ActionEditCustomCard from mtg_proxy_printer.model.document_page import CardContainer, Page, PageColumns @@ -38,24 +43,14 @@ from mtg_proxy_printer.model.card import MTGSet, Card, AnyCardType, CustomCard from mtg_proxy_printer.model.page_layout import PageLayoutSettings from mtg_proxy_printer.model.imagedb import ImageDatabase from mtg_proxy_printer.logger import get_logger -from mtg_proxy_printer.document_controller import DocumentAction -from mtg_proxy_printer.document_controller.replace_card import ActionReplaceCard -from mtg_proxy_printer.document_controller.save_document import ActionSaveDocument - -PAGE_MOVE_MIME_TYPE = "application/x-MTGProxyPrinter-PageMove" logger = get_logger(__name__) del get_logger -if sys.version_info[:2] >= (3, 9): - Counter = collections.Counter -else: - Counter = Counter - __all__ = [ "Document", ] @@ -62,16 +57,23 @@ class DocumentColumns(enum.IntEnum): Page = 0 INVALID_INDEX = QModelIndex() -ActionStack = collections.deque[DocumentAction] +ActionStack = deque[DocumentAction] AnyIndex = QModelIndex | QPersistentModelIndex ItemDataRole = Qt.ItemDataRole Orientation = Qt.Orientation ItemFlag = Qt.ItemFlag BlockingQueuedConnection = Qt.ConnectionType.BlockingQueuedConnection +PAGE_MOVE_MIME_TYPE = "application/x-MTGProxyPrinter-PageMove" +CARD_MOVE_MIME_TYPE = "application/x-MTGProxyPrinter-CardMove" +DRAG_OPERATION_TYPE = Literal["application/x-MTGProxyPrinter-PageMove"] | Literal["application/x-MTGProxyPrinter-CardMove"] | None + +class CardMoveMimeData(typing.TypedDict): + page: int + cards: list[int] class Document(QAbstractItemModel): """ This holds a multi-page document that contains any number of same-size pages. @@ -100,20 +102,25 @@ PageColumns.Language: self.tr("Language"), PageColumns.Image: self.tr("Image"), PageColumns.IsFront: self.tr("Side"), } - self.undo_stack: ActionStack = collections.deque() - self.redo_stack: ActionStack = collections.deque() + self.undo_stack: ActionStack = deque() + self.redo_stack: ActionStack = deque() self.save_file_path: Path | None = None self.card_db = card_db self.image_db = image_db self.pages: list[Page] = [first_page := Page()] # Mapping from page id() to list index in the page list self.page_index_cache: dict[int, int] = {id(first_page): 0} self.currently_edited_page = first_page self.page_layout = PageLayoutSettings.create_from_settings() + # The last started drag operation affects the index flags(). + self.current_drag_operation: DRAG_OPERATION_TYPE = None + # Also note the size of dragged cards. This allows checking, if pages can accept the drop, + # to prevent creation of MIXED pages. + self.current_drag_card_size: PageType = PageType.UNDETERMINED logger.debug(f"Loaded document settings from configuration file: {self.page_layout}") logger.info(f"Created {self.__class__.__name__} instance") @Slot(DocumentAction) def apply(self, action: DocumentAction): @@ -246,14 +253,19 @@ index = self._to_index(index) data = index.internalPointer() flags = super().flags(index) if isinstance(data, CardContainer) and (index.column() in self.EDITABLE_COLUMNS or data.card.is_custom_card): flags |= ItemFlag.ItemIsEditable - if isinstance(data, Page): - flags |= ItemFlag.ItemIsDragEnabled # Pages can be moved - if not index.isValid(): - flags |= ItemFlag.ItemIsDropEnabled # Only the root can accept drops to not overwrite items + if isinstance(data, Page|CardContainer): + flags |= ItemFlag.ItemIsDragEnabled # Pages and cards can be moved + if (not index.isValid() # Top level can accept any drop, both pages and cards, where the latter gets a new page + or ( + isinstance(data, Page) + and self.current_drag_operation == CARD_MOVE_MIME_TYPE # Pages only accept cards … + and data.accepts_card(self.current_drag_card_size) # with an acceptable size. + )): + flags |= ItemFlag.ItemIsDropEnabled return flags def setData(self, index: AnyIndex, value: Any, role: ItemDataRole = ItemDataRole.EditRole) -> bool: index = self._to_index(index) data: CardContainer = index.internalPointer() @@ -287,37 +299,86 @@ task = SingleDownloadTask(self.image_db, action) task.request_action.connect(self.apply, BlockingQueuedConnection) self.request_run_async_task.emit(task) def mimeData(self, indexes: list[QModelIndex], /) -> QMimeData: - """Supports encoding the row of a singular QModelIndex as QMimeData. Used for moving Pages via drag&drop.""" - if len(indexes) != 1: - return QMimeData() - row = indexes[0].row() - logger.debug(f"Initiating drag for page {row}") + """ + Reads model data and converts them into QMimeData used for Drag&Drop. + Dragging a page encodes its initial position + Dragging cards encodes their shared page index, and a list of card indices. + """ mime_data = QMimeData() - mime_data.setData(PAGE_MOVE_MIME_TYPE, row.to_bytes(8)) + if not indexes: + return mime_data + + if not (first := indexes[0]).parent().isValid(): + row = first.row() + logger.debug(f"Initiating drag for page {row}") + mime_data.setData(PAGE_MOVE_MIME_TYPE, row.to_bytes(8)) + self.current_drag_operation = PAGE_MOVE_MIME_TYPE + return mime_data + page = first.parent().row() + cards = sorted(set(index.row() for index in indexes)) + logger.debug(f"Initiating drag for {len(cards)} cards on page {page}") + data: CardMoveMimeData = {"page": page, "cards": cards} + encoded_data = json.dumps(data).encode("utf-8") + mime_data.setData(CARD_MOVE_MIME_TYPE, encoded_data) + self.current_drag_operation = CARD_MOVE_MIME_TYPE + self.current_drag_card_size = self.pages[page].page_type() return mime_data def dropMimeData( self, data: QMimeData, action: Qt.DropAction, row: int, column: PageColumns | DocumentColumns, parent: QModelIndex, /): - """Supports dropping pages moved via drag&drop. Only Page moves supported at the moment.""" + """Supports dropping cards or pages moved via drag&drop.""" + + # https://doc.qt.io/qt-6/qabstractitemmodel.html#dropMimeData: + # "When row and column are -1 it means that the dropped data should be considered as + # dropped directly on parent. Usually this will mean appending the data as child items of parent. + # If row and column are greater than or equal zero, it means that the drop occurred just + # before the specified row and column in the specified parent." if data.hasFormat(PAGE_MOVE_MIME_TYPE): + # Here, parent is always invalid. row == column == -1 means the drop ended on empty space within the view. + # The only location with empty space is below the last page, so treat it as if the user dropped directly + # below the last page, and move the page to the end. + # If row != -1, row states the drop location, so use that. logger.debug(f"Received page drop onto {row=}") - if row == -1: # Drop onto empty space or after last entry. Append in this case + if row == -1: row = self.rowCount() source_row = int.from_bytes(data.data(PAGE_MOVE_MIME_TYPE).data()) self.apply(ActionMovePage(source_row, row)) + elif data.hasFormat(CARD_MOVE_MIME_TYPE): + # Here, parent may be valid, and there are two main cases, one of which has 2 subcases: + card_data: CardMoveMimeData = json.loads(data.data(CARD_MOVE_MIME_TYPE).data()) + logger.debug(f"Received card drop onto {row=}: {card_data}") + # Case 1: Cards are dropped onto an existing page, given by parent.row(). + if parent.isValid(): + if row == column == -1: + # The drop ended on empty space within the page card table view. + # Append the cards at the end of the given page + row = self.rowCount(parent) + if parent.row() == card_data["page"]: + action = ActionMoveCardsWithinPage(parent.row(), card_data["cards"], row) + else: + action = ActionMoveCardsBetweenPages(card_data["page"], card_data["cards"], parent.row(), None) + else: + # Case 2: Cards are dropped between pages, and a new page must be inserted for the dropped cards + if row == column == -1: + # Subcase 1: The drop ended on empty space within the view. Append a new page. + row = self.rowCount() + # Subcase 2: Cards are moved to row on the page given by parent + action = ActionMoveCardsBetweenPages(card_data["page"], card_data["cards"], row, -1) + self.apply(action) + return False # Move complete, so signal via False that the caller does not have to remove the source rows def supportedDropActions(self, /) -> Qt.DropAction: return Qt.DropAction.MoveAction def mimeTypes(self, /) -> list[str]: - """Supported mime types. Currently only supporting Page moves""" - return [PAGE_MOVE_MIME_TYPE] + """Supported mime types.""" + return [PAGE_MOVE_MIME_TYPE, CARD_MOVE_MIME_TYPE] @staticmethod def _to_index(other: QPersistentModelIndex | QModelIndex) -> QModelIndex: return QModelIndex(other) if isinstance(other, QPersistentModelIndex) else other @@ -376,11 +437,11 @@ return card.is_front if role == ItemDataRole.EditRole else ( self.tr("Front") if card.is_front else self.tr("Back")) return None def _get_page_preview(self, page: Page): - names = collections.Counter(container.card.name for container in page) + names = Counter(container.card.name for container in page) return "\n".join(self.tr( "%n× {name}", "Used to display a card name and amount of copies in the page overview. " "Only needs translation for RTL language support", count).format(name=name) for name, count in names.items() ) @@ -403,11 +464,11 @@ def compute_pages_saved_by_compacting(self) -> int: """ Computes the number of pages that can be saved by compacting the document. """ - cards: Counter[PageType] = collections.Counter() + cards: Counter[PageType] = Counter() for page in self.pages: cards[page.page_type()] += len(page) required_pages = ( math.ceil(cards[PageType.OVERSIZED] / self.page_layout.compute_page_card_capacity(PageType.OVERSIZED)) + math.ceil(cards[PageType.REGULAR] / self.page_layout.compute_page_card_capacity(PageType.REGULAR)) Index: mtg_proxy_printer/model/document_page.py ================================================================== --- mtg_proxy_printer/model/document_page.py +++ mtg_proxy_printer/model/document_page.py @@ -35,10 +35,13 @@ @dataclasses.dataclass class CardContainer: parent: "Page" card: AnyCardType + def __repr__(self): + return f"{self.__class__.__name__}(card={self.card})" + class Page(list[CardContainer]): def __init__(self, __iterable: Iterable[AnyCardType] = None): __iterable = __iterable or [] Index: mtg_proxy_printer/page_scene/page_scene.py ================================================================== --- mtg_proxy_printer/page_scene/page_scene.py +++ mtg_proxy_printer/page_scene/page_scene.py @@ -361,23 +361,15 @@ position = self._compute_position_for_image(index.row(), page_type) if index.data(ItemDataRole.DisplayRole) is not None: # Card has a QPixmap set card_item = CardItem(index, self.document) self.addItem(card_item) card_item.setPos(position) - if next_item is not None: - # See https://doc.qt.io/qt-6/qgraphicsitem.html#sorting - # "You can call stackBefore() to reorder the list of children. - # This will directly modify the insertion order." - # This is required to keep the card order consistent with the model when inserting cards in the - # middle of the page. This can happen when undoing a card removal. The caller has to supply the - # item which’s position the new item takes. - card_item.stackBefore(next_item) def update_card_positions(self): page_type: PageType = self.selected_page.data(ItemDataRole.UserRole) - for index, card in enumerate(self.card_items): - card.setPos(self._compute_position_for_image(index, page_type)) + for card in self.card_items: + card.setPos(self._compute_position_for_image(card.index.row(), page_type)) def _is_valid_page_index(self, index: QModelIndex | QPersistentModelIndex): return index.isValid() and not index.parent().isValid() and index.row() < self.document.rowCount() @Slot(QModelIndex) @@ -446,21 +438,31 @@ elif not parent.isValid(): # Page removed. Update the page number text, as it contains the total number of pages self._update_page_number_text() def on_rows_moved(self, parent: QModelIndex, start: int, end: int, destination: QModelIndex, row: int): - if parent.isValid() and parent.row() == self.selected_page.row(): + source_page_row = parent.row() + current_page_row = self.selected_page.row() + destination_page_row = destination.row() + if not parent.isValid(): + # Moved pages around. Needs to update the current page text + self._update_page_number_text() + return + # Parent is valid, thus start:end+1 point to cards on the page source_page_row. + if source_page_row != current_page_row == destination_page_row: + # Cards moved onto the current page are treated as if they were added + logger.debug("Cards moved onto the currently shown page, calling card insertion handler.") + self.on_rows_inserted(destination, row, row + end - start) + elif source_page_row == current_page_row != destination_page_row: # Cards moved away are treated as if they were deleted logger.debug("Cards moved away from the currently shown page, calling card removal handler.") self.on_rows_removed(parent, start, end) - if destination.isValid() and destination.row() == self.selected_page.row(): - # Moved in cards are treated as if they were added - logger.debug("Cards moved onto the currently shown page, calling card insertion handler.") - self.on_rows_inserted(destination, row, row+end-start) - if not parent.isValid() and not destination.isValid(): - # Moved pages around. Needs to update the current page text - self._update_page_number_text() + elif source_page_row == current_page_row == destination_page_row: + logger.debug("Cards moved within the current page, reordering them") + self.update_card_positions() + # Remaining cases are card moves happening "off-screen", so nothing has to be done on them. + @functools.lru_cache(None) def _compute_position_for_image(self, index_row: int, page_type: PageType) -> QPointF: """Returns the page-absolute position of the top-left pixel of the given image.""" page_layout: PageLayoutSettings = self.document.page_layout Index: resources/ui/central_widget/columnar.ui ================================================================== --- resources/ui/central_widget/columnar.ui +++ resources/ui/central_widget/columnar.ui @@ -92,10 +92,16 @@ Qt::ContextMenuPolicy::CustomContextMenu 0 + + QAbstractItemView::DragDropMode::DragDrop + + + Qt::DropAction::MoveAction + true QAbstractItemView::SelectionBehavior::SelectRows @@ -128,11 +134,11 @@ false - QAbstractItemView::DragDropMode::InternalMove + QAbstractItemView::DragDropMode::DragDrop Qt::DropAction::MoveAction Index: resources/ui/central_widget/grouped.ui ================================================================== --- resources/ui/central_widget/grouped.ui +++ resources/ui/central_widget/grouped.ui @@ -38,11 +38,11 @@ 3 0 - QAbstractItemView::DragDropMode::InternalMove + QAbstractItemView::DragDropMode::DragDrop Qt::DropAction::MoveAction @@ -130,10 +130,16 @@ Qt::ContextMenuPolicy::CustomContextMenu 0 + + QAbstractItemView::DragDropMode::DragDrop + + + Qt::DropAction::MoveAction + true QAbstractItemView::SelectionBehavior::SelectRows Index: resources/ui/central_widget/tabbed_vertical.ui ================================================================== --- resources/ui/central_widget/tabbed_vertical.ui +++ resources/ui/central_widget/tabbed_vertical.ui @@ -72,11 +72,11 @@ - QAbstractItemView::DragDropMode::InternalMove + QAbstractItemView::DragDropMode::DragDrop Qt::DropAction::MoveAction @@ -117,10 +117,16 @@ Qt::ContextMenuPolicy::CustomContextMenu 0 + + QAbstractItemView::DragDropMode::DragDrop + + + Qt::DropAction::MoveAction + true QAbstractItemView::SelectionBehavior::SelectRows Index: setup_cx_freeze.py ================================================================== --- setup_cx_freeze.py +++ setup_cx_freeze.py @@ -22,11 +22,11 @@ import sys from cx_Freeze import setup, Executable ROOT_DIR = pathlib.Path(__file__).parent -resource_path = ROOT_DIR / "resources.bak" +resource_path = ROOT_DIR / "resources" main_package = "mtg_proxy_printer" meta_data = (ROOT_DIR/main_package/"meta_data.py").read_text() version = re.search( r"""^__version__\s*=\s*"(.*)"\s*""", meta_data, @@ -86,10 +86,11 @@ excludes += [ "platformdirs.android", "platformdirs.macos", "platformdirs.unix", ] + def get_icon() -> str: icon_path = resource_path / "icons" / "MTGPP.png" if sys.platform == "win32": dest = ROOT_DIR/"MTGPP.ico" @@ -98,10 +99,11 @@ with Image.open(icon_path) as src: src.save(dest, "ICO") return str(dest) else: return str(icon_path) + icon = get_icon() setup_parameters = { "executables": [ Index: tests/conftest.py ================================================================== --- tests/conftest.py +++ tests/conftest.py @@ -58,11 +58,11 @@ db.execute("PRAGMA reverse_unordered_selects = TRUE") return db @pytest.fixture -def image_db(qtbot, tmp_path: Path): +def image_db(qtbot, tmp_path: Path) -> ImageDatabase: image_db = ImageDatabase(tmp_path) regular = image_db.get_blank(CardSizes.REGULAR) large = image_db.get_blank(CardSizes.OVERSIZED) for scryfall_id, is_front in itertools.product( ["0000579f-7b35-4ed3-b44c-db2a538066fe", "b3b87bfc-f97f-4734-94f6-e3e2f335fc4d"], [True, False]): @@ -74,19 +74,19 @@ # Oversized card images key = ImageKey(scryfall_id, True, True) image_db.loaded_images[key] = large.copy() image_db.images_on_disk.add(key) - yield image_db + return image_db @pytest.fixture def document(qtbot, card_db: CardDatabase, image_db: ImageDatabase) -> Document: fill_card_database_with_json_cards(qtbot, card_db, [ "regular_english_card", "oversized_card", "english_double_faced_card"]) - document = Document(card_db, image_db) - yield document + return Document(card_db, image_db) + @pytest.fixture def mock_imagedb(): mock_image_db = unittest.mock.NonCallableMagicMock(spec=ImageDatabase) blanks = { @@ -95,21 +95,21 @@ } blanks[CardSizes.REGULAR].fill(QColorConstants.Transparent) blanks[CardSizes.OVERSIZED].fill(QColorConstants.Transparent) mock_image_db.get_blank = blanks.get return mock_image_db + @pytest.fixture def document_light(qtbot, mock_imagedb) -> Document: mock_card_db = unittest.mock.NonCallableMagicMock() mock_card_db.db = mtg_proxy_printer.sqlite_helpers.create_in_memory_database( "carddb", check_same_thread=False) - document = Document(mock_card_db, mock_imagedb) - yield document + return Document(mock_card_db, mock_imagedb) @pytest.fixture def page_layout() -> PageLayoutSettings: layout = PageLayoutSettings.create_from_settings() defaults = PageLayoutSettings.create_from_settings(mtg_proxy_printer.settings.DEFAULT_SETTINGS) assert_that(layout, is_dataclass_equal_to(defaults)) return layout Index: tests/document_controller/test_action_compact_document.py ================================================================== --- tests/document_controller/test_action_compact_document.py +++ tests/document_controller/test_action_compact_document.py @@ -22,11 +22,11 @@ from mtg_proxy_printer.model.document import Document from mtg_proxy_printer.document_controller import IllegalStateError from mtg_proxy_printer.document_controller.card_actions import ActionAddCard from mtg_proxy_printer.document_controller.page_actions import ActionNewPage, ActionRemovePage from mtg_proxy_printer.document_controller.compact_document import ActionCompactDocument -from mtg_proxy_printer.document_controller.move_cards import ActionMoveCards +from mtg_proxy_printer.document_controller.move_cards import ActionMoveCardsBetweenPages from tests.helpers import create_card from .helpers import append_new_card_in_page, card_container_with OVERSIZED = CardSizes.OVERSIZED @@ -104,11 +104,11 @@ action = ActionCompactDocument() action.apply(document_light) assert_that( action.actions, contains_exactly( - *[instance_of(ActionMoveCards)]*4, + *[instance_of(ActionMoveCardsBetweenPages)] * 4, instance_of(ActionRemovePage) ) ) assert_that(pages, has_length(4)) assert_that( @@ -151,8 +151,8 @@ ) ) assert_that( action.actions, contains_exactly( - *[instance_of(ActionMoveCards)]*2, + *[instance_of(ActionMoveCardsBetweenPages)] * 2, instance_of(ActionRemovePage)) ) Index: tests/document_controller/test_action_edit_document_settings.py ================================================================== --- tests/document_controller/test_action_edit_document_settings.py +++ tests/document_controller/test_action_edit_document_settings.py @@ -25,11 +25,11 @@ from mtg_proxy_printer.units_and_sizes import PageType, unit_registry, CardSizes from mtg_proxy_printer.model.page_layout import PageLayoutSettings from mtg_proxy_printer.model.document import Document from mtg_proxy_printer.document_controller.page_actions import ActionNewPage from mtg_proxy_printer.document_controller.card_actions import ActionAddCard -from mtg_proxy_printer.document_controller.move_cards import ActionMoveCards +from mtg_proxy_printer.document_controller.move_cards import ActionMoveCardsBetweenPages from mtg_proxy_printer.document_controller.edit_document_settings import ActionEditDocumentSettings from tests.helpers import create_card from .helpers import card_container_with, append_new_card_in_page @@ -203,11 +203,11 @@ action._already_applied = True old_settings = action.old_settings = copy.copy(document_light.page_layout) document_light.page_layout.page_height += 1*unit_registry.mm action.reflow_actions += [ new_page, - ActionMoveCards(0, range(7, 10), 1) + ActionMoveCardsBetweenPages(0, range(7, 10), 1) ] for sub_action in action.reflow_actions: sub_action._already_applied = True action.new_settings = document_light.page_layout Index: tests/document_controller/test_action_move_cards.py ================================================================== --- tests/document_controller/test_action_move_cards.py +++ tests/document_controller/test_action_move_cards.py @@ -12,11 +12,11 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . from functools import partial -from typing import Optional +from typing import Optional, Sequence import pytest from hamcrest import * from PySide6.QtCore import QModelIndex from pytestqt.qtbot import QtBot @@ -24,14 +24,15 @@ from mtg_proxy_printer.units_and_sizes import CardSizes, IntList from mtg_proxy_printer.model.document_page import PageType from mtg_proxy_printer.model.document import Document from mtg_proxy_printer.document_controller import IllegalStateError from mtg_proxy_printer.document_controller.page_actions import ActionNewPage -from mtg_proxy_printer.document_controller.move_cards import ActionMoveCards +from mtg_proxy_printer.document_controller.move_cards import ActionMoveCardsBetweenPages, ActionMoveCardsWithinPage from .helpers import card_container_with, append_new_card_in_page, card_container_with_name OptInt = Optional[int] + def validate_qt_model_move_signal_parameter( expected_source: int, expected_row_start: int, expected_row_end: int, expected_target: int, expected_target_row: int, @@ -62,11 +63,11 @@ ActionNewPage().apply(document_light) append_new_card_in_page(document_light.pages[0], "Normal", CardSizes.REGULAR) append_new_card_in_page(document_light.pages[1], "Large", CardSizes.OVERSIZED) assert_that(document_light.pages[0].page_type(), is_(PageType.REGULAR)) assert_that(document_light.pages[1].page_type(), is_(PageType.OVERSIZED)) - assert_that(calling(ActionMoveCards(0, [0], 1).apply).with_args(document_light), raises(IllegalStateError)) + assert_that(calling(ActionMoveCardsBetweenPages(0, [0], 1).apply).with_args(document_light), raises(IllegalStateError)) def test_apply_move_all_cards_onto_empty_page(qtbot: QtBot, document_light: Document): pages = document_light.pages ActionNewPage().apply(document_light) @@ -74,11 +75,11 @@ row_move_validator = partial(validate_qt_model_move_signal_parameter, 0, 0, 0, 1, 0) with qtbot.wait_signals( [document_light.page_type_changed] * 2 + [document_light.rowsAboutToBeMoved, document_light.rowsMoved], timeout=1000, check_params_cbs=[lambda index: index.row() == 0, lambda index: index.row() == 1] + [row_move_validator] * 2): - ActionMoveCards(0, [0], 1).apply(document_light) + ActionMoveCardsBetweenPages(0, [0], 1).apply(document_light) assert_that( pages, contains_exactly( empty(), @@ -97,11 +98,11 @@ row_move_validator = partial(validate_qt_model_move_signal_parameter, 0, 0, 0, 1, 1) with qtbot.wait_signals( [document_light.page_type_changed, document_light.rowsAboutToBeMoved, document_light.rowsMoved], timeout=1000, check_params_cbs=[lambda index: index.row() == 0] + [row_move_validator] * 2): - ActionMoveCards(0, [0], 1).apply(document_light) + ActionMoveCardsBetweenPages(0, [0], 1).apply(document_light) assert_that( pages, contains_exactly( empty(), @@ -123,11 +124,11 @@ row_move_validator = partial(validate_qt_model_move_signal_parameter, 0, 0, 0, 1, 0) with qtbot.wait_signals( [document_light.page_type_changed, document_light.rowsAboutToBeMoved, document_light.rowsMoved], timeout=1000, check_params_cbs=[lambda index: index.row() == 1] + [row_move_validator] * 2): - ActionMoveCards(0, [0], 1).apply(document_light) + ActionMoveCardsBetweenPages(0, [0], 1).apply(document_light) assert_that( pages, contains_exactly( contains_exactly( @@ -148,11 +149,11 @@ row_move_validator = partial(validate_qt_model_move_signal_parameter, 0, 0, 0, 1, 1) with qtbot.assert_not_emitted(document_light.page_type_changed), \ qtbot.wait_signals( [document_light.rowsAboutToBeMoved, document_light.rowsMoved], timeout=1000, check_params_cbs=[row_move_validator] * 2): - ActionMoveCards(0, [0], 1).apply(document_light) + ActionMoveCardsBetweenPages(0, [0], 1).apply(document_light) assert_that( pages, contains_exactly( contains_exactly( @@ -192,11 +193,11 @@ row_move_validator = partial(validate_qt_model_move_signal_parameter, 0, 1, 2, 1, 2) with qtbot.assert_not_emitted(document_light.page_type_changed), \ qtbot.wait_signals( [document_light.rowsAboutToBeMoved, document_light.rowsMoved], timeout=1000, check_params_cbs=[row_move_validator] * 2): - ActionMoveCards(0, [1, 2], 1).apply(document_light) + ActionMoveCardsBetweenPages(0, [1, 2], 1).apply(document_light) assert_that( pages, contains_exactly( contains_exactly( @@ -235,11 +236,11 @@ row_move_validator_2 = partial(validate_qt_model_move_signal_parameter, 0, 0, 0, 1, 1) with qtbot.assert_not_emitted(document_light.page_type_changed), \ qtbot.wait_signals( [document_light.rowsAboutToBeMoved, document_light.rowsMoved]*2, timeout=1000, check_params_cbs=[row_move_validator_1] * 2 + [row_move_validator_2] * 2): - ActionMoveCards(0, [0, 2], 1).apply(document_light) + ActionMoveCardsBetweenPages(0, [0, 2], 1).apply(document_light) assert_that( pages, contains_exactly( contains_exactly( card_container_with(on_page_0_0, pages[0])), @@ -256,11 +257,11 @@ pages = document_light.pages ActionNewPage().apply(document_light) card_1 = append_new_card_in_page(pages[0], "Move1") not_moved = append_new_card_in_page(pages[0], "Stay on 0") card_2 = append_new_card_in_page(pages[1], "After") - action = ActionMoveCards(0, [0], 1, 0) + action = ActionMoveCardsBetweenPages(0, [0], 1, 0) row_move_validator = partial(validate_qt_model_move_signal_parameter, 0, 0, 0, 1, 0) with qtbot.assert_not_emitted(document_light.page_type_changed), \ qtbot.wait_signals( [document_light.rowsAboutToBeMoved, document_light.rowsMoved], timeout=1000, check_params_cbs=[row_move_validator] * 2): @@ -285,11 +286,11 @@ ActionNewPage().apply(document_light) card_1 = append_new_card_in_page(pages[0], "Move1") not_moved = append_new_card_in_page(pages[0], "Stay on 0") card_2 = append_new_card_in_page(pages[1], "Before") card_3 = append_new_card_in_page(pages[1], "After") - action = ActionMoveCards(0, [0], 1, 1) + action = ActionMoveCardsBetweenPages(0, [0], 1, 1) row_move_validator = partial(validate_qt_model_move_signal_parameter, 0, 0, 0, 1, 1) with qtbot.assert_not_emitted(document_light.page_type_changed), \ qtbot.wait_signals( [document_light.rowsAboutToBeMoved, document_light.rowsMoved], timeout=1000, check_params_cbs=[row_move_validator] * 2): @@ -315,11 +316,11 @@ pages = document_light.pages ActionNewPage().apply(document_light) card_1 = append_new_card_in_page(pages[0], "Move1") not_moved = append_new_card_in_page(pages[0], "Stay on 0") card_2 = append_new_card_in_page(pages[1], "Before") - action = ActionMoveCards(0, [0], 1, target_row) + action = ActionMoveCardsBetweenPages(0, [0], 1, target_row) row_move_validator = partial(validate_qt_model_move_signal_parameter, 0, 0, 0, 1, 1) with qtbot.assert_not_emitted(document_light.page_type_changed), \ qtbot.wait_signals( [document_light.rowsAboutToBeMoved, document_light.rowsMoved], timeout=1000, check_params_cbs=[row_move_validator] * 2): @@ -343,11 +344,11 @@ def test_apply_without_indices_does_nothing(qtbot: QtBot, document_light: Document, target_row: OptInt): pages = document_light.pages ActionNewPage().apply(document_light) card_1 = append_new_card_in_page(pages[0], "Move1") card_2 = append_new_card_in_page(pages[1], "After") - action = ActionMoveCards(0, [], 1, target_row) + action = ActionMoveCardsBetweenPages(0, [], 1, target_row) with qtbot.assert_not_emitted(document_light.page_type_changed), \ qtbot.assert_not_emitted(document_light.rowsAboutToBeMoved), \ qtbot.assert_not_emitted(document_light.rowsMoved): action.apply(document_light) assert_that( @@ -364,18 +365,18 @@ ) @pytest.mark.parametrize("indices", [[], [0], [1], [0, 1], [0, 2], [0, 1, 2], [0, 1, 3, 4]]) def test___total_moved_cards(indices: IntList): - action = ActionMoveCards(0, indices, 0) + action = ActionMoveCardsBetweenPages(0, indices, 0) expected_result = len(indices) assert_that(action._total_moved_cards(), is_(equal_to(expected_result))) def _create_applied_action( - source: int, cards_to_move: list[int], target_page: int, target_row: int = None) -> ActionMoveCards: - action = ActionMoveCards(source, cards_to_move, target_page, target_row) + source: int, cards_to_move: list[int], target_page: int, target_row: int = None) -> ActionMoveCardsBetweenPages: + action = ActionMoveCardsBetweenPages(source, cards_to_move, target_page, target_row) action._already_applied = True return action def test_undo_resets_already_applied(document_light: Document): @@ -602,5 +603,131 @@ card_container_with(c3, pages[1]), ) ), "Incorrect card move" ) + +@pytest.fixture() +def document_with_cards(document_light: Document) -> Document: + ActionNewPage(count=3, content=[[], [], []]).apply(document_light) + pages = document_light.pages + content: list[tuple[int, str]] = [ + (0, "A1"), + (0, "A2"), + (0, "A3"), + (1, "B1"), + (1, "B2"), + (1, "B3"), + (3, "D1"), + (3, "D2"), + (3, "D3"), + (3, "D4"), + (3, "D5"), + (3, "D6"), + ] + for page, name in content: + append_new_card_in_page(pages[page], name) + return document_light + + +def gather_card_names(document: Document) -> Sequence[str]: + return [ + ",".join(container.card.name for container in page) + for page in document.pages + ] + +def generate_test_cases_for_card_moves_between_pages(): + # Tuples source, cards_to_move, target_page, target_row, expected + # Origin card order: ["A1,A2,A3", "B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"] + + # Move onto another existing page + yield 0, [0], 1, None, ["A2,A3", "B1,B2,B3,A1", "", "D1,D2,D3,D4,D5,D6"] + yield 0, [0], 1, 3, ["A2,A3", "B1,B2,B3,A1", "", "D1,D2,D3,D4,D5,D6"] + yield 0, [0], 1, 0, ["A2,A3", "A1,B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"] + yield 0, [0, 2], 1, None, ["A2", "B1,B2,B3,A1,A3", "", "D1,D2,D3,D4,D5,D6"] + yield 0, [1], 1, None, ["A1,A3", "B1,B2,B3,A2", "", "D1,D2,D3,D4,D5,D6"] + yield 0, [0, 1, 2], 1, None, ["", "B1,B2,B3,A1,A2,A3", "", "D1,D2,D3,D4,D5,D6"] + yield 0, [0, 1, 2], 1, 1, ["", "B1,A1,A2,A3,B2,B3", "", "D1,D2,D3,D4,D5,D6"] + yield 1, [0], 2, None, ["A1,A2,A3", "B2,B3", "B1", "D1,D2,D3,D4,D5,D6"] + yield 1, [0], 3, None, ["A1,A2,A3", "B2,B3", "", "D1,D2,D3,D4,D5,D6,B1"] + yield 3, [0, 2], 0, None, ["A1,A2,A3,D1,D3", "B1,B2,B3", "", "D2,D4,D5,D6"] + # Move to new page before target_page + yield 3, [0, 2, 3, 5], 0, -1, ["D1,D3,D4,D6", "A1,A2,A3", "B1,B2,B3", "", "D2,D5"] + yield 3, [0, 2, 3, 5], 1, -1, ["A1,A2,A3", "D1,D3,D4,D6", "B1,B2,B3", "", "D2,D5"] + # Move to new page at document end + yield 3, [0, 2, 3, 5], 4, -1, ["A1,A2,A3", "B1,B2,B3", "", "D2,D5", "D1,D3,D4,D6"] + + +@pytest.mark.parametrize( + "source, cards_to_move, target_page, target_row, expected", + generate_test_cases_for_card_moves_between_pages()) +def test_ActionMoveCardsBetweenPages_apply( + document_with_cards: Document, + source: int, cards_to_move: list[int], target_page: int, target_row: int|None, + expected: list[str]): + action = ActionMoveCardsBetweenPages(source, cards_to_move, target_page, target_row) + action.apply(document_with_cards) + result = gather_card_names(document_with_cards) + assert_that(result, contains_exactly(*expected), f"Got: {result}") + + +@pytest.mark.parametrize( + "source, cards_to_move, target_page, target_row, expected", + generate_test_cases_for_card_moves_between_pages()) +def test_ActionMoveCardsBetweenPages_undo( + document_with_cards: Document, + source: int, cards_to_move: list[int], target_page: int, target_row: int|None, + expected: list[str]): + action = ActionMoveCardsBetweenPages(source, cards_to_move, target_page, target_row) + action.apply(document_with_cards) + try: + assert_that(gather_card_names(document_with_cards), contains_exactly(*expected)) + except AssertionError: + pytest.skip("Test setup failed") + action.undo(document_with_cards) + result = gather_card_names(document_with_cards) + assert_that(result, contains_exactly("A1,A2,A3", "B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"), f"Got: {result}") + +def generate_test_cases_for_card_moves_within_page(): + # Tuples page, cards_to_move, target_row, expected + # Origin card order: ["A1,A2,A3", "B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"] + + yield 0, [0], None, ["A2,A3,A1", "B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"] # 0 + yield 0, [0], 0, ["A1,A2,A3", "B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"] # 1 + yield 0, [0], 1, ["A1,A2,A3", "B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"] # 2 + yield 0, [0], 2, ["A2,A1,A3", "B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"] # 3 + yield 0, [0], 3, ["A2,A3,A1", "B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"] # 4 + yield 3, [0, 1, 5], None, ["A1,A2,A3", "B1,B2,B3", "", "D3,D4,D5,D1,D2,D6"] # 5 + yield 3, [0, 5], 0, ["A1,A2,A3", "B1,B2,B3", "", "D1,D6,D2,D3,D4,D5"] # 6 + yield 3, [0, 5], 3, ["A1,A2,A3", "B1,B2,B3", "", "D2,D3,D1,D6,D4,D5"] # 7 + yield 3, [0, 2], 4, ["A1,A2,A3", "B1,B2,B3", "", "D2,D4,D1,D3,D5,D6"] # 8 + yield 3, [3, 5], 0, ["A1,A2,A3", "B1,B2,B3", "", "D4,D6,D1,D2,D3,D5"] # 9 + yield 3, [1, 2, 4, 5], 0, ["A1,A2,A3", "B1,B2,B3", "", "D2,D3,D5,D6,D1,D4"] # 10 + yield 0, [2], 0, ["A3,A1,A2", "B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"] # 11 + yield 0, [2], 1, ["A1,A3,A2", "B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"] # 12 + yield 0, [2], 2, ["A1,A2,A3", "B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"] # 13 + yield 0, [2], 3, ["A1,A2,A3", "B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"] # 14 + + +@pytest.mark.parametrize( + "page, cards_to_move, target_row, expected", + generate_test_cases_for_card_moves_within_page()) +def test_ActionMoveCardsWithinPage_apply(document_with_cards: Document, page: int, cards_to_move: list[int], + target_row: int|None, expected: list[str]): + ActionMoveCardsWithinPage(page, cards_to_move, target_row).apply(document_with_cards) + result = gather_card_names(document_with_cards) + assert_that(result, contains_exactly(*expected), f"Got: {result}") + + +@pytest.mark.parametrize( + "page, cards_to_move, target_row, after_apply", + generate_test_cases_for_card_moves_within_page()) +def test_ActionMoveCardsWithinPage_undo(document_with_cards: Document, page: int, cards_to_move: list[int], + target_row: int|None, after_apply: list[str]): + (action := ActionMoveCardsWithinPage(page, cards_to_move, target_row)).apply(document_with_cards) + try: + assert_that(gather_card_names(document_with_cards), contains_exactly(*after_apply)) + except AssertionError: + pytest.skip("Test setup failed") + action.undo(document_with_cards) + result = gather_card_names(document_with_cards) + assert_that(result, contains_exactly("A1,A2,A3", "B1,B2,B3", "", "D1,D2,D3,D4,D5,D6"), f"Got: {result}")