From 381eaed40ade8b3e562deb5bf2446c5373278895 Mon Sep 17 00:00:00 2001
From: Kieran Eglin <kieran.eglin@gmail.com>
Date: Sat, 2 Mar 2024 11:40:18 -0800
Subject: [PATCH] Fixed bug where indexing job wouldn't run for the first time

---
 .../workers/media_indexing_worker.ex          | 28 +++++++++----
 .../workers/media_indexing_worker_test.exs    | 42 +++++++++++--------
 2 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/lib/pinchflat/workers/media_indexing_worker.ex b/lib/pinchflat/workers/media_indexing_worker.ex
index 6dc0c32..ae54ac1 100644
--- a/lib/pinchflat/workers/media_indexing_worker.ex
+++ b/lib/pinchflat/workers/media_indexing_worker.ex
@@ -15,8 +15,9 @@ defmodule Pinchflat.Workers.MediaIndexingWorker do
   @doc """
   The ID is that of a source _record_, not a YouTube channel/playlist ID. Indexes
   the provided source, kicks off downloads for each new MediaItem, and
-  reschedules the job to run again in the future (as determined by the
-  souce's `index_frequency_minutes` field).
+  reschedules the job to run again in the future. It will ALWAYS index a source
+  if it's never been indexed before, but rescheduling is determined by the
+  `index_frequency_minutes` field.
 
   README: Re-scheduling here works a little different than you may expect.
   The reschedule time is relative to the time the job has actually _completed_.
@@ -39,18 +40,31 @@ defmodule Pinchflat.Workers.MediaIndexingWorker do
   def perform(%Oban.Job{args: %{"id" => source_id}}) do
     source = Sources.get_source!(source_id)
 
-    if source.index_frequency_minutes <= 0 do
-      :ok
-    else
-      index_media_and_reschedule(source)
+    case {source.index_frequency_minutes, source.last_indexed_at} do
+      {index_freq, _} when index_freq > 0 ->
+        # If the indexing is on a schedule simply run indexing and reschedule
+        index_media(source)
+        reschedule_indexing(source)
+
+      {_, nil} ->
+        # If the source has never been indexed, index it once
+        # even if it's not meant to reschedule
+        index_media(source)
+
+      _ ->
+        # If the source HAS been indexed and is not meant to reschedule,
+        # perform a no-op
+        :ok
     end
   end
 
-  defp index_media_and_reschedule(source) do
+  defp index_media(source) do
     SourceTasks.index_media_items(source)
     # This method handles the case where a source is set to not download media
     SourceTasks.enqueue_pending_media_tasks(source)
+  end
 
+  defp reschedule_indexing(source) do
     source
     |> Map.take([:id])
     |> MediaIndexingWorker.new(schedule_in: source.index_frequency_minutes * 60)
diff --git a/test/pinchflat/workers/media_indexing_worker_test.exs b/test/pinchflat/workers/media_indexing_worker_test.exs
index 176fa56..9fdc00a 100644
--- a/test/pinchflat/workers/media_indexing_worker_test.exs
+++ b/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)