Merge branch 'issue/2115' into 'develop'

[#2115] added paginate links to headers for  /chats/:id/messages

See merge request pleroma/pleroma!2981

CHANGELOG.md

@@ -28,6 +28,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 switched to a new configuration mechanism, however it was not officially removed until now.
 
 ### Fixed
+
+- Add documented-but-missing chat pagination.
 - Allow sending out emails again.
 
 ## [2.1.2] - 2020-09-17

lib/pleroma/chat.ex

@@ -18,6 +18,7 @@ defmodule Pleroma.Chat do
   It is a helper only, to make it easy to display a list of chats with other people, ordered by last bump. The actual messages are retrieved by querying the recipients of the ChatMessages.
   """
 
+  @type t :: %__MODULE__{}
   @primary_key {:id, FlakeId.Ecto.CompatType, autogenerate: true}
 
   schema "chats" do
@@ -41,16 +42,28 @@ def changeset(struct, params) do
     |> unique_constraint(:user_id, name: :chats_user_id_recipient_index)
   end
 
+  @spec get_by_user_and_id(User.t(), FlakeId.Ecto.CompatType.t()) ::
+          {:ok, t()} | {:error, :not_found}
+  def get_by_user_and_id(%User{id: user_id}, id) do
+    from(c in __MODULE__,
+      where: c.id == ^id,
+      where: c.user_id == ^user_id
+    )
+    |> Repo.find_resource()
+  end
+
+  @spec get_by_id(FlakeId.Ecto.CompatType.t()) :: t() | nil
   def get_by_id(id) do
-    __MODULE__
-    |> Repo.get(id)
+    Repo.get(__MODULE__, id)
   end
 
+  @spec get(FlakeId.Ecto.CompatType.t(), String.t()) :: t() | nil
   def get(user_id, recipient) do
-    __MODULE__
-    |> Repo.get_by(user_id: user_id, recipient: recipient)
+    Repo.get_by(__MODULE__, user_id: user_id, recipient: recipient)
   end
 
+  @spec get_or_create(FlakeId.Ecto.CompatType.t(), String.t()) ::
+          {:ok, t()} | {:error, Ecto.Changeset.t()}
   def get_or_create(user_id, recipient) do
     %__MODULE__{}
     |> changeset(%{user_id: user_id, recipient: recipient})
@@ -62,6 +75,8 @@ def get_or_create(user_id, recipient) do
     )
   end
 
+  @spec bump_or_create(FlakeId.Ecto.CompatType.t(), String.t()) ::
+          {:ok, t()} | {:error, Ecto.Changeset.t()}
   def bump_or_create(user_id, recipient) do
     %__MODULE__{}
     |> changeset(%{user_id: user_id, recipient: recipient})

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

@@ -158,7 +158,8 @@ def messages_operation do
             "The messages in the chat",
             "application/json",
             chat_messages_response()
-          )
+          ),
+        404 => Operation.response("Not Found", "application/json", ApiError)
       },
       security: [
         %{

lib/pleroma/web/controller_helper.ex

@@ -48,13 +48,13 @@ defp param_to_integer(val, default) when is_binary(val) do
 
   defp param_to_integer(_, default), do: default
 
-  def add_link_headers(conn, activities, extra_params \\ %{})
+  def add_link_headers(conn, entries, extra_params \\ %{})
 
-  def add_link_headers(%{assigns: %{skip_link_headers: true}} = conn, _activities, _extra_params),
+  def add_link_headers(%{assigns: %{skip_link_headers: true}} = conn, _entries, _extra_params),
     do: conn
 
-  def add_link_headers(conn, activities, extra_params) do
-    case get_pagination_fields(conn, activities, extra_params) do
+  def add_link_headers(conn, entries, extra_params) do
+    case get_pagination_fields(conn, entries, extra_params) do
       %{"next" => next_url, "prev" => prev_url} ->
         put_resp_header(conn, "link", "<#{next_url}>; rel=\"next\", <#{prev_url}>; rel=\"prev\"")
 
@@ -78,19 +78,15 @@ defp build_pagination_fields(conn, min_id, max_id, extra_params) do
     }
   end
 
-  def get_pagination_fields(conn, activities, extra_params \\ %{}) do
-    case List.last(activities) do
+  def get_pagination_fields(conn, entries, extra_params \\ %{}) do
+    case List.last(entries) do
       %{pagination_id: max_id} when not is_nil(max_id) ->
-        %{pagination_id: min_id} =
-          activities
-          |> List.first()
+        %{pagination_id: min_id} = List.first(entries)
 
         build_pagination_fields(conn, min_id, max_id, extra_params)
 
       %{id: max_id} ->
-        %{id: min_id} =
-          activities
-          |> List.first()
+        %{id: min_id} = List.first(entries)
 
         build_pagination_fields(conn, min_id, max_id, extra_params)
 

lib/pleroma/web/pleroma_api/controllers/chat_controller.ex

@@ -4,6 +4,8 @@
 defmodule Pleroma.Web.PleromaAPI.ChatController do
   use Pleroma.Web, :controller
 
+  import Pleroma.Web.ControllerHelper, only: [add_link_headers: 2]
+
   alias Pleroma.Activity
   alias Pleroma.Chat
   alias Pleroma.Chat.MessageReference
@@ -47,7 +49,7 @@ def delete_message(%{assigns: %{user: %{id: user_id} = user}} = conn, %{
       }) do
     with %MessageReference{} = cm_ref <-
            MessageReference.get_by_id(message_id),
-         ^chat_id <- cm_ref.chat_id |> to_string(),
+         ^chat_id <- to_string(cm_ref.chat_id),
          %Chat{user_id: ^user_id} <- Chat.get_by_id(chat_id),
          {:ok, _} <- remove_or_delete(cm_ref, user) do
       conn
@@ -68,18 +70,13 @@ defp remove_or_delete(
     end
   end
 
-  defp remove_or_delete(cm_ref, _) do
-    cm_ref
-    |> MessageReference.delete()
-  end
+  defp remove_or_delete(cm_ref, _), do: MessageReference.delete(cm_ref)
 
   def post_chat_message(
-        %{body_params: params, assigns: %{user: %{id: user_id} = user}} = conn,
-        %{
-          id: id
-        }
+        %{body_params: params, assigns: %{user: user}} = conn,
+        %{id: id}
       ) do
-    with %Chat{} = chat <- Repo.get_by(Chat, id: id, user_id: user_id),
+    with {:ok, chat} <- Chat.get_by_user_and_id(user, id),
          %User{} = recipient <- User.get_cached_by_ap_id(chat.recipient),
          {:ok, activity} <-
            CommonAPI.post_chat_message(user, recipient, params[:content],
@@ -103,13 +100,12 @@ def post_chat_message(
     end
   end
 
-  def mark_message_as_read(%{assigns: %{user: %{id: user_id}}} = conn, %{
-        id: chat_id,
-        message_id: message_id
-      }) do
-    with %MessageReference{} = cm_ref <-
-           MessageReference.get_by_id(message_id),
-         ^chat_id <- cm_ref.chat_id |> to_string(),
+  def mark_message_as_read(
+        %{assigns: %{user: %{id: user_id}}} = conn,
+        %{id: chat_id, message_id: message_id}
+      ) do
+    with %MessageReference{} = cm_ref <- MessageReference.get_by_id(message_id),
+         ^chat_id <- to_string(cm_ref.chat_id),
          %Chat{user_id: ^user_id} <- Chat.get_by_id(chat_id),
          {:ok, cm_ref} <- MessageReference.mark_as_read(cm_ref) do
       conn
@@ -119,36 +115,28 @@ def mark_message_as_read(%{assigns: %{user: %{id: user_id}}} = conn, %{
   end
 
   def mark_as_read(
-        %{
-          body_params: %{last_read_id: last_read_id},
-          assigns: %{user: %{id: user_id}}
-        } = conn,
+        %{body_params: %{last_read_id: last_read_id}, assigns: %{user: user}} = conn,
         %{id: id}
       ) do
-    with %Chat{} = chat <- Repo.get_by(Chat, id: id, user_id: user_id),
-         {_n, _} <-
-           MessageReference.set_all_seen_for_chat(chat, last_read_id) do
+    with {:ok, chat} <- Chat.get_by_user_and_id(user, id),
+         {_n, _} <- MessageReference.set_all_seen_for_chat(chat, last_read_id) do
       conn
       |> put_view(ChatView)
       |> render("show.json", chat: chat)
     end
   end
 
-  def messages(%{assigns: %{user: %{id: user_id}}} = conn, %{id: id} = params) do
-    with %Chat{} = chat <- Repo.get_by(Chat, id: id, user_id: user_id) do
-      cm_refs =
+  def messages(%{assigns: %{user: user}} = conn, %{id: id} = params) do
+    with {:ok, chat} <- Chat.get_by_user_and_id(user, id) do
+      chat_message_refs =
         chat
         |> MessageReference.for_chat_query()
         |> Pagination.fetch_paginated(params)
 
       conn
+      |> add_link_headers(chat_message_refs)
       |> put_view(MessageReferenceView)
-      |> render("index.json", chat_message_references: cm_refs)
-    else
-      _ ->
-        conn
-        |> put_status(:not_found)
-        |> json(%{error: "not found"})
+      |> render("index.json", chat_message_references: chat_message_refs)
     end
   end
 
@@ -165,8 +153,8 @@ def index(%{assigns: %{user: %{id: user_id} = user}} = conn, _params) do
     |> render("index.json", chats: chats)
   end
 
-  def create(%{assigns: %{user: user}} = conn, params) do
-    with %User{ap_id: recipient} <- User.get_by_id(params[:id]),
+  def create(%{assigns: %{user: user}} = conn, %{id: id}) do
+    with %User{ap_id: recipient} <- User.get_cached_by_id(id),
          {:ok, %Chat{} = chat} <- Chat.get_or_create(user.id, recipient) do
       conn
       |> put_view(ChatView)
@@ -174,8 +162,8 @@ def create(%{assigns: %{user: user}} = conn, params) do
     end
   end
 
-  def show(%{assigns: %{user: user}} = conn, params) do
-    with %Chat{} = chat <- Repo.get_by(Chat, user_id: user.id, id: params[:id]) do
+  def show(%{assigns: %{user: user}} = conn, %{id: id}) do
+    with {:ok, chat} <- Chat.get_by_user_and_id(user, id) do
       conn
       |> put_view(ChatView)
       |> render("show.json", chat: chat)

test/web/pleroma_api/controllers/chat_controller_test.exs

@@ -2,7 +2,7 @@
 # Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/>
 # SPDX-License-Identifier: AGPL-3.0-only
 defmodule Pleroma.Web.PleromaAPI.ChatControllerTest do
-  use Pleroma.Web.ConnCase, async: true
+  use Pleroma.Web.ConnCase
 
   alias Pleroma.Chat
   alias Pleroma.Chat.MessageReference
@@ -201,17 +201,39 @@ test "it paginates", %{conn: conn, user: user} do
 
       chat = Chat.get(user.id, recipient.ap_id)
 
-      result =
-        conn
-        |> get("/api/v1/pleroma/chats/#{chat.id}/messages")
-        |> json_response_and_validate_schema(200)
+      response = get(conn, "/api/v1/pleroma/chats/#{chat.id}/messages")
+      result = json_response_and_validate_schema(response, 200)
+
+      [next, prev] = get_resp_header(response, "link") |> hd() |> String.split(", ")
+      api_endpoint = "/api/v1/pleroma/chats/"
+
+      assert String.match?(
+               next,
+               ~r(#{api_endpoint}.*/messages\?id=.*&limit=\d+&max_id=.*; rel=\"next\"$)
+             )
+
+      assert String.match?(
+               prev,
+               ~r(#{api_endpoint}.*/messages\?id=.*&limit=\d+&min_id=.*; rel=\"prev\"$)
+             )
 
       assert length(result) == 20
 
-      result =
-        conn
-        |> get("/api/v1/pleroma/chats/#{chat.id}/messages?max_id=#{List.last(result)["id"]}")
-        |> json_response_and_validate_schema(200)
+      response =
+        get(conn, "/api/v1/pleroma/chats/#{chat.id}/messages?max_id=#{List.last(result)["id"]}")
+
+      result = json_response_and_validate_schema(response, 200)
+      [next, prev] = get_resp_header(response, "link") |> hd() |> String.split(", ")
+
+      assert String.match?(
+               next,
+               ~r(#{api_endpoint}.*/messages\?id=.*&limit=\d+&max_id=.*; rel=\"next\"$)
+             )
+
+      assert String.match?(
+               prev,
+               ~r(#{api_endpoint}.*/messages\?id=.*&limit=\d+&max_id=.*&min_id=.*; rel=\"prev\"$)
+             )
 
       assert length(result) == 10
     end
@@ -240,12 +262,10 @@ test "it returns the messages for a given chat", %{conn: conn, user: user} do
       assert length(result) == 3
 
       # Trying to get the chat of a different user
-      result =
       conn
       |> assign(:user, other_user)
       |> get("/api/v1/pleroma/chats/#{chat.id}/messages")
-
-      assert result |> json_response(404)
+      |> json_response_and_validate_schema(404)
     end
   end