Addressed a bunch of TODO comments

ideas.md

@@ -0,0 +1,3 @@
+- Write media datbase ID as metadata/to file/whatever so it gives us an option to retroactively match media to the DB down the line. Useful if someone moves the media without informing the UI
+   - Use a UUID for the media database ID (or at least alongside it)
+- Look into this and its recommended plugins https://hexdocs.pm/ex_check/readme.html

lib/pinchflat/downloader/backends/yt_dlp/command_runner.ex

@@ -11,7 +11,7 @@ defmodule Pinchflat.Downloader.Backends.YtDlp.CommandRunner do
   @doc """
   Runs a yt-dlp command and returns the string output
 
-  # TODO: deduplicate command opts, keeping the last one on conflict
+  # IDEA: deduplicate command opts, keeping the last one on conflict
           although possibly not needed (and a LOT easier) if yt-dlp
           just ignores duplicate options (ie: look into that)
   """

lib/pinchflat/downloader/backends/yt_dlp/video.ex

@@ -4,23 +4,16 @@ defmodule Pinchflat.Downloader.Backends.YtDlp.Video do
   """
 
   @doc """
-  Downloads a single video (and possible metadata) to the tmp directory.
+  Downloads a single video (and possible metadata) directly to its
-  Videos are downloaded in the following format:
+  final destination. Returns the parsed JSON output from yt-dlp.
-    `tmp/yt-dlp/<video_id>/<video_id>.<ext>`
-
-  The video will be moved to its final destination... elsewhere
-  # TODO: update these docs when I figure out a module to move videos
-  # TODO: test
-  # NOTE: maybe instead of moving it to the tempdir, I can just download it
-          to the final destination by using the `output` option. The
-          parser could be updated to generate a value for the output option.
-          This way, advanced users can just the yt-dlp output syntax and
-          newer users can use the easier liquid-like syntax.
   """
   def download(url, command_opts \\ []) do
-    # TODO: if this stays this simple, consider not abstracting it
+    opts = [:no_simulate, :dump_json] ++ command_opts
-    #       HOWEVER - this module does provide clarity of intent so maybe keep?
+
-    backend_runner().run(url, command_opts)
+    case backend_runner().run(url, opts) do
+      {:ok, output} -> Phoenix.json_library().decode(output)
+      err -> err
+    end
   end
 
   defp backend_runner do

lib/pinchflat/downloader/video_downloader.ex

@@ -8,16 +8,15 @@ defmodule Pinchflat.Downloader.VideoDownloader do
   it open-ish for future expansion (just in case).
   """
 
+  alias Pinchflat.Profiles.MediaProfile
+
   alias Pinchflat.Downloader.Backends.YtDlp.Video, as: YtDlpVideo
   alias Pinchflat.Profiles.Options.YtDlp.OptionBuilder, as: YtDlpOptionBuilder
 
   @doc """
   Downloads a single video based on the settings in the given media profile.
-
-  # TODO: implement media profiles - so far this is a glorified mock
-  # TODO: test
   """
-  def download_for_media_profile(url, media_profile, backend \\ :yt_dlp) do
+  def download_for_media_profile(url, %MediaProfile{} = media_profile, backend \\ :yt_dlp) do
     option_builder = option_builder(backend)
     video_backend = video_backend(backend)
     {:ok, options} = option_builder.build(media_profile)
@@ -25,13 +24,13 @@ defmodule Pinchflat.Downloader.VideoDownloader do
     video_backend.download(url, options)
   end
 
-  def option_builder(backend) do
+  defp option_builder(backend) do
     case backend do
       :yt_dlp -> YtDlpOptionBuilder
     end
   end
 
-  def video_backend(backend) do
+  defp video_backend(backend) do
     case backend do
       :yt_dlp -> YtDlpVideo
     end

lib/pinchflat/profiles.ex

@@ -9,13 +9,7 @@ defmodule Pinchflat.Profiles do
   alias Pinchflat.Profiles.MediaProfile
 
   @doc """
-  Returns the list of media_profiles.
+  Returns the list of media_profiles. Returns [%MediaProfile{}, ...]
-
-  ## Examples
-
-      iex> list_media_profiles()
-      [%MediaProfile{}, ...]
-
   """
   def list_media_profiles do
     Repo.all(MediaProfile)
@@ -24,30 +18,12 @@ defmodule Pinchflat.Profiles do
   @doc """
   Gets a single media_profile.
 
-  Raises `Ecto.NoResultsError` if the Media profile does not exist.
+  Returns %MediaProfile{}. Raises `Ecto.NoResultsError` if the Media profile does not exist.
-
-  ## Examples
-
-      iex> get_media_profile!(123)
-      %MediaProfile{}
-
-      iex> get_media_profile!(456)
-      ** (Ecto.NoResultsError)
-
   """
   def get_media_profile!(id), do: Repo.get!(MediaProfile, id)
 
   @doc """
-  Creates a media_profile.
+  Creates a media_profile. Returns {:ok, %MediaProfile{}} | {:error, %Ecto.Changeset{}}
-
-  ## Examples
-
-      iex> create_media_profile(%{field: value})
-      {:ok, %MediaProfile{}}
-
-      iex> create_media_profile(%{field: bad_value})
-      {:error, %Ecto.Changeset{}}
-
   """
   def create_media_profile(attrs \\ %{}) do
     %MediaProfile{}
@@ -56,16 +32,7 @@ defmodule Pinchflat.Profiles do
   end
 
   @doc """
-  Updates a media_profile.
+  Updates a media_profile. Returns {:ok, %MediaProfile{}} | {:error, %Ecto.Changeset{}}
-
-  ## Examples
-
-      iex> update_media_profile(media_profile, %{field: new_value})
-      {:ok, %MediaProfile{}}
-
-      iex> update_media_profile(media_profile, %{field: bad_value})
-      {:error, %Ecto.Changeset{}}
-
   """
   def update_media_profile(%MediaProfile{} = media_profile, attrs) do
     media_profile
@@ -74,16 +41,7 @@ defmodule Pinchflat.Profiles do
   end
 
   @doc """
-  Deletes a media_profile.
+  Deletes a media_profile. Returns {:ok, %MediaProfile{}} | {:error, %Ecto.Changeset{}}
-
-  ## Examples
-
-      iex> delete_media_profile(media_profile)
-      {:ok, %MediaProfile{}}
-
-      iex> delete_media_profile(media_profile)
-      {:error, %Ecto.Changeset{}}
-
   """
   def delete_media_profile(%MediaProfile{} = media_profile) do
     Repo.delete(media_profile)
@@ -91,12 +49,6 @@ defmodule Pinchflat.Profiles do
 
   @doc """
   Returns an `%Ecto.Changeset{}` for tracking media_profile changes.
-
-  ## Examples
-
-      iex> change_media_profile(media_profile)
-      %Ecto.Changeset{data: %MediaProfile{}}
-
   """
   def change_media_profile(%MediaProfile{} = media_profile, attrs \\ %{}) do
     MediaProfile.changeset(media_profile, attrs)

lib/pinchflat/profiles/media_profile.ex

@@ -1,4 +1,8 @@
 defmodule Pinchflat.Profiles.MediaProfile do
+  @moduledoc """
+  A media profile is a set of settings that can be applied to many media sources
+  """
+
   use Ecto.Schema
   import Ecto.Changeset
 

lib/pinchflat/profiles/options/yt_dlp/option_builder.ex

@@ -2,20 +2,19 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilder do
   @moduledoc """
   Builds the options for yt-dlp based on the given media profile.
 
-  TODO: probably make this a behaviour so I can add other backends later
+  IDEA: consider making this a behaviour so I can add other backends later
   """
 
+  alias Pinchflat.Profiles.MediaProfile
   alias Pinchflat.Profiles.Options.YtDlp.OutputPathBuilder
 
   @doc """
   Builds the options for yt-dlp based on the given media profile.
 
-  TODO: add a guard to ensure argument is a media profile
+  IDEA: consider adding the ability to pass in a second argument to override
-  TODO: consider adding the ability to pass in a second argument to override
         these options
-  TODO: test
   """
-  def build(media_profile) do
+  def build(%MediaProfile{} = media_profile) do
     {:ok, output_path} = OutputPathBuilder.build(media_profile.output_path_template)
 
     # NOTE: I'll be hardcoding most things for now (esp. options to help me test) -

lib/pinchflat/profiles/options/yt_dlp/output_path_builder.ex

@@ -2,7 +2,7 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OutputPathBuilder do
   @moduledoc """
   Builds yt-dlp-friendly output paths for downloaded media
 
-  TODO: probably make this a behaviour so I can add other backends later
+  IDEA: consider making this a behaviour so I can add other backends later
   """
 
   alias Pinchflat.RenderedString.Parser, as: TemplateParser
@@ -12,8 +12,6 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OutputPathBuilder do
 
   Translates liquid-style templates into yt-dlp-style templates,
   leaving yt-dlp syntax intact.
-
-  TODO: test
   """
   def build(template_string) do
     TemplateParser.parse(template_string, full_yt_dlp_options_map())
@@ -58,9 +56,9 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OutputPathBuilder do
   defp custom_yt_dlp_option_map do
     %{
       # Individual parts of the upload date
-      "upload_year" => "%(upload_date>%Y)s",
+      "upload_year" => "%(upload_date>%Y)S",
-      "upload_month" => "%(upload_date>%m)s",
+      "upload_month" => "%(upload_date>%m)S",
-      "upload_day" => "%(upload_date>%d)s"
+      "upload_day" => "%(upload_date>%d)S"
     }
   end
 end

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

@@ -0,0 +1,25 @@
+defmodule Pinchflat.Downloader.Backends.YtDlp.OutputPathBuilderTest do
+  use ExUnit.Case, async: true
+
+  alias Pinchflat.Profiles.Options.YtDlp.OutputPathBuilder
+
+  describe "build/1" do
+    test "it expands 'standard' curly brace variables in the template" do
+      assert {:ok, res} = OutputPathBuilder.build("/videos/{{ title }}.{{ ext }}")
+
+      assert res == "/videos/%(title)S.%(ext)S"
+    end
+
+    test "it expands 'custom' curly brace variables in the template" do
+      assert {:ok, res} = OutputPathBuilder.build("/videos/{{ upload_year }}.{{ ext }}")
+
+      assert res == "/videos/%(upload_date>%Y)S.%(ext)S"
+    end
+
+    test "it leaves yt-dlp variables alone" do
+      assert {:ok, res} = OutputPathBuilder.build("/videos/%(title)s.%(ext)s")
+
+      assert res == "/videos/%(title)s.%(ext)s"
+    end
+  end
+end

test/pinchflat/downloader/backends/yt_dlp/video_collection_test.exs

@@ -2,7 +2,7 @@ defmodule Pinchflat.Downloader.Backends.YtDlp.VideoCollectionTest do
   use ExUnit.Case, async: true
   import Mox
 
-  alias Pinchflat.Downloader.Backends.YtDlp.VideoCollection, as: VideoCollection
+  alias Pinchflat.Downloader.Backends.YtDlp.VideoCollection
 
   @channel_url "https://www.youtube.com/@TheUselessTrials"
 

test/pinchflat/downloader/backends/yt_dlp/video_test.exs

@@ -0,0 +1,44 @@
+defmodule Pinchflat.Downloader.Backends.YtDlp.VideoTest do
+  use ExUnit.Case, async: true
+  import Mox
+
+  alias Pinchflat.Downloader.Backends.YtDlp.Video
+
+  @video_url "https://www.youtube.com/watch?v=TiZPUDkDYbk"
+
+  setup :verify_on_exit!
+
+  describe "download/2" do
+    test "it calls the backend runner with the expected arguments" do
+      expect(CommandRunnerMock, :run, fn @video_url, opts ->
+        assert opts == [:no_simulate, :dump_json]
+
+        {:ok, "{}"}
+      end)
+
+      assert {:ok, _} = Video.download(@video_url)
+    end
+
+    test "it passes along additional options" do
+      expect(CommandRunnerMock, :run, fn _url, opts ->
+        assert opts == [:no_simulate, :dump_json, :custom_arg]
+
+        {:ok, "{}"}
+      end)
+
+      assert {:ok, _} = Video.download(@video_url, [:custom_arg])
+    end
+
+    test "it parses the output as JSON" do
+      expect(CommandRunnerMock, :run, fn _url, _opts -> {:ok, "{\"title\": \"Test\"}"} end)
+
+      assert {:ok, %{"title" => "Test"}} = Video.download(@video_url)
+    end
+
+    test "it directly passes along any errors" do
+      expect(CommandRunnerMock, :run, fn _url, _opts -> {:error, "Big issue", 1} end)
+
+      assert {:error, "Big issue", 1} = Video.download(@video_url)
+    end
+  end
+end

test/pinchflat/downloader/video_downloader_test.exs

@@ -0,0 +1,35 @@
+defmodule Pinchflat.Downloader.VideoDownloaderTest do
+  use ExUnit.Case, async: true
+  import Mox
+
+  alias Pinchflat.Profiles.MediaProfile
+  alias Pinchflat.Downloader.VideoDownloader
+
+  @video_url "https://www.youtube.com/watch?v=TiZPUDkDYbk"
+  @media_profile %MediaProfile{
+    output_path_template: "videos/{{ title }}.%(ext)s"
+  }
+
+  setup :verify_on_exit!
+
+  describe "download_for_media_profile/3" do
+    test "it calls the backend runner with the arguments built from the media profile" do
+      expect(CommandRunnerMock, :run, fn @video_url, opts ->
+        assert :no_simulate in opts
+        assert :dump_json in opts
+        assert {:output, "/tmp/yt-dlp/videos/%(title)S.%(ext)s"} in opts
+
+        {:ok, "{}"}
+      end)
+
+      assert {:ok, _} = VideoDownloader.download_for_media_profile(@video_url, @media_profile)
+    end
+
+    test "it returns the parsed JSON output" do
+      expect(CommandRunnerMock, :run, fn _url, _opts -> {:ok, "{\"title\": \"Test\"}"} end)
+
+      assert {:ok, %{"title" => "Test"}} =
+               VideoDownloader.download_for_media_profile(@video_url, @media_profile)
+    end
+  end
+end

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

@@ -0,0 +1,18 @@
+defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do
+  use ExUnit.Case, async: true
+
+  alias Pinchflat.Profiles.MediaProfile
+  alias Pinchflat.Profiles.Options.YtDlp.OptionBuilder
+
+  @media_profile %MediaProfile{
+    output_path_template: "videos/{{ title }}.%(ext)s"
+  }
+
+  describe "build/1" do
+    test "it generates an expanded output path based on the given template" do
+      assert {:ok, res} = OptionBuilder.build(@media_profile)
+
+      assert {:output, "/tmp/yt-dlp/videos/%(title)S.%(ext)s"} in res
+    end
+  end
+end

test/pinchflat/utils/string_utils_test.exs

@@ -1,7 +1,7 @@
 defmodule Pinchflat.Utils.StringUtilsTest do
   use ExUnit.Case, async: true
 
-  alias Pinchflat.Utils.StringUtils, as: StringUtils
+  alias Pinchflat.Utils.StringUtils
 
   describe "to_kebab_case/1" do
     test "converts a space-delimited string to kebab-case" do