Hooked up collection_type to form; refactored enqueue_pending_media_downloads

.iex.exs

@@ -2,6 +2,7 @@ alias Pinchflat.Repo
 
 alias Pinchflat.Tasks.Task
 alias Pinchflat.Media.MediaItem
+alias Pinchflat.Tasks.SourceTasks
 alias Pinchflat.Media.MediaMetadata
 alias Pinchflat.MediaSource.Source
 alias Pinchflat.Profiles.MediaProfile

lib/pinchflat/media_source.ex

@@ -100,6 +100,8 @@ defmodule Pinchflat.MediaSource do
   Note that this fetches source details as long as the `original_url` is present.
   This means that it'll go for it even if a changeset is otherwise invalid. This
   is pretty easy to change, but for MVP I'm not concerned.
+
+  IDEA: Maybe I could discern `collection_type` based on the original URL?
   """
   def change_source_from_url(%Source{} = source, attrs) do
     case change_source(source, attrs) do

lib/pinchflat/media_source/source.ex

@@ -9,7 +9,16 @@ defmodule Pinchflat.MediaSource.Source do
   alias Pinchflat.Media.MediaItem
   alias Pinchflat.Profiles.MediaProfile
 
-  @allowed_fields ~w(collection_name collection_id collection_type index_frequency_minutes original_url media_profile_id)a
+  @allowed_fields ~w(
+    collection_name
+    collection_id
+    collection_type
+    friendly_name
+    index_frequency_minutes
+    original_url
+    media_profile_id
+  )a
+
   @required_fields @allowed_fields -- ~w(index_frequency_minutes friendly_name)a
 
   schema "sources" do
@@ -17,7 +26,7 @@ defmodule Pinchflat.MediaSource.Source do
     field :collection_name, :string
     field :collection_id, :string
     field :collection_type, Ecto.Enum, values: [:channel, :playlist]
-    field :index_frequency_minutes, :integer
+    field :index_frequency_minutes, :integer, default: 60 * 24
     # This should only be used for user reference going forward
     # as the collection_id should be used for all API calls
     field :original_url, :string

lib/pinchflat/tasks/source_tasks.ex

@@ -3,9 +3,11 @@ defmodule Pinchflat.Tasks.SourceTasks do
   This module contains methods for managing tasks (workers) related to sources.
   """
 
+  alias Pinchflat.Media
   alias Pinchflat.Tasks
   alias Pinchflat.MediaSource.Source
   alias Pinchflat.Workers.MediaIndexingWorker
+  alias Pinchflat.Workers.VideoDownloadWorker
 
   @doc """
   Starts tasks for indexing a source's media.
@@ -30,4 +32,27 @@ defmodule Pinchflat.Tasks.SourceTasks do
       end
     end
   end
+
+  @doc """
+  Starts tasks for downloading videos for any of a sources _pending_ media items.
+
+  NOTE: this starts a download for each media item that is pending,
+  not just the ones that were indexed in this job run. This should ensure
+  that any stragglers are caught if, for some reason, they weren't enqueued
+  or somehow got de-queued.
+
+  I'm not sure of a case where this would happen, but it's cheap insurance.
+
+  Returns :ok
+  """
+  def enqueue_pending_media_downloads(%Source{} = source) do
+    source
+    |> Media.list_pending_media_items_for()
+    |> Enum.each(fn media_item ->
+      media_item
+      |> Map.take([:id])
+      |> VideoDownloadWorker.new()
+      |> Tasks.create_job_with_task(media_item)
+    end)
+  end
 end

lib/pinchflat/workers/media_indexing_worker.ex

@@ -7,10 +7,9 @@ defmodule Pinchflat.Workers.MediaIndexingWorker do
     tags: ["media_source", "media_indexing"]
 
   alias __MODULE__
-  alias Pinchflat.Media
   alias Pinchflat.Tasks
   alias Pinchflat.MediaSource
-  alias Pinchflat.Workers.VideoDownloadWorker
+  alias Pinchflat.Tasks.SourceTasks
 
   @impl Oban.Worker
   @doc """
@@ -49,7 +48,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorker do
 
   defp index_media_and_reschedule(source) do
     MediaSource.index_media_items(source)
-    enqueue_video_downloads(source)
+    SourceTasks.enqueue_pending_media_downloads(source)
 
     source
     |> Map.take([:id])
@@ -60,21 +59,4 @@ defmodule Pinchflat.Workers.MediaIndexingWorker do
       {:error, :duplicate_job} -> {:ok, :job_exists}
     end
   end
-
-  # NOTE: this starts a download for each media item that is pending,
-  # not just the ones that were indexed in this job run. This should ensure
-  # that any stragglers are caught if, for some reason, they weren't enqueued
-  # or somehow got de-queued.
-  #
-  # I'm not sure of a case where this would happen, but it's cheap insurance.
-  defp enqueue_video_downloads(source) do
-    source
-    |> Media.list_pending_media_items_for()
-    |> Enum.each(fn media_item ->
-      media_item
-      |> Map.take([:id])
-      |> VideoDownloadWorker.new()
-      |> Tasks.create_job_with_task(media_item)
-    end)
-  end
 end

lib/pinchflat_web/controllers/media_sources/source_html.ex

@@ -24,4 +24,11 @@ defmodule PinchflatWeb.MediaSources.SourceHTML do
       {"Monthly", 30 * 24 * 60}
     ]
   end
+
+  def friendly_collection_types do
+    [
+      {"Channel", "channel"},
+      {"Playlist", "playlist"}
+    ]
+  end
 end

lib/pinchflat_web/controllers/media_sources/source_html/source_form.html.heex

@@ -10,7 +10,13 @@
     label="Media Profile"
   />
 
-  <.input field={f[:collection_type]} type="text" label="Collection Type" />
+  <.input
+    field={f[:collection_type]}
+    options={friendly_collection_types()}
+    type="select"
+    label="Collection Type"
+  />
+
   <.input field={f[:original_url]} type="text" label="Source URL" />
 
   <.input

test/pinchflat/tasks/source_tasks_test.exs

@@ -2,11 +2,14 @@ defmodule Pinchflat.Tasks.SourceTasksTest do
   use Pinchflat.DataCase
 
   import Pinchflat.TasksFixtures
+  import Pinchflat.MediaFixtures
   import Pinchflat.MediaSourceFixtures
 
+  alias Pinchflat.Tasks
   alias Pinchflat.Tasks.Task
   alias Pinchflat.Tasks.SourceTasks
   alias Pinchflat.Workers.MediaIndexingWorker
+  alias Pinchflat.Workers.VideoDownloadWorker
 
   describe "kickoff_indexing_task/1" do
     test "it does not schedule a job if the interval is <= 0" do
@@ -42,4 +45,35 @@ defmodule Pinchflat.Tasks.SourceTasksTest do
       assert_raise Ecto.NoResultsError, fn -> Repo.reload!(task) end
     end
   end
+
+  describe "enqueue_pending_media_downloads/1" do
+    test "it enqueues a job for each pending media item" do
+      source = source_fixture()
+      media_item = media_item_fixture(source_id: source.id, media_filepath: nil)
+
+      assert :ok = SourceTasks.enqueue_pending_media_downloads(source)
+
+      assert_enqueued(worker: VideoDownloadWorker, args: %{"id" => media_item.id})
+    end
+
+    test "it does not enqueue a job for media items with a filepath" do
+      source = source_fixture()
+      _media_item = media_item_fixture(source_id: source.id, media_filepath: "some/filepath.mp4")
+
+      assert :ok = SourceTasks.enqueue_pending_media_downloads(source)
+
+      refute_enqueued(worker: VideoDownloadWorker)
+    end
+
+    test "it attaches a task to each enqueued job" 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 :ok = SourceTasks.enqueue_pending_media_downloads(source)
+
+      assert [_] = Tasks.list_tasks_for(:media_item_id, media_item.id)
+    end
+  end
 end