Merge pull request #1296 from code-corps/1288-conversation-index

Conversation index endpoint with supporting infrastructure

Author: joshsmith

lib/code_corps/messages/conversation_query.ex

@@ -0,0 +1,46 @@
+defmodule CodeCorps.Messages.ConversationQuery do
+  @moduledoc ~S"""
+  Holds helpers to query `CodeCorps.Conversation` records using a map of params.
+  """
+
+  import Ecto.Query
+
+  alias CodeCorps.{Conversation, ConversationPart, Message, Repo}
+  alias Ecto.Queryable
+
+  @doc ~S"""
+  Narrows down a `CodeCorps.Conversation` query by `project_id` of the parent
+  `CodeCorps.Message`, if specified in a params map
+  """
+  @spec project_filter(Queryable.t, map) :: Queryable.t
+  def project_filter(queryable, %{"project_id" => project_id}) do
+    queryable
+    |> join(:left, [c], m in Message, c.message_id == m.id)
+    |> where([_c, m], m.project_id == ^project_id)
+  end
+  def project_filter(queryable, %{}), do: queryable
+
+
+  @doc ~S"""
+  Narrows down a `CodeCorps.Conversation` query to return only those records
+  considered to have a specific status.
+
+  The status of `active` means that only those records are included which either
+  - belong to a `CodeCorps.Message` initiated by user
+  - belong to a `CodeCorps.Message` initiated by admin, with at least a single
+    reply in the form of a `CodeCorps.ConversationPart`
+  """
+  @spec status_filter(Queryable.t, map) :: Queryable.t
+  def status_filter(queryable, %{"status" => "active"}) do
+    prefiltered_ids = queryable |> select([c], c.id) |> Repo.all
+
+    Conversation
+    |> where([c], c.id in ^prefiltered_ids)
+    |> join(:left, [c], m in Message, c.message_id == m.id)
+    |> join(:left, [c, _m], cp in ConversationPart, c.id == cp.conversation_id)
+    |> group_by([c, m, _cp], [c.id, m.initiated_by])
+    |> having([_c, m, _cp], m.initiated_by == "user")
+    |> or_having([c, m, cp], m.initiated_by == "admin" and count(cp.id) > 0)
+  end
+  def status_filter(query, %{}), do: query
+end

lib/code_corps/messages/messages.ex

@@ -3,7 +3,7 @@ defmodule CodeCorps.Messages do
   Main context for work with the Messaging feature.
   """
 
-  alias CodeCorps.{Helpers.Query, Message, Messages, Repo}
+  alias CodeCorps.{Conversation, Helpers.Query, Message, Messages, Repo}
   alias Ecto.{Changeset, Queryable}
 
   @doc ~S"""
@@ -18,6 +18,17 @@ defmodule CodeCorps.Messages do
     |> Repo.all()
   end
 
+  @doc ~S"""
+  Lists pre-scoped `CodeCorps.Conversation` records filtered by parameters
+  """
+  @spec list_conversations(Queryable.t, map) :: list(Conversation.t)
+  def list_conversations(scope, %{} = params) do
+    scope
+    |> Messages.ConversationQuery.project_filter(params)
+    |> Messages.ConversationQuery.status_filter(params)
+    |> Repo.all()
+  end
+
   @doc ~S"""
   Creates a `CodeCorps.Message` from a set of parameters.
   """

lib/code_corps/model/conversation.ex

@@ -16,8 +16,8 @@ defmodule CodeCorps.Conversation do
   @type t :: %__MODULE__{}
 
   schema "conversations" do
-    field :status, :string, null: false, default: "open"
     field :read_at, :utc_datetime, null: true
+    field :status, :string, null: false, default: "open"
 
     belongs_to :message, CodeCorps.Message
     belongs_to :user, CodeCorps.User

lib/code_corps/policy/conversation.ex

@@ -0,0 +1,24 @@
+defmodule CodeCorps.Policy.Conversation do
+  @moduledoc ~S"""
+  Handles `CodeCorps.User` authorization of actions on `CodeCorps.Conversation`
+  records.
+  """
+
+  import Ecto.Query
+
+  alias CodeCorps.{Message, Policy, Repo, User}
+
+  @spec scope(Ecto.Queryable.t, User.t) :: Ecto.Queryable.t
+  def scope(queryable, %User{admin: true}), do: queryable
+  def scope(queryable, %User{id: id} = current_user) do
+    scoped_message_ids =
+      Message
+      |> Policy.Message.scope(current_user)
+      |> select([m], m.id)
+      |> Repo.all
+
+    queryable
+    |> where(user_id: ^id)
+    |> or_where([c], c.message_id in ^scoped_message_ids)
+  end
+end

lib/code_corps/policy/policy.ex

@@ -3,7 +3,7 @@ defmodule CodeCorps.Policy do
   Handles authorization for various API actions performed on objects in the database.
   """
 
-  alias CodeCorps.{Category, Comment, DonationGoal, GithubAppInstallation, GithubEvent, GithubRepo, Message, Organization, OrganizationInvite, OrganizationGithubAppInstallation, Preview, Project, ProjectCategory, ProjectSkill, ProjectUser, Role, RoleSkill, Skill, StripeConnectAccount, StripeConnectPlan, StripeConnectSubscription, StripePlatformCard, StripePlatformCustomer, Task, TaskSkill, User, UserCategory, UserRole, UserSkill, UserTask}
+  alias CodeCorps.{Category, Comment, Conversation, DonationGoal, GithubAppInstallation, GithubEvent, GithubRepo, Message, Organization, OrganizationInvite, OrganizationGithubAppInstallation, Preview, Project, ProjectCategory, ProjectSkill, ProjectUser, Role, RoleSkill, Skill, StripeConnectAccount, StripeConnectPlan, StripeConnectSubscription, StripePlatformCard, StripePlatformCustomer, Task, TaskSkill, User, UserCategory, UserRole, UserSkill, UserTask}
 
   alias CodeCorps.Policy
 
@@ -28,6 +28,7 @@ defmodule CodeCorps.Policy do
   """
   @spec scope(module, User.t) :: Ecto.Queryable.t
   def scope(Message, %User{} = current_user), do: Message |> Policy.Message.scope(current_user)
+  def scope(Conversation, %User{} = current_user), do: Conversation |> Policy.Conversation.scope(current_user)
 
   @spec can?(User.t, atom, struct, map) :: boolean
 

lib/code_corps_web/controllers/conversation_controller.ex

@@ -0,0 +1,22 @@
+defmodule CodeCorpsWeb.ConversationController do
+  @moduledoc false
+  use CodeCorpsWeb, :controller
+
+  alias CodeCorps.{
+    Conversation,
+    Messages,
+    User
+  }
+
+  action_fallback CodeCorpsWeb.FallbackController
+  plug CodeCorpsWeb.Plug.DataToAttributes
+  plug CodeCorpsWeb.Plug.IdsToIntegers
+
+  @spec index(Conn.t, map) :: Conn.t
+  def index(%Conn{} = conn, %{} = params) do
+    with %User{} = current_user <- conn |> CodeCorps.Guardian.Plug.current_resource,
+         conversations <- Conversation |> Policy.scope(current_user) |> Messages.list_conversations(params) do
+      conn |> render("index.json-api", data: conversations)
+    end
+  end
+end

lib/code_corps_web/router.ex

@@ -71,6 +71,7 @@ defmodule CodeCorpsWeb.Router do
 
     resources "/categories", CategoryController, only: [:create, :update]
     resources "/comments", CommentController, only: [:create, :update]
+    resources "/conversations", ConversationController, only: [:index]
     resources "/donation-goals", DonationGoalController, only: [:create, :update, :delete]
     post "/oauth/github", UserController, :github_oauth
     resources "/github-app-installations", GithubAppInstallationController, only: [:create]

lib/code_corps_web/views/conversation_view.ex

@@ -0,0 +1,10 @@
+defmodule CodeCorpsWeb.ConversationView do
+  @moduledoc false
+  use CodeCorpsWeb, :view
+  use JaSerializer.PhoenixView
+
+  attributes [:read_at, :status, :inserted_at, :updated_at]
+
+  has_one :user, type: "user", field: :user_id
+  has_one :message, type: "message", field: :message_id
+end

test/lib/code_corps/messages/messages_test.exs

@@ -5,13 +5,13 @@ defmodule CodeCorps.MessagesTest do
 
   import Ecto.Query, only: [where: 2]
 
-  alias CodeCorps.{Message, Messages}
+  alias CodeCorps.{Conversation, Message, Messages}
 
-  describe "list" do
-    defp get_and_sort_ids(records) do
-      records |> Enum.map(&Map.get(&1, :id)) |> Enum.sort
-    end
+  defp get_and_sort_ids(records) do
+    records |> Enum.map(&Map.get(&1, :id)) |> Enum.sort
+  end
 
+  describe "list" do
     test "returns all records by default" do
       insert_list(3, :message)
       assert Message |> Messages.list(%{}) |> Enum.count == 3
@@ -121,4 +121,165 @@ defmodule CodeCorps.MessagesTest do
       refute message_p2_a2.id in result_ids
     end
   end
+
+  describe "list_conversations/2" do
+    test "returns all records by default" do
+      insert_list(3, :conversation)
+      assert Conversation |> Messages.list_conversations(%{}) |> Enum.count == 3
+    end
+
+    test "can filter by project" do
+      [%{project: project_1} = message_1, %{project: project_2} = message_2] =
+        insert_pair(:message)
+
+      conversation_1 = insert(:conversation, message: message_1)
+      conversation_2 = insert(:conversation, message: message_2)
+
+      result_ids =
+        Conversation
+        |> Messages.list_conversations(%{"project_id" => project_1.id})
+        |> get_and_sort_ids()
+
+      assert result_ids |> Enum.count == 1
+      assert conversation_1.id in result_ids
+      refute conversation_2.id in result_ids
+
+      result_ids =
+        Conversation
+        |> Messages.list_conversations(%{"project_id" => project_2.id})
+        |> get_and_sort_ids()
+
+      assert result_ids |> Enum.count == 1
+      refute conversation_1.id in result_ids
+      assert conversation_2.id in result_ids
+    end
+
+    test "can filter by status" do
+      message_started_by_admin = insert(:message, initiated_by: "admin")
+      message_started_by_user = insert(:message, initiated_by: "user")
+
+      conversation_started_by_admin_without_reply =
+        insert(:conversation, message: message_started_by_admin)
+      conversation_started_by_admin_with_reply =
+        insert(:conversation, message: message_started_by_admin)
+      insert(
+        :conversation_part,
+        conversation: conversation_started_by_admin_with_reply
+      )
+
+      conversation_started_by_user_without_reply =
+        insert(:conversation, message: message_started_by_user)
+      conversation_started_by_user_with_reply =
+        insert(:conversation, message: message_started_by_user)
+      insert(
+        :conversation_part,
+        conversation: conversation_started_by_user_with_reply
+      )
+
+      result_ids =
+        Conversation
+        |> Messages.list_conversations(%{"status" => "active"})
+        |> get_and_sort_ids()
+
+      refute conversation_started_by_admin_without_reply.id in result_ids
+      assert conversation_started_by_admin_with_reply.id in result_ids
+      assert conversation_started_by_user_without_reply.id in result_ids
+      assert conversation_started_by_user_with_reply.id in result_ids
+
+      result_ids =
+        Conversation
+        |> Messages.list_conversations(%{"status" => "any"})
+        |> get_and_sort_ids()
+
+      assert conversation_started_by_admin_without_reply.id in result_ids
+      assert conversation_started_by_admin_with_reply.id in result_ids
+      assert conversation_started_by_user_without_reply.id in result_ids
+      assert conversation_started_by_user_with_reply.id in result_ids
+    end
+
+    test "builds upon the provided scope" do
+      [project_1, project_2] = insert_pair(:project)
+      [user_1, user_2] = insert_pair(:user)
+
+      message_p1 = insert(:message, project: project_1)
+      message_p2 = insert(:message, project: project_2)
+
+      conversation_u1_p1 =
+        insert(:conversation, user: user_1, message: message_p1)
+      conversation_u1_p2 =
+        insert(:conversation, user: user_1, message: message_p2)
+      conversation_u2_p1 =
+        insert(:conversation, user: user_2, message: message_p1)
+      conversation_u2_p2 =
+        insert(:conversation, user: user_2, message: message_p2)
+
+      params = %{"project_id" => project_1.id}
+      result_ids =
+        Conversation
+        |> where(user_id: ^user_1.id)
+        |> Messages.list_conversations(params)
+        |> get_and_sort_ids()
+
+      assert conversation_u1_p1.id in result_ids
+      refute conversation_u1_p2.id in result_ids
+      refute conversation_u2_p1.id in result_ids
+      refute conversation_u2_p2.id in result_ids
+    end
+
+    test "supports multiple filters at once" do
+      ## we create two messages started by admin, each on a different project
+      %{project: project_1} = message_1_started_by_admin =
+        insert(:message, initiated_by: "admin")
+      %{project: project_2} = message_2_started_by_admin =
+        insert(:message, initiated_by: "admin")
+
+      # we create one conversation without a reply, to test the "status" filter
+
+      conversation_started_by_admin_without_reply =
+        insert(:conversation, message: message_1_started_by_admin)
+
+      # we create two conversations with replies, on on each message
+      # since the messages are on different projects, this allows us to
+      # test the project filter
+
+      conversation_started_by_admin_with_reply =
+        insert(:conversation, message: message_1_started_by_admin)
+      insert(
+        :conversation_part,
+        conversation: conversation_started_by_admin_with_reply
+      )
+      other_conversation_started_by_admin_with_reply =
+        insert(:conversation, message: message_2_started_by_admin)
+      insert(
+        :conversation_part,
+        conversation: other_conversation_started_by_admin_with_reply
+      )
+
+      params = %{"status" => "active", "project_id" => project_1.id}
+      result_ids =
+        Conversation
+        |> Messages.list_conversations(params)
+        |> get_and_sort_ids()
+
+      # this means the status filter worked, because the first conv. belongs to
+      # the message with the correct project
+      refute conversation_started_by_admin_without_reply.id in result_ids
+      # this conversation is active and belongs to the message with the
+      # correct project
+      assert conversation_started_by_admin_with_reply.id in result_ids
+      # this conversation is active, but belongs to a message with a different
+      # project
+      refute other_conversation_started_by_admin_with_reply.id in result_ids
+
+      params = %{"status" => "active", "project_id" => project_2.id}
+      result_ids =
+        Conversation
+        |> Messages.list_conversations(params)
+        |> get_and_sort_ids()
+
+      refute conversation_started_by_admin_without_reply.id in result_ids
+      refute conversation_started_by_admin_with_reply.id in result_ids
+      assert other_conversation_started_by_admin_with_reply.id in result_ids
+    end
+  end
 end