Updated task listing fns to take a record directly

lib/pinchflat/tasks/tasks.ex

@@ -20,13 +20,17 @@ defmodule Pinchflat.Tasks do
   Returns the list of tasks for a given record type and ID. Optionally allows you to specify
   which worker or job states to include.
 
-  IDEA: this should be updated to take a struct instead of a record type and ID
-
   Returns [%Task{}, ...]
   """
-  def list_tasks_for(attached_record_type, attached_record_id, worker_name \\ nil, job_states \\ Oban.Job.states()) do
+  def list_tasks_for(record, worker_name \\ nil, job_states \\ Oban.Job.states()) do
     stringified_states = Enum.map(job_states, &to_string/1)
 
+    record_type =
+      case record do
+        %Source{} -> :source_id
+        %MediaItem{} -> :media_item_id
+      end
+
     worker_name_finder =
       if worker_name do
         # Workers are the full module name - we want to match on the string ENDING with
@@ -43,7 +47,7 @@ defmodule Pinchflat.Tasks do
     Repo.all(
       from t in Task,
         join: j in assoc(t, :job),
-        where: field(t, ^attached_record_type) == ^attached_record_id,
+        where: field(t, ^record_type) == ^record.id,
         where: ^worker_name_finder,
         where: j.state in ^stringified_states
     )
@@ -55,10 +59,9 @@ defmodule Pinchflat.Tasks do
 
   Returns [%Task{}, ...]
   """
-  def list_pending_tasks_for(attached_record_type, attached_record_id, worker_name \\ nil) do
+  def list_pending_tasks_for(record, worker_name \\ nil) do
     list_tasks_for(
-      attached_record_type,
+      record,
-      attached_record_id,
       worker_name,
       [:available, :scheduled, :retryable]
     )
@@ -128,14 +131,10 @@ defmodule Pinchflat.Tasks do
 
   Returns :ok
   """
-  def delete_tasks_for(attached_record, worker_name \\ nil) do
+  def delete_tasks_for(record, worker_name \\ nil) do
-    tasks =
+    record
-      case attached_record do
+    |> list_tasks_for(worker_name)
-        %Source{} = source -> list_tasks_for(:source_id, source.id, worker_name)
+    |> Enum.each(&delete_task/1)
-        %MediaItem{} = media_item -> list_tasks_for(:media_item_id, media_item.id, worker_name)
-      end
-
-    Enum.each(tasks, &delete_task/1)
   end
 
   @doc """
@@ -144,14 +143,10 @@ defmodule Pinchflat.Tasks do
 
   Returns :ok
   """
-  def delete_pending_tasks_for(attached_record, worker_name \\ nil) do
+  def delete_pending_tasks_for(record, worker_name \\ nil) do
-    tasks =
+    record
-      case attached_record do
+    |> list_pending_tasks_for(worker_name)
-        %Source{} = source -> list_pending_tasks_for(:source_id, source.id, worker_name)
+    |> Enum.each(&delete_task/1)
-        %MediaItem{} = media_item -> list_pending_tasks_for(:media_item_id, media_item.id, worker_name)
-      end
-
-    Enum.each(tasks, &delete_task/1)
   end
 
   @doc """

lib/pinchflat/yt_dlp/command_runner.ex

@@ -37,7 +37,8 @@ defmodule Pinchflat.YtDlp.CommandRunner do
 
     case System.cmd(command, formatted_command_opts, stderr_to_stdout: true) do
       {_, 0} ->
-        # IDEA: consider deleting the file after reading it
+        # IDEA: consider deleting the file after reading it. It's in the tmp dir, so it's not
+        # a huge deal, but it's still a good idea to clean up after ourselves.
         # (even on error? especially on error?)
         File.read(output_filepath)
 

lib/pinchflat_web/controllers/sources/source_controller.ex

@@ -46,7 +46,7 @@ defmodule PinchflatWeb.Sources.SourceController do
   def show(conn, %{"id" => id}) do
     source = Repo.preload(Sources.get_source!(id), :media_profile)
 
-    pending_tasks = Repo.preload(Tasks.list_pending_tasks_for(:source_id, source.id), :job)
+    pending_tasks = Repo.preload(Tasks.list_pending_tasks_for(source), :job)
     pending_media = Media.list_pending_media_items_for(source, limit: 100)
     downloaded_media = Media.list_downloaded_media_items_for(source, limit: 100)
 

test/pinchflat/downloading/downloading_helpers_test.exs

@@ -34,11 +34,11 @@ defmodule Pinchflat.Downloading.DownloadingHelpersTest do
       source = source_fixture()
       media_item = media_item_fixture(source_id: source.id, media_filepath: nil)
 
-      assert [] = Tasks.list_tasks_for(:media_item_id, media_item.id)
+      assert [] = Tasks.list_tasks_for(media_item)
 
       assert :ok = DownloadingHelpers.enqueue_pending_download_tasks(source)
 
-      assert [_] = Tasks.list_tasks_for(:media_item_id, media_item.id)
+      assert [_] = Tasks.list_tasks_for(media_item)
     end
 
     test "it does not create a job if the source is set to not download" do
@@ -54,7 +54,7 @@ defmodule Pinchflat.Downloading.DownloadingHelpersTest do
       media_item = media_item_fixture(source_id: source.id, media_filepath: nil)
 
       assert :ok = DownloadingHelpers.enqueue_pending_download_tasks(source)
-      assert [] = Tasks.list_tasks_for(:media_item_id, media_item.id)
+      assert [] = Tasks.list_tasks_for(media_item)
     end
   end
 
@@ -69,7 +69,7 @@ defmodule Pinchflat.Downloading.DownloadingHelpersTest do
       assert :ok = DownloadingHelpers.dequeue_pending_download_tasks(source)
 
       refute_enqueued(worker: MediaDownloadWorker)
-      assert [] = Tasks.list_tasks_for(:media_item_id, media_item.id)
+      assert [] = Tasks.list_tasks_for(media_item)
     end
   end
 end

test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs

@@ -104,7 +104,7 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpersTest do
     test "creates a download task record", %{source: source} do
       assert {:ok, media_item} = FastIndexingHelpers.index_and_enqueue_download_for_media_item(source, @media_url)
 
-      assert [_] = Tasks.list_tasks_for(:media_item_id, media_item.id, "MediaDownloadWorker")
+      assert [_] = Tasks.list_tasks_for(media_item, "MediaDownloadWorker")
     end
 
     test "does not enqueue a download job if the source does not allow it" do

test/pinchflat/slow_indexing/media_collection_indexing_worker_test.exs

@@ -103,7 +103,7 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorkerTest do
       source = source_fixture(index_frequency_minutes: 10)
 
       task_count_fetcher = fn ->
-        Enum.count(Tasks.list_tasks_for(:source_id, source.id, "MediaCollectionIndexingWorker"))
+        Enum.count(Tasks.list_tasks_for(source, "MediaCollectionIndexingWorker"))
       end
 
       assert_changed([from: 0, to: 1], task_count_fetcher, fn ->

test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs

@@ -149,7 +149,7 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpersTest do
 
       SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source)
 
-      assert [] = Tasks.list_tasks_for(:media_item_id, media_item.id)
+      assert [] = Tasks.list_tasks_for(media_item)
     end
 
     test "it doesn't blow up if a media item cannot be coerced into a struct", %{source: source} do

test/pinchflat/tasks_test.exs

@@ -36,53 +36,59 @@ defmodule Pinchflat.TasksTest do
     end
   end
 
-  describe "list_tasks_for/4" do
+  describe "list_tasks_for/3" do
     test "it lets you specify which record type/ID to join on" do
-      task = task_fixture()
+      source = source_fixture()
+      task = task_fixture(source_id: source.id)
 
-      assert Tasks.list_tasks_for(:source_id, task.source_id) == [task]
+      assert Tasks.list_tasks_for(source, nil, [:available]) == [task]
     end
 
     test "it lets you specify which job states to include" do
-      task = task_fixture()
+      source = source_fixture()
+      task = task_fixture(source_id: source.id)
 
-      assert Tasks.list_tasks_for(:source_id, task.source_id, nil, [:available]) == [task]
+      assert Tasks.list_tasks_for(source, nil, [:available]) == [task]
-      assert Tasks.list_tasks_for(:source_id, task.source_id, nil, [:cancelled]) == []
+      assert Tasks.list_tasks_for(source, nil, [:cancelled]) == []
     end
 
     test "it lets you specify which worker to include" do
-      task = task_fixture()
+      source = source_fixture()
+      task = task_fixture(source_id: source.id)
 
-      assert Tasks.list_tasks_for(:source_id, task.source_id, "TestJobWorker") == [task]
+      assert Tasks.list_tasks_for(source, "TestJobWorker") == [task]
-      assert Tasks.list_tasks_for(:source_id, task.source_id, "FooBarWorker") == []
+      assert Tasks.list_tasks_for(source, "FooBarWorker") == []
     end
 
     test "it includes all workers if no worker is specified" do
-      task = task_fixture()
+      source = source_fixture()
+      task = task_fixture(source_id: source.id)
 
-      assert Tasks.list_tasks_for(:source_id, task.source_id, nil) == [task]
+      assert Tasks.list_tasks_for(source, nil) == [task]
     end
   end
 
   describe "list_pending_tasks_for/3" do
     test "it lists pending tasks" do
-      task = task_fixture()
+      source = source_fixture()
+      task = task_fixture(source_id: source.id)
 
-      assert Tasks.list_pending_tasks_for(:source_id, task.source_id) == [task]
+      assert Tasks.list_pending_tasks_for(source) == [task]
     end
 
     test "it does not list non-pending tasks" do
-      task = Repo.preload(task_fixture(), :job)
+      task = Repo.preload(task_fixture(), [:job, :source])
       :ok = Oban.cancel_job(task.job)
 
-      assert Tasks.list_pending_tasks_for(:source_id, task.source_id) == []
+      assert Tasks.list_pending_tasks_for(task.source) == []
     end
 
     test "it lets you specify which worker to include" do
-      task = task_fixture()
+      source = source_fixture()
+      task = task_fixture(source_id: source.id)
 
-      assert Tasks.list_pending_tasks_for(:source_id, task.source_id, "TestJobWorker") == [task]
+      assert Tasks.list_pending_tasks_for(source, "TestJobWorker") == [task]
-      assert Tasks.list_pending_tasks_for(:source_id, task.source_id, "FooBarWorker") == []
+      assert Tasks.list_pending_tasks_for(source, "FooBarWorker") == []
     end
   end