Merge branch 'fix/2449-scheduled-poll-bug' into 'develop'

Fix for scheduled post with poll

Closes #2449

See merge request pleroma/pleroma!3294

CHANGELOG.md

@@ -80,6 +80,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   - Mastodon API: Fixed last_status.account being not filled with account data.
   - Mastodon API: Fix not being able to add or remove multiple users at once in lists.
   - Mastodon API: Fixed own_votes being not returned with poll data.
+  - Mastodon API: Fixed creation of scheduled posts with polls.
 </details>
 
 ## Unreleased (Patch)

lib/pleroma/web/api_spec/operations/status_operation.ex

@@ -413,34 +413,7 @@ defp create_request do
           items: %Schema{type: :string},
           description: "Array of Attachment ids to be attached as media."
         },
-        poll: %Schema{
-          nullable: true,
-          type: :object,
-          required: [:options],
-          properties: %{
-            options: %Schema{
-              type: :array,
-              items: %Schema{type: :string},
-              description: "Array of possible answers. Must be provided with `poll[expires_in]`."
-            },
-            expires_in: %Schema{
-              type: :integer,
-              nullable: true,
-              description:
-                "Duration the poll should be open, in seconds. Must be provided with `poll[options]`"
-            },
-            multiple: %Schema{
-              allOf: [BooleanLike],
-              nullable: true,
-              description: "Allow multiple choices?"
-            },
-            hide_totals: %Schema{
-              allOf: [BooleanLike],
-              nullable: true,
-              description: "Hide vote counts until the poll ends?"
-            }
-          }
-        },
+        poll: poll_params(),
         in_reply_to_id: %Schema{
           nullable: true,
           allOf: [FlakeID],
@@ -522,6 +495,37 @@ defp create_request do
     }
   end
 
+  def poll_params do
+    %Schema{
+      nullable: true,
+      type: :object,
+      required: [:options, :expires_in],
+      properties: %{
+        options: %Schema{
+          type: :array,
+          items: %Schema{type: :string},
+          description: "Array of possible answers. Must be provided with `poll[expires_in]`."
+        },
+        expires_in: %Schema{
+          type: :integer,
+          nullable: true,
+          description:
+            "Duration the poll should be open, in seconds. Must be provided with `poll[options]`"
+        },
+        multiple: %Schema{
+          allOf: [BooleanLike],
+          nullable: true,
+          description: "Allow multiple choices?"
+        },
+        hide_totals: %Schema{
+          allOf: [BooleanLike],
+          nullable: true,
+          description: "Hide vote counts until the poll ends?"
+        }
+      }
+    }
+  end
+
   def id_param do
     Operation.parameter(:id, :path, FlakeID, "Status ID",
       example: "9umDrYheeY451cQnEe",

lib/pleroma/web/api_spec/schemas/scheduled_status.ex

@@ -5,8 +5,8 @@
 defmodule Pleroma.Web.ApiSpec.Schemas.ScheduledStatus do
   alias OpenApiSpex.Schema
   alias Pleroma.Web.ApiSpec.Schemas.Attachment
-  alias Pleroma.Web.ApiSpec.Schemas.Poll
   alias Pleroma.Web.ApiSpec.Schemas.VisibilityScope
+  alias Pleroma.Web.ApiSpec.StatusOperation
 
   require OpenApiSpex
 
@@ -29,7 +29,7 @@ defmodule Pleroma.Web.ApiSpec.Schemas.ScheduledStatus do
           spoiler_text: %Schema{type: :string, nullable: true},
           visibility: %Schema{allOf: [VisibilityScope], nullable: true},
           scheduled_at: %Schema{type: :string, format: :"date-time", nullable: true},
-          poll: %Schema{allOf: [Poll], nullable: true},
+          poll: StatusOperation.poll_params(),
           in_reply_to_id: %Schema{type: :string, nullable: true}
         }
       }

lib/pleroma/workers/scheduled_activity_worker.ex

@@ -9,38 +9,50 @@ defmodule Pleroma.Workers.ScheduledActivityWorker do
 
   use Pleroma.Workers.WorkerHelper, queue: "scheduled_activities"
 
-  alias Pleroma.Config
+  alias Pleroma.Repo
   alias Pleroma.ScheduledActivity
   alias Pleroma.User
-  alias Pleroma.Web.CommonAPI
 
   require Logger
 
   @impl Oban.Worker
   def perform(%Job{args: %{"activity_id" => activity_id}}) do
-    if Config.get([ScheduledActivity, :enabled]) do
-      case Pleroma.Repo.get(ScheduledActivity, activity_id) do
-        %ScheduledActivity{} = scheduled_activity ->
-          post_activity(scheduled_activity)
+    with %ScheduledActivity{} = scheduled_activity <- find_scheduled_activity(activity_id),
+         %User{} = user <- find_user(scheduled_activity.user_id) do
+      params = atomize_keys(scheduled_activity.params)
 
-        _ ->
-          Logger.error("#{__MODULE__} Couldn't find scheduled activity: #{activity_id}")
-      end
-    end
-  end
-
-  defp post_activity(%ScheduledActivity{user_id: user_id, params: params} = scheduled_activity) do
-    params = Map.new(params, fn {key, value} -> {String.to_existing_atom(key), value} end)
-
-    with {:delete, {:ok, _}} <- {:delete, ScheduledActivity.delete(scheduled_activity)},
-         {:user, %User{} = user} <- {:user, User.get_cached_by_id(user_id)},
-         {:post, {:ok, _}} <- {:post, CommonAPI.post(user, params)} do
-      :ok
+      Repo.transaction(fn ->
+        {:ok, activity} = Pleroma.Web.CommonAPI.post(user, params)
+        {:ok, _} = ScheduledActivity.delete(scheduled_activity)
+        activity
+      end)
     else
-      error ->
-        Logger.error(
-          "#{__MODULE__} Couldn't create a status from the scheduled activity: #{inspect(error)}"
-        )
+      {:error, :scheduled_activity_not_found} = error ->
+        Logger.error("#{__MODULE__} Couldn't find scheduled activity: #{activity_id}")
+        error
+
+      {:error, :user_not_found} = error ->
+        Logger.error("#{__MODULE__} Couldn't find user for scheduled activity: #{activity_id}")
+        error
     end
   end
+
+  defp find_scheduled_activity(id) do
+    with nil <- Repo.get(ScheduledActivity, id) do
+      {:error, :scheduled_activity_not_found}
+    end
+  end
+
+  defp find_user(id) do
+    with nil <- User.get_cached_by_id(id) do
+      {:error, :user_not_found}
+    end
+  end
+
+  defp atomize_keys(map) do
+    Map.new(map, fn
+      {key, value} when is_map(value) -> {String.to_existing_atom(key), atomize_keys(value)}
+      {key, value} -> {String.to_existing_atom(key), value}
+    end)
+  end
 end

test/pleroma/config/deprecation_warnings_test.exs

@@ -87,7 +87,7 @@ test "check_hellthread_threshold/0" do
   end
 
   test "check_activity_expiration_config/0" do
-    clear_config(Pleroma.ActivityExpiration, enabled: true)
+    clear_config([Pleroma.ActivityExpiration], enabled: true)
 
     assert capture_log(fn ->
              DeprecationWarnings.check_activity_expiration_config()
@@ -95,7 +95,7 @@ test "check_activity_expiration_config/0" do
   end
 
   test "check_uploders_s3_public_endpoint/0" do
-    clear_config(Pleroma.Uploaders.S3, public_endpoint: "https://fake.amazonaws.com/bucket/")
+    clear_config([Pleroma.Uploaders.S3], public_endpoint: "https://fake.amazonaws.com/bucket/")
 
     assert capture_log(fn ->
              DeprecationWarnings.check_uploders_s3_public_endpoint()

test/pleroma/web/mastodon_api/controllers/status_controller_test.exs

@@ -516,7 +516,7 @@ test "posting a poll", %{conn: conn} do
              end)
 
       assert NaiveDateTime.diff(NaiveDateTime.from_iso8601!(response["poll"]["expires_at"]), time) in 420..430
-      refute response["poll"]["expred"]
+      assert response["poll"]["expired"] == false
 
       question = Object.get_by_id(response["poll"]["id"])
 
@@ -592,6 +592,44 @@ test "maximum date limit is enforced", %{conn: conn} do
       %{"error" => error} = json_response_and_validate_schema(conn, 422)
       assert error == "Expiration date is too far in the future"
     end
+
+    test "scheduled poll", %{conn: conn} do
+      clear_config([ScheduledActivity, :enabled], true)
+
+      scheduled_at =
+        NaiveDateTime.add(NaiveDateTime.utc_now(), :timer.minutes(6), :millisecond)
+        |> NaiveDateTime.to_iso8601()
+        |> Kernel.<>("Z")
+
+      %{"id" => scheduled_id} =
+        conn
+        |> put_req_header("content-type", "application/json")
+        |> post("/api/v1/statuses", %{
+          "status" => "very cool poll",
+          "poll" => %{
+            "options" => ~w(a b c),
+            "expires_in" => 420
+          },
+          "scheduled_at" => scheduled_at
+        })
+        |> json_response_and_validate_schema(200)
+
+      assert {:ok, %{id: activity_id}} =
+               perform_job(Pleroma.Workers.ScheduledActivityWorker, %{
+                 activity_id: scheduled_id
+               })
+
+      assert Repo.all(Oban.Job) == []
+
+      object =
+        Activity
+        |> Repo.get(activity_id)
+        |> Object.normalize()
+
+      assert object.data["content"] == "very cool poll"
+      assert object.data["type"] == "Question"
+      assert length(object.data["oneOf"]) == 3
+    end
   end
 
   test "get a status" do

test/pleroma/workers/scheduled_activity_worker_test.exs

@@ -11,10 +11,9 @@ defmodule Pleroma.Workers.ScheduledActivityWorkerTest do
   import Pleroma.Factory
   import ExUnit.CaptureLog
 
-  setup do: clear_config([ScheduledActivity, :enabled])
+  setup do: clear_config([ScheduledActivity, :enabled], true)
 
   test "creates a status from the scheduled activity" do
-    clear_config([ScheduledActivity, :enabled], true)
     user = insert(:user)
 
     naive_datetime =
@@ -32,18 +31,22 @@ test "creates a status from the scheduled activity" do
         params: %{status: "hi"}
       )
 
+    {:ok, %{id: activity_id}} =
       ScheduledActivityWorker.perform(%Oban.Job{args: %{"activity_id" => scheduled_activity.id}})
 
     refute Repo.get(ScheduledActivity, scheduled_activity.id)
-    activity = Repo.all(Pleroma.Activity) |> Enum.find(&(&1.actor == user.ap_id))
-    assert Pleroma.Object.normalize(activity, fetch: false).data["content"] == "hi"
+
+    object =
+      Pleroma.Activity
+      |> Repo.get(activity_id)
+      |> Pleroma.Object.normalize()
+
+    assert object.data["content"] == "hi"
   end
 
-  test "adds log message if ScheduledActivity isn't find" do
-    clear_config([ScheduledActivity, :enabled], true)
-
+  test "error message for non-existent scheduled activity" do
     assert capture_log([level: :error], fn ->
              ScheduledActivityWorker.perform(%Oban.Job{args: %{"activity_id" => 42}})
-           end) =~ "Couldn't find scheduled activity"
+           end) =~ "Couldn't find scheduled activity: 42"
   end
 end