From 23332bec28ce958dc80c9c4cabce2e5301b73a03 Mon Sep 17 00:00:00 2001 From: Liam Date: Sun, 14 Jul 2024 19:18:25 -0400 Subject: [PATCH] Move conversation controller functionality into context --- lib/philomena/conversations.ex | 183 ++++++++++++++---- lib/philomena/conversations/conversation.ex | 31 +-- lib/philomena/users.ex | 16 ++ .../conversation/message_controller.ex | 16 +- .../controllers/conversation_controller.ex | 77 ++------ .../templates/conversation/index.html.slime | 8 +- 6 files changed, 197 insertions(+), 134 deletions(-) diff --git a/lib/philomena/conversations.ex b/lib/philomena/conversations.ex index 1de8fe1c..aacf6b94 100644 --- a/lib/philomena/conversations.ex +++ b/lib/philomena/conversations.ex @@ -9,45 +9,7 @@ defmodule Philomena.Conversations do alias Philomena.Conversations.Conversation alias Philomena.Conversations.Message alias Philomena.Reports - - @doc """ - Creates a conversation. - - ## Examples - - iex> create_conversation(%{field: value}) - {:ok, %Conversation{}} - - iex> create_conversation(%{field: bad_value}) - {:error, %Ecto.Changeset{}} - - """ - def create_conversation(from, attrs \\ %{}) do - %Conversation{} - |> Conversation.creation_changeset(from, attrs) - |> Repo.insert() - |> case do - {:ok, conversation} -> - report_non_approved_message(hd(conversation.messages)) - {:ok, conversation} - - error -> - error - end - end - - @doc """ - Returns an `%Ecto.Changeset{}` for tracking conversation changes. - - ## Examples - - iex> change_conversation(conversation) - %Ecto.Changeset{source: %Conversation{}} - - """ - def change_conversation(%Conversation{} = conversation) do - Conversation.changeset(conversation, %{}) - end + alias Philomena.Users @doc """ Returns the number of unread conversations for the given user. @@ -75,6 +37,96 @@ defmodule Philomena.Conversations do |> Repo.aggregate(:count) end + @doc """ + Returns a `m:Scrivener.Page` of conversations between the partner and the user. + + ## Examples + + iex> list_conversations_with("123", %User{}, page_size: 10) + %Scrivener.Page{} + + """ + def list_conversations_with(partner_id, user, pagination) do + query = + from c in Conversation, + where: + (c.from_id == ^partner_id and c.to_id == ^user.id) or + (c.to_id == ^partner_id and c.from_id == ^user.id) + + list_conversations(query, user, pagination) + end + + @doc """ + Returns a `m:Scrivener.Page` of conversations sent by or received from the user. + + ## Examples + + iex> list_conversations_with("123", %User{}, page_size: 10) + %Scrivener.Page{} + + """ + def list_conversations(queryable \\ Conversation, user, pagination) do + query = + from c in queryable, + as: :conversations, + where: + (c.from_id == ^user.id and not c.from_hidden) or + (c.to_id == ^user.id and not c.to_hidden), + inner_lateral_join: + cnt in subquery( + from m in Message, + where: m.conversation_id == parent_as(:conversations).id, + select: %{count: count()} + ), + on: true, + order_by: [desc: :last_message_at], + preload: [:to, :from], + select: %{c | message_count: cnt.count} + + Repo.paginate(query, pagination) + end + + @doc """ + Creates a conversation. + + ## Examples + + iex> create_conversation(from, to, %{field: value}) + {:ok, %Conversation{}} + + iex> create_conversation(from, to, %{field: bad_value}) + {:error, %Ecto.Changeset{}} + + """ + def create_conversation(from, attrs \\ %{}) do + to = Users.get_user_by_name(attrs["recipient"]) + + %Conversation{} + |> Conversation.creation_changeset(from, to, attrs) + |> Repo.insert() + |> case do + {:ok, conversation} -> + report_non_approved_message(hd(conversation.messages)) + {:ok, conversation} + + error -> + error + end + end + + @doc """ + Returns an `%Ecto.Changeset{}` for tracking conversation changes. + + ## Examples + + iex> change_conversation(conversation) + %Ecto.Changeset{source: %Conversation{}} + + """ + def change_conversation(%Conversation{} = conversation) do + Conversation.changeset(conversation, %{}) + end + @doc """ Marks a conversation as read or unread from the perspective of the given user. @@ -138,6 +190,59 @@ defmodule Philomena.Conversations do end end + @doc """ + Returns the number of messages in the given conversation. + + ## Example + + iex> count_messages(%Conversation{}) + 3 + + """ + def count_messages(conversation) do + Message + |> where(conversation_id: ^conversation.id) + |> Repo.aggregate(:count) + end + + @doc """ + Returns a `m:Scrivener.Page` of 2-tuples of messages and rendered output + within a conversation. + + Messages are ordered by user message preference (`messages_newest_first`). + + When coerced to a list and rendered as Markdown, the result may look like: + + [ + {%Message{body: "hello *world*"}, "hello world"} + ] + + ## Example + + iex> list_messages(%Conversation{}, %User{}, & &1.body, page_size: 10) + %Scrivener.Page{} + + """ + def list_messages(conversation, user, collection_renderer, pagination) do + direction = + if user.messages_newest_first do + :desc + else + :asc + end + + query = + from m in Message, + where: m.conversation_id == ^conversation.id, + order_by: [{^direction, :created_at}], + preload: :from + + messages = Repo.paginate(query, pagination) + rendered = collection_renderer.(messages) + + put_in(messages.entries, Enum.zip(messages.entries, rendered)) + end + @doc """ Creates a message within a conversation. diff --git a/lib/philomena/conversations/conversation.ex b/lib/philomena/conversations/conversation.ex index d48aa0b9..b0122eb2 100644 --- a/lib/philomena/conversations/conversation.ex +++ b/lib/philomena/conversations/conversation.ex @@ -4,7 +4,6 @@ defmodule Philomena.Conversations.Conversation do alias Philomena.Users.User alias Philomena.Conversations.Message - alias Philomena.Repo @derive {Phoenix.Param, key: :slug} @@ -20,6 +19,8 @@ defmodule Philomena.Conversations.Conversation do field :from_hidden, :boolean, default: false field :slug, :string field :last_message_at, :utc_datetime + + field :message_count, :integer, virtual: true field :recipient, :string, virtual: true timestamps(inserted_at: :created_at, type: :utc_datetime) @@ -43,17 +44,17 @@ defmodule Philomena.Conversations.Conversation do end @doc false - def creation_changeset(conversation, from, attrs) do + def creation_changeset(conversation, from, to, attrs) do conversation - |> cast(attrs, [:title, :recipient]) - |> validate_required([:title, :recipient]) - |> validate_length(:title, max: 300, count: :bytes) + |> cast(attrs, [:title]) |> put_assoc(:from, from) - |> put_recipient() - |> set_slug() - |> set_last_message() + |> put_assoc(:to, to) + |> put_change(:slug, Ecto.UUID.generate()) |> cast_assoc(:messages, with: &Message.creation_changeset(&1, &2, from)) + |> set_last_message() |> validate_length(:messages, is: 1) + |> validate_length(:title, max: 300, count: :bytes) + |> validate_required([:title, :from, :to]) end @doc false @@ -64,21 +65,7 @@ defmodule Philomena.Conversations.Conversation do |> set_last_message() end - defp set_slug(changeset) do - changeset - |> change(slug: Ecto.UUID.generate()) - end - defp set_last_message(changeset) do change(changeset, last_message_at: DateTime.utc_now(:second)) end - - defp put_recipient(changeset) do - recipient = changeset |> get_field(:recipient) - user = Repo.get_by(User, name: recipient) - - changeset - |> put_change(:to, user) - |> validate_required(:to) - end end diff --git a/lib/philomena/users.ex b/lib/philomena/users.ex index 013b6173..575552aa 100644 --- a/lib/philomena/users.ex +++ b/lib/philomena/users.ex @@ -55,6 +55,22 @@ defmodule Philomena.Users do Repo.get_by(User, email: email) end + @doc """ + Gets a user by name. + + ## Examples + + iex> get_user_by_name("Administrator") + %User{} + + iex> get_user_by_name("nonexistent") + nil + + """ + def get_user_by_name(name) when is_binary(name) do + Repo.get_by(User, name: name) + end + @doc """ Gets a user by email and password. diff --git a/lib/philomena_web/controllers/conversation/message_controller.ex b/lib/philomena_web/controllers/conversation/message_controller.ex index 2d6d2ba2..f6f7fdd1 100644 --- a/lib/philomena_web/controllers/conversation/message_controller.ex +++ b/lib/philomena_web/controllers/conversation/message_controller.ex @@ -1,10 +1,8 @@ defmodule PhilomenaWeb.Conversation.MessageController do use PhilomenaWeb, :controller - alias Philomena.Conversations.{Conversation, Message} + alias Philomena.Conversations.Conversation alias Philomena.Conversations - alias Philomena.Repo - import Ecto.Query plug PhilomenaWeb.FilterBannedUsersPlug plug PhilomenaWeb.CanaryMapPlug, create: :show @@ -15,20 +13,16 @@ defmodule PhilomenaWeb.Conversation.MessageController do id_field: "slug", persisted: true + @page_size 25 + def create(conn, %{"message" => message_params}) do conversation = conn.assigns.conversation user = conn.assigns.current_user case Conversations.create_message(conversation, user, message_params) do {:ok, _message} -> - count = - Message - |> where(conversation_id: ^conversation.id) - |> Repo.aggregate(:count, :id) - - page = - Float.ceil(count / 25) - |> round() + count = Conversations.count_messages(conversation) + page = div(count + @page_size - 1, @page_size) conn |> put_flash(:info, "Message successfully sent.") diff --git a/lib/philomena_web/controllers/conversation_controller.ex b/lib/philomena_web/controllers/conversation_controller.ex index f893fa1f..0e4abdf0 100644 --- a/lib/philomena_web/controllers/conversation_controller.ex +++ b/lib/philomena_web/controllers/conversation_controller.ex @@ -4,8 +4,6 @@ defmodule PhilomenaWeb.ConversationController do alias PhilomenaWeb.NotificationCountPlug alias Philomena.{Conversations, Conversations.Conversation, Conversations.Message} alias PhilomenaWeb.MarkdownRenderer - alias Philomena.Repo - import Ecto.Query plug PhilomenaWeb.FilterBannedUsersPlug when action in [:new, :create] @@ -19,42 +17,17 @@ defmodule PhilomenaWeb.ConversationController do only: :show, preload: [:to, :from] - def index(conn, %{"with" => partner}) do + def index(conn, params) do user = conn.assigns.current_user - Conversation - |> where( - [c], - (c.from_id == ^user.id and c.to_id == ^partner and not c.from_hidden) or - (c.to_id == ^user.id and c.from_id == ^partner and not c.to_hidden) - ) - |> load_conversations(conn) - end - - def index(conn, _params) do - user = conn.assigns.current_user - - Conversation - |> where( - [c], - (c.from_id == ^user.id and not c.from_hidden) or (c.to_id == ^user.id and not c.to_hidden) - ) - |> load_conversations(conn) - end - - defp load_conversations(queryable, conn) do conversations = - queryable - |> join( - :inner_lateral, - [c], - _ in fragment("SELECT COUNT(*) FROM messages m WHERE m.conversation_id = ?", c.id), - on: true - ) - |> order_by(desc: :last_message_at) - |> preload([:to, :from]) - |> select([c, cnt], {c, cnt.count}) - |> Repo.paginate(conn.assigns.scrivener) + case params do + %{"with" => partner_id} -> + Conversations.list_conversations_with(partner_id, user, conn.assigns.scrivener) + + _ -> + Conversations.list_conversations(user, conn.assigns.scrivener) + end render(conn, "index.html", title: "Conversations", conversations: conversations) end @@ -62,27 +35,17 @@ defmodule PhilomenaWeb.ConversationController do def show(conn, _params) do conversation = conn.assigns.conversation user = conn.assigns.current_user - pref = load_direction(user) messages = - Message - |> where(conversation_id: ^conversation.id) - |> order_by([{^pref, :created_at}]) - |> preload([:from]) - |> Repo.paginate(conn.assigns.scrivener) + Conversations.list_messages( + conversation, + user, + &MarkdownRenderer.render_collection(&1, conn), + conn.assigns.scrivener + ) - rendered = - messages.entries - |> MarkdownRenderer.render_collection(conn) - - messages = %{messages | entries: Enum.zip(messages.entries, rendered)} - - changeset = - %Message{} - |> Conversations.change_message() - - conversation - |> Conversations.mark_conversation_read(user) + changeset = Conversations.change_message(%Message{}) + Conversations.mark_conversation_read(conversation, user) # Update the conversation ticker in the header conn = NotificationCountPlug.call(conn) @@ -96,9 +59,10 @@ defmodule PhilomenaWeb.ConversationController do end def new(conn, params) do - changeset = + conversation = %Conversation{recipient: params["recipient"], messages: [%Message{}]} - |> Conversations.change_conversation() + + changeset = Conversations.change_conversation(conversation) render(conn, "new.html", title: "New Conversation", changeset: changeset) end @@ -116,7 +80,4 @@ defmodule PhilomenaWeb.ConversationController do render(conn, "new.html", changeset: changeset) end end - - defp load_direction(%{messages_newest_first: false}), do: :asc - defp load_direction(_user), do: :desc end diff --git a/lib/philomena_web/templates/conversation/index.html.slime b/lib/philomena_web/templates/conversation/index.html.slime index 611610e0..a7c33f33 100644 --- a/lib/philomena_web/templates/conversation/index.html.slime +++ b/lib/philomena_web/templates/conversation/index.html.slime @@ -20,14 +20,14 @@ h1 My Conversations th.table--communication-list__stats With th.table--communication-list__options Options tbody - = for {c, count} <- @conversations do + = for c <- @conversations do tr class=conversation_class(@conn.assigns.current_user, c) td.table--communication-list__name => link c.title, to: ~p"/conversations/#{c}" .small-text.hide-mobile - => count - = pluralize("message", "messages", count) + => c.message_count + = pluralize("message", "messages", c.message_count) ' ; started = pretty_time(c.created_at) ' , last message @@ -36,7 +36,7 @@ h1 My Conversations td.table--communication-list__stats = render PhilomenaWeb.UserAttributionView, "_user.html", object: %{user: other_party(@current_user, c)}, conn: @conn td.table--communication-list__options - => link "Last message", to: last_message_path(c, count) + => link "Last message", to: last_message_path(c, c.message_count) ' • => link "Hide", to: ~p"/conversations/#{c}/hide", data: [method: "post"], data: [confirm: "Are you really, really sure?"]