diff --git a/circle.yml b/circle.yml index 9a005620c..024c8a436 100644 --- a/circle.yml +++ b/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: - | diff --git a/lib/code_corps/accounts/changesets.ex b/lib/code_corps/accounts/changesets.ex index 34c7124fe..88a59df25 100644 --- a/lib/code_corps/accounts/changesets.ex +++ b/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) diff --git a/lib/code_corps/accounts/users.ex b/lib/code_corps/accounts/users.ex new file mode 100644 index 000000000..68ad119ff --- /dev/null +++ b/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 \ No newline at end of file diff --git a/lib/code_corps/github/adapters/user.ex b/lib/code_corps/github/adapters/user.ex index aada4984f..2a766907d 100644 --- a/lib/code_corps/github/adapters/user.ex +++ b/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 diff --git a/lib/code_corps_web/controllers/user_controller.ex b/lib/code_corps_web/controllers/user_controller.ex index d463a4297..51a163a71 100644 --- a/lib/code_corps_web/controllers/user_controller.ex +++ b/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() diff --git a/test/lib/code_corps/accounts/changesets_test.exs b/test/lib/code_corps/accounts/changesets_test.exs index 184825552..752924bcb 100644 --- a/test/lib/code_corps/accounts/changesets_test.exs +++ b/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 diff --git a/test/lib/code_corps/accounts/users_test.exs b/test/lib/code_corps/accounts/users_test.exs new file mode 100644 index 000000000..375dc8ce2 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/test/lib/code_corps/github/adapters/user_test.exs b/test/lib/code_corps/github/adapters/user_test.exs index cbee88d22..dafb84028 100644 --- a/test/lib/code_corps/github/adapters/user_test.exs +++ b/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 diff --git a/test/lib/code_corps/github/sync/user/record_linker_test.exs b/test/lib/code_corps/github/sync/user/record_linker_test.exs index 714e11940..c619e26fb 100644 --- a/test/lib/code_corps/github/sync/user/record_linker_test.exs +++ b/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 diff --git a/test/lib/code_corps_web/controllers/user_controller_test.exs b/test/lib/code_corps_web/controllers/user_controller_test.exs index 4cb0d8a6f..08daa529a 100644 --- a/test/lib/code_corps_web/controllers/user_controller_test.exs +++ b/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) diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index 7aa51fe29..661c3a7ef 100644 --- a/test/support/test_helpers.ex +++ b/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