Applies cutoff date logic to pending media logic

lib/pinchflat/media.ex

@@ -64,6 +64,7 @@ defmodule Pinchflat.Media do
     MediaItem
     |> where([mi], mi.source_id == ^source.id and is_nil(mi.media_filepath))
     |> where(^build_format_clauses(media_profile))
+    |> where(^maybe_apply_cutoff_date(source))
     |> Repo.maybe_limit(limit)
     |> Repo.all()
   end
@@ -92,11 +93,12 @@ defmodule Pinchflat.Media do
   Returns boolean()
   """
   def pending_download?(%MediaItem{} = media_item) do
-    media_profile = Repo.preload(media_item, source: :media_profile).source.media_profile
+    media_item = Repo.preload(media_item, source: :media_profile)
 
     MediaItem
     |> where([mi], mi.id == ^media_item.id and is_nil(mi.media_filepath))
-    |> where(^build_format_clauses(media_profile))
+    |> where(^build_format_clauses(media_item.source.media_profile))
+    |> where(^maybe_apply_cutoff_date(media_item.source))
     |> Repo.exists?()
   end
 
@@ -260,6 +262,14 @@ defmodule Pinchflat.Media do
     {:ok, media_item}
   end
 
+  defp maybe_apply_cutoff_date(source) do
+    if source.download_cutoff_date do
+      dynamic([mi], mi.upload_date >= ^source.download_cutoff_date)
+    else
+      dynamic(true)
+    end
+  end
+
   defp build_format_clauses(media_profile) do
     mapped_struct = Map.from_struct(media_profile)
 

lib/pinchflat/sources.ex

@@ -41,13 +41,24 @@ defmodule Pinchflat.Sources do
   original_url (if provided). Will attempt to start indexing the source's
   media if successfully inserted.
 
+  Runs an initial `change_source` check to ensure most of the source is valid
+  before making an expensive API call. Runs it through `Repo.insert` even
+  though we know it's going to fail so it picks up any addl. database errors
+  and fulfills our return contract.
+
   Returns {:ok, %Source{}} | {:error, %Ecto.Changeset{}}
   """
   def create_source(attrs) do
-    %Source{}
-    |> change_source_from_url(attrs)
-    |> maybe_change_indexing_frequency()
-    |> commit_and_handle_tasks()
+    case change_source(%Source{}, attrs, :initial) do
+      %Ecto.Changeset{valid?: true} ->
+        %Source{}
+        |> change_source_from_url(attrs)
+        |> maybe_change_indexing_frequency()
+        |> commit_and_handle_tasks()
+
+      changeset ->
+        Repo.insert(changeset)
+    end
   end
 
   @doc """
@@ -58,13 +69,24 @@ defmodule Pinchflat.Sources do
   Existing indexing tasks will be cancelled if the indexing frequency has been
   changed (logic in `SourceTasks.kickoff_indexing_task`)
 
+  Runs an initial `change_source` check to ensure most of the source is valid
+  before making an expensive API call. Runs it through `Repo.update` even
+  though we know it's going to fail so it picks up any addl. database errors
+  and fulfills our return contract.
+
   Returns {:ok, %Source{}} | {:error, %Ecto.Changeset{}}
   """
   def update_source(%Source{} = source, attrs) do
-    source
-    |> change_source_from_url(attrs)
-    |> maybe_change_indexing_frequency()
-    |> commit_and_handle_tasks()
+    case change_source(source, attrs, :initial) do
+      %Ecto.Changeset{valid?: true} ->
+        source
+        |> change_source_from_url(attrs)
+        |> maybe_change_indexing_frequency()
+        |> commit_and_handle_tasks()
+
+      changeset ->
+        Repo.update(changeset)
+    end
   end
 
   @doc """
@@ -89,8 +111,8 @@ defmodule Pinchflat.Sources do
   @doc """
   Returns an `%Ecto.Changeset{}` for tracking source changes.
   """
-  def change_source(%Source{} = source, attrs \\ %{}) do
-    Source.changeset(source, attrs)
+  def change_source(%Source{} = source, attrs \\ %{}, validation_stage \\ :pre_insert) do
+    Source.changeset(source, attrs, validation_stage)
   end
 
   @doc """

lib/pinchflat/sources/source.ex

@@ -25,11 +25,11 @@ defmodule Pinchflat.Sources.Source do
     media_profile_id
   )a
 
-  @required_fields ~w(
-    collection_name
-    collection_id
-    collection_type
-    custom_name
+  # Expensive API calls are made when a source is inserted/updated so
+  # we want to ensure that the source is valid before making the call.
+  # This way, we check that the other attributes are valid before ensuring
+  # that all fields are valid.
+  @initially_required_fields ~w(
     index_frequency_minutes
     fast_index
     download_media
@@ -37,6 +37,14 @@ defmodule Pinchflat.Sources.Source do
     media_profile_id
   )a
 
+  @pre_insert_required_fields @initially_required_fields ++
+                                ~w(
+                                  custom_name
+                                  collection_name
+                                  collection_id
+                                  collection_type
+                                )a
+
   schema "sources" do
     field :custom_name, :string
     field :collection_name, :string
@@ -59,11 +67,19 @@ defmodule Pinchflat.Sources.Source do
   end
 
   @doc false
-  def changeset(source, attrs) do
+  def changeset(source, attrs, validation_stage) do
+    # See above for rationale
+    required_fields =
+      if validation_stage == :initial do
+        @initially_required_fields
+      else
+        @pre_insert_required_fields
+      end
+
     source
     |> cast(attrs, @allowed_fields)
     |> dynamic_default(:custom_name, fn cs -> get_field(cs, :collection_name) end)
-    |> validate_required(@required_fields)
+    |> validate_required(required_fields)
     |> unique_constraint([:collection_id, :media_profile_id])
   end
 

test/pinchflat/media_test.exs

@@ -215,6 +215,30 @@ defmodule Pinchflat.MediaTest do
     end
   end
 
+  describe "list_pending_media_items_for/1 when testing cutoff dates" do
+    test "does not return media items with an upload date before the cutoff date" do
+      source = source_fixture(%{download_cutoff_date: now_minus(1, :day)})
+
+      _old_media_item =
+        media_item_fixture(%{source_id: source.id, media_filepath: nil, upload_date: now_minus(2, :days)})
+
+      new_media_item = media_item_fixture(%{source_id: source.id, media_filepath: nil, upload_date: now()})
+
+      assert Media.list_pending_media_items_for(source) == [new_media_item]
+    end
+
+    test "does not apply a cutoff if there is no cutoff date" do
+      source = source_fixture(%{download_cutoff_date: nil})
+
+      old_media_item =
+        media_item_fixture(%{source_id: source.id, media_filepath: nil, upload_date: now_minus(2, :days)})
+
+      new_media_item = media_item_fixture(%{source_id: source.id, media_filepath: nil, upload_date: now()})
+
+      assert Media.list_pending_media_items_for(source) == [old_media_item, new_media_item]
+    end
+  end
+
   describe "list_downloaded_media_items_for/1" do
     test "returns only media items with a media_filepath" do
       source = source_fixture()
@@ -260,6 +284,27 @@ defmodule Pinchflat.MediaTest do
 
       refute Media.pending_download?(media_item)
     end
+
+    test "returns true if there is a cutoff date before the media's upload date" do
+      source = source_fixture(%{download_cutoff_date: now_minus(2, :days)})
+      media_item = media_item_fixture(%{source_id: source.id, media_filepath: nil, upload_date: now_minus(1, :day)})
+
+      assert Media.pending_download?(media_item)
+    end
+
+    test "returns false if there is a cutoff date after the media's upload date" do
+      source = source_fixture(%{download_cutoff_date: now_minus(1, :day)})
+      media_item = media_item_fixture(%{source_id: source.id, media_filepath: nil, upload_date: now_minus(2, :days)})
+
+      refute Media.pending_download?(media_item)
+    end
+
+    test "returns true if there is no cutoff date" do
+      source = source_fixture(%{download_cutoff_date: nil})
+      media_item = media_item_fixture(%{source_id: source.id, media_filepath: nil, upload_date: now_minus(1, :day)})
+
+      assert Media.pending_download?(media_item)
+    end
   end
 
   describe "search/1" do

test/pinchflat/sources_test.exs

@@ -115,6 +115,12 @@ defmodule Pinchflat.SourcesTest do
       assert {:error, %Ecto.Changeset{}} = Sources.create_source(@invalid_source_attrs)
     end
 
+    test "creation with invalid data fails fast and does not call the runner" do
+      expect(YtDlpRunnerMock, :run, 0, &channel_mock/3)
+
+      assert {:error, %Ecto.Changeset{}} = Sources.create_source(@invalid_source_attrs)
+    end
+
     test "creation enforces uniqueness of collection_id scoped to the media_profile" do
       expect(YtDlpRunnerMock, :run, 2, fn _url, _opts, _ot ->
         {:ok,
@@ -225,6 +231,14 @@ defmodule Pinchflat.SourcesTest do
       assert source.collection_name == "some updated name"
     end
 
+    test "updates with invalid data fails fast and does not call the runner" do
+      expect(YtDlpRunnerMock, :run, 0, &channel_mock/3)
+
+      source = source_fixture()
+
+      assert {:error, %Ecto.Changeset{}} = Sources.update_source(source, @invalid_source_attrs)
+    end
+
     test "updating the original_url will re-fetch the source details for channels" do
       expect(YtDlpRunnerMock, :run, &channel_mock/3)
 
@@ -430,7 +444,7 @@ defmodule Pinchflat.SourcesTest do
     end
   end
 
-  describe "change_source/2" do
+  describe "change_source/3" do
     test "it returns a changeset" do
       source = source_fixture()
 

test/pinchflat/yt_dlp/backend/media_test.exs

@@ -140,5 +140,18 @@ defmodule Pinchflat.YtDlp.Backend.MediaTest do
 
       assert %Media{short_form_content: false} = Media.response_to_struct(response)
     end
+
+    test "parses the upload date" do
+      response = %{
+        "webpage_url" => "https://www.youtube.com/watch?v=TiZPUDkDYbk",
+        "aspect_ratio" => 1.0,
+        "duration" => 61,
+        "upload_date" => "20210101"
+      }
+
+      expected_date = Date.from_iso8601!("2021-01-01")
+
+      assert %Media{upload_date: ^expected_date} = Media.response_to_struct(response)
+    end
   end
 end

test/support/fixtures/sources_fixtures.ex

@@ -15,15 +15,19 @@ defmodule Pinchflat.SourcesFixtures do
     {:ok, source} =
       %Source{}
       |> Source.changeset(
-        Enum.into(attrs, %{
-          collection_name: "Source ##{:rand.uniform(1_000_000)}",
-          collection_id: Base.encode16(:crypto.hash(:md5, "#{:rand.uniform(1_000_000)}")),
-          collection_type: "channel",
-          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
-        })
+        Enum.into(
+          attrs,
+          %{
+            collection_name: "Source ##{:rand.uniform(1_000_000)}",
+            collection_id: Base.encode16(:crypto.hash(:md5, "#{:rand.uniform(1_000_000)}")),
+            collection_type: "channel",
+            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
+          }
+        ),
+        :pre_insert
       )
       |> Repo.insert()
 

test/support/testing_helper_methods.ex

@@ -11,6 +11,14 @@ defmodule Pinchflat.TestingHelperMethods do
     DateTime.add(now(), offset, :minute)
   end
 
+  def now_minus(offset, unit) when unit in [:minute, :minutes] do
+    DateTime.add(now(), -offset, :minute)
+  end
+
+  def now_minus(offset, unit) when unit in [:day, :days] do
+    DateTime.add(now(), -offset, :day)
+  end
+
   def assert_changed(checker_fun, action_fn) do
     before_res = checker_fun.()
     action_fn.()