Merge pull request #329 from philomena-dev/conversations-cleanup

Conversations cleanup

lib/philomena/conversations.ex

@@ -6,75 +6,112 @@ defmodule Philomena.Conversations do
   import Ecto.Query, warn: false
   alias Ecto.Multi
   alias Philomena.Repo
-  alias Philomena.Reports
   alias Philomena.Conversations.Conversation
+  alias Philomena.Conversations.Message
+  alias Philomena.Reports
+  alias Philomena.Users
 
   @doc """
-  Gets a single conversation.
+  Returns the number of unread conversations for the given user.
 
-  Raises `Ecto.NoResultsError` if the Conversation does not exist.
+  Conversations hidden by the given user are not counted.
 
   ## Examples
 
-      iex> get_conversation!(123)
+      iex> count_unread_conversations(user1)
-      %Conversation{}
+      0
 
-      iex> get_conversation!(456)
+      iex> count_unread_conversations(user2)
-      ** (Ecto.NoResultsError)
+      7
 
   """
-  def get_conversation!(id), do: Repo.get!(Conversation, id)
+  def count_unread_conversations(user) do
+    Conversation
+    |> where(
+      [c],
+      ((c.to_id == ^user.id and c.to_read == false) or
+         (c.from_id == ^user.id and c.from_read == false)) and
+        not ((c.to_id == ^user.id and c.to_hidden == true) or
+               (c.from_id == ^user.id and c.from_hidden == true))
+    )
+    |> 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(%{field: value})
+      iex> create_conversation(from, to, %{field: value})
       {:ok, %Conversation{}}
 
-      iex> create_conversation(%{field: bad_value})
+      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, attrs)
+    |> Conversation.creation_changeset(from, to, attrs)
     |> Repo.insert()
+    |> case do
+      {:ok, conversation} ->
+        report_non_approved_message(hd(conversation.messages))
+        {:ok, conversation}
+
+      error ->
+        error
     end
-
-  @doc """
-  Updates a conversation.
-
-  ## Examples
-
-      iex> update_conversation(conversation, %{field: new_value})
-      {:ok, %Conversation{}}
-
-      iex> update_conversation(conversation, %{field: bad_value})
-      {:error, %Ecto.Changeset{}}
-
-  """
-  def update_conversation(%Conversation{} = conversation, attrs) do
-    conversation
-    |> Conversation.changeset(attrs)
-    |> Repo.update()
-  end
-
-  @doc """
-  Deletes a Conversation.
-
-  ## Examples
-
-      iex> delete_conversation(conversation)
-      {:ok, %Conversation{}}
-
-      iex> delete_conversation(conversation)
-      {:error, %Ecto.Changeset{}}
-
-  """
-  def delete_conversation(%Conversation{} = conversation) do
-    Repo.delete(conversation)
   end
 
   @doc """
@@ -90,196 +127,221 @@ defmodule Philomena.Conversations do
     Conversation.changeset(conversation, %{})
   end
 
-  def count_unread_conversations(user) do
-    Conversation
-    |> where(
-      [c],
-      ((c.to_id == ^user.id and c.to_read == false) or
-         (c.from_id == ^user.id and c.from_read == false)) and
-        not ((c.to_id == ^user.id and c.to_hidden == true) or
-               (c.from_id == ^user.id and c.from_hidden == true))
-    )
-    |> Repo.aggregate(:count, :id)
-  end
-
-  def mark_conversation_read(conversation, user, read \\ true)
-
-  def mark_conversation_read(
-        %Conversation{to_id: user_id, from_id: user_id} = conversation,
-        %{id: user_id},
-        read
-      ) do
-    conversation
-    |> Conversation.read_changeset(%{to_read: read, from_read: read})
-    |> Repo.update()
-  end
-
-  def mark_conversation_read(%Conversation{to_id: user_id} = conversation, %{id: user_id}, read) do
-    conversation
-    |> Conversation.read_changeset(%{to_read: read})
-    |> Repo.update()
-  end
-
-  def mark_conversation_read(%Conversation{from_id: user_id} = conversation, %{id: user_id}, read) do
-    conversation
-    |> Conversation.read_changeset(%{from_read: read})
-    |> Repo.update()
-  end
-
-  def mark_conversation_read(_conversation, _user, _read), do: {:ok, nil}
-
-  def mark_conversation_hidden(conversation, user, hidden \\ true)
-
-  def mark_conversation_hidden(
-        %Conversation{to_id: user_id} = conversation,
-        %{id: user_id},
-        hidden
-      ) do
-    conversation
-    |> Conversation.hidden_changeset(%{to_hidden: hidden})
-    |> Repo.update()
-  end
-
-  def mark_conversation_hidden(
-        %Conversation{from_id: user_id} = conversation,
-        %{id: user_id},
-        hidden
-      ) do
-    conversation
-    |> Conversation.hidden_changeset(%{from_hidden: hidden})
-    |> Repo.update()
-  end
-
-  def mark_conversation_hidden(_conversation, _user, _read), do: {:ok, nil}
-
-  alias Philomena.Conversations.Message
-
   @doc """
-  Gets a single message.
+  Marks a conversation as read or unread from the perspective of the given user.
-
-  Raises `Ecto.NoResultsError` if the Message does not exist.
 
   ## Examples
 
-      iex> get_message!(123)
+      iex> mark_conversation_read(conversation, user, true)
-      %Message{}
+      {:ok, %Conversation{}}
 
-      iex> get_message!(456)
+      iex> mark_conversation_read(conversation, user, false)
-      ** (Ecto.NoResultsError)
+      {:ok, %Conversation{}}
+
+      iex> mark_conversation_read(conversation, %User{}, true)
+      {:error, %Ecto.Changeset{}}
 
   """
-  def get_message!(id), do: Repo.get!(Message, id)
+  def mark_conversation_read(%Conversation{} = conversation, user, read \\ true) do
+    changes =
+      %{}
+      |> put_conditional(:to_read, read, conversation.to_id == user.id)
+      |> put_conditional(:from_read, read, conversation.from_id == user.id)
+
+    conversation
+    |> Conversation.read_changeset(changes)
+    |> Repo.update()
+  end
 
   @doc """
-  Creates a message.
+  Marks a conversation as hidden or visible from the perspective of the given user.
+
+  Hidden conversations are not shown in the list of conversations for the user, and
+  are not counted when retrieving the number of unread conversations.
 
   ## Examples
 
-      iex> create_message(%{field: value})
+      iex> mark_conversation_hidden(conversation, user, true)
+      {:ok, %Conversation{}}
+
+      iex> mark_conversation_hidden(conversation, user, false)
+      {:ok, %Conversation{}}
+
+      iex> mark_conversation_hidden(conversation, %User{}, true)
+      {:error, %Ecto.Changeset{}}
+
+  """
+  def mark_conversation_hidden(%Conversation{} = conversation, user, hidden \\ true) do
+    changes =
+      %{}
+      |> put_conditional(:to_hidden, hidden, conversation.to_id == user.id)
+      |> put_conditional(:from_hidden, hidden, conversation.from_id == user.id)
+
+    conversation
+    |> Conversation.hidden_changeset(changes)
+    |> Repo.update()
+  end
+
+  defp put_conditional(map, key, value, condition) do
+    if condition do
+      Map.put(map, key, value)
+    else
+      map
+    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 <strong>world</strong>"}
+      ]
+
+  ## 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.
+
+  ## Examples
+
+      iex> create_message(%Conversation{}, %User{}, %{field: value})
       {:ok, %Message{}}
 
-      iex> create_message(%{field: bad_value})
+      iex> create_message(%Conversation{}, %User{}, %{field: bad_value})
       {:error, %Ecto.Changeset{}}
 
   """
   def create_message(conversation, user, attrs \\ %{}) do
-    message =
+    message_changeset =
-      Ecto.build_assoc(conversation, :messages)
+      conversation
+      |> Ecto.build_assoc(:messages)
       |> Message.creation_changeset(attrs, user)
 
-    show_as_read =
+    conversation_changeset =
-      case message do
+      Conversation.new_message_changeset(conversation)
-        %{changes: %{approved: true}} -> false
-        _ -> true
-      end
-
-    conversation_query =
-      Conversation
-      |> where(id: ^conversation.id)
-
-    now = DateTime.utc_now()
 
     Multi.new()
-    |> Multi.insert(:message, message)
+    |> Multi.insert(:message, message_changeset)
-    |> Multi.update_all(:conversation, conversation_query,
+    |> Multi.update(:conversation, conversation_changeset)
-      set: [from_read: show_as_read, to_read: show_as_read, last_message_at: now]
-    )
     |> Repo.transaction()
+    |> case do
+      {:ok, %{message: message}} ->
+        report_non_approved_message(message)
+        {:ok, message}
+
+      _error ->
+        {:error, message_changeset}
+    end
   end
 
-  def approve_conversation_message(message, user) do
+  @doc """
-    reports_query = Reports.close_report_query({"Conversation", message.conversation_id}, user)
+  Approves a previously-posted message which was not approved at post time.
 
-    message_query =
+  ## Examples
-      message
-      |> Message.approve_changeset()
 
-    conversation_query =
+      iex> approve_message(%Message{}, %User{})
-      Conversation
+      {:ok, %Message{}}
-      |> where(id: ^message.conversation_id)
+
+      iex> approve_message(%Message{}, %User{})
+      {:error, %Ecto.Changeset{}}
+
+  """
+  def approve_message(message, approving_user) do
+    message_changeset = Message.approve_changeset(message)
+
+    conversation_update_query =
+      from c in Conversation,
+        where: c.id == ^message.conversation_id,
+        update: [set: [from_read: false, to_read: false]]
+
+    reports_query =
+      Reports.close_report_query({"Conversation", message.conversation_id}, approving_user)
 
     Multi.new()
-    |> Multi.update(:message, message_query)
+    |> Multi.update(:message, message_changeset)
-    |> Multi.update_all(:conversation, conversation_query, set: [to_read: false])
+    |> Multi.update_all(:conversation, conversation_update_query, [])
     |> Multi.update_all(:reports, reports_query, [])
     |> Repo.transaction()
     |> case do
-      {:ok, %{reports: {_count, reports}} = result} ->
+      {:ok, %{reports: {_count, reports}, message: message}} ->
         Reports.reindex_reports(reports)
 
-        {:ok, result}
+        message
 
-      error ->
+      _error ->
-        error
+        {:error, message_changeset}
     end
   end
 
-  def report_non_approved(id) do
+  @doc """
+  Generates a system report for an unapproved message.
+
+  This is called by `create_conversation/2` and `create_message/3`, so it normally does not
+  need to be called explicitly.
+
+  ## Examples
+
+      iex> report_non_approved_message(%Message{approved: false})
+      {:ok, %Report{}}
+
+      iex> report_non_approved_message(%Message{approved: true})
+      {:ok, nil}
+
+  """
+  def report_non_approved_message(message) do
+    if message.approved do
+      {:ok, nil}
+    else
       Reports.create_system_report(
-      {"Conversation", id},
+        {"Conversation", message.conversation_id},
         "Approval",
         "PM contains externally-embedded images and has been flagged for review."
       )
     end
-
-  def set_as_read(conversation) do
-    conversation
-    |> Conversation.to_read_changeset()
-    |> Repo.update()
-  end
-
-  @doc """
-  Updates a message.
-
-  ## Examples
-
-      iex> update_message(message, %{field: new_value})
-      {:ok, %Message{}}
-
-      iex> update_message(message, %{field: bad_value})
-      {:error, %Ecto.Changeset{}}
-
-  """
-  def update_message(%Message{} = message, attrs) do
-    message
-    |> Message.changeset(attrs)
-    |> Repo.update()
-  end
-
-  @doc """
-  Deletes a Message.
-
-  ## Examples
-
-      iex> delete_message(message)
-      {:ok, %Message{}}
-
-      iex> delete_message(message)
-      {:error, %Ecto.Changeset{}}
-
-  """
-  def delete_message(%Message{} = message) do
-    Repo.delete(message)
   end
 
   @doc """

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)
@@ -32,50 +33,39 @@ defmodule Philomena.Conversations.Conversation do
     |> validate_required([])
   end
 
+  @doc false
   def read_changeset(conversation, attrs) do
-    conversation
+    cast(conversation, attrs, [:from_read, :to_read])
-    |> cast(attrs, [:from_read, :to_read])
-  end
-
-  def to_read_changeset(conversation) do
-    change(conversation)
-    |> put_change(:to_read, true)
-  end
-
-  def hidden_changeset(conversation, attrs) do
-    conversation
-    |> cast(attrs, [:from_hidden, :to_hidden])
   end
 
   @doc false
-  def creation_changeset(conversation, from, attrs) do
+  def hidden_changeset(conversation, attrs) do
-    conversation
+    cast(conversation, attrs, [:from_hidden, :to_hidden])
-    |> cast(attrs, [:title, :recipient])
-    |> validate_required([:title, :recipient])
-    |> validate_length(:title, max: 300, count: :bytes)
-    |> put_assoc(:from, from)
-    |> put_recipient()
-    |> set_slug()
-    |> set_last_message()
-    |> cast_assoc(:messages, with: &Message.creation_changeset(&1, &2, from))
-    |> validate_length(:messages, is: 1)
   end
 
-  defp set_slug(changeset) do
+  @doc false
-    changeset
+  def creation_changeset(conversation, from, to, attrs) do
-    |> change(slug: Ecto.UUID.generate())
+    conversation
+    |> cast(attrs, [:title])
+    |> put_assoc(:from, from)
+    |> 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
+  def new_message_changeset(conversation) do
+    conversation
+    |> change(from_read: false)
+    |> change(to_read: false)
+    |> set_last_message()
   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

lib/philomena/conversations/message.ex

@@ -33,6 +33,7 @@ defmodule Philomena.Conversations.Message do
     |> Approval.maybe_put_approval(user)
   end
 
+  @doc false
   def approve_changeset(message) do
     change(message, approved: true)
   end

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.
 

lib/philomena_web/controllers/conversation/message/approve_controller.ex

@@ -16,7 +16,7 @@ defmodule PhilomenaWeb.Conversation.Message.ApproveController do
     message = conn.assigns.message
 
     {:ok, _message} =
-      Conversations.approve_conversation_message(message, conn.assigns.current_user)
+      Conversations.approve_message(message, conn.assigns.current_user)
 
     conn
     |> put_flash(:info, "Conversation message approved.")

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,24 +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: message}} ->
+      {:ok, _message} ->
-        if not message.approved do
+        count = Conversations.count_messages(conversation)
-          Conversations.report_non_approved(message.conversation_id)
+        page = div(count + @page_size - 1, @page_size)
-        end
-
-        count =
-          Message
-          |> where(conversation_id: ^conversation.id)
-          |> Repo.aggregate(:count, :id)
-
-        page =
-          Float.ceil(count / 25)
-          |> round()
 
         conn
         |> put_flash(:info, "Message successfully sent.")

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
+      case params do
-      |> join(
+        %{"with" => partner_id} ->
-        :inner_lateral,
+          Conversations.list_conversations_with(partner_id, user, conn.assigns.scrivener)
-        [c],
+
-        _ in fragment("SELECT COUNT(*) FROM messages m WHERE m.conversation_id = ?", c.id),
+        _ ->
-        on: true
+          Conversations.list_conversations(user, conn.assigns.scrivener)
-      )
+      end
-      |> order_by(desc: :last_message_at)
-      |> preload([:to, :from])
-      |> select([c, cnt], {c, cnt.count})
-      |> Repo.paginate(conn.assigns.scrivener)
 
     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
+      Conversations.list_messages(
-      |> where(conversation_id: ^conversation.id)
+        conversation,
-      |> order_by([{^pref, :created_at}])
+        user,
-      |> preload([:from])
+        &MarkdownRenderer.render_collection(&1, conn),
-      |> Repo.paginate(conn.assigns.scrivener)
+        conn.assigns.scrivener
+      )
 
-    rendered =
+    changeset = Conversations.change_message(%Message{})
-      messages.entries
+    Conversations.mark_conversation_read(conversation, user)
-      |> MarkdownRenderer.render_collection(conn)
-
-    messages = %{messages | entries: Enum.zip(messages.entries, rendered)}
-
-    changeset =
-      %Message{}
-      |> Conversations.change_message()
-
-    conversation
-    |> Conversations.mark_conversation_read(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
@@ -108,21 +72,12 @@ defmodule PhilomenaWeb.ConversationController do
 
     case Conversations.create_conversation(user, conversation_params) do
       {:ok, conversation} ->
-        if not hd(conversation.messages).approved do
-          Conversations.report_non_approved(conversation.id)
-          Conversations.set_as_read(conversation)
-        end
-
         conn
         |> put_flash(:info, "Conversation successfully created.")
         |> redirect(to: ~p"/conversations/#{conversation}")
 
       {:error, changeset} ->
-        conn
+        render(conn, "new.html", changeset: changeset)
-        |> render("new.html", changeset: changeset)
     end
   end
-
-  defp load_direction(%{messages_newest_first: false}), do: :asc
-  defp load_direction(_user), do: :desc
 end

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
+                => c.message_count
-                = pluralize("message", "messages", 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)
               ' &bull;
               => link "Hide", to: ~p"/conversations/#{c}/hide", data: [method: "post"], data: [confirm: "Are you really, really sure?"]