Got 'enabled' logic working re: downloading pending media

lib/pinchflat/sources/sources.ex

@@ -255,19 +255,40 @@ defmodule Pinchflat.Sources do
     end
   end
 
-  # If the source is NOT new (ie: updated) and the download_media flag has changed,
+  # If the source is new (ie: not persisted), do nothing
-  # enqueue or dequeue media download tasks as necessary.
+  defp maybe_handle_media_tasks(%{data: %{__meta__: %{state: state}}}, _source) when state != :loaded do
-  defp maybe_handle_media_tasks(changeset, source) do
-    case {changeset.data, changeset.changes} do
-      {%{__meta__: %{state: :loaded}}, %{download_media: true}} ->
-        DownloadingHelpers.enqueue_pending_download_tasks(source)
-
-      {%{__meta__: %{state: :loaded}}, %{download_media: false}} ->
-        DownloadingHelpers.dequeue_pending_download_tasks(source)
-
-      _ ->
     :ok
   end
+
+  # If the source is NOT new (ie: updated),
+  # enqueue or dequeue media download tasks as necessary.
+  defp maybe_handle_media_tasks(changeset, source) do
+    current_changes = changeset.changes
+    applied_changes = Ecto.Changeset.apply_changes(changeset)
+
+    # We need both current_changes and applied_changes to determine
+    # the course of action to take. For example, we only care if a source is supposed
+    # to be `enabled` or not - we don't care if that information comes from the
+    # current changes or if that's how it already was in the database.
+    # Rephrased, we're essentially using it in place of `get_field/2`
+    case {current_changes, applied_changes} do
+      {%{download_media: false}, _} ->
+        DownloadingHelpers.dequeue_pending_download_tasks(source)
+
+      {%{download_media: true}, %{enabled: true}} ->
+        DownloadingHelpers.enqueue_pending_download_tasks(source)
+
+      {%{enabled: false}, _} ->
+        DownloadingHelpers.dequeue_pending_download_tasks(source)
+
+      {%{enabled: true}, %{download_media: true}} ->
+        DownloadingHelpers.enqueue_pending_download_tasks(source)
+
+      _ ->
+        nil
+    end
+
+    :ok
   end
 
   defp maybe_run_indexing_task(changeset, source) do

lib/pinchflat_web/controllers/sources/source_html/index_table_live.ex

@@ -7,10 +7,10 @@ defmodule PinchflatWeb.Sources.IndexTableLive do
   alias Pinchflat.Sources.Source
   alias Pinchflat.Media.MediaItem
 
-  # TODO: test
+  # TODO: test (and maybe remove existing index tests)
   def render(assigns) do
     ~H"""
-    <.table rows={@sources} table_class="text-black dark:text-white">
+    <.table rows={@sources} table_class="text-white">
       <:col :let={source} label="Name">
         <.subtle_link href={~p"/sources/#{source.id}"}>
           <%= StringUtils.truncate(source.custom_name || source.collection_name, 35) %>

test/pinchflat/sources_test.exs

@@ -418,6 +418,100 @@ defmodule Pinchflat.SourcesTest do
       assert {:ok, %Source{}} = Sources.update_source(source, update_attrs)
     end
 
+    test "updates with invalid data returns error changeset" do
+      source = source_fixture()
+
+      assert {:error, %Ecto.Changeset{}} =
+               Sources.update_source(source, @invalid_source_attrs)
+
+      assert source == Sources.get_source!(source.id)
+    end
+
+    test "updating will kickoff a metadata storage worker if the original_url changes" do
+      expect(YtDlpRunnerMock, :run, &playlist_mock/4)
+      source = source_fixture()
+      update_attrs = %{original_url: "https://www.youtube.com/channel/cba321"}
+
+      assert {:ok, %Source{} = source} = Sources.update_source(source, update_attrs)
+
+      assert_enqueued(worker: SourceMetadataStorageWorker, args: %{"id" => source.id})
+    end
+
+    test "updating will not kickoff a metadata storage worker other attrs change" do
+      source = source_fixture()
+      update_attrs = %{name: "some new name"}
+
+      assert {:ok, %Source{}} = Sources.update_source(source, update_attrs)
+
+      refute_enqueued(worker: SourceMetadataStorageWorker)
+    end
+  end
+
+  describe "update_source/3 when testing media download tasks" do
+    test "enabling the download_media attribute will schedule a download task" do
+      source = source_fixture(download_media: false)
+      media_item = media_item_fixture(source_id: source.id, media_filepath: nil)
+      update_attrs = %{download_media: true}
+
+      refute_enqueued(worker: MediaDownloadWorker)
+      assert {:ok, %Source{}} = Sources.update_source(source, update_attrs)
+      assert_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id})
+    end
+
+    test "disabling the download_media attribute will cancel the download task" do
+      source = source_fixture(download_media: true, enabled: true)
+      media_item = media_item_fixture(source_id: source.id, media_filepath: nil)
+      update_attrs = %{download_media: false}
+      DownloadingHelpers.enqueue_pending_download_tasks(source)
+
+      assert_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id})
+      assert {:ok, %Source{}} = Sources.update_source(source, update_attrs)
+      refute_enqueued(worker: MediaDownloadWorker)
+    end
+
+    test "enabling download_media will not schedule a task if the source is disabled" do
+      source = source_fixture(download_media: false, enabled: false)
+      _media_item = media_item_fixture(source_id: source.id, media_filepath: nil)
+      update_attrs = %{download_media: true}
+
+      refute_enqueued(worker: MediaDownloadWorker)
+      assert {:ok, %Source{}} = Sources.update_source(source, update_attrs)
+      refute_enqueued(worker: MediaDownloadWorker)
+    end
+
+    test "disabling a source will cancel any pending download tasks" do
+      source = source_fixture(download_media: true, enabled: true)
+      media_item = media_item_fixture(source_id: source.id, media_filepath: nil)
+      update_attrs = %{enabled: false}
+      DownloadingHelpers.enqueue_pending_download_tasks(source)
+
+      assert_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id})
+      assert {:ok, %Source{}} = Sources.update_source(source, update_attrs)
+      refute_enqueued(worker: MediaDownloadWorker)
+    end
+
+    test "enabling a source will schedule a download task if download_media is true" do
+      source = source_fixture(download_media: true, enabled: false)
+      media_item = media_item_fixture(source_id: source.id, media_filepath: nil)
+      update_attrs = %{enabled: true}
+
+      refute_enqueued(worker: MediaDownloadWorker)
+      assert {:ok, %Source{}} = Sources.update_source(source, update_attrs)
+      assert_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id})
+    end
+
+    test "enabling a source will not schedule a download task if download_media is false" do
+      source = source_fixture(download_media: false, enabled: false)
+      _media_item = media_item_fixture(source_id: source.id, media_filepath: nil)
+      update_attrs = %{enabled: true}
+
+      refute_enqueued(worker: MediaDownloadWorker)
+      assert {:ok, %Source{}} = Sources.update_source(source, update_attrs)
+      refute_enqueued(worker: MediaDownloadWorker)
+    end
+  end
+
+  describe "update_source/3 when testing indexing" do
     test "updating the index frequency to >0 will re-schedule the indexing task" do
       source = source_fixture()
       update_attrs = %{index_frequency_minutes: 123}
@@ -462,27 +556,6 @@ defmodule Pinchflat.SourcesTest do
       refute_enqueued(worker: MediaCollectionIndexingWorker, args: %{"id" => source.id})
     end
 
-    test "enabling the download_media attribute will schedule a download task" do
-      source = source_fixture(download_media: false)
-      media_item = media_item_fixture(source_id: source.id, media_filepath: nil)
-      update_attrs = %{download_media: true}
-
-      refute_enqueued(worker: MediaDownloadWorker)
-      assert {:ok, %Source{}} = Sources.update_source(source, update_attrs)
-      assert_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id})
-    end
-
-    test "disabling the download_media attribute will cancel the download task" do
-      source = source_fixture(download_media: true)
-      media_item = media_item_fixture(source_id: source.id, media_filepath: nil)
-      update_attrs = %{download_media: false}
-      DownloadingHelpers.enqueue_pending_download_tasks(source)
-
-      assert_enqueued(worker: MediaDownloadWorker, args: %{"id" => media_item.id})
-      assert {:ok, %Source{}} = Sources.update_source(source, update_attrs)
-      refute_enqueued(worker: MediaDownloadWorker)
-    end
-
     test "enabling fast_index will schedule a fast indexing task" do
       source = source_fixture(fast_index: false)
       update_attrs = %{fast_index: true}
@@ -503,15 +576,6 @@ defmodule Pinchflat.SourcesTest do
       refute_enqueued(worker: FastIndexingWorker)
     end
 
-    test "updates with invalid data returns error changeset" do
-      source = source_fixture()
-
-      assert {:error, %Ecto.Changeset{}} =
-               Sources.update_source(source, @invalid_source_attrs)
-
-      assert source == Sources.get_source!(source.id)
-    end
-
     test "fast_index forces the index frequency to be a default value" do
       source = source_fixture(%{fast_index: true})
       update_attrs = %{index_frequency_minutes: 0}
@@ -529,25 +593,6 @@ defmodule Pinchflat.SourcesTest do
 
       assert source.index_frequency_minutes == 0
     end
-
-    test "updating will kickoff a metadata storage worker if the original_url changes" do
-      expect(YtDlpRunnerMock, :run, &playlist_mock/4)
-      source = source_fixture()
-      update_attrs = %{original_url: "https://www.youtube.com/channel/cba321"}
-
-      assert {:ok, %Source{} = source} = Sources.update_source(source, update_attrs)
-
-      assert_enqueued(worker: SourceMetadataStorageWorker, args: %{"id" => source.id})
-    end
-
-    test "updating will not kickoff a metadata storage worker other attrs change" do
-      source = source_fixture()
-      update_attrs = %{name: "some new name"}
-
-      assert {:ok, %Source{}} = Sources.update_source(source, update_attrs)
-
-      refute_enqueued(worker: SourceMetadataStorageWorker)
-    end
   end
 
   describe "update_source/3 when testing options" do

test/support/fixtures/sources_fixtures.ex

@@ -20,6 +20,7 @@ defmodule Pinchflat.SourcesFixtures do
         Enum.into(
           attrs,
           %{
+            enabled: true,
             collection_name: "Source ##{:rand.uniform(1_000_000)}",
             collection_id: Base.encode16(:crypto.hash(:md5, "#{:rand.uniform(1_000_000)}")),
             collection_type: "channel",