StatusController: Correctly paginate favorites.

Favorites were paginating wrongly, because the pagination headers
where using the id of the id of the `Create` activity, while the
ordering was by the id of the `Like` activity. This isn't easy to
notice in most cases, as they usually have a similar order because
people tend to favorite posts as they come in. This commit adds a
way to give different pagination ids to the pagination helper, so
we can paginate correctly in cases like this.

4 files changed, 42 insertions, 28 deletions

diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex
index 6213d0eb7..f800447fd 100644
--- a/lib/pleroma/activity.ex
+++ b/lib/pleroma/activity.ex
@@ -41,6 +41,10 @@ defmodule Pleroma.Activity do
     field(:recipients, {:array, :string}, default: [])
     field(:thread_muted?, :boolean, virtual: true)
 
+    # A field that can be used if you need to join some kind of other
+    # id to order / paginate this field by
+    field(:pagination_id, :string, virtual: true)
+
     # This is a fake relation,
     # do not use outside of with_preloaded_user_actor/with_joined_user_actor
     has_one(:user_actor, User, on_delete: :nothing, foreign_key: :id)
diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex
index eb73c95fe..cc883ccce 100644
--- a/lib/pleroma/web/activity_pub/activity_pub.ex
+++ b/lib/pleroma/web/activity_pub/activity_pub.ex
@@ -1138,12 +1138,11 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
     |> Activity.Queries.by_type("Like")
     |> Activity.with_joined_object()
     |> Object.with_joined_activity()
-    |> select([_like, object, activity], %{activity | object: object})
+    |> select([like, object, activity], %{activity | object: object, pagination_id: like.id})
     |> order_by([like, _, _], desc_nulls_last: like.id)
     |> Pagination.fetch_paginated(
       Map.merge(params, %{skip_order: true}),
-      pagination,
-      :object_activity
+      pagination
     )
   end
 
diff --git a/lib/pleroma/web/api_spec/operations/status_operation.ex b/lib/pleroma/web/api_spec/operations/status_operation.ex
index ca9db01e5..0b7fad793 100644
--- a/lib/pleroma/web/api_spec/operations/status_operation.ex
+++ b/lib/pleroma/web/api_spec/operations/status_operation.ex
@@ -333,7 +333,8 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do
     %Operation{
       tags: ["Statuses"],
       summary: "Favourited statuses",
-      description: "Statuses the user has favourited",
+      description:
+        "Statuses the user has favourited. Please note that you have to use the link headers to paginate this. You can not build the query parameters yourself.",
       operationId: "StatusController.favourites",
       parameters: pagination_params(),
       security: [%{"oAuth" => ["read:favourites"]}],
diff --git a/lib/pleroma/web/controller_helper.ex b/lib/pleroma/web/controller_helper.ex
index 5d67d75b5..5e33e0810 100644
--- a/lib/pleroma/web/controller_helper.ex
+++ b/lib/pleroma/web/controller_helper.ex
@@ -57,35 +57,45 @@ defmodule Pleroma.Web.ControllerHelper do
     end
   end
 
+  defp build_pagination_fields(conn, min_id, max_id, extra_params) do
+    params =
+      conn.params
+      |> Map.drop(Map.keys(conn.path_params))
+      |> Map.merge(extra_params)
+      |> Map.drop(Pagination.page_keys() -- ["limit", "order"])
+
+    fields = %{
+      "next" => current_url(conn, Map.put(params, :max_id, max_id)),
+      "prev" => current_url(conn, Map.put(params, :min_id, min_id))
+    }
+
+    #  Generating an `id` without already present pagination keys would
+    # need a query-restriction with an `q.id >= ^id` or `q.id <= ^id`
+    # instead of the `q.id > ^min_id` and `q.id < ^max_id`.
+    #  This is because we only have ids present inside of the page, while
+    # `min_id`, `since_id` and `max_id` requires to know one outside of it.
+    if Map.take(conn.params, Pagination.page_keys() -- ["limit", "order"]) != [] do
+      Map.put(fields, "id", current_url(conn, conn.params))
+    else
+      fields
+    end
+  end
+
   def get_pagination_fields(conn, activities, extra_params \\ %{}) do
     case List.last(activities) do
-      %{id: max_id} ->
-        params =
-          conn.params
-          |> Map.drop(Map.keys(conn.path_params))
-          |> Map.merge(extra_params)
-          |> Map.drop(Pagination.page_keys() -- ["limit", "order"])
+      %{pagination_id: max_id} when not is_nil(max_id) ->
+        %{pagination_id: min_id} =
+          activities
+          |> List.first()
+
+        build_pagination_fields(conn, min_id, max_id, extra_params)
 
-        min_id =
+      %{id: max_id} ->
+        %{id: min_id} =
           activities
           |> List.first()
-          |> Map.get(:id)
-
-        fields = %{
-          "next" => current_url(conn, Map.put(params, :max_id, max_id)),
-          "prev" => current_url(conn, Map.put(params, :min_id, min_id))
-        }
-
-        #  Generating an `id` without already present pagination keys would
-        # need a query-restriction with an `q.id >= ^id` or `q.id <= ^id`
-        # instead of the `q.id > ^min_id` and `q.id < ^max_id`.
-        #  This is because we only have ids present inside of the page, while
-        # `min_id`, `since_id` and `max_id` requires to know one outside of it.
-        if Map.take(conn.params, Pagination.page_keys() -- ["limit", "order"]) != [] do
-          Map.put(fields, "id", current_url(conn, conn.params))
-        else
-          fields
-        end
+
+        build_pagination_fields(conn, min_id, max_id, extra_params)
 
       _ ->
         %{}