Changed the purpose of the 'culled_at' flag. renamed some methods

lib/pinchflat/downloading/downloading_helpers.ex

@@ -91,8 +91,7 @@ defmodule Pinchflat.Downloading.DownloadingHelpers do
         [m, s, mp],
         ^MediaQuery.for_source(source) and
           ^MediaQuery.downloaded() and
-          not (^MediaQuery.download_prevented()) and
-          not (^MediaQuery.culled())
+          not (^MediaQuery.download_prevented())
       )
     )
     |> Repo.all()

lib/pinchflat/downloading/media_quality_upgrade_worker.ex

@@ -23,10 +23,10 @@ defmodule Pinchflat.Downloading.MediaQualityUpgradeWorker do
   """
   @impl Oban.Worker
   def perform(%Oban.Job{}) do
-    redownloadable_media = Media.list_redownloadable_media_items()
-    Logger.info("Redownloading #{length(redownloadable_media)} media items")
+    upgradable_media = Media.list_upgradeable_media_items()
+    Logger.info("Redownloading #{length(upgradable_media)} media items")
 
-    Enum.each(redownloadable_media, fn media_item ->
+    Enum.each(upgradable_media, fn media_item ->
       MediaDownloadWorker.kickoff_with_task(media_item, %{quality_upgrade?: true})
     end)
   end

lib/pinchflat/downloading/media_retention_worker.ex

@@ -6,8 +6,11 @@ defmodule Pinchflat.Downloading.MediaRetentionWorker do
     unique: [period: :infinity, states: [:available, :scheduled, :retryable, :executing]],
     tags: ["media_item", "local_metadata"]
 
+  use Pinchflat.Media.MediaQuery
+
   require Logger
 
+  alias Pinchflat.Repo
   alias Pinchflat.Media
 
   @doc """
@@ -20,14 +23,52 @@ defmodule Pinchflat.Downloading.MediaRetentionWorker do
   """
   @impl Oban.Worker
   def perform(%Oban.Job{}) do
+    cull_cullable_media_items()
+    # delete_media_items_from_before_cutoff()
+
+    :ok
+  end
+
+  defp cull_cullable_media_items do
+    # TODO: consider bringing `list_cullable_media_items` into this module since it's only used here
     cullable_media = Media.list_cullable_media_items()
     Logger.info("Culling #{length(cullable_media)} media items past their retention date")
 
     Enum.each(cullable_media, fn media_item ->
+      # Setting `prevent_download` does what it says on the tin,
+      # TODO: finish these docs
       Media.delete_media_files(media_item, %{
         prevent_download: true,
         culled_at: DateTime.utc_now()
       })
     end)
   end
+
+  # TODO: test
+  defp delete_media_items_from_before_cutoff do
+    deletable_media =
+      MediaQuery.new()
+      |> MediaQuery.require_assoc(:source)
+      |> where(^MediaQuery.deletable_from_source_cutoff())
+      |> Repo.all()
+
+    Logger.info("Deleting #{length(deletable_media)} media items that are from before the source cutoff")
+    # TODO: Maybe I should be setting `culled_at` here since it does capture what's actually happening.
+    # There are also a few spots in code that check for `culled_at` to determine if a media item is
+    # eligible for redownload, for instance. I should re-evalute what `culled_at` actually "means" and,
+    # since culled_at really only gets set for items that we prevent download on OR items that shouldn't
+    # be redownloaded anyway, maybe it should become purely informational rather than functional.
+    #
+    # TODO: depending on the above, maybe I should ensure that `culled_at` is set to nil if the media item
+    # gets re-downloaded. Because in this case the user could just change the cutoff date and re-download
+    # and I don't think it makes sense to still indicate that the media item was culled.
+    Enum.each(deletable_media, fn media_item ->
+      # Note that I'm not setting any attributes like `prevent_download` on the media_item here.
+      # That's because cutoff_date can easily change and it's a valid behavior to re-download older
+      # media items if the cutoff_date changes.
+      # Download is ultimately prevented because `MediaQuery.pending()` only returns media items
+      # from after the cutoff date (among other things).
+      Media.delete_media_files(media_item)
+    end)
+  end
 end

lib/pinchflat/media/media.ex

@@ -38,8 +38,10 @@ defmodule Pinchflat.Media do
   end
 
   @doc """
-  Returns a list of media_items that are redownloadable based on the redownload delay
-  of the media_profile their source belongs to.
+  Returns a list of media_items that are upgradeable based on the redownload delay
+  of the media_profile their source belongs to. In this context, upgradeable means
+  that it's been long enough since upload that the video may be in a higher quality
+  or have better sponsorblock segments (or similar).
 
   The logic is that a media_item is past_redownload_delay if the media_item's uploaded_at is
   at least redownload_delay_days ago AND `media_downloaded_at` - `redownload_delay_days`
@@ -52,10 +54,10 @@ defmodule Pinchflat.Media do
 
   Returns [%MediaItem{}, ...]
   """
-  def list_redownloadable_media_items do
+  def list_upgradeable_media_items do
     MediaQuery.new()
     |> MediaQuery.require_assoc(:media_profile)
-    |> where(^MediaQuery.redownloadable())
+    |> where(^MediaQuery.upgradeable())
     |> Repo.all()
   end
 

lib/pinchflat/media/media_query.ex

@@ -45,6 +45,14 @@ defmodule Pinchflat.Media.MediaQuery do
     )
   end
 
+  # def upload_date_before_source_cutoff do
+  #   dynamic(
+  #     [mi, source],
+  #     not is_nil(source.download_cutoff_date) and
+  #       fragment("date(?) < ?", mi.uploaded_at, source.download_cutoff_date)
+  #   )
+  # end
+
   def format_matching_profile_preference do
     dynamic(
       [mi, source, media_profile],
@@ -108,6 +116,15 @@ defmodule Pinchflat.Media.MediaQuery do
     )
   end
 
+  def deletable_from_source_cutoff do
+    dynamic(
+      [mi, source],
+      ^downloaded() and
+        not (^upload_date_after_source_cutoff()) and
+        not (^culling_prevented())
+    )
+  end
+
   def pending do
     dynamic(
       [mi],
@@ -119,12 +136,11 @@ defmodule Pinchflat.Media.MediaQuery do
     )
   end
 
-  def redownloadable do
+  def upgradeable do
     dynamic(
       [mi, source],
       ^downloaded() and
         not (^download_prevented()) and
-        not (^culled()) and
         not (^redownloaded()) and
         ^past_redownload_delay()
     )

test/pinchflat/downloading/downloading_helpers_test.exs

@@ -130,9 +130,6 @@ defmodule Pinchflat.Downloading.DownloadingHelpersTest do
       _download_prevented =
         media_item_fixture(source_id: source.id, media_filepath: "some/filepath.mp4", prevent_download: true)
 
-      _culled =
-        media_item_fixture(source_id: source.id, media_filepath: "some/filepath.mp4", culled_at: now())
-
       assert [] = DownloadingHelpers.kickoff_redownload_for_existing_media(source)
 
       refute_enqueued(worker: MediaDownloadWorker)

test/pinchflat/media_test.exs

@@ -133,7 +133,7 @@ defmodule Pinchflat.MediaTest do
     end
   end
 
-  describe "list_redownloadable_media_items/0" do
+  describe "list_upgradeable_media_items/0" do
     setup do
       media_profile = media_profile_fixture(%{redownload_delay_days: 4})
       source = source_fixture(%{media_profile_id: media_profile.id, inserted_at: now_minus(10, :days)})
@@ -149,7 +149,7 @@ defmodule Pinchflat.MediaTest do
           media_downloaded_at: now_minus(5, :days)
         })
 
-      assert Media.list_redownloadable_media_items() == [media_item]
+      assert Media.list_upgradeable_media_items() == [media_item]
     end
 
     test "returns media items that were downloaded in past but still meet redownload delay", %{source: source} do
@@ -160,7 +160,7 @@ defmodule Pinchflat.MediaTest do
           media_downloaded_at: now_minus(19, :days)
         })
 
-      assert Media.list_redownloadable_media_items() == [media_item]
+      assert Media.list_upgradeable_media_items() == [media_item]
     end
 
     test "does not return media items without a media_downloaded_at", %{source: source} do
@@ -171,7 +171,7 @@ defmodule Pinchflat.MediaTest do
           media_downloaded_at: nil
         })
 
-      assert Media.list_redownloadable_media_items() == []
+      assert Media.list_upgradeable_media_items() == []
     end
 
     test "does not return media items that are set to prevent download", %{source: source} do
@@ -183,7 +183,7 @@ defmodule Pinchflat.MediaTest do
           prevent_download: true
         })
 
-      assert Media.list_redownloadable_media_items() == []
+      assert Media.list_upgradeable_media_items() == []
     end
 
     test "does not return media items that have been culled", %{source: source} do
@@ -195,7 +195,7 @@ defmodule Pinchflat.MediaTest do
           culled_at: now()
         })
 
-      assert Media.list_redownloadable_media_items() == []
+      assert Media.list_upgradeable_media_items() == []
     end
 
     test "does not return media items before the download delay", %{source: source} do
@@ -206,7 +206,7 @@ defmodule Pinchflat.MediaTest do
           media_downloaded_at: now_minus(3, :days)
         })
 
-      assert Media.list_redownloadable_media_items() == []
+      assert Media.list_upgradeable_media_items() == []
     end
 
     test "does not return media items that have already been redownloaded", %{source: source} do
@@ -218,7 +218,7 @@ defmodule Pinchflat.MediaTest do
           media_redownloaded_at: now()
         })
 
-      assert Media.list_redownloadable_media_items() == []
+      assert Media.list_upgradeable_media_items() == []
     end
 
     test "does not return media items that were first downloaded well after the uploaded_at", %{source: source} do
@@ -229,7 +229,7 @@ defmodule Pinchflat.MediaTest do
           uploaded_at: now_minus(20, :days)
         })
 
-      assert Media.list_redownloadable_media_items() == []
+      assert Media.list_upgradeable_media_items() == []
     end
 
     test "does not return media items that were recently uploaded", %{source: source} do
@@ -240,7 +240,7 @@ defmodule Pinchflat.MediaTest do
           uploaded_at: now_minus(2, :days)
         })
 
-      assert Media.list_redownloadable_media_items() == []
+      assert Media.list_upgradeable_media_items() == []
     end
 
     test "does not return media items without a redownload delay" do
@@ -254,7 +254,7 @@ defmodule Pinchflat.MediaTest do
           media_downloaded_at: now_minus(5, :days)
         })
 
-      assert Media.list_redownloadable_media_items() == []
+      assert Media.list_upgradeable_media_items() == []
     end
   end