Check if a refresh is permitted by comparing timestamps before attempting to insert an Oban job

It's better to avoid inserting an Oban job that will just be rejected if it's not expensive to check.

3 files changed, 16 insertions, 9 deletions

diff --git a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex
index 495f89278..4b347a6a7 100644
--- a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex
+++ b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex
@@ -32,12 +32,10 @@ defmodule Pleroma.Web.MastodonAPI.PollController do
   @doc "GET /api/v1/polls/:id"
   def show(%{assigns: %{user: user}, private: %{open_api_spex: %{params: %{id: id}}}} = conn, _) do
     with %Object{} = object <- Object.get_by_id(id),
-         %Activity{} = activity <- Activity.get_create_by_object_ap_id(object.data["id"]),
+         %Activity{} = activity <-
+           Activity.get_create_by_object_ap_id_with_object(object.data["id"]),
          true <- Visibility.visible_for_user?(activity, user) do
-      unless activity.local do
-        PollWorker.new(%{"op" => "refresh", "activity_id" => activity.id})
-        |> Oban.insert(unique: [period: 60])
-      end
+      maybe_refresh_poll(activity)
 
       try_render(conn, "show.json", %{object: object, for: user})
     else
@@ -76,4 +74,13 @@ defmodule Pleroma.Web.MastodonAPI.PollController do
       end
     end)
   end
+
+  defp maybe_refresh_poll(%Activity{object: %Object{} = object} = activity) do
+    with false <- activity.local,
+         {:ok, end_time} <- NaiveDateTime.from_iso8601(object.data["closed"]),
+         {_, :lt} <- {:closed_compare, NaiveDateTime.compare(object.updated_at, end_time)} do
+      PollWorker.new(%{"op" => "refresh", "activity_id" => activity.id})
+      |> Oban.insert(unique: [period: 60])
+    end
+  end
 end
diff --git a/lib/pleroma/workers/poll_worker.ex b/lib/pleroma/workers/poll_worker.ex
index f70ab48a4..bb92634c9 100644
--- a/lib/pleroma/workers/poll_worker.ex
+++ b/lib/pleroma/workers/poll_worker.ex
@@ -36,8 +36,6 @@ defmodule Pleroma.Workers.PollWorker do
   def perform(%Job{args: %{"op" => "refresh", "activity_id" => activity_id}}) do
     with {_, %Activity{object: object}} <-
            {:activity, Activity.get_by_id_with_object(activity_id)},
-         {:ok, end_time} <- NaiveDateTime.from_iso8601(object.data["closed"]),
-         {_, :lt} <- {:closed_compare, NaiveDateTime.compare(object.updated_at, end_time)},
          {_, {:ok, _object}} <- {:refetch, Fetcher.refetch_object(object)} do
       stream_update(activity_id)
 
@@ -45,7 +43,6 @@ defmodule Pleroma.Workers.PollWorker do
     else
       {:activity, nil} -> {:cancel, :poll_activity_not_found}
       {:refetch, _} = e -> {:cancel, e}
-      {:closed_compare, _} -> {:cancel, :poll_finalized}
       e -> {:error, e}
     end
   end
diff --git a/test/pleroma/web/mastodon_api/controllers/poll_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/poll_controller_test.exs
index 4b236678c..51af87742 100644
--- a/test/pleroma/web/mastodon_api/controllers/poll_controller_test.exs
+++ b/test/pleroma/web/mastodon_api/controllers/poll_controller_test.exs
@@ -29,13 +29,16 @@ defmodule Pleroma.Web.MastodonAPI.PollControllerTest do
       id = to_string(object.id)
       assert %{"id" => ^id, "expired" => false, "multiple" => false} = response
 
+      # Local activities should not generate an Oban job to refresh
+      assert activity.local
+
       refute_enqueued(
         worker: Pleroma.Workers.PollWorker,
         args: %{"op" => "refresh", "activity_id" => activity.id}
       )
     end
 
-    test "does not create oban job to refresh poll if activity is local", %{conn: conn} do
+    test "creates an oban job to refresh poll if activity is remote", %{conn: conn} do
       user = insert(:user, local: false)
       question = insert(:question, user: user)
       activity = insert(:question_activity, question: question, local: false)