Source attributes in output template (#45)

* Updated download options to take a media item; allowed for specifying custom output path template options

* Fixed bug where indexing job wouldn't run for the first time

* Renamed friendly_name to custom_name

* Added new options to default template and UI

* Improved explainer UI

test/pinchflat/media_client/backends/yt_dlp/output_path_builder_test.exs

@@ -3,7 +3,7 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.OutputPathBuilderTest do
 
   alias Pinchflat.Profiles.Options.YtDlp.OutputPathBuilder
 
-  describe "build/1" do
+  describe "build/2" do
     test "it expands 'standard' curly brace variables in the template" do
       assert {:ok, res} = OutputPathBuilder.build("/videos/{{ title }}.{{ ext }}")
 
@@ -16,6 +16,12 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.OutputPathBuilderTest do
       assert res == "/videos/%(upload_date>%Y)S.%(ext)S"
     end
 
+    test "it respects additional options" do
+      assert {:ok, res} = OutputPathBuilder.build("/videos/{{ custom }}.{{ ext }}", %{"custom" => "test"})
+
+      assert res == "/videos/test.%(ext)S"
+    end
+
     test "it leaves yt-dlp variables alone" do
       assert {:ok, res} = OutputPathBuilder.build("/videos/%(title)s.%(ext)s")
 

test/pinchflat/profiles/options/yt_dlp/download_option_builder_test.exs

@@ -1,24 +1,40 @@
 defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilderTest do
-  use ExUnit.Case, async: true
+  use Pinchflat.DataCase
+  import Pinchflat.MediaFixtures
+  import Pinchflat.ProfilesFixtures
+  import Pinchflat.SourcesFixtures
 
-  alias Pinchflat.Profiles.MediaProfile
+  alias Pinchflat.Profiles
   alias Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilder
 
-  @media_profile %MediaProfile{
-    output_path_template: "{{ title }}.%(ext)s"
-  }
+  setup do
+    media_profile = media_profile_fixture(%{output_path_template: "{{ title }}.%(ext)s"})
+    source = source_fixture(%{media_profile_id: media_profile.id, custom_name: "my source"})
+    media_item = Repo.preload(media_item_fixture(source_id: source.id), source: :media_profile)
 
-  describe "build/1" do
-    test "it generates an expanded output path based on the given template" do
-      assert {:ok, res} = DownloadOptionBuilder.build(@media_profile)
+    {:ok, media_item: media_item}
+  end
+
+  describe "build/1 when testing output options" do
+    test "it generates an expanded output path based on the given template", %{media_item: media_item} do
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert {:output, "/tmp/test/videos/%(title)S.%(ext)s"} in res
     end
+
+    test "it respects custom output path options", %{media_item: media_item} do
+      media_item =
+        update_media_profile_attribute(media_item, %{output_path_template: "{{ source_custom_name }}.%(ext)s"})
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
+
+      assert {:output, "/tmp/test/videos/#{media_item.source.custom_name}.%(ext)s"} in res
+    end
   end
 
   describe "build/1 when testing default options" do
-    test "it includes default options" do
-      assert {:ok, res} = DownloadOptionBuilder.build(@media_profile)
+    test "it includes default options", %{media_item: media_item} do
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert :no_progress in res
       assert :windows_filenames in res
@@ -26,109 +42,93 @@ defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilderTest do
   end
 
   describe "build/1 when testing subtitle options" do
-    test "includes :write_subs option when specified" do
-      media_profile = %MediaProfile{@media_profile | download_subs: true}
+    test "includes :write_subs option when specified", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{download_subs: true})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert :write_subs in res
     end
 
-    test "forces SRT format when download_subs is true" do
-      media_profile = %MediaProfile{@media_profile | download_subs: true}
+    test "forces SRT format when download_subs is true", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{download_subs: true})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert {:convert_subs, "srt"} in res
     end
 
-    test "includes :write_auto_subs option when specified" do
-      media_profile = %MediaProfile{@media_profile | download_subs: true, download_auto_subs: true}
+    test "includes :write_auto_subs option when specified", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{download_subs: true, download_auto_subs: true})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert :write_auto_subs in res
     end
 
-    test "doesn't include :write_auto_subs option when download_subs is false" do
-      media_profile = %MediaProfile{@media_profile | download_subs: false, download_auto_subs: true}
+    test "doesn't include :write_auto_subs option when download_subs is false", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{download_subs: false, download_auto_subs: true})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       refute :write_auto_subs in res
     end
 
-    test "includes :embed_subs option when specified" do
-      media_profile = %MediaProfile{@media_profile | embed_subs: true}
+    test "includes :embed_subs option when specified", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{embed_subs: true})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert :embed_subs in res
     end
 
-    test "includes sub_langs option when download_subs is true" do
-      media_profile = %MediaProfile{@media_profile | download_subs: true, sub_langs: "en"}
+    test "includes sub_langs option when download_subs is true", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{download_subs: true, sub_langs: "en"})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert {:sub_langs, "en"} in res
     end
 
-    test "includes sub_langs option when embed_subs is true" do
-      media_profile = %MediaProfile{@media_profile | embed_subs: true, sub_langs: "en"}
+    test "includes sub_langs option when embed_subs is true", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{embed_subs: true, sub_langs: "en"})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert {:sub_langs, "en"} in res
     end
 
-    test "doesn't include sub_langs option when neither downloading nor embedding" do
-      media_profile = %MediaProfile{
-        @media_profile
-        | embed_subs: false,
-          download_subs: false,
-          sub_langs: "en"
-      }
+    test "doesn't include sub_langs option when neither downloading nor embedding", %{media_item: media_item} do
+      media_item =
+        update_media_profile_attribute(media_item, %{embed_subs: false, download_subs: false, sub_langs: "en"})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       refute {:sub_langs, "en"} in res
     end
-
-    test "other struct attributes are ignored" do
-      media_profile = %MediaProfile{@media_profile | id: -1}
-
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
-
-      refute {:id, -1} in res
-    end
   end
 
   describe "build/1 when testing thumbnail options" do
-    test "includes :write_thumbnail option when specified" do
-      media_profile = %MediaProfile{@media_profile | download_thumbnail: true}
+    test "includes :write_thumbnail option when specified", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{download_thumbnail: true})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert :write_thumbnail in res
     end
 
-    test "includes :embed_thumbnail option when specified" do
-      media_profile = %MediaProfile{@media_profile | embed_thumbnail: true}
+    test "includes :embed_thumbnail option when specified", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{embed_thumbnail: true})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert :embed_thumbnail in res
     end
 
-    test "doesn't include these options when not specified" do
-      media_profile = %MediaProfile{
-        @media_profile
-        | embed_thumbnail: false,
-          download_thumbnail: false
-      }
+    test "doesn't include these options when not specified", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{embed_thumbnail: false, download_thumbnail: false})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       refute :write_thumbnail in res
       refute :embed_thumbnail in res
@@ -136,31 +136,27 @@ defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilderTest do
   end
 
   describe "build/1 when testing metadata options" do
-    test "includes :write_info_json option when specified" do
-      media_profile = %MediaProfile{@media_profile | download_metadata: true}
+    test "includes :write_info_json option when specified", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{download_metadata: true})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert :write_info_json in res
       assert :clean_info_json in res
     end
 
-    test "includes :embed_metadata option when specified" do
-      media_profile = %MediaProfile{@media_profile | embed_metadata: true}
+    test "includes :embed_metadata option when specified", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{embed_metadata: true})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert :embed_metadata in res
     end
 
-    test "doesn't include these options when not specified" do
-      media_profile = %MediaProfile{
-        @media_profile
-        | embed_metadata: false,
-          download_metadata: false
-      }
+    test "doesn't include these options when not specified", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{embed_metadata: false, download_metadata: false})
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_profile)
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       refute :write_info_json in res
       refute :clean_info_json in res
@@ -169,10 +165,22 @@ defmodule Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilderTest do
   end
 
   describe "build/1 when testing quality options" do
-    test "it includes quality options" do
-      assert {:ok, res} = DownloadOptionBuilder.build(@media_profile)
+    test "it includes quality options", %{media_item: media_item} do
+      media_item = update_media_profile_attribute(media_item, %{preferred_resolution: :"1080p"})
+
+      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert {:format_sort, "res:1080,+codec:avc:m4a"} in res
     end
   end
+
+  defp update_media_profile_attribute(media_item_with_preloads, attrs) do
+    media_item_with_preloads.source.media_profile
+    |> Profiles.change_media_profile(attrs)
+    |> Repo.update!()
+
+    media_item_with_preloads
+    |> Repo.reload()
+    |> Repo.preload(source: :media_profile)
+  end
 end

test/pinchflat/sources_test.exs

@@ -66,18 +66,18 @@ defmodule Pinchflat.SourcesTest do
       assert String.starts_with?(source.collection_id, "some_playlist_id_")
     end
 
-    test "you can specify a custom friendly_name" do
+    test "you can specify a custom custom_name" do
       expect(YtDlpRunnerMock, :run, &channel_mock/3)
 
       valid_attrs = %{
         media_profile_id: media_profile_fixture().id,
         original_url: "https://www.youtube.com/channel/abc123",
-        friendly_name: "some custom name"
+        custom_name: "some custom name"
       }
 
       assert {:ok, %Source{} = source} = Sources.create_source(valid_attrs)
 
-      assert source.friendly_name == "some custom name"
+      assert source.custom_name == "some custom name"
     end
 
     test "friendly name is pulled from collection_name if not specified" do
@@ -90,7 +90,7 @@ defmodule Pinchflat.SourcesTest do
 
       assert {:ok, %Source{} = source} = Sources.create_source(valid_attrs)
 
-      assert source.friendly_name == "some channel name"
+      assert source.custom_name == "some channel name"
     end
 
     test "collection_type is inferred from source details" do

test/pinchflat/workers/media_indexing_worker_test.exs

@@ -12,23 +12,6 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do
   setup :verify_on_exit!
 
   describe "perform/1" do
-    test "it does not do any indexing if the source shouldn't be indexed" do
-      expect(YtDlpRunnerMock, :run, 0, fn _url, _opts, _ot -> {:ok, ""} end)
-
-      source = source_fixture(index_frequency_minutes: -1)
-
-      perform_job(MediaIndexingWorker, %{id: source.id})
-    end
-
-    test "it does not reschedule if the source shouldn't be indexed" do
-      expect(YtDlpRunnerMock, :run, 0, fn _url, _opts, _ot -> {:ok, ""} end)
-
-      source = source_fixture(index_frequency_minutes: -1)
-      perform_job(MediaIndexingWorker, %{id: source.id})
-
-      refute_enqueued(worker: MediaIndexingWorker, args: %{"id" => source.id})
-    end
-
     test "it indexes the source if it should be indexed" do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, ""} end)
 
@@ -37,6 +20,31 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do
       perform_job(MediaIndexingWorker, %{id: source.id})
     end
 
+    test "it indexes the source no matter what if the source has never been indexed before" do
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, ""} end)
+
+      source = source_fixture(index_frequency_minutes: 0, last_indexed_at: nil)
+
+      perform_job(MediaIndexingWorker, %{id: source.id})
+    end
+
+    test "it does not do any indexing if the source has been indexed and shouldn't be rescheduled" do
+      expect(YtDlpRunnerMock, :run, 0, fn _url, _opts, _ot -> {:ok, ""} end)
+
+      source = source_fixture(index_frequency_minutes: -1, last_indexed_at: DateTime.utc_now())
+
+      perform_job(MediaIndexingWorker, %{id: source.id})
+    end
+
+    test "it does not reschedule if the source shouldn't be indexed" do
+      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, ""} end)
+
+      source = source_fixture(index_frequency_minutes: -1)
+      perform_job(MediaIndexingWorker, %{id: source.id})
+
+      refute_enqueued(worker: MediaIndexingWorker, args: %{"id" => source.id})
+    end
+
     test "it kicks off a download job for each pending media item" do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, source_attributes_return_fixture()} end)
 

test/support/fixtures/sources_fixtures.ex

@@ -19,7 +19,7 @@ defmodule Pinchflat.SourcesFixtures do
           collection_name: "Source ##{:rand.uniform(1_000_000)}",
           collection_id: Base.encode16(:crypto.hash(:md5, "#{:rand.uniform(1_000_000)}")),
           collection_type: "channel",
-          friendly_name: "Cool and good internal name!",
+          custom_name: "Cool and good internal name!",
           original_url: "https://www.youtube.com/channel/#{Faker.String.base64(12)}",
           media_profile_id: ProfilesFixtures.media_profile_fixture().id,
           index_frequency_minutes: 60