Merge pull request #255 from fabiomanz/fix/import-dialog-subdeck-counts

Fix import dialog showing 0 notes/media for subdeck-only decks

Author: aplaice

crowd_anki/importer/anki_importer.py

@@ -27,15 +27,15 @@ def load_deck(self, directory_path) -> bool:
         :param directory_path: Path
         """
         deck_json = self.read_deck(self.get_deck_path(directory_path))
+        deck = deck_initializer.from_json(deck_json)
 
-        import_config = self.read_import_config(directory_path, deck_json)
+        import_config = self.read_import_config(directory_path, deck)
         if import_config is None:
             return False
 
         if aqt.mw:
             aqt.mw.create_backup_now()
         try:
-            deck = deck_initializer.from_json(deck_json)
             deck.save_to_collection(self.collection, import_config=import_config)
 
             if import_config.use_media:
@@ -84,7 +84,7 @@ def read_deck(file_path: Path):
             return json.load(deck_file)
 
     @staticmethod
-    def read_import_config(directory_path, deck_json):
+    def read_import_config(directory_path, deck):
         file_path = directory_path.joinpath(IMPORT_CONFIG_NAME)
 
         if not file_path.exists():
@@ -93,7 +93,7 @@ def read_import_config(directory_path, deck_json):
             with file_path.open(encoding='utf8') as meta_file:
                 import_dict = yaml.full_load(meta_file)
 
-        import_dialog = ImportDialog(deck_json, import_dict)
+        import_dialog = ImportDialog(deck, import_dict)
         if import_dialog.exec() == QDialog.DialogCode.Rejected:
             return None
 

crowd_anki/importer/import_dialog.py

@@ -66,13 +66,13 @@ class ImportConfig(PersonalFieldsHolder):
 
 
 class ImportDialog(QDialog):
-    def __init__(self, deck_json, config, parent=None):
+    def __init__(self, deck, config, parent=None):
         super().__init__(None)
         self.parent = parent
         self.form = ConfigUI()
         self.form.setupUi(self)
         self.userConfig = ConfigSettings.get_instance()
-        self.deck_json = deck_json
+        self.deck = deck
         self.import_defaults = ImportDefaults.from_dict(config)
         self.personal_field_ui_dict = defaultdict(dict)
         self.ui_initial_setup()
@@ -120,12 +120,12 @@ def on_click(item):
                     item.setCheckState(Qt.CheckState.Checked)
         self.form.list_personal_fields.itemClicked.connect(on_click)
 
-        for model in self.deck_json["note_models"]:
-            model_name = model["name"]
-            model_id = model[UUID_FIELD_NAME]
+        for model in self.deck.metadata.models.values():
+            model_name = model.anki_dict["name"]
+            model_id = model.anki_dict[UUID_FIELD_NAME]
             add_header(model_name)
 
-            for field in model["flds"]:
+            for field in model.anki_dict["flds"]:
                 field_name = field["name"]
                 field_ui = add_field(field_name, self.import_defaults.is_personal_field(model_name, field_name))
                 self.personal_field_ui_dict[model_name].setdefault(field_name, field_ui)
@@ -149,10 +149,10 @@ def set_checked_and_text(checkbox, text, count, checked: bool = True):
                 text = f"{text}: {'{:,}'.format(count)}"
             checkbox.setText(text)
 
-        set_checked_and_text(self.form.cb_notes, "Notes", len(self.deck_json['notes']))
-        set_checked_and_text(self.form.cb_media, "Media Files", len(self.deck_json['media_files']))
+        set_checked_and_text(self.form.cb_notes, "Notes", self.deck.get_note_count())
+        set_checked_and_text(self.form.cb_media, "Media Files", self.deck.get_media_file_count())
 
-        # TODO: Deck Parts to Use, check which are actually in the deck_json
+        # TODO: Deck Parts to Use, check which are actually in the deck
 
     def read_import_config(self):
         config = ImportConfig(

crowd_anki/representation/deck.py

@@ -51,6 +51,7 @@ def __init__(self,
 
         self.collection = None
         self.notes = []
+        self.media_files = []
         self.children = []
         self.metadata = None
         self.deck_config_uuid = None
@@ -69,6 +70,21 @@ def flatten(self):
     def get_note_count(self):
         return len(self.notes) + sum(child.get_note_count() for child in self.children)
 
+    def get_media_file_count(self):
+        return len(self._collect_media_files())
+
+    def _collect_media_files(self):
+        """Unique media file names in this deck and all its subdecks.
+
+        Media files are deduplicated by name, since the same file (e.g.
+        re-used media or shared `_xxx` media) can appear in multiple
+        decks and must only be counted once.
+        """
+        media = set(self.media_files)
+        for child in self.children:
+            media |= child._collect_media_files()
+        return media
+
     def _update_db(self):
         # Introduce uuid field for unique identification of entities
         utils.add_column(self.collection.db, "notes", UUID_FIELD_NAME)

crowd_anki/representation/deck_initializer.py

@@ -45,6 +45,7 @@ def from_json(json_dict, deck_metadata=None) -> Deck:
 
     deck.deck_config_uuid = json_dict["deck_config_uuid"]
     deck.notes = [Note.from_json(json_note) for json_note in json_dict["notes"]]
+    deck.media_files = json_dict.get("media_files", [])
     deck.children = [from_json(child, deck_metadata=deck.metadata) for child in json_dict["children"]]
 
     deck.post_import_filter()

test/representation/deck_initializer_spec.py

@@ -1,4 +1,4 @@
-from expects import expect, be
+from expects import expect, be, equal
 from mamba import description, it
 from unittest.mock import MagicMock
 
@@ -8,10 +8,31 @@
 
 TEST_DECK = "test deck"
 
+
+def _deck_json(media_files=None, children=None):
+    return {
+        "deck_config_uuid": "config-uuid",
+        "notes": [],
+        "media_files": media_files or [],
+        "children": children or [],
+    }
+
+
 with description("Initializer from deck") as self:
     with it("should return None when trying to export dynamic deck"):
         collection = MagicMock()
 
         collection.decks.byName.return_value = DYNAMIC_DECK
 
         expect(deck_initializer.from_collection(collection, TEST_DECK)).to(be(None))
+
+    with description("from_json") as self:
+        with it("populates media_files from JSON, including subdecks"):
+            json_dict = _deck_json(
+                media_files=["a.png", "b.png"],
+                children=[_deck_json(media_files=["c.png"])],
+            )
+
+            deck = deck_initializer.from_json(json_dict)
+
+            expect(deck.get_media_file_count()).to(equal(3))

test/representation/deck_spec.py

@@ -0,0 +1,62 @@
+from mamba import description, it
+from expects import expect, equal
+
+from crowd_anki.representation.deck import Deck
+
+
+def _make_deck(notes=0, media_files=None, children=None):
+    deck = Deck(file_provider_supplier=None)
+    deck.notes = [None] * notes
+    deck.media_files = list(media_files or [])
+    deck.children = children or []
+    return deck
+
+
+with description(Deck) as self:
+    with description(".get_note_count") as self:
+        with it("returns 0 for an empty deck"):
+            expect(_make_deck().get_note_count()).to(equal(0))
+
+        with it("counts notes in a flat deck"):
+            expect(_make_deck(notes=3).get_note_count()).to(equal(3))
+
+        with it("counts notes recursively across subdecks"):
+            deck = _make_deck(notes=1, children=[
+                _make_deck(notes=2, children=[
+                    _make_deck(notes=3),
+                ]),
+            ])
+            expect(deck.get_note_count()).to(equal(6))
+
+        with it("counts notes in subdecks even when the parent has none"):
+            deck = _make_deck(notes=0, children=[_make_deck(notes=5)])
+            expect(deck.get_note_count()).to(equal(5))
+
+    with description(".get_media_file_count") as self:
+        with it("returns 0 for an empty deck"):
+            expect(_make_deck().get_media_file_count()).to(equal(0))
+
+        with it("counts media files in a flat deck"):
+            expect(_make_deck(media_files=['a.jpg', 'b.jpg', 'c.jpg']).get_media_file_count()).to(equal(3))
+
+        with it("counts media files recursively across subdecks"):
+            deck = _make_deck(media_files=['a.jpg'], children=[
+                _make_deck(media_files=['b.jpg', 'c.jpg'], children=[
+                    _make_deck(media_files=['d.jpg', 'e.jpg', 'f.jpg']),
+                ]),
+            ])
+            expect(deck.get_media_file_count()).to(equal(6))
+
+        with it("counts media files in subdecks even when the parent has none"):
+            deck = _make_deck(children=[_make_deck(media_files=['a.jpg', 'b.jpg', 'c.jpg', 'd.jpg'])])
+            expect(deck.get_media_file_count()).to(equal(4))
+
+        with it("counts media files shared across subdecks only once"):
+            deck = _make_deck(media_files=['shared.jpg'], children=[
+                _make_deck(media_files=['shared.jpg', 'a.jpg']),
+                _make_deck(media_files=['shared.jpg', 'b.jpg']),
+            ])
+            expect(deck.get_media_file_count()).to(equal(3))
+
+        with it("counts media files duplicated within a single deck only once"):
+            expect(_make_deck(media_files=['a.jpg', 'a.jpg']).get_media_file_count()).to(equal(1))