diff --git a/ankihub/gui/block_exam_dialog.py b/ankihub/gui/block_exam_dialog.py index 16032a954..16aee33dd 100644 --- a/ankihub/gui/block_exam_dialog.py +++ b/ankihub/gui/block_exam_dialog.py @@ -28,6 +28,7 @@ add_notes_to_block_exam_subdeck, create_block_exam_subdeck, ) +from ..main.subdecks import is_tag_based_subdeck from ..settings import BlockExamSubdeckConfigOrigin, config from .utils import clear_layout @@ -368,19 +369,19 @@ def _populate_subdeck_list(self): """Populate the subdeck list widget.""" self.subdeck_list.clear() - # Get ALL subdecks under the root deck (including nested ones) - all_subdecks = [] - - for deck_name, deck_id in aqt.mw.col.decks.children(self.root_deck_id): - subdeck_path = deck_name[len(self.root_deck["name"]) + 2 :] - all_subdecks.append((subdeck_path, deck_id)) + # Get ALL subdecks under the root deck (including nested ones), excluding tag-based subdecks + all_subdecks = [ + (deck_name, deck_id) + for deck_name, deck_id in aqt.mw.col.decks.children(self.root_deck_id) + if not is_tag_based_subdeck(deck_id) # Skip tag-based subdecks + ] # Sort subdecks alphabetically by name all_subdecks.sort(key=lambda x: x[0].lower()) # Add all subdecks to the list for subdeck_name, subdeck_id in all_subdecks: - item = QListWidgetItem(subdeck_name) + item = QListWidgetItem(subdeck_name[len(self.root_deck["name"]) + 2 :]) item.setData(Qt.ItemDataRole.UserRole, subdeck_id) # Mark block exam subdecks differently (optional visual indication) diff --git a/ankihub/gui/deckbrowser.py b/ankihub/gui/deckbrowser.py index 0418c048a..ab1a5670e 100644 --- a/ankihub/gui/deckbrowser.py +++ b/ankihub/gui/deckbrowser.py @@ -9,8 +9,9 @@ from aqt import QMenu, gui_hooks, qconnect from aqt.qt import QDialog, QDialogButtonBox, QFont -from ..main.block_exam_subdecks import get_subdeck_name_without_parent, move_subdeck_to_main_deck +from ..main.block_exam_subdecks import dissolve_block_exam_subdeck, get_subdeck_name_without_parent from ..main.deck_unsubscribtion import unsubscribe_from_deck_and_uninstall +from ..main.subdecks import is_tag_based_subdeck from ..settings import config from .operations.user_details import check_user_feature_access from .subdeck_due_date_dialog import DatePickerDialog @@ -70,7 +71,7 @@ def on_button_clicked(button_index: int) -> None: if button_index != 1: return - note_count = move_subdeck_to_main_deck(subdeck_id) + note_count = dissolve_block_exam_subdeck(subdeck_id) aqt.mw.deckBrowser.refresh() show_tooltip(f"{note_count} notes merged into the main deck", parent=aqt.mw) @@ -98,23 +99,30 @@ def on_button_clicked(button_index: int) -> None: ) -def _setup_update_subdeck_due_date(menu: QMenu, subdeck_did: DeckId) -> None: +def _setup_update_subdeck_due_date(menu: QMenu, subdeck_did: DeckId, is_tag_based: bool) -> None: action = menu.addAction("Set due date") - initial_due_date = config.get_block_exam_subdeck_due_date(subdeck_did) - - action.setToolTip("Set the due date of this subdeck.") - qconnect( - action.triggered, - lambda: _open_date_picker_dialog_for_subdeck(subdeck_did, initial_due_date), - ) + if is_tag_based: + action.setEnabled(False) + action.setToolTip("This subdeck is tag-based and cannot be managed as a block exam subdeck.") + else: + initial_due_date = config.get_block_exam_subdeck_due_date(subdeck_did) + action.setToolTip("Set the due date of this subdeck.") + qconnect( + action.triggered, + lambda: _open_date_picker_dialog_for_subdeck(subdeck_did, initial_due_date), + ) -def _setup_remove_block_exam_subdeck(menu: QMenu, subdeck_did: DeckId) -> None: +def _setup_remove_block_exam_subdeck(menu: QMenu, subdeck_did: DeckId, is_tag_based: bool) -> None: action = menu.addAction("Merge into parent deck") - action.setToolTip("Deletes the subdeck and moves all notes back into the main deck.") - qconnect(action.triggered, lambda: _open_remove_block_exam_subdeck_dialog(subdeck_did)) + if is_tag_based: + action.setEnabled(False) + action.setToolTip("This subdeck is tag-based and cannot be managed as a block exam subdeck.") + else: + action.setToolTip("Deletes the subdeck and moves all notes back into the main deck.") + qconnect(action.triggered, lambda: _open_remove_block_exam_subdeck_dialog(subdeck_did)) def _initialize_subdeck_context_menu_actions(menu: QMenu, deck_id: int) -> None: @@ -137,8 +145,9 @@ def on_access_granted(_: dict) -> None: font.setPointSize(10) label_action.setFont(font) - _setup_update_subdeck_due_date(menu, did) - _setup_remove_block_exam_subdeck(menu, did) + is_tag_based = is_tag_based_subdeck(did) + _setup_update_subdeck_due_date(menu, did, is_tag_based) + _setup_remove_block_exam_subdeck(menu, did, is_tag_based) check_user_feature_access( feature_key="has_flashcard_selector_access", diff --git a/ankihub/gui/subdeck_due_date_dialog.py b/ankihub/gui/subdeck_due_date_dialog.py index 5c6916b9a..e632ba605 100644 --- a/ankihub/gui/subdeck_due_date_dialog.py +++ b/ankihub/gui/subdeck_due_date_dialog.py @@ -13,8 +13,8 @@ from .. import LOGGER from ..main.block_exam_subdecks import ( check_block_exam_subdeck_due_dates, + dissolve_block_exam_subdeck, get_subdeck_name_without_parent, - move_subdeck_to_main_deck, set_subdeck_due_date, ) from ..settings import BlockExamSubdeckConfig, BlockExamSubdeckConfigOrigin, config @@ -113,7 +113,7 @@ def _setup_ui(self): def _on_move_to_main_deck(self): """Handle moving subdeck to main deck.""" - note_count = move_subdeck_to_main_deck(self.subdeck_config.subdeck_id) + note_count = dissolve_block_exam_subdeck(self.subdeck_config.subdeck_id) tooltip(f"{note_count} notes merged into the parent deck", parent=aqt.mw) self.accept() aqt.mw.deckBrowser.refresh() diff --git a/ankihub/main/block_exam_subdecks.py b/ankihub/main/block_exam_subdecks.py index 646fc31cc..db1596137 100644 --- a/ankihub/main/block_exam_subdecks.py +++ b/ankihub/main/block_exam_subdecks.py @@ -11,7 +11,10 @@ from .. import LOGGER from ..settings import BlockExamSubdeckConfig, BlockExamSubdeckConfigOrigin, config -from .utils import move_notes_to_decks_while_respecting_odid, note_ids_in_deck_hierarchy +from .utils import ( + move_notes_to_decks_while_respecting_odid, + note_ids_in_deck_hierarchy, +) def get_root_deck_id_from_subdeck(subdeck_id: DeckId) -> DeckId: @@ -189,15 +192,17 @@ def check_block_exam_subdeck_due_dates() -> List[BlockExamSubdeckConfig]: return expired_subdecks -def move_subdeck_to_main_deck(subdeck_id: DeckId) -> int: - """Move all notes from a subdeck back to the root deck, delete the subdeck, - and remove its configuration (if it exists). +def dissolve_block_exam_subdeck(subdeck_id: DeckId) -> int: + """Remove a block exam subdeck and move its notes back to the root deck. + + If the root deck is an AnkiHub deck with subdecks enabled, notes will be + redistributed to tag-based subdecks after being moved to the root deck. Args: subdeck_id: The Anki subdeck ID Returns: - The number of notes moved to the root deck + The number of notes moved from the subdeck Raises: ValueError: If the provided deck is a root deck (not a subdeck) @@ -224,6 +229,14 @@ def move_subdeck_to_main_deck(subdeck_id: DeckId) -> int: aqt.mw.col.decks.remove([subdeck_id]) + # If the root deck is managed by AnkiHub and has subdecks enabled, move notes to subdecks + # based on subdeck tags + ah_did = config.get_deck_uuid_by_did(root_deck_id) + if ah_did and config.deck_config(ah_did).subdecks_enabled and note_ids: + from .subdecks import build_subdecks_and_move_cards_to_them + + build_subdecks_and_move_cards_to_them(ah_did, note_ids) + LOGGER.info("Successfully moved subdeck to root deck", subdeck_name=subdeck["name"]) if subdeck_config: diff --git a/ankihub/main/subdecks.py b/ankihub/main/subdecks.py index 47518173b..1744d15f1 100644 --- a/ankihub/main/subdecks.py +++ b/ankihub/main/subdecks.py @@ -21,7 +21,11 @@ from ..db.models import AnkiHubNote from ..settings import config from .block_exam_subdecks import get_exam_subdecks -from .utils import move_notes_to_decks_while_respecting_odid, nids_in_deck_but_not_in_subdeck, note_ids_in_decks +from .utils import ( + move_notes_to_decks_while_respecting_odid, + nids_in_deck_but_not_in_subdeck, + note_ids_in_decks, +) # root tag for tags that indicate which subdeck a note belongs to SUBDECK_TAG = "AnkiHub_Subdeck" @@ -289,3 +293,40 @@ def _subdeck_name_to_tag(deck_name: str) -> str: result = re.sub("_+", "_", result) return result + + +def is_tag_based_subdeck(subdeck_id: DeckId) -> bool: + """Check if a subdeck is tag-based. + + A subdeck is considered tag-based if there exists at least one note anywhere + in the collection with a subdeck tag matching the subdeck's path. + + Args: + subdeck_id: The subdeck ID to check + + Returns: + True if the subdeck is tag-based, False otherwise. + """ + # TODO We need to restrict the notes to those belonging to the AnkiHub deck somehow + # or restrict the subdeck tags. + # Otherwise there could be false positives if there are multiple AnkiHub decks + # with subdecks and notes from one deck have subdeck tags matching subdecks + # of another deck. + # We can probably use note_ids_in_deck_hierarchy + # It's probably better to use tag-information from the ankihub_db instead + # Or both? + + # Get subdeck path relative to root + full_subdeck_name = aqt.mw.col.decks.name(subdeck_id) + + # Check if this is a subdeck (has a parent) + if "::" not in full_subdeck_name: + return False + + subdeck_path = full_subdeck_name.split("::", maxsplit=1)[1] + + # Check if any notes have subdeck tags matching this path + tag_pattern = f"^{SUBDECK_TAG}::[^:]+::{re.escape(subdeck_path)}$" + matching_nids = aqt.mw.col.find_notes(f'tag:"re:{tag_pattern}"') + + return bool(matching_nids) diff --git a/tests/addon/test_integration.py b/tests/addon/test_integration.py index 5731f7187..9264f0ff6 100644 --- a/tests/addon/test_integration.py +++ b/tests/addon/test_integration.py @@ -68,7 +68,7 @@ add_notes_to_block_exam_subdeck, check_block_exam_subdeck_due_dates, create_block_exam_subdeck, - move_subdeck_to_main_deck, + dissolve_block_exam_subdeck, set_subdeck_due_date, ) @@ -8994,13 +8994,13 @@ def test_set_due_date_on_regular_subdeck( saved_due_date = subdeck_config.due_date if subdeck_config else None assert saved_due_date == due_date - def test_move_subdeck_to_main_deck_with_nested_subdecks( + def test_dissolve_block_exam_subdeck_with_nested_subdecks( self, anki_session_with_addon_data: AnkiSession, install_ah_deck: InstallAHDeck, add_anki_note: AddAnkiNote, ): - """Test moving a subdeck with nested children moves all notes to main deck.""" + """Test dissolving a subdeck with nested children moves all notes to main deck.""" with anki_session_with_addon_data.profile_loaded(): ah_did = install_ah_deck() deck_config = config.deck_config(ah_did) @@ -9029,8 +9029,8 @@ def test_move_subdeck_to_main_deck_with_nested_subdecks( assert note_subdeck.cards()[0].did == subdeck_id assert note_nested.cards()[0].did == nested_subdeck_id - # Move subdeck to main deck - result = move_subdeck_to_main_deck(subdeck_id) + # Dissolve subdeck + result = dissolve_block_exam_subdeck(subdeck_id) # Verify return value - should be 2 notes (subdeck + nested) assert result == 2 @@ -9052,12 +9052,12 @@ def test_move_subdeck_to_main_deck_with_nested_subdecks( subdeck_config = config.get_block_exam_subdeck_config(subdeck_id) assert subdeck_config is None - def test_move_subdeck_to_main_deck_without_config( + def test_dissolve_block_exam_subdeck_without_config( self, anki_session_with_addon_data: AnkiSession, add_anki_note: AddAnkiNote, ): - """Test removing a regular subdeck that doesn't have a BlockExamSubdeckConfig.""" + """Test dissolving a regular subdeck that doesn't have a BlockExamSubdeckConfig.""" with anki_session_with_addon_data.profile_loaded(): main_deck_id = create_anki_deck("Test Deck") main_deck_name = aqt.mw.col.decks.name_if_exists(main_deck_id) @@ -9073,8 +9073,8 @@ def test_move_subdeck_to_main_deck_without_config( subdeck_config = config.get_block_exam_subdeck_config(subdeck_id) assert subdeck_config is None - # Move subdeck to main deck (should work without config) - result = move_subdeck_to_main_deck(subdeck_id) + # Dissolve subdeck (should work without config) + result = dissolve_block_exam_subdeck(subdeck_id) # Verify return value - should be 1 note assert result == 1 @@ -9086,17 +9086,17 @@ def test_move_subdeck_to_main_deck_without_config( # Verify subdeck was deleted assert aqt.mw.col.decks.id_for_name(subdeck_name) is None - def test_move_subdeck_to_main_deck_with_root_deck( + def test_dissolve_block_exam_subdeck_with_root_deck( self, anki_session_with_addon_data: AnkiSession, ): - """Test calling move_subdeck_to_main_deck on a root deck only removes config.""" + """Test calling dissolve_block_exam_subdeck on a root deck raises ValueError.""" with anki_session_with_addon_data.profile_loaded(): root_deck_id = create_anki_deck("Test Deck") with pytest.raises(ValueError, match="The provided deck isn't a subdeck."): - # Call move_subdeck_to_main_deck on the root deck - move_subdeck_to_main_deck(root_deck_id) + # Call dissolve_block_exam_subdeck on the root deck + dissolve_block_exam_subdeck(root_deck_id) class TestBlockExamSubdeckDialog: diff --git a/tests/addon/test_unit.py b/tests/addon/test_unit.py index 514559a92..adde2e6a4 100644 --- a/tests/addon/test_unit.py +++ b/tests/addon/test_unit.py @@ -33,7 +33,7 @@ handle_expired_subdeck, ) from ankihub.main.block_exam_subdecks import ( - move_subdeck_to_main_deck, + dissolve_block_exam_subdeck, set_subdeck_due_date, ) from ankihub.settings import BlockExamSubdeckConfig, BlockExamSubdeckConfigOrigin @@ -158,6 +158,7 @@ SUBDECK_TAG, add_subdeck_tags_to_notes, deck_contains_subdeck_tags, + is_tag_based_subdeck, ) from ankihub.main.suggestions import ChangeSuggestionResult from ankihub.main.utils import ( @@ -662,6 +663,45 @@ def test_add_subdeck_tags_to_notes_with_spaces_in_deck_name( assert note3.tags == [f"{SUBDECK_TAG}::AA::b_b::c_c"] +@pytest.mark.parametrize( + "deck_name,tag,expected_is_tag_based_subdeck", + [ + ("TestDeck::SubA", f"{SUBDECK_TAG}::AnkiHubDeckName::SubA", True), + ("TestDeck::SubA::SubB", f"{SUBDECK_TAG}::AnkiHubDeckName::SubA::SubB", True), + ("TestDeck::SubC", None, False), # No matching tag + ("TestDeck", None, False), # Root deck is not a subdeck + ], +) +def test_is_tag_based_subdeck( + anki_session_with_addon_data: AnkiSession, + install_ah_deck: InstallAHDeck, + add_anki_note: AddAnkiNote, + deck_name: str, + tag: Optional[str], + expected_is_tag_based_subdeck: bool, +): + """Test that is_tag_based_subdeck correctly identifies subdecks with matching subdeck tags.""" + with anki_session_with_addon_data.profile_loaded(): + # Install an AnkiHub deck with subdecks enabled + ah_did = install_ah_deck(anki_deck_name="TestDeck") + root_deck_id = config.deck_config(ah_did).anki_id + + # Create the deck (or use root deck if it's TestDeck) + if deck_name == "TestDeck": + deck_id = root_deck_id + else: + deck_id = create_anki_deck(deck_name) + + # Add a note with the tag if specified + if tag is not None: + note = add_anki_note() + note.tags = [tag] + aqt.mw.col.update_note(note) + + # Check if the deck is tag-based + assert is_tag_based_subdeck(deck_id) is expected_is_tag_based_subdeck + + class TestAnkiHubSignOut: @pytest.mark.parametrize("confirmed_sign_out,expected_logged_in_state", [(True, False), (False, True)]) def test_sign_out( @@ -3488,21 +3528,21 @@ def test_deck_with_skipped_notes(self, mock_dependencies: Dict[str, Any], qtbot: assert "see this topic" in message -class TestMoveSubdeckToMainDeck: - """Tests for move_subdeck_to_main_deck function.""" +class TestDissolveBlockExamSubdeck: + """Tests for dissolve_block_exam_subdeck function.""" @patch("ankihub.main.block_exam_subdecks.note_ids_in_deck_hierarchy") @patch("ankihub.main.block_exam_subdecks.move_notes_to_decks_while_respecting_odid") @patch("ankihub.main.block_exam_subdecks.aqt") @patch("ankihub.main.block_exam_subdecks.config") - def test_move_subdeck_to_main_deck_success( + def test_dissolve_block_exam_subdeck_success( self, mock_config, mock_aqt, mock_move_notes, mock_note_ids_in_deck_hierarchy, ): - """Test successfully moving subdeck to main deck.""" + """Test successfully dissolving a block exam subdeck.""" # Setup mocks mock_subdeck = {"name": "Test Deck::Subdeck", "id": 456} mock_aqt.mw.col.decks.get.return_value = mock_subdeck @@ -3512,20 +3552,75 @@ def test_move_subdeck_to_main_deck_success( mock_parent_deck = {"name": "Test Deck", "id": 123} mock_aqt.mw.col.decks.parents.return_value = [mock_parent_deck] + # Mock config.get_deck_uuid_by_did to return None (no AnkiHub deck) + mock_config.get_deck_uuid_by_did.return_value = None + subdeck_config = BlockExamSubdeckConfig(subdeck_id=DeckId(456), due_date="2024-12-31") mock_config.get_block_exam_subdeck_config.return_value = subdeck_config - result = move_subdeck_to_main_deck(DeckId(456)) + result = dissolve_block_exam_subdeck(DeckId(456)) assert result == 3 # Should return the number of notes moved mock_note_ids_in_deck_hierarchy.assert_called_once_with(456) mock_move_notes.assert_called_once_with({1: 123, 2: 123, 3: 123}) mock_aqt.mw.col.decks.remove.assert_called_once_with([456]) mock_config.remove_block_exam_subdeck.assert_called_once_with(DeckId(456)) + # Verify get_deck_uuid_by_did was called with root_deck_id + mock_config.get_deck_uuid_by_did.assert_called_once_with(123) + + @patch("ankihub.main.subdecks.build_subdecks_and_move_cards_to_them") + @patch("ankihub.main.block_exam_subdecks.note_ids_in_deck_hierarchy") + @patch("ankihub.main.block_exam_subdecks.move_notes_to_decks_while_respecting_odid") + @patch("ankihub.main.block_exam_subdecks.aqt") + @patch("ankihub.main.block_exam_subdecks.config") + def test_dissolve_subdeck_with_subdecks_enabled( + self, + mock_config, + mock_aqt, + mock_move_notes, + mock_note_ids_in_deck_hierarchy, + mock_build_subdecks, + ): + """Test dissolving subdeck that belongs to an AnkiHub deck rebuilds subdecks.""" + # Setup mocks + mock_subdeck = {"name": "Test Deck::Subdeck", "id": 456} + mock_aqt.mw.col.decks.get.return_value = mock_subdeck + mock_note_ids_in_deck_hierarchy.return_value = [1, 2, 3] + + # Mock the parent deck + mock_parent_deck = {"name": "Test Deck", "id": 123} + mock_aqt.mw.col.decks.parents.return_value = [mock_parent_deck] + + # Mock AnkiHub deck ID (indicates this is an AnkiHub deck) + test_ah_did = uuid.uuid4() + mock_config.get_deck_uuid_by_did.return_value = test_ah_did + + # Mock deck_config to return a config with subdecks_enabled=True + mock_deck_config = Mock() + mock_deck_config.subdecks_enabled = True + mock_config.deck_config.return_value = mock_deck_config + + subdeck_config = BlockExamSubdeckConfig(subdeck_id=DeckId(456), due_date="2024-12-31") + mock_config.get_block_exam_subdeck_config.return_value = subdeck_config + + result = dissolve_block_exam_subdeck(DeckId(456)) + + # Verify normal operations happened + assert result == 3 + mock_note_ids_in_deck_hierarchy.assert_called_once_with(456) + mock_move_notes.assert_called_once_with({1: 123, 2: 123, 3: 123}) + mock_aqt.mw.col.decks.remove.assert_called_once_with([456]) + mock_config.remove_block_exam_subdeck.assert_called_once_with(DeckId(456)) + # Verify get_deck_uuid_by_did was called with root_deck_id + mock_config.get_deck_uuid_by_did.assert_called_once_with(123) + # Verify deck_config was called with test_ah_did + mock_config.deck_config.assert_called_once_with(test_ah_did) + # Verify subdeck rebuilding was called + mock_build_subdecks.assert_called_once_with(test_ah_did, [1, 2, 3]) @patch("ankihub.main.block_exam_subdecks.aqt") @patch("ankihub.main.block_exam_subdecks.config") - def test_move_subdeck_to_main_deck_subdeck_not_found( + def test_dissolve_subdeck_not_found( self, mock_config, mock_aqt, @@ -3536,7 +3631,7 @@ def test_move_subdeck_to_main_deck_subdeck_not_found( subdeck_config = BlockExamSubdeckConfig(subdeck_id=DeckId(456), due_date="2024-12-31") mock_config.get_block_exam_subdeck_config.return_value = subdeck_config - result = move_subdeck_to_main_deck(DeckId(456)) + result = dissolve_block_exam_subdeck(DeckId(456)) assert result == 0 # Should return 0 when subdeck not found mock_config.remove_block_exam_subdeck.assert_called_once_with(DeckId(456)) @@ -3590,7 +3685,7 @@ def test_handle_expired_subdeck_success(self, mock_aqt, mock_dialog_class, mock_ mock_dialog_class.assert_called_once_with(subdeck_config, parent=mock_aqt.mw) mock_dialog.show.assert_called_once() - @patch("ankihub.gui.subdeck_due_date_dialog.config", create=True) + @patch("ankihub.gui.subdeck_due_date_dialog.config") @patch("ankihub.gui.subdeck_due_date_dialog.aqt") def test_handle_expired_subdeck_not_found(self, mock_aqt, mock_config): """Test handling when expired subdeck not found in Anki."""