diff --git a/lib/pinchflat/sources/sources.ex b/lib/pinchflat/sources/sources.ex index 161b957..7052614 100644 --- a/lib/pinchflat/sources/sources.ex +++ b/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 + defp maybe_handle_media_tasks(%{data: %{__meta__: %{state: state}}}, _source) when state != :loaded do + :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 - case {changeset.data, changeset.changes} do - {%{__meta__: %{state: :loaded}}, %{download_media: true}} -> - DownloadingHelpers.enqueue_pending_download_tasks(source) + current_changes = changeset.changes + applied_changes = Ecto.Changeset.apply_changes(changeset) - {%{__meta__: %{state: :loaded}}, %{download_media: false}} -> + # 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) + _ -> - :ok + nil end + + :ok end defp maybe_run_indexing_task(changeset, source) do diff --git a/lib/pinchflat_web/controllers/sources/source_html/index_table_live.ex b/lib/pinchflat_web/controllers/sources/source_html/index_table_live.ex index 434cbb7..eae6613 100644 --- a/lib/pinchflat_web/controllers/sources/source_html/index_table_live.ex +++ b/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) %> diff --git a/test/pinchflat/sources_test.exs b/test/pinchflat/sources_test.exs index ee57e17..f365a41 100644 --- a/test/pinchflat/sources_test.exs +++ b/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 diff --git a/test/support/fixtures/sources_fixtures.ex b/test/support/fixtures/sources_fixtures.ex index d54ba5f..9d699dc 100644 --- a/test/support/fixtures/sources_fixtures.ex +++ b/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",