Merge branch 'develop' into limit-part-scope

circle.yml

@@ -10,7 +10,7 @@ dependencies:
   pre:
     - ./bin/circle_pre_build.sh
     - mix deps.compile
-    - mix compile
+    - mix compile --warnings-as-errors
 test:
   override:
     - |

lib/code_corps/accounts/changesets.ex

@@ -14,8 +14,14 @@ defmodule CodeCorps.Accounts.Changesets do
   """
   @spec create_from_github_changeset(struct, map) :: Changeset.t
   def create_from_github_changeset(struct, %{} = params) do
+    params =
+      params
+      |> Adapters.User.to_user()
+      |> Enum.reject(fn {_, v} -> is_nil(v) end)
+      |> Map.new()
+
     struct
-    |> Changeset.change(params |> Adapters.User.to_user())
+    |> Changeset.change(params)
     |> Changeset.put_change(:sign_up_context, "github")
     |> Changeset.validate_inclusion(:type, ["bot", "user"])
     |> RandomIconColor.generate_icon_color(:default_color)

lib/code_corps/accounts/users.ex

@@ -0,0 +1,11 @@
+defmodule CodeCorps.Accounts.Users do
+  alias CodeCorps.ProjectUser
+
+  import Ecto.Query
+
+  def project_filter(query, %{"project_id" => project_id}) do
+    from user in query,
+      join: pu in ProjectUser, on: pu.user_id == user.id and pu.project_id == ^project_id
+  end
+  def project_filter(query, _), do: query
+end

lib/code_corps/github/adapters/user.ex

@@ -21,17 +21,12 @@ defmodule CodeCorps.GitHub.Adapters.User do
   Converts a Github user payload into a map of attributes suitable for creating
   or updating a `CodeCorps.User`
 
-  Any `nil` values are removed. For example, we don't want to delete an
-  existing email just because it's `nil` in the payload.
-
   The `type` gets transformed to match our expected values for user type.
   """
   @spec to_user(map) :: map
   def to_user(%{} = payload) do
     payload
     |> CodeCorps.Adapter.MapTransformer.transform(@user_mapping)
-    |> Enum.reject(fn {_, v} -> is_nil(v) end)
-    |> Map.new
     |> transform_type
   end
 

lib/code_corps_web/controllers/user_controller.ex

@@ -7,7 +7,8 @@ defmodule CodeCorpsWeb.UserController do
     GitHub,
     Helpers.Query,
     Services.UserService,
-    User
+    User,
+    Accounts
   }
 
   action_fallback CodeCorpsWeb.FallbackController
@@ -20,6 +21,7 @@ defmodule CodeCorpsWeb.UserController do
       |> Query.id_filter(params)
       |> Query.limit_filter(params)
       |> Query.user_filter(params)
+      |> Accounts.Users.project_filter(params)
       |> Repo.all()
       |> preload()
 

test/lib/code_corps/accounts/changesets_test.exs

@@ -18,6 +18,16 @@ defmodule CodeCorps.Accounts.ChangesetsTest do
       changeset = Changesets.create_from_github_changeset(%User{}, %{})
       assert changeset.changes.default_color
     end
+
+    test "ensures nil values are omitted" do
+      params = %{"email" => nil, "github_avatar_url" => nil, "type" => "bot"}
+
+      changeset = Changesets.create_from_github_changeset(%User{}, params)
+
+      refute changeset.changes[:email]
+      refute changeset.changes[:github_avatar_url]
+    end
+
   end
 
   describe "update_from_github_oauth_changeset/2" do

test/lib/code_corps/accounts/users_test.exs

@@ -0,0 +1,28 @@
+defmodule CodeCorps.Accounts.UsersTest do
+  @moduledoc false
+
+  use CodeCorps.DbAccessCase
+  import CodeCorps.TestHelpers, only: [assert_ids_from_query: 2]
+
+  alias CodeCorps.{Accounts, User}
+
+  describe "project_filter/2" do
+    test "filters users by project filter" do
+      user_1 = insert(:user)
+      user_2 = insert(:user)
+
+      project = insert(:project)
+
+      insert(:project_user, user: user_1, project: project)
+      insert(:project_user, user: user_2, project: project)
+      insert(:project_user)
+
+      result =
+        User
+        |> Accounts.Users.project_filter(%{"project_id" => project.id})
+        |> Repo.all()
+
+      assert_ids_from_query(result, [user_1.id, user_2.id])
+     end
+  end
+end

test/lib/code_corps/github/adapters/user_test.exs

@@ -7,11 +7,22 @@ defmodule CodeCorps.GitHub.Adapters.UserTest do
 
   alias CodeCorps.GitHub.Adapters.User
 
+  defp expected_payload(type) do
+    %{
+      email: nil,
+      github_id: nil,
+      github_username: nil,
+      github_avatar_url: nil,
+      type: type
+    }
+  end
+
   describe "to_user/1" do
     test "maps API payload" do
       %{"issue" => %{"user" => payload}} = load_event_fixture("issues_opened")
 
       assert User.to_user(payload) == %{
+        email: nil,
         github_id: payload["id"],
         github_username: payload["login"],
         github_avatar_url: payload["avatar_url"],
@@ -20,15 +31,15 @@ defmodule CodeCorps.GitHub.Adapters.UserTest do
     end
 
     test "maps Bot type" do
-      assert User.to_user(%{"type" => "Bot"}) == %{type: "bot"}
+      assert User.to_user(%{"type" => "Bot"}) == expected_payload("bot")
     end
 
     test "maps User type" do
-      assert User.to_user(%{"type" => "User"}) == %{type: "user"}
+      assert User.to_user(%{"type" => "User"}) == expected_payload("user")
     end
 
     test "does not map Organization type" do
-      assert User.to_user(%{"type" => "Organization"}) == %{type: "Organization"}
+      assert User.to_user(%{"type" => "Organization"}) == expected_payload("Organization")
     end
   end
 end

test/lib/code_corps/github/sync/user/record_linker_test.exs

@@ -12,6 +12,12 @@ defmodule CodeCorps.Sync.User.RecordLinkerTest do
   }
   alias CodeCorps.GitHub.Adapters.User, as: UserAdapter
 
+  def remove_nils(payload) do
+    payload
+    |> Enum.reject(fn {_, v} -> is_nil(v) end)
+    |> Map.new()
+  end
+
   describe "link_to/2 for comments" do
     @payload load_event_fixture("issue_comment_created")
     @bot_payload load_event_fixture("issue_comment_created_by_bot")
@@ -43,7 +49,7 @@ defmodule CodeCorps.Sync.User.RecordLinkerTest do
 
     test "finds user by github id if none is found by comment association" do
       %{"comment" => %{"id" => github_id} = comment} = @payload
-      attributes = UserAdapter.to_user(@user_payload)
+      attributes = @user_payload |> UserAdapter.to_user() |> remove_nils()
       preinserted_user = insert(:user, attributes)
       github_comment = insert(:github_comment, github_id: github_id)
 
@@ -58,7 +64,7 @@ defmodule CodeCorps.Sync.User.RecordLinkerTest do
       github_comment = insert(:github_comment, github_id: github_id)
       {:ok, %User{} = returned_user} = RecordLinker.link_to(github_comment, comment)
 
-      created_attributes = UserAdapter.to_user(@user_payload)
+      created_attributes = @user_payload |> UserAdapter.to_user() |> remove_nils()
       created_user = Repo.get_by(User, created_attributes)
       assert created_user.id == returned_user.id
     end
@@ -86,7 +92,7 @@ defmodule CodeCorps.Sync.User.RecordLinkerTest do
       github_comment = insert(:github_comment, github_id: github_id)
       {:ok, %User{} = returned_user} = RecordLinker.link_to(github_comment, comment)
 
-      created_attributes = UserAdapter.to_user(@bot_user_payload)
+      created_attributes = @bot_user_payload |> UserAdapter.to_user() |> remove_nils()
       created_user = Repo.get_by(User, created_attributes)
       assert created_user.id == returned_user.id
     end
@@ -142,7 +148,7 @@ defmodule CodeCorps.Sync.User.RecordLinkerTest do
 
     test "returns user by github id if no user by task association found" do
       %{"issue" => %{"number" => number} = issue} = @payload
-      attributes = UserAdapter.to_user(@user_payload)
+      attributes = @user_payload |> UserAdapter.to_user() |> remove_nils()
       preinserted_user = insert(:user, attributes)
       github_issue = insert(:github_issue, number: number)
 
@@ -157,7 +163,7 @@ defmodule CodeCorps.Sync.User.RecordLinkerTest do
       github_issue = insert(:github_issue, number: number)
       {:ok, %User{} = returned_user} = RecordLinker.link_to(github_issue, issue)
 
-      created_attributes = UserAdapter.to_user(@user_payload)
+      created_attributes = @user_payload |> UserAdapter.to_user() |> remove_nils()
       created_user = Repo.get_by(User, created_attributes)
       assert created_user.id == returned_user.id
     end
@@ -191,7 +197,7 @@ defmodule CodeCorps.Sync.User.RecordLinkerTest do
       github_issue = insert(:github_issue, number: number)
       {:ok, %User{} = returned_user} = RecordLinker.link_to(github_issue, issue)
 
-      created_attributes = UserAdapter.to_user(@bot_user_payload)
+      created_attributes =  @bot_user_payload |> UserAdapter.to_user() |> remove_nils()
       created_user = Repo.get_by(User, created_attributes)
       assert created_user.id == returned_user.id
     end

test/lib/code_corps_web/controllers/user_controller_test.exs

@@ -62,6 +62,25 @@ defmodule CodeCorpsWeb.UserControllerTest do
       |> assert_ids_from_response([user_1.id, user_2.id, user_3.id])
     end
 
+    test "returns search result on project filter", %{conn: conn} do
+      user_1 = insert(:user)
+      user_2 = insert(:user)
+
+      project = insert(:project)
+
+      insert(:project_user, user: user_1, project: project)
+      insert(:project_user, user: user_2, project: project)
+      insert(:project_user)
+
+      params = %{"project_id" => project.id}
+      path = conn |> user_path(:index, params)
+
+      conn
+      |> get(path)
+      |> json_response(200)
+      |> assert_ids_from_response([user_1.id, user_2.id])
+    end
+
     test "limit filter limits results on index", %{conn: conn} do
       insert_list(6, :user)
 

test/support/test_helpers.ex

@@ -8,6 +8,10 @@ defmodule CodeCorps.TestHelpers do
     end
   end
 
+  def assert_ids_from_query(query, ids) do
+    assert query |> Enum.map(&Map.get(&1, :id)) |> Enum.sort == ids |> Enum.sort()
+  end
+
   def assert_id_from_response(response, id) do
     assert String.to_integer(response["data"]["id"]) == id
     response