Merge pull request #1305 from code-corps/fix-messages

Fix message queries

Author: joshsmith

lib/code_corps/messages/conversation_query.ex

@@ -20,6 +20,17 @@ defmodule CodeCorps.Messages.ConversationQuery do
   end
   def project_filter(queryable, %{}), do: queryable
 
+  @doc ~S"""
+  Narrows down a `CodeCorps.Conversation` query by `user_id`, if specified in a
+  params map
+  """
+  @spec user_filter(Queryable.t, map) :: Queryable.t
+  def user_filter(queryable, %{"user_id" => user_id}) do
+    queryable
+    |> where([c], c.user_id == ^user_id)
+  end
+  def user_filter(queryable, %{}), do: queryable
+
 
   @doc ~S"""
   Narrows down a `CodeCorps.Conversation` query to return only those records

lib/code_corps/messages/messages.ex

@@ -33,6 +33,7 @@ defmodule CodeCorps.Messages do
     scope
     |> Messages.ConversationQuery.project_filter(params)
     |> Messages.ConversationQuery.status_filter(params)
+    |> Messages.ConversationQuery.user_filter(params)
     |> Repo.all()
   end
 

lib/code_corps/policy/conversation.ex

@@ -8,14 +8,23 @@ defmodule CodeCorps.Policy.Conversation do
     only: [administered_by?: 2, get_message: 1, get_project: 1]
   import Ecto.Query
 
-  alias CodeCorps.{Conversation, Message, Policy, Repo, User}
+  alias CodeCorps.{Conversation, Message, Project, ProjectUser, 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
+  def scope(queryable, %User{id: id}) do
+    projects_administered_by_user_ids =
+      Project
+      |> join(:inner, [p], pu in ProjectUser, pu.project_id == p.id)
+      |> where([_p, pu], pu.user_id == ^id)
+      |> where([_p, pu], pu.role in ~w(admin owner))
+      |> select([p], p.id)
+      |> Repo.all
+
     scoped_message_ids =
       Message
-      |> Policy.Message.scope(current_user)
+      |> where([m], m.author_id == ^id)
+      |> or_where([m], m.project_id in ^projects_administered_by_user_ids)
       |> select([m], m.id)
       |> Repo.all
 

lib/code_corps/policy/message.ex

@@ -6,7 +6,7 @@ defmodule CodeCorps.Policy.Message do
   import CodeCorps.Policy.Helpers, only: [administered_by?: 2, get_project: 1]
   import Ecto.Query
 
-  alias CodeCorps.{Message, Project, ProjectUser, User, Repo}
+  alias CodeCorps.{Conversation, Message, Project, ProjectUser, Repo, User}
 
   @spec scope(Ecto.Queryable.t, User.t) :: Ecto.Queryable.t
   def scope(queryable, %User{admin: true}), do: queryable
@@ -19,17 +19,32 @@ defmodule CodeCorps.Policy.Message do
       |> select([p], p.id)
       |> Repo.all
 
+    messages_targeted_at_user_ids =
+      Message
+      |> join(:inner, [m], c in Conversation, c.message_id == m.id)
+      |> where([_m, c], c.user_id == ^id)
+      |> select([m, _c], m.id)
+      |> Repo.all
+
     queryable
     |> where([m], m.author_id == ^id)
+    |> or_where([m], m.id in ^messages_targeted_at_user_ids)
     |> or_where([m], m.project_id in ^projects_administered_by_user_ids)
   end
 
-  def show?(%User{id: user_id}, %{initiated_by: "user", author_id: author_id})
+  def show?(
+    %User{id: user_id},
+    %Message{initiated_by: "user", author_id: author_id})
     when user_id == author_id do
+
     true
   end
   def show?(%User{} = user, %Message{} = message) do
-    message |> get_project() |> administered_by?(user)
+    cond do
+      message |> get_project() |> administered_by?(user) -> true
+      message.conversations |> Enum.any?(fn c -> c.user_id == user.id end) -> true
+      true -> false
+    end
   end
   def show?(_, _), do: false
 

test/lib/code_corps/policy/message_test.exs

@@ -21,7 +21,7 @@ defmodule CodeCorps.Policy.MessageTest do
       assert Message |> scope(user) |> Repo.all |> Enum.count == 3
     end
 
-    test "returns records where user is the author or they administer the project" do
+    test "returns records where user is the author or they administer the project, or they are the target of the conversation" do
       user = insert(:user, admin: false)
 
       %{project: project_user_applied_to} =
@@ -58,6 +58,9 @@ defmodule CodeCorps.Policy.MessageTest do
       message_from_owned_project =
         insert(:message, project: project_user_owns)
 
+      %{message: message_where_user_is_target} =
+        insert(:conversation, user: user)
+
       result_ids =
         Message
         |> scope(user)
@@ -71,6 +74,7 @@ defmodule CodeCorps.Policy.MessageTest do
       assert message_from_other_administered_project.id in result_ids
       assert message_from_owned_project.id in result_ids
       refute some_other_message.id in result_ids
+      assert message_where_user_is_target.id in result_ids
     end
   end
 
@@ -86,35 +90,42 @@ defmodule CodeCorps.Policy.MessageTest do
       user = insert(:user)
       message = insert(:message, initiated_by: "user")
 
-      refute show?(user, message)
+      refute show?(user, message |> Repo.preload(:conversations))
     end
 
     test "returns false when user is a pending project member" do
       %{project: project, user: user} = insert(:project_user, role: "pending")
       message = insert(:message, initiated_by: "user", project: project)
 
-      refute show?(user, message)
+      refute show?(user, message |> Repo.preload(:conversations))
     end
 
     test "returns false when user is a project contributor" do
       %{project: project, user: user} = insert(:project_user, role: "contributor")
       message = insert(:message, initiated_by: "user", project: project)
 
-      refute show?(user, message)
+      refute show?(user, message |> Repo.preload(:conversations))
     end
 
     test "returns true when user is a project admin" do
       %{project: project, user: user} = insert(:project_user, role: "admin")
       message = insert(:message, initiated_by: "user", project: project)
 
-      assert show?(user, message)
+      assert show?(user, message |> Repo.preload(:conversations))
     end
 
     test "returns true when user is project owner" do
       %{project: project, user: user} = insert(:project_user, role: "owner")
       message = insert(:message, initiated_by: "user", project: project)
 
-      assert show?(user, message)
+      assert show?(user, message |> Repo.preload(:conversations))
+    end
+
+    test "returns true when message conversation is targeted at user" do
+      user = insert(:user)
+      %{message: message} = insert(:conversation, user: user)
+
+      assert show?(user, message |> Repo.preload(:conversations))
     end
   end
 

test/lib/code_corps_web/controllers/conversation_controller_test.exs

@@ -29,6 +29,18 @@ defmodule CodeCorpsWeb.ConversationControllerTest do
       |> json_response(200)
       |> assert_ids_from_response([conversation_1.id, conversation_2.id])
     end
+
+    @tag authenticated: :admin
+    test "lists all entries by status", %{conn: conn} do
+      insert_pair(:conversation)
+      user = insert(:user)
+      conversation_other = insert(:conversation, user: user)
+
+      conn
+      |> get("conversations?user_id=#{user.id}")
+      |> json_response(200)
+      |> assert_ids_from_response([conversation_other.id])
+    end
   end
 
   describe "show" do