Merge pull request #229 from pSpitzner/restore_state_candidate_search

Fixes around Candidate Search

Author: pSpitzner

CHANGELOG.md

@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Archive files can now be deleted [#217](https://github.com/pSpitzner/beets-flask/issues/217)
 - Import Bootleg Button now works as expected [#218](https://github.com/pSpitzner/beets-flask/issues/218)
 - Startup script was not executed correctly if placed in `/config/beets-flask/startup.sh` [#227](https://github.com/pSpitzner/beets-flask/pull/227)
+- Another state-related bug around Searching for Candidates [#225](https://github.com/pSpitzner/beets-flask/issues/225). We now no longer require a certaint type of state before allowing to add candidates.
 
 ### Added
 

backend/beets_flask/importer/session.py

@@ -62,6 +62,7 @@
 from .pipeline import AsyncPipeline
 from .stages import (
     StageOrder,
+    finalize,
     group_albums,
     identify_duplicates,
     lookup_candidates,
@@ -355,6 +356,10 @@ def lookup_candidates(self, task: importer.ImportTask):
             f"This session should not reach this stage. {self.__class__.__name__}"
         )
 
+    def finalize(self, task: importer.ImportTask):
+        """Last stage called and customizable any session."""
+        self.logger.debug(f"Finalized {self} {task}")
+
     # ---------------------------------- Run --------------------------------- #
 
     def run_sync(self) -> SessionState:
@@ -457,6 +462,7 @@ def stages(self) -> StageOrder:
             stages.append(lookup_candidates(self))
 
         stages.append(identify_duplicates(self))
+        stages.append(finalize(self))
 
         return stages
 
@@ -502,6 +508,7 @@ class AddCandidatesSession(PreviewSession):
     """
 
     search: TaskIdMapping[Search | Literal["skip"]]
+    initial_task_states: dict[str, ProgressState]
 
     def __init__(
         self,
@@ -512,16 +519,17 @@ def __init__(
     ):
         super().__init__(state, config_overlay, **kwargs)
 
-        if state.progress != Progress.PREVIEW_COMPLETED:
-            raise ValueError("Cannot run AddCandidatesSession on non-preview state.")
-
         # None means skip search for this task
         self.search = parse_task_id_mapping(search, "skip")
+        self.initial_task_states = {}
 
-        # Reset task progress only for tasks that have search values
-        # other tasks are skipped
         for task in self.state.task_states:
+            self.initial_task_states[task.id] = deepcopy(task.progress)
             if task.progress >= Progress.PREVIEW_COMPLETED:
+                # Reset task progress only for tasks that have search values
+                # other tasks are skipped
+                # This should only be relevant for searches on multiple tasks
+                # (i.e. folders)
                 s = self.search[task.id]
                 if s != "skip":
                     task.set_progress(Progress.LOOKING_UP_CANDIDATES - 1)
@@ -537,13 +545,18 @@ def lookup_candidates(self, task: importer.ImportTask):
             log.debug(f"Skipping search for {task_state.id=}")
             return
 
+        # Beets treats empty strings like real strings, which is not what we want here.
+        # TODO: revisit once PR merged upstream: https://github.com/beetbox/beets/pull/6117
         if (
             search["search_artist"] is not None
             and search["search_artist"].strip() == ""
         ):
             search["search_artist"] = None
         if search["search_album"] is not None and search["search_album"].strip() == "":
             search["search_album"] = None
+        search["search_ids"] = list(
+            filter(lambda x: x.strip() != "", search["search_ids"])
+        )
 
         log.debug(f"Using {search=} for {task_state.id=}, {task_state.paths=}")
 
@@ -606,6 +619,20 @@ def lookup_candidates(self, task: importer.ImportTask):
             )
             self.state.exc = None
 
+    def finalize(self, task: importer.ImportTask):
+        """Restore initial taks and session states."""
+
+        task_state = self.state.get_task_state_for_task_raise(task)
+        if task_state.id in self.initial_task_states:
+            task_state.set_progress(self.initial_task_states[task_state.id])
+        else:
+            log.warning(
+                f"Task {task_state.id} not in initial task states. "
+                + "Cannot restore previous progress."
+            )
+
+        self.logger.debug(f"Finalized {self} {task}")
+
 
 class ImportSession(BaseSession):
     """
@@ -730,6 +757,7 @@ def stages(self):
 
         # finally, move files
         stages.append(manipulate_files(self))
+        stages.append(finalize(self))
 
         return stages
 
@@ -1062,6 +1090,7 @@ async def run_async(self) -> SessionState:
 
     @property
     def stages(self):
+        # This tweaked session skips the pipeline
         return StageOrder()
 
 

backend/beets_flask/importer/stages.py

@@ -38,6 +38,7 @@
 from beets_flask import log
 from beets_flask.server.exceptions import (
     NoCandidatesFoundException,
+    NotImportedException,
 )
 
 from .progress import Progress, ProgressState
@@ -549,7 +550,10 @@ def plugin_stage(
 
 @mutator_stage
 @skip_until(Progress.MATCH_THRESHOLD)
-@set_progress(Progress.MATCH_THRESHOLD)
+@set_progress(
+    Progress.MATCH_THRESHOLD,
+    on_error={NotImportedException: Progress.PREVIEW_COMPLETED},
+)
 def match_threshold(
     session: AutoImportSession,
     task: ImportTask,
@@ -630,6 +634,16 @@ def manipulate_files(
     return task
 
 
+@stage
+def finalize(
+    session: BaseSession,
+    task: ImportTask,
+) -> ImportTask:
+    """Give our custom sessions a way to do something at the very end."""
+    session.finalize(task)
+    return task
+
+
 def _apply_choice(session: ImportSession, task: ImportTask):
     # tweaked version of beets apply_choices.
     # we do not want to rely on the global config object

backend/beets_flask/invoker/enqueue.py

@@ -36,7 +36,6 @@
     UndoSession,
     delete_from_beets,
 )
-from beets_flask.importer.states import Progress
 from beets_flask.importer.types import DuplicateAction
 from beets_flask.logger import log
 from beets_flask.redis import import_queue, preview_queue
@@ -491,12 +490,6 @@ async def run_preview_add_candidates(
     with db_session_factory() as db_session:
         s_state_live = _get_live_state_by_folder(hash, path, db_session)
 
-        if s_state_live.progress != Progress.PREVIEW_COMPLETED:
-            raise InvalidUsageException(
-                f"Session state not in preview completed state for {hash=} "
-                + f"Found state: {s_state_live.progress}"
-            )
-
         a_session = AddCandidatesSession(
             s_state_live,
             search=search,

backend/tests/integration/test_flows.py

@@ -20,14 +20,13 @@
     SessionStateInDb,
 )
 from beets_flask.disk import Folder
-from beets_flask.importer.progress import FolderStatus
+from beets_flask.importer.progress import FolderStatus, Progress
 from beets_flask.importer.session import (
     CandidateChoice,
     TaskIdMappingArg,
 )
 from beets_flask.importer.types import DuplicateAction
 from beets_flask.invoker.enqueue import (
-    Progress,
     run_import_auto,
     run_import_bootleg,
     run_import_candidate,

backend/tests/integration/test_routes/test_exceptions.py

@@ -1,5 +1,3 @@
-
-
 class TestErrorHandling:
     """Tests our exception handling capabilities."""
 

backend/tests/unit/test_setup.py

@@ -14,8 +14,6 @@ def test_log():
     assert log.name == "beets-flask"
 
 
-
-
 def test_config():
     """Test that config is correctly set up for testing."""
     import tempfile

frontend/src/components/common/chips.tsx

@@ -230,6 +230,8 @@ export function FolderStatusChip({
             status_name = "Failed";
             if (folderStatus.exc?.type === "NoCandidatesFoundException") {
                 status_name = "No Match";
+            } else if (folderStatus.exc?.type === "NotImportedException") {
+                status_name = "Threshold";
             }
             break;
         default: