From df090d758c232ef5a67fb56a9f66961ba1df26c3 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Thu, 28 Dec 2017 17:52:26 +0100 Subject: [PATCH 01/24] Add Accounts.create_invite - Add UserInvite schema and model - Add Accounts.UserInvites subcontext - Add Accounts.create_invite tests --- lib/code_corps/accounts/accounts.ex | 5 + lib/code_corps/accounts/user_invites.ex | 15 +++ lib/code_corps/model/user_invite.ex | 17 +++ .../20171228163712_add_user_invites.exs | 17 +++ priv/repo/structure.sql | 81 +++++++++++- .../lib/code_corps/accounts/accounts_test.exs | 121 +++++++++++++++++- 6 files changed, 252 insertions(+), 4 deletions(-) create mode 100644 lib/code_corps/accounts/user_invites.ex create mode 100644 lib/code_corps/model/user_invite.ex create mode 100644 priv/repo/migrations/20171228163712_add_user_invites.exs diff --git a/lib/code_corps/accounts/accounts.ex b/lib/code_corps/accounts/accounts.ex index c8e75eced..da7c6fe9f 100644 --- a/lib/code_corps/accounts/accounts.ex +++ b/lib/code_corps/accounts/accounts.ex @@ -6,6 +6,7 @@ defmodule CodeCorps.Accounts do """ alias CodeCorps.{ + Accounts, Accounts.Changesets, Comment, GitHub.Adapters, @@ -14,6 +15,7 @@ defmodule CodeCorps.Accounts do Processor, Task, User, + UserInvite, Repo } alias Ecto.{Changeset, Multi} @@ -184,4 +186,7 @@ defmodule CodeCorps.Accounts do |> Repo.update_all(updates, update_options) |> (fn {_count, comments} -> {:ok, comments} end).() end + + @spec create_invite(map) :: {:ok, UserInvite.t} | {:error, Changeset.t} + defdelegate create_invite(params), to: Accounts.UserInvites end diff --git a/lib/code_corps/accounts/user_invites.ex b/lib/code_corps/accounts/user_invites.ex new file mode 100644 index 000000000..336fbf371 --- /dev/null +++ b/lib/code_corps/accounts/user_invites.ex @@ -0,0 +1,15 @@ +defmodule CodeCorps.Accounts.UserInvites do + alias CodeCorps.{Repo, UserInvite} + alias Ecto.Changeset + + @spec create_invite(map) :: {:ok, UserInvite.t} | {:error, Changeset.t} + def create_invite(%{} = params) do + %UserInvite{} + |> Changeset.cast(params, [:email, :name, :role, :inviter_id, :project_id]) + |> Changeset.validate_required([:email, :inviter_id]) + |> Changeset.validate_inclusion(:role, ~w(contributor admin owner)) + |> Changeset.assoc_constraint(:inviter) + |> Changeset.assoc_constraint(:project) + |> Repo.insert + end +end diff --git a/lib/code_corps/model/user_invite.ex b/lib/code_corps/model/user_invite.ex new file mode 100644 index 000000000..913fa7c83 --- /dev/null +++ b/lib/code_corps/model/user_invite.ex @@ -0,0 +1,17 @@ +defmodule CodeCorps.UserInvite do + use CodeCorps.Model + + @type t :: %__MODULE__{} + + schema "user_invites" do + field :email, :string, null: false + field :role, :string, default: "contributor" + field :name, :string + + belongs_to :project, CodeCorps.Project + belongs_to :inviter, CodeCorps.User + belongs_to :invitee, CodeCorps.User + + timestamps() + end +end diff --git a/priv/repo/migrations/20171228163712_add_user_invites.exs b/priv/repo/migrations/20171228163712_add_user_invites.exs new file mode 100644 index 000000000..712dead7b --- /dev/null +++ b/priv/repo/migrations/20171228163712_add_user_invites.exs @@ -0,0 +1,17 @@ +defmodule CodeCorps.Repo.Migrations.AddUserInvites do + use Ecto.Migration + + def change do + create table(:user_invites) do + add :email, :string, null: false + add :name, :string, null: true + add :role, :string, null: true, default: "contributor" + + add :inviter_id, references(:users) + add :invitee_id, references(:users) + add :project_id, references(:projects) + + timestamps() + end + end +end diff --git a/priv/repo/structure.sql b/priv/repo/structure.sql index cfe9bc6e9..0269d747f 100644 --- a/priv/repo/structure.sql +++ b/priv/repo/structure.sql @@ -2,8 +2,8 @@ -- PostgreSQL database dump -- --- Dumped from database version 10.1 --- Dumped by pg_dump version 10.1 +-- Dumped from database version 10.0 +-- Dumped by pg_dump version 10.0 SET statement_timeout = 0; SET lock_timeout = 0; @@ -1727,6 +1727,42 @@ CREATE SEQUENCE user_categories_id_seq ALTER SEQUENCE user_categories_id_seq OWNED BY user_categories.id; +-- +-- Name: user_invites; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE user_invites ( + id bigint NOT NULL, + email character varying(255) NOT NULL, + name character varying(255), + role character varying(255) DEFAULT 'contributor'::character varying, + inviter_id bigint, + invitee_id bigint, + project_id bigint, + inserted_at timestamp without time zone NOT NULL, + updated_at timestamp without time zone NOT NULL +); + + +-- +-- Name: user_invites_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE user_invites_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: user_invites_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE user_invites_id_seq OWNED BY user_invites.id; + + -- -- Name: user_roles; Type: TABLE; Schema: public; Owner: - -- @@ -2175,6 +2211,13 @@ ALTER TABLE ONLY tasks ALTER COLUMN id SET DEFAULT nextval('tasks_id_seq'::regcl ALTER TABLE ONLY user_categories ALTER COLUMN id SET DEFAULT nextval('user_categories_id_seq'::regclass); +-- +-- Name: user_invites id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY user_invites ALTER COLUMN id SET DEFAULT nextval('user_invites_id_seq'::regclass); + + -- -- Name: user_roles id; Type: DEFAULT; Schema: public; Owner: - -- @@ -2507,6 +2550,14 @@ ALTER TABLE ONLY user_categories ADD CONSTRAINT user_categories_pkey PRIMARY KEY (id); +-- +-- Name: user_invites user_invites_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY user_invites + ADD CONSTRAINT user_invites_pkey PRIMARY KEY (id); + + -- -- Name: user_roles user_roles_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -4099,6 +4150,30 @@ ALTER TABLE ONLY user_categories ADD CONSTRAINT user_categories_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id); +-- +-- Name: user_invites user_invites_invitee_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY user_invites + ADD CONSTRAINT user_invites_invitee_id_fkey FOREIGN KEY (invitee_id) REFERENCES users(id); + + +-- +-- Name: user_invites user_invites_inviter_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY user_invites + ADD CONSTRAINT user_invites_inviter_id_fkey FOREIGN KEY (inviter_id) REFERENCES users(id); + + +-- +-- Name: user_invites user_invites_project_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY user_invites + ADD CONSTRAINT user_invites_project_id_fkey FOREIGN KEY (project_id) REFERENCES projects(id); + + -- -- Name: user_roles user_roles_role_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -4159,5 +4234,5 @@ ALTER TABLE ONLY users -- PostgreSQL database dump complete -- -INSERT INTO "schema_migrations" (version) VALUES (20160723215749), (20160804000000), (20160804001111), (20160805132301), (20160805203929), (20160808143454), (20160809214736), (20160810124357), (20160815125009), (20160815143002), (20160816020347), (20160816034021), (20160817220118), (20160818000944), (20160818132546), (20160820113856), (20160820164905), (20160822002438), (20160822004056), (20160822011624), (20160822020401), (20160822044612), (20160830081224), (20160830224802), (20160911233738), (20160912002705), (20160912145957), (20160918003206), (20160928232404), (20161003185918), (20161019090945), (20161019110737), (20161020144622), (20161021131026), (20161031001615), (20161121005339), (20161121014050), (20161121043941), (20161121045709), (20161122015942), (20161123081114), (20161123150943), (20161124085742), (20161125200620), (20161126045705), (20161127054559), (20161205024856), (20161207112519), (20161209192504), (20161212005641), (20161214005935), (20161215052051), (20161216051447), (20161218005913), (20161219160401), (20161219163909), (20161220141753), (20161221085759), (20161226213600), (20161231063614), (20170102130055), (20170102181053), (20170104113708), (20170104212623), (20170104235423), (20170106013143), (20170115035159), (20170115230549), (20170121014100), (20170131234029), (20170201014901), (20170201025454), (20170201035458), (20170201183258), (20170220032224), (20170224233516), (20170226050552), (20170228085250), (20170308214128), (20170308220713), (20170308222552), (20170313130611), (20170318032449), (20170318082740), (20170324194827), (20170424215355), (20170501225441), (20170505224222), (20170526095401), (20170602000208), (20170622205732), (20170626231059), (20170628092119), (20170628213609), (20170629183404), (20170630140136), (20170706132431), (20170707213648), (20170711122252), (20170717092127), (20170725060612), (20170727052644), (20170731130121), (20170814131722), (20170913114958), (20170921014405), (20170925214512), (20170925230419), (20170926134646), (20170927100300), (20170928234412), (20171003134956), (20171003225853), (20171006063358), (20171006161407), (20171012215106), (20171012221231), (20171016125229), (20171016125516), (20171016223356), (20171016235656), (20171017235433), (20171019191035), (20171025184225), (20171026010933), (20171027061833), (20171028011642), (20171028173508), (20171030182857), (20171031232023), (20171031234356), (20171101023309), (20171104013543), (20171106045740), (20171106050209), (20171106103153), (20171106200036), (20171109231538), (20171110001134), (20171114010851), (20171114033357), (20171114225214), (20171114225713), (20171114232534), (20171115201624), (20171115225358), (20171119004204), (20171121075226), (20171121144138), (20171123065902), (20171127215847), (20171201073818), (20171205161052), (20171213062707), (20171220154922); +INSERT INTO "schema_migrations" (version) VALUES (20160723215749), (20160804000000), (20160804001111), (20160805132301), (20160805203929), (20160808143454), (20160809214736), (20160810124357), (20160815125009), (20160815143002), (20160816020347), (20160816034021), (20160817220118), (20160818000944), (20160818132546), (20160820113856), (20160820164905), (20160822002438), (20160822004056), (20160822011624), (20160822020401), (20160822044612), (20160830081224), (20160830224802), (20160911233738), (20160912002705), (20160912145957), (20160918003206), (20160928232404), (20161003185918), (20161019090945), (20161019110737), (20161020144622), (20161021131026), (20161031001615), (20161121005339), (20161121014050), (20161121043941), (20161121045709), (20161122015942), (20161123081114), (20161123150943), (20161124085742), (20161125200620), (20161126045705), (20161127054559), (20161205024856), (20161207112519), (20161209192504), (20161212005641), (20161214005935), (20161215052051), (20161216051447), (20161218005913), (20161219160401), (20161219163909), (20161220141753), (20161221085759), (20161226213600), (20161231063614), (20170102130055), (20170102181053), (20170104113708), (20170104212623), (20170104235423), (20170106013143), (20170115035159), (20170115230549), (20170121014100), (20170131234029), (20170201014901), (20170201025454), (20170201035458), (20170201183258), (20170220032224), (20170224233516), (20170226050552), (20170228085250), (20170308214128), (20170308220713), (20170308222552), (20170313130611), (20170318032449), (20170318082740), (20170324194827), (20170424215355), (20170501225441), (20170505224222), (20170526095401), (20170602000208), (20170622205732), (20170626231059), (20170628092119), (20170628213609), (20170629183404), (20170630140136), (20170706132431), (20170707213648), (20170711122252), (20170717092127), (20170725060612), (20170727052644), (20170731130121), (20170814131722), (20170913114958), (20170921014405), (20170925214512), (20170925230419), (20170926134646), (20170927100300), (20170928234412), (20171003134956), (20171003225853), (20171006063358), (20171006161407), (20171012215106), (20171012221231), (20171016125229), (20171016125516), (20171016223356), (20171016235656), (20171017235433), (20171019191035), (20171025184225), (20171026010933), (20171027061833), (20171028011642), (20171028173508), (20171030182857), (20171031232023), (20171031234356), (20171101023309), (20171104013543), (20171106045740), (20171106050209), (20171106103153), (20171106200036), (20171109231538), (20171110001134), (20171114010851), (20171114033357), (20171114225214), (20171114225713), (20171114232534), (20171115201624), (20171115225358), (20171119004204), (20171121075226), (20171121144138), (20171123065902), (20171127215847), (20171201073818), (20171205161052), (20171213062707), (20171220154922), (20171228163712); diff --git a/test/lib/code_corps/accounts/accounts_test.exs b/test/lib/code_corps/accounts/accounts_test.exs index 7c268b29a..6ef68c307 100644 --- a/test/lib/code_corps/accounts/accounts_test.exs +++ b/test/lib/code_corps/accounts/accounts_test.exs @@ -3,7 +3,9 @@ defmodule CodeCorps.AccountsTest do use CodeCorps.DbAccessCase - alias CodeCorps.{Accounts, Comment, Task, User, GitHub.TestHelpers} + alias CodeCorps.{ + Accounts, Comment, Task, GitHub.TestHelpers, User, UserInvite + } alias Ecto.Changeset describe "create_from_github/1" do @@ -120,4 +122,121 @@ defmodule CodeCorps.AccountsTest do assert user.cloudinary_public_id end end + + describe "create_invite/1" do + @base_attrs %{email: "foo@example.com"} + + test "creates a user invite" do + %{id: inviter_id} = insert(:user) + + {:ok, %UserInvite{} = user_invite} = + @base_attrs + |> Map.put(:inviter_id, inviter_id) + |> Accounts.create_invite + + assert Repo.one(UserInvite).id == user_invite.id + end + + test "requires email" do + {:error, changeset} = + @base_attrs + |> Map.delete(:email) + |> Accounts.create_invite + + refute changeset.valid? + assert changeset.errors[:email] + end + + test "requires valid inviter id" do + {:error, changeset} = + @base_attrs + |> Accounts.create_invite + + refute changeset.valid? + assert changeset.errors[:inviter_id] + + {:error, changeset} = + @base_attrs + |> Map.put(:inviter_id, -1) + |> Accounts.create_invite + + refute changeset.valid? + refute changeset.errors[:inviter_id] + assert changeset.errors[:inviter] + end + + test "allows specifying name" do + %{id: inviter_id} = insert(:user) + + {:ok, %UserInvite{} = user_invite} = + @base_attrs + |> Map.put(:inviter_id, inviter_id) + |> Map.put(:name, "John") + |> Accounts.create_invite + + assert user_invite.name == "John" + end + + test "allows specifying role" do + %{id: inviter_id} = insert(:user) + + {:ok, %UserInvite{} = user_invite} = + @base_attrs + |> Map.put(:inviter_id, inviter_id) + |> Map.put(:role, "admin") + |> Accounts.create_invite + + assert user_invite.role == "admin" + end + + test "defaults role to 'contributor'" do + %{id: inviter_id} = insert(:user) + + {:ok, %UserInvite{} = user_invite} = + @base_attrs + |> Map.put(:inviter_id, inviter_id) + |> Accounts.create_invite + + assert user_invite.role == "contributor" + end + + test "does not allow invalid roles" do + %{id: inviter_id} = insert(:user) + + {:error, changeset} = + @base_attrs + |> Map.put(:inviter_id, inviter_id) + |> Map.put(:role, "foo") + |> Accounts.create_invite + + refute changeset.valid? + assert changeset.errors[:role] + end + + test "associates with project if id provided" do + %{id: inviter_id} = insert(:user) + %{id: project_id} = insert(:project) + + {:ok, %UserInvite{} = user_invite} = + @base_attrs + |> Map.put(:inviter_id, inviter_id) + |> Map.put(:project_id, project_id) + |> Accounts.create_invite + + assert user_invite.project_id == project_id + end + + test "requires valid project id" do + %{id: inviter_id} = insert(:user) + + {:error, changeset} = + @base_attrs + |> Map.put(:inviter_id, inviter_id) + |> Map.put(:project_id, -1) + |> Accounts.create_invite + + refute changeset.valid? + assert changeset.errors[:project] + end + end end From 6c6d22e4ae8d44734a3b50528a007b20701f3101 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Thu, 28 Dec 2017 18:45:57 +0100 Subject: [PATCH 02/24] Add Accounts.claim_invite/1 - Add Accounts.claim_invite/1 tests - Add validation step to Accounts.create_invite, to ensure invitee is not a member of the project already --- lib/code_corps/accounts/accounts.ex | 3 + lib/code_corps/accounts/user_invites.ex | 77 ++++++++++++++++++- .../lib/code_corps/accounts/accounts_test.exs | 43 ++++++++++- test/support/factories.ex | 8 ++ 4 files changed, 128 insertions(+), 3 deletions(-) diff --git a/lib/code_corps/accounts/accounts.ex b/lib/code_corps/accounts/accounts.ex index da7c6fe9f..08e9fe455 100644 --- a/lib/code_corps/accounts/accounts.ex +++ b/lib/code_corps/accounts/accounts.ex @@ -189,4 +189,7 @@ defmodule CodeCorps.Accounts do @spec create_invite(map) :: {:ok, UserInvite.t} | {:error, Changeset.t} defdelegate create_invite(params), to: Accounts.UserInvites + + @spec claim_invite(UserInvite.t) :: {:ok, User.t} + defdelegate claim_invite(user_invite), to: Accounts.UserInvites end diff --git a/lib/code_corps/accounts/user_invites.ex b/lib/code_corps/accounts/user_invites.ex index 336fbf371..fa977fb40 100644 --- a/lib/code_corps/accounts/user_invites.ex +++ b/lib/code_corps/accounts/user_invites.ex @@ -1,6 +1,8 @@ defmodule CodeCorps.Accounts.UserInvites do - alias CodeCorps.{Repo, UserInvite} - alias Ecto.Changeset + alias CodeCorps.{Project, ProjectUser, Repo, User, UserInvite} + alias Ecto.{Changeset, Multi} + + import Ecto.Query @spec create_invite(map) :: {:ok, UserInvite.t} | {:error, Changeset.t} def create_invite(%{} = params) do @@ -10,6 +12,77 @@ defmodule CodeCorps.Accounts.UserInvites do |> Changeset.validate_inclusion(:role, ~w(contributor admin owner)) |> Changeset.assoc_constraint(:inviter) |> Changeset.assoc_constraint(:project) + |> ensure_email_not_owned_by_member() |> Repo.insert end + + @spec ensure_email_not_owned_by_member(Changeset.t) :: Changeset.t + defp ensure_email_not_owned_by_member(%Changeset{} = changeset) do + email = changeset |> Changeset.get_change(:email) + project_id = changeset |> Changeset.get_change(:project_id) + + case [email, project_id] do + [nil, _] -> changeset + [_, nil] -> changeset + [email, project_id] -> + count = + ProjectUser + |> where(project_id: ^project_id) + |> join(:inner, [pu], u in User, pu.user_id == u.id) + |> where([_pu, u], u.email == ^email) + |> select([pu, _U], count(pu.id)) + |> Repo.one + + if count > 0 do + changeset |> Changeset.add_error(:email, "Already associated with a project member") + else + changeset + end + end + end + + @spec claim_invite(UserInvite.t) :: {:ok, User.t} + def claim_invite(%UserInvite{} = user_invite) do + Multi.new + |> Multi.run(:user, fn %{} -> user_invite |> claim_user() end) + |> Multi.run(:project_user, fn %{user: user} -> user |> join_project(user_invite) end) + |> Multi.run(:user_invite, fn %{user: user} -> user_invite |> associate_invitee(user) end) + |> Repo.transaction + |> normalize_success() + end + + @spec claim_user(UserInvite.t) :: {:ok, User.t} + defp claim_user(%UserInvite{email: email, name: name}) do + case User |> Repo.get_by(email: email) do + %User{} = user -> {:ok, user} + nil -> %User{} |> Changeset.change(%{email: email, name: name}) |> Repo.insert + end + end + + @spec join_project(User.t, UserInvite.t) :: {:ok, ProjectUser.t} | {:error, Changeset.t} + defp join_project(%User{} = user, %UserInvite{role: role, project: %Project{} = project}) do + case ProjectUser |> Repo.get_by(user_id: user.id, project_id: project.id) do + %ProjectUser{} = project_user -> + {:ok, project_user} + nil -> + %ProjectUser{} + |> Changeset.change(%{role: role}) + |> Changeset.put_assoc(:project, project) + |> Changeset.put_assoc(:user, user) + |> Repo.insert + end + end + defp join_project(%User{}, %UserInvite{}), do: {:ok, nil} + + @spec associate_invitee(UserInvite.t, User.t) :: ProjectUser.t + defp associate_invitee(%UserInvite{invitee: nil} = invite, %User{} = user) do + invite + |> Changeset.change(%{}) + |> Changeset.put_assoc(:invitee, user) + |> Repo.update + end + + @spec normalize_success(tuple) :: tuple + defp normalize_success({:ok, %{user: user}}), do: {:ok, user |> Repo.preload(:project_users)} + defp normalize_success(other_tuple), do: other_tuple end diff --git a/test/lib/code_corps/accounts/accounts_test.exs b/test/lib/code_corps/accounts/accounts_test.exs index 6ef68c307..607e06b7a 100644 --- a/test/lib/code_corps/accounts/accounts_test.exs +++ b/test/lib/code_corps/accounts/accounts_test.exs @@ -4,7 +4,7 @@ defmodule CodeCorps.AccountsTest do use CodeCorps.DbAccessCase alias CodeCorps.{ - Accounts, Comment, Task, GitHub.TestHelpers, User, UserInvite + Accounts, Comment, ProjectUser, Task, GitHub.TestHelpers, User, UserInvite } alias Ecto.Changeset @@ -238,5 +238,46 @@ defmodule CodeCorps.AccountsTest do refute changeset.valid? assert changeset.errors[:project] end + + test "throws error if user is already member of project" do + %{id: inviter_id} = insert(:user) + %{id: project_id} = project = insert(:project) + %{email: email} = user = insert(:user) + + project_user = insert(:project_user, user: user, project: project) + + {:error, changeset} = + %{email: email} + |> Map.put(:inviter_id, inviter_id) + |> Map.put(:project_id, project_id) + |> Accounts.create_invite + + refute changeset.valid? + assert changeset.errors[:email] + end + end + + describe "claim_invite/1" do + test "creates user" do + invite = insert(:user_invite, invitee: nil, project: nil) + + {:ok, %User{} = user} = invite |> Accounts.claim_invite + assert Repo.get(User, user.id) + end + + test "associates invite with user" do + invite = insert(:user_invite, invitee: nil, project: nil) + + {:ok, %User{} = user} = invite |> Accounts.claim_invite + assert Repo.one(UserInvite).invitee_id == user.id + end + + test "creates project membership if project provided" do + project = insert(:project) + invite = insert(:user_invite, invitee: nil, project: project) + + {:ok, %User{} = user} = invite |> Accounts.claim_invite + assert Repo.get_by(ProjectUser, user_id: user.id, project_id: project.id) + end end end diff --git a/test/support/factories.ex b/test/support/factories.ex index 967d20a4e..1f626cce8 100644 --- a/test/support/factories.ex +++ b/test/support/factories.ex @@ -370,6 +370,14 @@ defmodule CodeCorps.Factories do } end + def user_invite_factory do + %CodeCorps.UserInvite{ + inviter: build(:user), + name: sequence(:name, &"Name#{&1}"), + email: sequence(:email, &"email-#{&1}@example.com") + } + end + def user_skill_factory do %CodeCorps.UserSkill{ user: build(:user), From c9ce8fc091ca9784dae0c6d2b4245a825f7af334 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Thu, 28 Dec 2017 19:05:27 +0100 Subject: [PATCH 03/24] Switch Accounts.claim_invite to accept a params map --- lib/code_corps/accounts/accounts.ex | 4 +- lib/code_corps/accounts/user_invites.ex | 40 ++++++++++++------- .../lib/code_corps/accounts/accounts_test.exs | 29 ++++++++++++-- 3 files changed, 53 insertions(+), 20 deletions(-) diff --git a/lib/code_corps/accounts/accounts.ex b/lib/code_corps/accounts/accounts.ex index 08e9fe455..2a9a21b7e 100644 --- a/lib/code_corps/accounts/accounts.ex +++ b/lib/code_corps/accounts/accounts.ex @@ -190,6 +190,6 @@ defmodule CodeCorps.Accounts do @spec create_invite(map) :: {:ok, UserInvite.t} | {:error, Changeset.t} defdelegate create_invite(params), to: Accounts.UserInvites - @spec claim_invite(UserInvite.t) :: {:ok, User.t} - defdelegate claim_invite(user_invite), to: Accounts.UserInvites + @spec claim_invite(map) :: {:ok, User.t} + defdelegate claim_invite(map), to: Accounts.UserInvites end diff --git a/lib/code_corps/accounts/user_invites.ex b/lib/code_corps/accounts/user_invites.ex index fa977fb40..09c3a7014 100644 --- a/lib/code_corps/accounts/user_invites.ex +++ b/lib/code_corps/accounts/user_invites.ex @@ -41,23 +41,34 @@ defmodule CodeCorps.Accounts.UserInvites do end end - @spec claim_invite(UserInvite.t) :: {:ok, User.t} - def claim_invite(%UserInvite{} = user_invite) do + @spec claim_invite(map) :: {:ok, User.t} + def claim_invite(%{} = params) do Multi.new - |> Multi.run(:user, fn %{} -> user_invite |> claim_user() end) - |> Multi.run(:project_user, fn %{user: user} -> user |> join_project(user_invite) end) - |> Multi.run(:user_invite, fn %{user: user} -> user_invite |> associate_invitee(user) end) + |> Multi.run(:load_invite, fn %{} -> params |> load_invite() end) + |> Multi.run(:user, fn %{} -> params |> claim_new_user() end) + |> Multi.run(:project_user, fn %{user: user, load_invite: user_invite} -> + user |> join_project(user_invite) + end) + |> Multi.run(:user_invite, fn %{user: user, load_invite: user_invite} -> + user_invite |> associate_invitee(user) + end) |> Repo.transaction - |> normalize_success() + |> marshall_response() end - @spec claim_user(UserInvite.t) :: {:ok, User.t} - defp claim_user(%UserInvite{email: email, name: name}) do - case User |> Repo.get_by(email: email) do - %User{} = user -> {:ok, user} - nil -> %User{} |> Changeset.change(%{email: email, name: name}) |> Repo.insert + @spec load_invite(map) :: {:ok, UserInvite.t} | {:error, :not_found} + defp load_invite(%{"invite_id" => invite_id}) do + case UserInvite |> Repo.get(invite_id) |> Repo.preload([:invitee, :project]) do + nil -> {:error, :not_found} + %UserInvite{} = invite -> {:ok, invite} end end + defp load_invite(%{}), do: {:error, :not_found} + + @spec claim_new_user(map) :: {:ok, User.t} + defp claim_new_user(%{} = params) do + %User{} |> User.registration_changeset(params) |> Repo.insert + end @spec join_project(User.t, UserInvite.t) :: {:ok, ProjectUser.t} | {:error, Changeset.t} defp join_project(%User{} = user, %UserInvite{role: role, project: %Project{} = project}) do @@ -82,7 +93,8 @@ defmodule CodeCorps.Accounts.UserInvites do |> Repo.update end - @spec normalize_success(tuple) :: tuple - defp normalize_success({:ok, %{user: user}}), do: {:ok, user |> Repo.preload(:project_users)} - defp normalize_success(other_tuple), do: other_tuple + @spec marshall_response(tuple) :: tuple + defp marshall_response({:ok, %{user: user}}), do: {:ok, user |> Repo.preload(:project_users)} + defp marshall_response({:error, :load_invite, :not_found, _}), do: {:error, :invite_not_found} + defp marshall_response(other_tuple), do: other_tuple end diff --git a/test/lib/code_corps/accounts/accounts_test.exs b/test/lib/code_corps/accounts/accounts_test.exs index 607e06b7a..7916bcef2 100644 --- a/test/lib/code_corps/accounts/accounts_test.exs +++ b/test/lib/code_corps/accounts/accounts_test.exs @@ -244,7 +244,7 @@ defmodule CodeCorps.AccountsTest do %{id: project_id} = project = insert(:project) %{email: email} = user = insert(:user) - project_user = insert(:project_user, user: user, project: project) + insert(:project_user, user: user, project: project) {:error, changeset} = %{email: email} @@ -258,17 +258,28 @@ defmodule CodeCorps.AccountsTest do end describe "claim_invite/1" do + @valid_user_params %{ + "email" => "test@user.com", + "password" => "somepassword", + "username" => "testuser" + } test "creates user" do invite = insert(:user_invite, invitee: nil, project: nil) - {:ok, %User{} = user} = invite |> Accounts.claim_invite + {:ok, %User{} = user} = + @valid_user_params + |> Map.put("invite_id", invite.id) + |> Accounts.claim_invite assert Repo.get(User, user.id) end test "associates invite with user" do invite = insert(:user_invite, invitee: nil, project: nil) - {:ok, %User{} = user} = invite |> Accounts.claim_invite + {:ok, %User{} = user} = + @valid_user_params + |> Map.put("invite_id", invite.id) + |> Accounts.claim_invite assert Repo.one(UserInvite).invitee_id == user.id end @@ -276,8 +287,18 @@ defmodule CodeCorps.AccountsTest do project = insert(:project) invite = insert(:user_invite, invitee: nil, project: project) - {:ok, %User{} = user} = invite |> Accounts.claim_invite + {:ok, %User{} = user} = + @valid_user_params + |> Map.put("invite_id", invite.id) + |> Accounts.claim_invite assert Repo.get_by(ProjectUser, user_id: user.id, project_id: project.id) end + + test "returns :invite_not_found if bad id provided" do + assert {:error, :invite_not_found} = + @valid_user_params + |> Map.put("invite_id", -1) + |> Accounts.claim_invite + end end end From bf72b97edc0172fd0db5f04bf467371318dae984 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Fri, 29 Dec 2017 18:37:33 +0100 Subject: [PATCH 04/24] Use ProjectUser as provider of valid roles for UserInvite validation --- lib/code_corps/accounts/user_invites.ex | 41 +++++++++++++++---------- lib/code_corps/model/project_user.ex | 15 +++++---- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/lib/code_corps/accounts/user_invites.ex b/lib/code_corps/accounts/user_invites.ex index 09c3a7014..8c733e556 100644 --- a/lib/code_corps/accounts/user_invites.ex +++ b/lib/code_corps/accounts/user_invites.ex @@ -4,26 +4,30 @@ defmodule CodeCorps.Accounts.UserInvites do import Ecto.Query - @spec create_invite(map) :: {:ok, UserInvite.t} | {:error, Changeset.t} + @spec create_invite(map) :: {:ok, UserInvite.t()} | {:error, Changeset.t()} def create_invite(%{} = params) do %UserInvite{} |> Changeset.cast(params, [:email, :name, :role, :inviter_id, :project_id]) |> Changeset.validate_required([:email, :inviter_id]) - |> Changeset.validate_inclusion(:role, ~w(contributor admin owner)) + |> Changeset.validate_inclusion(:role, ProjectUser.roles()) |> Changeset.assoc_constraint(:inviter) |> Changeset.assoc_constraint(:project) |> ensure_email_not_owned_by_member() - |> Repo.insert + |> Repo.insert() end - @spec ensure_email_not_owned_by_member(Changeset.t) :: Changeset.t + @spec ensure_email_not_owned_by_member(Changeset.t()) :: Changeset.t() defp ensure_email_not_owned_by_member(%Changeset{} = changeset) do email = changeset |> Changeset.get_change(:email) project_id = changeset |> Changeset.get_change(:project_id) case [email, project_id] do - [nil, _] -> changeset - [_, nil] -> changeset + [nil, _] -> + changeset + + [_, nil] -> + changeset + [email, project_id] -> count = ProjectUser @@ -31,7 +35,7 @@ defmodule CodeCorps.Accounts.UserInvites do |> join(:inner, [pu], u in User, pu.user_id == u.id) |> where([_pu, u], u.email == ^email) |> select([pu, _U], count(pu.id)) - |> Repo.one + |> Repo.one() if count > 0 do changeset |> Changeset.add_error(:email, "Already associated with a project member") @@ -41,9 +45,9 @@ defmodule CodeCorps.Accounts.UserInvites do end end - @spec claim_invite(map) :: {:ok, User.t} + @spec claim_invite(map) :: {:ok, User.t()} def claim_invite(%{} = params) do - Multi.new + Multi.new() |> Multi.run(:load_invite, fn %{} -> params |> load_invite() end) |> Multi.run(:user, fn %{} -> params |> claim_new_user() end) |> Multi.run(:project_user, fn %{user: user, load_invite: user_invite} -> @@ -52,45 +56,48 @@ defmodule CodeCorps.Accounts.UserInvites do |> Multi.run(:user_invite, fn %{user: user, load_invite: user_invite} -> user_invite |> associate_invitee(user) end) - |> Repo.transaction + |> Repo.transaction() |> marshall_response() end - @spec load_invite(map) :: {:ok, UserInvite.t} | {:error, :not_found} + @spec load_invite(map) :: {:ok, UserInvite.t()} | {:error, :not_found} defp load_invite(%{"invite_id" => invite_id}) do case UserInvite |> Repo.get(invite_id) |> Repo.preload([:invitee, :project]) do nil -> {:error, :not_found} %UserInvite{} = invite -> {:ok, invite} end end + defp load_invite(%{}), do: {:error, :not_found} - @spec claim_new_user(map) :: {:ok, User.t} + @spec claim_new_user(map) :: {:ok, User.t()} defp claim_new_user(%{} = params) do - %User{} |> User.registration_changeset(params) |> Repo.insert + %User{} |> User.registration_changeset(params) |> Repo.insert() end - @spec join_project(User.t, UserInvite.t) :: {:ok, ProjectUser.t} | {:error, Changeset.t} + @spec join_project(User.t(), UserInvite.t()) :: {:ok, ProjectUser.t()} | {:error, Changeset.t()} defp join_project(%User{} = user, %UserInvite{role: role, project: %Project{} = project}) do case ProjectUser |> Repo.get_by(user_id: user.id, project_id: project.id) do %ProjectUser{} = project_user -> {:ok, project_user} + nil -> %ProjectUser{} |> Changeset.change(%{role: role}) |> Changeset.put_assoc(:project, project) |> Changeset.put_assoc(:user, user) - |> Repo.insert + |> Repo.insert() end end + defp join_project(%User{}, %UserInvite{}), do: {:ok, nil} - @spec associate_invitee(UserInvite.t, User.t) :: ProjectUser.t + @spec associate_invitee(UserInvite.t(), User.t()) :: ProjectUser.t() defp associate_invitee(%UserInvite{invitee: nil} = invite, %User{} = user) do invite |> Changeset.change(%{}) |> Changeset.put_assoc(:invitee, user) - |> Repo.update + |> Repo.update() end @spec marshall_response(tuple) :: tuple diff --git a/lib/code_corps/model/project_user.ex b/lib/code_corps/model/project_user.ex index b99ae388d..96ebd0c35 100644 --- a/lib/code_corps/model/project_user.ex +++ b/lib/code_corps/model/project_user.ex @@ -8,15 +8,14 @@ defmodule CodeCorps.ProjectUser do @type t :: %__MODULE__{} schema "project_users" do - field :role, :string + field(:role, :string) - belongs_to :project, CodeCorps.Project - belongs_to :user, CodeCorps.User + belongs_to(:project, CodeCorps.Project) + belongs_to(:user, CodeCorps.User) timestamps() end - @doc """ Builds a changeset to create a pending membership """ @@ -55,7 +54,11 @@ defmodule CodeCorps.ProjectUser do |> validate_inclusion(:role, roles()) end - defp roles do - ~w{ pending contributor admin owner } + @doc ~S""" + Returns supported project user role types + """ + @spec roles :: list(String.t()) + def roles do + ~w{pending contributor admin owner} end end From 3aa1a0f46b79871e8de3dceab70b9be605feb68b Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Fri, 29 Dec 2017 18:44:15 +0100 Subject: [PATCH 05/24] Remove default "contributor" role from UserInvite --- lib/code_corps/model/user_invite.ex | 2 +- .../20171228163712_add_user_invites.exs | 12 ++++++------ priv/repo/structure.sql | 2 +- test/lib/code_corps/accounts/accounts_test.exs | 15 ++------------- 4 files changed, 10 insertions(+), 21 deletions(-) diff --git a/lib/code_corps/model/user_invite.ex b/lib/code_corps/model/user_invite.ex index 913fa7c83..391430b48 100644 --- a/lib/code_corps/model/user_invite.ex +++ b/lib/code_corps/model/user_invite.ex @@ -5,7 +5,7 @@ defmodule CodeCorps.UserInvite do schema "user_invites" do field :email, :string, null: false - field :role, :string, default: "contributor" + field :role, :string field :name, :string belongs_to :project, CodeCorps.Project diff --git a/priv/repo/migrations/20171228163712_add_user_invites.exs b/priv/repo/migrations/20171228163712_add_user_invites.exs index 712dead7b..9e29a2809 100644 --- a/priv/repo/migrations/20171228163712_add_user_invites.exs +++ b/priv/repo/migrations/20171228163712_add_user_invites.exs @@ -3,13 +3,13 @@ defmodule CodeCorps.Repo.Migrations.AddUserInvites do def change do create table(:user_invites) do - add :email, :string, null: false - add :name, :string, null: true - add :role, :string, null: true, default: "contributor" + add(:email, :string, null: false) + add(:name, :string, null: true) + add(:role, :string, null: true) - add :inviter_id, references(:users) - add :invitee_id, references(:users) - add :project_id, references(:projects) + add(:inviter_id, references(:users)) + add(:invitee_id, references(:users)) + add(:project_id, references(:projects)) timestamps() end diff --git a/priv/repo/structure.sql b/priv/repo/structure.sql index 0269d747f..8883dd3e4 100644 --- a/priv/repo/structure.sql +++ b/priv/repo/structure.sql @@ -1735,7 +1735,7 @@ CREATE TABLE user_invites ( id bigint NOT NULL, email character varying(255) NOT NULL, name character varying(255), - role character varying(255) DEFAULT 'contributor'::character varying, + role character varying(255), inviter_id bigint, invitee_id bigint, project_id bigint, diff --git a/test/lib/code_corps/accounts/accounts_test.exs b/test/lib/code_corps/accounts/accounts_test.exs index 7916bcef2..7531326d6 100644 --- a/test/lib/code_corps/accounts/accounts_test.exs +++ b/test/lib/code_corps/accounts/accounts_test.exs @@ -189,17 +189,6 @@ defmodule CodeCorps.AccountsTest do assert user_invite.role == "admin" end - test "defaults role to 'contributor'" do - %{id: inviter_id} = insert(:user) - - {:ok, %UserInvite{} = user_invite} = - @base_attrs - |> Map.put(:inviter_id, inviter_id) - |> Accounts.create_invite - - assert user_invite.role == "contributor" - end - test "does not allow invalid roles" do %{id: inviter_id} = insert(:user) @@ -285,13 +274,13 @@ defmodule CodeCorps.AccountsTest do test "creates project membership if project provided" do project = insert(:project) - invite = insert(:user_invite, invitee: nil, project: project) + invite = insert(:user_invite, invitee: nil, project: project, role: "admin") {:ok, %User{} = user} = @valid_user_params |> Map.put("invite_id", invite.id) |> Accounts.claim_invite - assert Repo.get_by(ProjectUser, user_id: user.id, project_id: project.id) + assert Repo.get_by(ProjectUser, user_id: user.id, project_id: project.id, role: "admin") end test "returns :invite_not_found if bad id provided" do From 9e137cafcbc099290f2e2b803396208aaae3396a Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Fri, 29 Dec 2017 19:12:33 +0100 Subject: [PATCH 06/24] Add tests for project user invite --- lib/code_corps/accounts/user_invites.ex | 28 ++++++ .../lib/code_corps/accounts/accounts_test.exs | 88 +++++++++++++------ 2 files changed, 87 insertions(+), 29 deletions(-) diff --git a/lib/code_corps/accounts/user_invites.ex b/lib/code_corps/accounts/user_invites.ex index 8c733e556..c40f09e15 100644 --- a/lib/code_corps/accounts/user_invites.ex +++ b/lib/code_corps/accounts/user_invites.ex @@ -1,4 +1,8 @@ defmodule CodeCorps.Accounts.UserInvites do + @moduledoc ~S""" + Subcontext for managing of `UserInvite` records + """ + alias CodeCorps.{Project, ProjectUser, Repo, User, UserInvite} alias Ecto.{Changeset, Multi} @@ -13,6 +17,7 @@ defmodule CodeCorps.Accounts.UserInvites do |> Changeset.assoc_constraint(:inviter) |> Changeset.assoc_constraint(:project) |> ensure_email_not_owned_by_member() + |> ensure_role_and_project() |> Repo.insert() end @@ -45,6 +50,29 @@ defmodule CodeCorps.Accounts.UserInvites do end end + @spec ensure_role_and_project(Changeset.t()) :: Changeset.t() + defp ensure_role_and_project(%Changeset{} = changeset) do + changes = [ + changeset |> Changeset.get_field(:role), + changeset |> Changeset.get_field(:project_id) + ] + + case changes do + [nil, nil] -> + changeset + + [nil, _project_id] -> + changeset |> Changeset.add_error(:role, "Needs to be specified for a project invite") + + [_role, nil] -> + changeset + |> Changeset.add_error(:project_id, "Needs to be specified for a project invite") + + [_role, _project_id] -> + changeset + end + end + @spec claim_invite(map) :: {:ok, User.t()} def claim_invite(%{} = params) do Multi.new() diff --git a/test/lib/code_corps/accounts/accounts_test.exs b/test/lib/code_corps/accounts/accounts_test.exs index 7531326d6..47efe09ef 100644 --- a/test/lib/code_corps/accounts/accounts_test.exs +++ b/test/lib/code_corps/accounts/accounts_test.exs @@ -4,16 +4,23 @@ defmodule CodeCorps.AccountsTest do use CodeCorps.DbAccessCase alias CodeCorps.{ - Accounts, Comment, ProjectUser, Task, GitHub.TestHelpers, User, UserInvite + Accounts, + Comment, + ProjectUser, + Task, + GitHub.TestHelpers, + User, + UserInvite } + alias Ecto.Changeset describe "create_from_github/1" do test "creates proper user from provided payload" do {:ok, %User{} = user} = "user" - |> TestHelpers.load_endpoint_fixture - |> Accounts.create_from_github + |> TestHelpers.load_endpoint_fixture() + |> Accounts.create_from_github() assert user.id assert user.default_color @@ -29,7 +36,7 @@ defmodule CodeCorps.AccountsTest do {:error, %Changeset{} = changeset} = payload - |> Accounts.create_from_github + |> Accounts.create_from_github() assert changeset.errors[:email] == {"has already been taken", []} end @@ -42,7 +49,7 @@ defmodule CodeCorps.AccountsTest do {:error, %Changeset{} = changeset} = payload - |> Accounts.create_from_github + |> Accounts.create_from_github() assert changeset.errors[:github_id] == {"account is already connected to someone else", []} end @@ -50,8 +57,8 @@ defmodule CodeCorps.AccountsTest do test "uploads photo from GitHub avatar" do {:ok, %User{} = user} = "user" - |> TestHelpers.load_endpoint_fixture - |> Accounts.create_from_github + |> TestHelpers.load_endpoint_fixture() + |> Accounts.create_from_github() user = Repo.get(User, user.id) assert user.cloudinary_public_id @@ -132,7 +139,7 @@ defmodule CodeCorps.AccountsTest do {:ok, %UserInvite{} = user_invite} = @base_attrs |> Map.put(:inviter_id, inviter_id) - |> Accounts.create_invite + |> Accounts.create_invite() assert Repo.one(UserInvite).id == user_invite.id end @@ -141,7 +148,7 @@ defmodule CodeCorps.AccountsTest do {:error, changeset} = @base_attrs |> Map.delete(:email) - |> Accounts.create_invite + |> Accounts.create_invite() refute changeset.valid? assert changeset.errors[:email] @@ -150,7 +157,7 @@ defmodule CodeCorps.AccountsTest do test "requires valid inviter id" do {:error, changeset} = @base_attrs - |> Accounts.create_invite + |> Accounts.create_invite() refute changeset.valid? assert changeset.errors[:inviter_id] @@ -158,7 +165,7 @@ defmodule CodeCorps.AccountsTest do {:error, changeset} = @base_attrs |> Map.put(:inviter_id, -1) - |> Accounts.create_invite + |> Accounts.create_invite() refute changeset.valid? refute changeset.errors[:inviter_id] @@ -172,63 +179,83 @@ defmodule CodeCorps.AccountsTest do @base_attrs |> Map.put(:inviter_id, inviter_id) |> Map.put(:name, "John") - |> Accounts.create_invite + |> Accounts.create_invite() assert user_invite.name == "John" end - test "allows specifying role" do + test "creates a user invite for a project" do %{id: inviter_id} = insert(:user) + %{id: project_id} = insert(:project) {:ok, %UserInvite{} = user_invite} = @base_attrs |> Map.put(:inviter_id, inviter_id) |> Map.put(:role, "admin") - |> Accounts.create_invite + |> Map.put(:project_id, project_id) + |> Accounts.create_invite() assert user_invite.role == "admin" + assert user_invite.project_id == project_id end test "does not allow invalid roles" do %{id: inviter_id} = insert(:user) + %{id: project_id} = insert(:project) {:error, changeset} = @base_attrs |> Map.put(:inviter_id, inviter_id) |> Map.put(:role, "foo") - |> Accounts.create_invite + |> Map.put(:project_id, project_id) + |> Accounts.create_invite() refute changeset.valid? assert changeset.errors[:role] end - test "associates with project if id provided" do + test "requires valid project id" do + %{id: inviter_id} = insert(:user) + + {:error, changeset} = + @base_attrs + |> Map.put(:inviter_id, inviter_id) + |> Map.put(:project_id, -1) + |> Map.put(:role, "contributor") + |> Accounts.create_invite() + + refute changeset.valid? + assert changeset.errors[:project] + end + + test "requires role if project is specified" do %{id: inviter_id} = insert(:user) %{id: project_id} = insert(:project) - {:ok, %UserInvite{} = user_invite} = + {:error, changeset} = @base_attrs |> Map.put(:inviter_id, inviter_id) |> Map.put(:project_id, project_id) - |> Accounts.create_invite + |> Accounts.create_invite() - assert user_invite.project_id == project_id + refute changeset.valid? + assert changeset.errors[:role] end - test "requires valid project id" do + test "requires project_id if role is specified" do %{id: inviter_id} = insert(:user) {:error, changeset} = @base_attrs |> Map.put(:inviter_id, inviter_id) - |> Map.put(:project_id, -1) - |> Accounts.create_invite + |> Map.put(:role, "contributor") + |> Accounts.create_invite() refute changeset.valid? - assert changeset.errors[:project] + assert changeset.errors[:project_id] end - test "throws error if user is already member of project" do + test "requires user not to be member of project" do %{id: inviter_id} = insert(:user) %{id: project_id} = project = insert(:project) %{email: email} = user = insert(:user) @@ -239,7 +266,7 @@ defmodule CodeCorps.AccountsTest do %{email: email} |> Map.put(:inviter_id, inviter_id) |> Map.put(:project_id, project_id) - |> Accounts.create_invite + |> Accounts.create_invite() refute changeset.valid? assert changeset.errors[:email] @@ -258,7 +285,8 @@ defmodule CodeCorps.AccountsTest do {:ok, %User{} = user} = @valid_user_params |> Map.put("invite_id", invite.id) - |> Accounts.claim_invite + |> Accounts.claim_invite() + assert Repo.get(User, user.id) end @@ -268,7 +296,8 @@ defmodule CodeCorps.AccountsTest do {:ok, %User{} = user} = @valid_user_params |> Map.put("invite_id", invite.id) - |> Accounts.claim_invite + |> Accounts.claim_invite() + assert Repo.one(UserInvite).invitee_id == user.id end @@ -279,7 +308,8 @@ defmodule CodeCorps.AccountsTest do {:ok, %User{} = user} = @valid_user_params |> Map.put("invite_id", invite.id) - |> Accounts.claim_invite + |> Accounts.claim_invite() + assert Repo.get_by(ProjectUser, user_id: user.id, project_id: project.id, role: "admin") end @@ -287,7 +317,7 @@ defmodule CodeCorps.AccountsTest do assert {:error, :invite_not_found} = @valid_user_params |> Map.put("invite_id", -1) - |> Accounts.claim_invite + |> Accounts.claim_invite() end end end From 21fe4d482fcc6fadb6ad6a91aa6197f8fc730167 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Tue, 2 Jan 2018 17:54:44 +0100 Subject: [PATCH 07/24] Update user requirement on create to prevent invite of existing users --- lib/code_corps/accounts/user_invites.ex | 39 +++++-------------- .../lib/code_corps/accounts/accounts_test.exs | 9 ++--- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/lib/code_corps/accounts/user_invites.ex b/lib/code_corps/accounts/user_invites.ex index c40f09e15..7920a8dc2 100644 --- a/lib/code_corps/accounts/user_invites.ex +++ b/lib/code_corps/accounts/user_invites.ex @@ -6,8 +6,6 @@ defmodule CodeCorps.Accounts.UserInvites do alias CodeCorps.{Project, ProjectUser, Repo, User, UserInvite} alias Ecto.{Changeset, Multi} - import Ecto.Query - @spec create_invite(map) :: {:ok, UserInvite.t()} | {:error, Changeset.t()} def create_invite(%{} = params) do %UserInvite{} @@ -16,40 +14,23 @@ defmodule CodeCorps.Accounts.UserInvites do |> Changeset.validate_inclusion(:role, ProjectUser.roles()) |> Changeset.assoc_constraint(:inviter) |> Changeset.assoc_constraint(:project) - |> ensure_email_not_owned_by_member() + |> ensure_email_not_owned_by_user() |> ensure_role_and_project() |> Repo.insert() end - @spec ensure_email_not_owned_by_member(Changeset.t()) :: Changeset.t() - defp ensure_email_not_owned_by_member(%Changeset{} = changeset) do - email = changeset |> Changeset.get_change(:email) - project_id = changeset |> Changeset.get_change(:project_id) - - case [email, project_id] do - [nil, _] -> - changeset - - [_, nil] -> - changeset - - [email, project_id] -> - count = - ProjectUser - |> where(project_id: ^project_id) - |> join(:inner, [pu], u in User, pu.user_id == u.id) - |> where([_pu, u], u.email == ^email) - |> select([pu, _U], count(pu.id)) - |> Repo.one() - - if count > 0 do - changeset |> Changeset.add_error(:email, "Already associated with a project member") - else - changeset - end + @spec ensure_email_not_owned_by_user(Changeset.t()) :: Changeset.t() + defp ensure_email_not_owned_by_user(%Changeset{changes: %{email: email}} = changeset) + when not is_nil(email) do + if User |> Repo.get_by(email: email) do + changeset |> Changeset.add_error(:email, "Already associated with a user") + else + changeset end end + defp ensure_email_not_owned_by_user(%Changeset{} = changeset), do: changeset + @spec ensure_role_and_project(Changeset.t()) :: Changeset.t() defp ensure_role_and_project(%Changeset{} = changeset) do changes = [ diff --git a/test/lib/code_corps/accounts/accounts_test.exs b/test/lib/code_corps/accounts/accounts_test.exs index 47efe09ef..a4064ae45 100644 --- a/test/lib/code_corps/accounts/accounts_test.exs +++ b/test/lib/code_corps/accounts/accounts_test.exs @@ -255,17 +255,13 @@ defmodule CodeCorps.AccountsTest do assert changeset.errors[:project_id] end - test "requires user not to be member of project" do + test "requires user not to be registered with email" do %{id: inviter_id} = insert(:user) - %{id: project_id} = project = insert(:project) - %{email: email} = user = insert(:user) - - insert(:project_user, user: user, project: project) + %{email: email} = insert(:user) {:error, changeset} = %{email: email} |> Map.put(:inviter_id, inviter_id) - |> Map.put(:project_id, project_id) |> Accounts.create_invite() refute changeset.valid? @@ -279,6 +275,7 @@ defmodule CodeCorps.AccountsTest do "password" => "somepassword", "username" => "testuser" } + test "creates user" do invite = insert(:user_invite, invitee: nil, project: nil) From 00d264668546ebb74930fd76bd93995d17516ed5 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Tue, 2 Jan 2018 18:14:24 +0100 Subject: [PATCH 08/24] Add invite claim support to user controller --- .../controllers/user_controller.ex | 7 ++++ .../controllers/user_controller_test.exs | 41 ++++++++++++++++++- test/support/test_helpers.ex | 2 +- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/lib/code_corps_web/controllers/user_controller.ex b/lib/code_corps_web/controllers/user_controller.ex index 51a163a71..8f5e03cdc 100644 --- a/lib/code_corps_web/controllers/user_controller.ex +++ b/lib/code_corps_web/controllers/user_controller.ex @@ -36,6 +36,13 @@ defmodule CodeCorpsWeb.UserController do end @spec create(Conn.t, map) :: Conn.t + def create(%Conn{} = conn, %{"invite_id" => _} = params) do + with {:ok, %User{} = user} <- Accounts.claim_invite(params), + user <- preload(user) + do + conn |> put_status(:created) |> render("show.json-api", data: user) + end + end def create(%Conn{} = conn, %{} = params) do with {:ok, %User{} = user} <- %User{} |> User.registration_changeset(params) |> Repo.insert(), user <- preload(user) 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 08daa529a..244020525 100644 --- a/test/lib/code_corps_web/controllers/user_controller_test.exs +++ b/test/lib/code_corps_web/controllers/user_controller_test.exs @@ -5,7 +5,7 @@ defmodule CodeCorpsWeb.UserControllerTest do import CodeCorps.GitHub.TestHelpers - alias CodeCorps.{User, Repo} + alias CodeCorps.{User, UserInvite, ProjectUser, Repo} @valid_attrs %{ email: "test@user.com", @@ -147,6 +147,45 @@ defmodule CodeCorpsWeb.UserControllerTest do assert conn |> json_response(422) end + + test "supports claiming a user invite if specified by an id parameter", %{conn: conn} do + %{id: invite_id, email: email} = insert(:user_invite) + attrs = + @valid_attrs + |> Map.merge(%{email: email, invite_id: invite_id, password: "password"}) + + path = conn |> user_path(:create) + json = conn |> post(path, attrs) |> json_response(201) + + created_user = Repo.get_by(User, email: email) + assert created_user + json |> assert_id_from_response(created_user.id) + + updated_invite = Repo.get(UserInvite, invite_id) + assert updated_invite.invitee_id == created_user.id + end + + test "supports claiming a user invite to a project if specified by an id parameter", %{conn: conn} do + project = insert(:project) + %{id: invite_id, email: email} = invite = insert(:user_invite, role: "contributor", project: project) + attrs = + @valid_attrs + |> Map.merge(%{email: email, invite_id: invite_id, password: "password"}) + + path = conn |> user_path(:create) + json = conn |> post(path, attrs) |> json_response(201) + + created_user = Repo.get_by(User, email: email) + assert created_user + json |> assert_id_from_response(created_user.id) + + updated_invite = Repo.get(UserInvite, invite_id) + assert updated_invite.invitee_id == created_user.id + + membership = Repo.get_by(ProjectUser, user_id: created_user.id) + assert membership.project_id == invite.project_id + assert membership.role == invite.role + end end describe "update" do diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index 661c3a7ef..ae8b70d43 100644 --- a/test/support/test_helpers.ex +++ b/test/support/test_helpers.ex @@ -13,7 +13,7 @@ defmodule CodeCorps.TestHelpers do end def assert_id_from_response(response, id) do - assert String.to_integer(response["data"]["id"]) == id + assert (String.to_integer(response["data"]["id"]) == id) response end From 64246b9f492eb7e06052fe4d1e805cca1b5928e8 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Tue, 2 Jan 2018 18:29:46 +0100 Subject: [PATCH 09/24] Refactor account creation by extracting into Accounts context --- lib/code_corps/accounts/accounts.ex | 18 ++- .../controllers/user_controller.ex | 15 +-- .../lib/code_corps/accounts/accounts_test.exs | 115 ++++++++++-------- 3 files changed, 85 insertions(+), 63 deletions(-) diff --git a/lib/code_corps/accounts/accounts.ex b/lib/code_corps/accounts/accounts.ex index 2a9a21b7e..c5c622d61 100644 --- a/lib/code_corps/accounts/accounts.ex +++ b/lib/code_corps/accounts/accounts.ex @@ -22,6 +22,21 @@ defmodule CodeCorps.Accounts do import Ecto.Query + @doc ~S""" + Creates a `CodeCorps.User` account when such a request is made from the client + application. + + If an `invite_id` attribute is provided in the request, treats the process as + claiming the specified invite. + """ + @spec create(map) :: {:ok, User.t} | {:error, Changeset.t} | {:error, :invite_not_found} + def create(%{"invite_id" => _} = params) do + params |> Accounts.UserInvites.claim_invite() + end + def create(%{} = params) do + %User{} |> User.registration_changeset(params) |> Repo.insert() + end + @doc ~S""" Creates a user record using attributes from a GitHub payload. """ @@ -189,7 +204,4 @@ defmodule CodeCorps.Accounts do @spec create_invite(map) :: {:ok, UserInvite.t} | {:error, Changeset.t} defdelegate create_invite(params), to: Accounts.UserInvites - - @spec claim_invite(map) :: {:ok, User.t} - defdelegate claim_invite(map), to: Accounts.UserInvites end diff --git a/lib/code_corps_web/controllers/user_controller.ex b/lib/code_corps_web/controllers/user_controller.ex index 8f5e03cdc..68c04501a 100644 --- a/lib/code_corps_web/controllers/user_controller.ex +++ b/lib/code_corps_web/controllers/user_controller.ex @@ -36,18 +36,11 @@ defmodule CodeCorpsWeb.UserController do end @spec create(Conn.t, map) :: Conn.t - def create(%Conn{} = conn, %{"invite_id" => _} = params) do - with {:ok, %User{} = user} <- Accounts.claim_invite(params), - user <- preload(user) - do - conn |> put_status(:created) |> render("show.json-api", data: user) - end - end def create(%Conn{} = conn, %{} = params) do - with {:ok, %User{} = user} <- %User{} |> User.registration_changeset(params) |> Repo.insert(), - user <- preload(user) - do - conn |> put_status(:created) |> render("show.json-api", data: user) + with {:ok, %User{} = user} <- params |> Accounts.create() do + conn + |> put_status(:created) + |> render("show.json-api", data: user |> preload()) end end diff --git a/test/lib/code_corps/accounts/accounts_test.exs b/test/lib/code_corps/accounts/accounts_test.exs index a4064ae45..8bd232254 100644 --- a/test/lib/code_corps/accounts/accounts_test.exs +++ b/test/lib/code_corps/accounts/accounts_test.exs @@ -15,6 +15,72 @@ defmodule CodeCorps.AccountsTest do alias Ecto.Changeset + describe "create/1" do + @valid_user_params %{ + "email" => "test@user.com", + "password" => "somepassword", + "username" => "testuser" + } + + test "creates user" do + {:ok, %User{} = user} = + @valid_user_params + |> Accounts.create() + + assert Repo.get(User, user.id) + end + + test "returns changeset if validation errors" do + {:error, %Changeset{} = changeset} = + @valid_user_params + |> Map.delete("email") + |> Accounts.create() + + refute changeset.valid? + end + + test "claims invite if id providedf" do + invite = insert(:user_invite, invitee: nil, project: nil) + + {:ok, %User{} = user} = + @valid_user_params + |> Map.put("invite_id", invite.id) + |> Accounts.create() + + assert Repo.get(User, user.id) + end + + test "associates invite with user" do + invite = insert(:user_invite, invitee: nil, project: nil) + + {:ok, %User{} = user} = + @valid_user_params + |> Map.put("invite_id", invite.id) + |> Accounts.create() + + assert Repo.one(UserInvite).invitee_id == user.id + end + + test "creates project membership if project provided with invite" do + project = insert(:project) + invite = insert(:user_invite, invitee: nil, project: project, role: "admin") + + {:ok, %User{} = user} = + @valid_user_params + |> Map.put("invite_id", invite.id) + |> Accounts.create() + + assert Repo.get_by(ProjectUser, user_id: user.id, project_id: project.id, role: "admin") + end + + test "returns :invite_not_found if bad invite id provided" do + assert {:error, :invite_not_found} = + @valid_user_params + |> Map.put("invite_id", -1) + |> Accounts.create() + end + end + describe "create_from_github/1" do test "creates proper user from provided payload" do {:ok, %User{} = user} = @@ -268,53 +334,4 @@ defmodule CodeCorps.AccountsTest do assert changeset.errors[:email] end end - - describe "claim_invite/1" do - @valid_user_params %{ - "email" => "test@user.com", - "password" => "somepassword", - "username" => "testuser" - } - - test "creates user" do - invite = insert(:user_invite, invitee: nil, project: nil) - - {:ok, %User{} = user} = - @valid_user_params - |> Map.put("invite_id", invite.id) - |> Accounts.claim_invite() - - assert Repo.get(User, user.id) - end - - test "associates invite with user" do - invite = insert(:user_invite, invitee: nil, project: nil) - - {:ok, %User{} = user} = - @valid_user_params - |> Map.put("invite_id", invite.id) - |> Accounts.claim_invite() - - assert Repo.one(UserInvite).invitee_id == user.id - end - - test "creates project membership if project provided" do - project = insert(:project) - invite = insert(:user_invite, invitee: nil, project: project, role: "admin") - - {:ok, %User{} = user} = - @valid_user_params - |> Map.put("invite_id", invite.id) - |> Accounts.claim_invite() - - assert Repo.get_by(ProjectUser, user_id: user.id, project_id: project.id, role: "admin") - end - - test "returns :invite_not_found if bad id provided" do - assert {:error, :invite_not_found} = - @valid_user_params - |> Map.put("invite_id", -1) - |> Accounts.claim_invite() - end - end end From c56eee76d019eed4b661ffa8b5124149ad554540 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Tue, 2 Jan 2018 19:08:52 +0100 Subject: [PATCH 10/24] Add policy and tests --- lib/code_corps/policy/user_invite.ex | 22 +++++++++ .../code_corps/policy/user_invite_test.exs | 49 +++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 lib/code_corps/policy/user_invite.ex create mode 100644 test/lib/code_corps/policy/user_invite_test.exs diff --git a/lib/code_corps/policy/user_invite.ex b/lib/code_corps/policy/user_invite.ex new file mode 100644 index 000000000..2d7545ede --- /dev/null +++ b/lib/code_corps/policy/user_invite.ex @@ -0,0 +1,22 @@ +defmodule CodeCorps.Policy.UserInvite do + alias CodeCorps.{Policy.Helpers, User} + + def create?(%User{} = user, %{"project_id" => _} = params) do + user_role = + params + |> Helpers.get_project() + |> Helpers.get_membership(user) + |> Helpers.get_role() + + new_role = Map.get(params, "role") + do_create?(user_role, new_role) + end + def create?(%User{}, %{}), do: true + + defp do_create?("admin", "pending"), do: true + defp do_create?("admin", "contributor"), do: true + defp do_create?("owner", "pending"), do: true + defp do_create?("owner", "contributor"), do: true + defp do_create?("owner", "admin"), do: true + defp do_create?(_, _), do: false +end diff --git a/test/lib/code_corps/policy/user_invite_test.exs b/test/lib/code_corps/policy/user_invite_test.exs new file mode 100644 index 000000000..c4747dcf3 --- /dev/null +++ b/test/lib/code_corps/policy/user_invite_test.exs @@ -0,0 +1,49 @@ +defmodule CodeCorps.Policy.UserInviteTest do + use CodeCorps.PolicyCase + + alias CodeCorps.{Policy.UserInvite, Repo} + alias Ecto.Changeset + + describe "create?" do + defp set_role(membership, role) do + membership |> Changeset.change(%{role: role}) |> Repo.update() + end + + test "returns true if no project/role specified" do + user = insert(:user) + assert user |> UserInvite.create?(%{}) == true + end + + test "when invite for project contributor, returns true if user is admin or higher" do + project = insert(:project) + user = insert(:user) + attrs = %{"project_id" => project.id, "role" => "contributor"} + + refute user |> UserInvite.create?(attrs) + membership = insert(:project_user, project: project, user: user, role: "pending") + refute user |> UserInvite.create?(attrs) + membership |> set_role("contributor") + refute user |> UserInvite.create?(attrs) + membership |> set_role("admin") + assert user |> UserInvite.create?(attrs) + membership |> set_role("owner") + assert user |> UserInvite.create?(attrs) + end + + test "when invite for project admin, returns true if user is owner" do + project = insert(:project) + user = insert(:user) + attrs = %{"project_id" => project.id, "role" => "admin"} + + refute user |> UserInvite.create?(attrs) + membership = insert(:project_user, project: project, user: user, role: "pending") + refute user |> UserInvite.create?(attrs) + membership |> set_role("contributor") + refute user |> UserInvite.create?(attrs) + membership |> set_role("admin") + refute user |> UserInvite.create?(attrs) + membership |> set_role("owner") + assert user |> UserInvite.create?(attrs) + end + end +end From 2969154b382695e55ac8dedf6d97dbf010b7c10b Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 10:40:08 +0100 Subject: [PATCH 11/24] Clean up formatting in UserController --- .../controllers/user_controller.ex | 45 +++++++++++-------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/lib/code_corps_web/controllers/user_controller.ex b/lib/code_corps_web/controllers/user_controller.ex index 68c04501a..dbcbc8c64 100644 --- a/lib/code_corps_web/controllers/user_controller.ex +++ b/lib/code_corps_web/controllers/user_controller.ex @@ -11,10 +11,10 @@ defmodule CodeCorpsWeb.UserController do Accounts } - action_fallback CodeCorpsWeb.FallbackController - plug CodeCorpsWeb.Plug.DataToAttributes + action_fallback(CodeCorpsWeb.FallbackController) + plug(CodeCorpsWeb.Plug.DataToAttributes) - @spec index(Conn.t, map) :: Conn.t + @spec index(Conn.t(), map) :: Conn.t() def index(%Conn{} = conn, %{} = params) do users = User @@ -28,14 +28,14 @@ defmodule CodeCorpsWeb.UserController do conn |> render("index.json-api", data: users) end - @spec show(Conn.t, map) :: Conn.t + @spec show(Conn.t(), map) :: Conn.t() def show(%Conn{} = conn, %{"id" => id}) do with %User{} = user <- User |> Repo.get(id) |> preload() do conn |> render("show.json-api", data: user) end end - @spec create(Conn.t, map) :: Conn.t + @spec create(Conn.t(), map) :: Conn.t() def create(%Conn{} = conn, %{} = params) do with {:ok, %User{} = user} <- params |> Accounts.create() do conn @@ -44,48 +44,55 @@ defmodule CodeCorpsWeb.UserController do end end - @spec update(Conn.t, map) :: Conn.t + @spec update(Conn.t(), map) :: Conn.t() def update(%Conn{} = conn, %{"id" => id} = params) do with %User{} = user <- User |> Repo.get(id), - %User{} = current_user <- conn |> CodeCorps.Guardian.Plug.current_resource, + %User{} = current_user <- conn |> CodeCorps.Guardian.Plug.current_resource(), {:ok, :authorized} <- current_user |> Policy.authorize(:update, user), {:ok, user, _, _} <- user |> UserService.update(params), - user <- preload(user) - do - conn |> render("show.json-api", data: user) + user <- preload(user) do + conn |> render("show.json-api", data: user) end end - @doc """ + @doc ~S""" Differs from other resources by path: `/oauth/github` """ - @spec github_oauth(Conn.t, map) :: Conn.t + @spec github_oauth(Conn.t(), map) :: Conn.t() def github_oauth(%Conn{} = conn, %{"code" => code, "state" => state}) do current_user = Guardian.Plug.current_resource(conn) + with {:ok, user} <- GitHub.API.User.connect(current_user, code, state), - user <- preload(user) - do + user <- preload(user) do Analytics.SegmentTracker.track(user.id, "Connected to GitHub", user) conn |> render("show.json-api", data: user) end end - @spec email_available(Conn.t, map) :: Conn.t + @spec email_available(Conn.t(), map) :: Conn.t() def email_available(%Conn{} = conn, %{"email" => email}) do hash = User.check_email_availability(email) conn |> json(hash) end - @spec username_available(Conn.t, map) :: Conn.t + @spec username_available(Conn.t(), map) :: Conn.t() def username_available(%Conn{} = conn, %{"username" => username}) do hash = User.check_username_availability(username) conn |> json(hash) end @preloads [ - :categories, :github_app_installations, :organizations, :project_users, - :slugged_route, :stripe_connect_subscriptions, :stripe_platform_card, - :stripe_platform_customer, :user_categories, :user_roles, :user_skills + :categories, + :github_app_installations, + :organizations, + :project_users, + :slugged_route, + :stripe_connect_subscriptions, + :stripe_platform_card, + :stripe_platform_customer, + :user_categories, + :user_roles, + :user_skills ] def preload(data) do From 93a6d67e3bacaa67fcf740d6549b74dc3be5d760 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 11:16:45 +0100 Subject: [PATCH 12/24] Fix moduledoc in Policy.ConversationPart --- lib/code_corps/policy/conversation_part.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/code_corps/policy/conversation_part.ex b/lib/code_corps/policy/conversation_part.ex index 8ca9161b7..8339a23dc 100644 --- a/lib/code_corps/policy/conversation_part.ex +++ b/lib/code_corps/policy/conversation_part.ex @@ -1,7 +1,7 @@ defmodule CodeCorps.Policy.ConversationPart do @moduledoc ~S""" - Handles `CodeCorps.User` authorization of actions on `CodeCorps.Conversation` - records. + Handles `CodeCorps.User` authorization of actions on + `CodeCorps.ConversationPart` records. """ import CodeCorps.Policy.Helpers, From 36eacd251cc97a53d271f5df6f73fce9bbfb20f7 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 11:17:16 +0100 Subject: [PATCH 13/24] Add UserInviteView and tests --- lib/code_corps_web/views/user_invite_view.ex | 11 ++ .../views/user_invite_view_test.exs | 139 ++++++++++++++++++ 2 files changed, 150 insertions(+) create mode 100644 lib/code_corps_web/views/user_invite_view.ex create mode 100644 test/lib/code_corps_web/views/user_invite_view_test.exs diff --git a/lib/code_corps_web/views/user_invite_view.ex b/lib/code_corps_web/views/user_invite_view.ex new file mode 100644 index 000000000..1c5da6c3e --- /dev/null +++ b/lib/code_corps_web/views/user_invite_view.ex @@ -0,0 +1,11 @@ +defmodule CodeCorpsWeb.UserInviteView do + @moduledoc false + use CodeCorpsWeb, :view + use JaSerializer.PhoenixView + + attributes [:email, :name, :role] + + has_one :invitee, type: "user", field: :invitee_id + has_one :inviter, type: "user", field: :inviter_id + has_one :project, type: "project", field: :project_id +end diff --git a/test/lib/code_corps_web/views/user_invite_view_test.exs b/test/lib/code_corps_web/views/user_invite_view_test.exs new file mode 100644 index 000000000..ef6417eaa --- /dev/null +++ b/test/lib/code_corps_web/views/user_invite_view_test.exs @@ -0,0 +1,139 @@ +defmodule CodeCorpsWeb.UserInviteViewTest do + @moduledoc false + + use CodeCorpsWeb.ViewCase + + test "renders all attributes and relationships properly for unclaimed plain user invite" do + user_invite = insert(:user_invite) + + rendered_json = render(CodeCorpsWeb.UserInviteView, "show.json-api", data: user_invite) + + expected_json = %{ + "data" => %{ + "id" => "#{user_invite.id}", + "type" => "user-invite", + "attributes" => %{ + "email" => user_invite.email, + "name" => user_invite.name, + "role" => nil + }, + "relationships" => %{ + "invitee" => %{"data" => nil}, + "inviter" => %{ + "data" => %{"id" => "#{user_invite.inviter_id}", "type" => "user"} + }, + "project" => %{"data" => nil} + } + }, + "jsonapi" => %{ + "version" => "1.0" + } + } + + assert rendered_json == expected_json + end + + test "renders all attributes and relationships properly for claimed plain user invite" do + user_invite = insert(:user_invite, invitee: :user |> build()) + + rendered_json = render(CodeCorpsWeb.UserInviteView, "show.json-api", data: user_invite) + + expected_json = %{ + "data" => %{ + "id" => "#{user_invite.id}", + "type" => "user-invite", + "attributes" => %{ + "email" => user_invite.email, + "name" => user_invite.name, + "role" => nil + }, + "relationships" => %{ + "invitee" => %{ + "data" => %{"id" => "#{user_invite.invitee_id}", "type" => "user"} + }, + "inviter" => %{ + "data" => %{"id" => "#{user_invite.inviter_id}", "type" => "user"} + }, + "project" => %{"data" => nil} + } + }, + "jsonapi" => %{ + "version" => "1.0" + } + } + + assert rendered_json == expected_json + end + + test "renders all attributes and relationships properly for unclaimed project user invite" do + user_invite = insert(:user_invite, project: :project |> build(), role: "contributor") + + rendered_json = render(CodeCorpsWeb.UserInviteView, "show.json-api", data: user_invite) + + expected_json = %{ + "data" => %{ + "id" => "#{user_invite.id}", + "type" => "user-invite", + "attributes" => %{ + "email" => user_invite.email, + "name" => user_invite.name, + "role" => user_invite.role + }, + "relationships" => %{ + "invitee" => %{"data" => nil}, + "inviter" => %{ + "data" => %{"id" => "#{user_invite.inviter_id}", "type" => "user"} + }, + "project" => %{ + "data" => %{"id" => "#{user_invite.project_id}", "type" => "project"} + } + } + }, + "jsonapi" => %{ + "version" => "1.0" + } + } + + assert rendered_json == expected_json + end + + test "renders all attributes and relationships properly for claimed project user invite" do + user_invite = + insert( + :user_invite, + invitee: :user |> build(), + project: :project |> build(), + role: "contributor" + ) + + rendered_json = render(CodeCorpsWeb.UserInviteView, "show.json-api", data: user_invite) + + expected_json = %{ + "data" => %{ + "id" => "#{user_invite.id}", + "type" => "user-invite", + "attributes" => %{ + "email" => user_invite.email, + "name" => user_invite.name, + "role" => user_invite.role + }, + "relationships" => %{ + "invitee" => %{ + "data" => %{"id" => "#{user_invite.invitee_id}", "type" => "user"} + }, + "inviter" => %{ + "data" => %{"id" => "#{user_invite.inviter_id}", "type" => "user"} + }, + "project" => %{ + "data" => %{"id" => "#{user_invite.project_id}", "type" => "project"} + } + } + }, + "jsonapi" => %{ + "version" => "1.0" + } + } + + assert rendered_json == expected_json + end +end From cba0b7a4045741bd3c4a8ce2403017878e183c0e Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 11:17:54 +0100 Subject: [PATCH 14/24] Correct Policy.UserInvite behavior --- lib/code_corps/policy/user_invite.ex | 24 +++++++++++++++++-- .../code_corps/policy/user_invite_test.exs | 18 ++++++++++---- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/lib/code_corps/policy/user_invite.ex b/lib/code_corps/policy/user_invite.ex index 2d7545ede..af2ea4591 100644 --- a/lib/code_corps/policy/user_invite.ex +++ b/lib/code_corps/policy/user_invite.ex @@ -1,7 +1,21 @@ defmodule CodeCorps.Policy.UserInvite do + @moduledoc ~S""" + Handles `CodeCorps.User` authorization of actions on `CodeCorps.UserInvite` + records. + """ + alias CodeCorps.{Policy.Helpers, User} - def create?(%User{} = user, %{"project_id" => _} = params) do + @doc ~S""" + Returns true if the specified `CodeCorps.User` is allowed to create a + `CodeCorps.UserInvite` using the specified attributes. + """ + @spec create?(User.t(), map) :: boolean + def create?( + %User{id: user_id} = user, + %{"project_id" => _, "inviter_id" => inviter_id} = params + ) + when user_id == inviter_id do user_role = params |> Helpers.get_project() @@ -11,8 +25,14 @@ defmodule CodeCorps.Policy.UserInvite do new_role = Map.get(params, "role") do_create?(user_role, new_role) end - def create?(%User{}, %{}), do: true + def create?(%User{id: user_id}, %{"inviter_id" => inviter_id}) + when user_id == inviter_id, + do: true + + def create?(%User{}, %{}), do: false + + @spec do_create?(String.t() | nil, String.t() | nil) :: boolean defp do_create?("admin", "pending"), do: true defp do_create?("admin", "contributor"), do: true defp do_create?("owner", "pending"), do: true diff --git a/test/lib/code_corps/policy/user_invite_test.exs b/test/lib/code_corps/policy/user_invite_test.exs index c4747dcf3..821045adb 100644 --- a/test/lib/code_corps/policy/user_invite_test.exs +++ b/test/lib/code_corps/policy/user_invite_test.exs @@ -9,15 +9,25 @@ defmodule CodeCorps.Policy.UserInviteTest do membership |> Changeset.change(%{role: role}) |> Repo.update() end - test "returns true if no project/role specified" do + test "returns true if current user matches inviter and no project/role specified" do user = insert(:user) - assert user |> UserInvite.create?(%{}) == true + assert user |> UserInvite.create?(%{"inviter_id" => user.id}) == true + end + + test "returns true if current user does not match inviter and no project/role specified" do + user = insert(:user) + assert user |> UserInvite.create?(%{"inviter_id" => -1}) == false + end + + test "returns false if no project/role or inviter specified" do + user = insert(:user) + assert user |> UserInvite.create?(%{}) == false end test "when invite for project contributor, returns true if user is admin or higher" do project = insert(:project) user = insert(:user) - attrs = %{"project_id" => project.id, "role" => "contributor"} + attrs = %{"project_id" => project.id, "role" => "contributor", "inviter_id" => user.id} refute user |> UserInvite.create?(attrs) membership = insert(:project_user, project: project, user: user, role: "pending") @@ -33,7 +43,7 @@ defmodule CodeCorps.Policy.UserInviteTest do test "when invite for project admin, returns true if user is owner" do project = insert(:project) user = insert(:user) - attrs = %{"project_id" => project.id, "role" => "admin"} + attrs = %{"project_id" => project.id, "role" => "admin", "inviter_id" => user.id} refute user |> UserInvite.create?(attrs) membership = insert(:project_user, project: project, user: user, role: "pending") From 88b103b072511257e5c2a04866da2e6388ef9587 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 11:18:18 +0100 Subject: [PATCH 15/24] Add inviter_id support to Plugs.IdsToInteger --- lib/code_corps_web/plugs/ids_to_integers.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/code_corps_web/plugs/ids_to_integers.ex b/lib/code_corps_web/plugs/ids_to_integers.ex index f6df5d0fe..9c279783b 100644 --- a/lib/code_corps_web/plugs/ids_to_integers.ex +++ b/lib/code_corps_web/plugs/ids_to_integers.ex @@ -40,6 +40,7 @@ defmodule CodeCorpsWeb.Plug.IdsToIntegers do defp convert?("donation_goal_id"), do: true defp convert?("github_app_installation_id"), do: true defp convert?("github_repo_id"), do: true + defp convert?("inviter_id"), do: true defp convert?("organization_github_app_installation_id"), do: true defp convert?("organization_invite_id"), do: true defp convert?("organization_id"), do: true From d7a7b1dc74aa827eff6e9a201fccdd240a72bf94 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 11:18:41 +0100 Subject: [PATCH 16/24] Add UserInvite support to CodeCorps.Policy.can? --- lib/code_corps/policy/policy.ex | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/code_corps/policy/policy.ex b/lib/code_corps/policy/policy.ex index 7c94df8de..bf207a60d 100644 --- a/lib/code_corps/policy/policy.ex +++ b/lib/code_corps/policy/policy.ex @@ -34,6 +34,7 @@ defmodule CodeCorps.Policy do TaskSkill, User, UserCategory, + UserInvite, UserRole, UserSkill, UserTask @@ -191,6 +192,9 @@ defmodule CodeCorps.Policy do defp can?(%User{} = current_user, :create, %UserCategory{}, %{} = params), do: Policy.UserCategory.create?(current_user, params) defp can?(%User{} = current_user, :delete, %UserCategory{} = user_category, %{}), do: Policy.UserCategory.delete?(current_user, user_category) + # UserInvite + defp can?(%User{} = current_user, :create, %UserInvite{}, %{} = params), do: Policy.UserInvite.create?(current_user, params) + # UserRole defp can?(%User{} = current_user, :create, %UserRole{}, %{} = params), do: Policy.UserRole.create?(current_user, params) defp can?(%User{} = current_user, :delete, %UserRole{} = user_role, %{}), do: Policy.UserRole.delete?(current_user, user_role) From 461c8a4949409940ae1ebcd978a028d9ed727c22 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 11:53:34 +0100 Subject: [PATCH 17/24] Add UserInviteController with create endpoint and associated tests - includes tracking of event --- .../analytics/segment_traits_builder.ex | 14 +++ lib/code_corps/validators/slug_validator.ex | 4 +- .../controllers/user_invite_controller.ex | 21 ++++ lib/code_corps_web/router.ex | 1 + .../analytics/segment_traits_builder_test.exs | 1 + .../user_invite_controller_test.exs | 114 ++++++++++++++++++ 6 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 lib/code_corps_web/controllers/user_invite_controller.ex create mode 100644 test/lib/code_corps_web/controllers/user_invite_controller_test.exs diff --git a/lib/code_corps/analytics/segment_traits_builder.ex b/lib/code_corps/analytics/segment_traits_builder.ex index 3000ba5c0..6dc383154 100644 --- a/lib/code_corps/analytics/segment_traits_builder.ex +++ b/lib/code_corps/analytics/segment_traits_builder.ex @@ -202,6 +202,20 @@ defmodule CodeCorps.Analytics.SegmentTraitsBuilder do category_id: user_category.category.id } end + defp traits(%CodeCorps.UserInvite{} = user_invite) do + user_invite = user_invite |> Repo.preload([:project, :inviter, :invitee]) + %{ + email: user_invite.email, + invitee: user_invite |> Map.get(:invitee) |> (&(&1 || %{})).() |> Map.get(:username), + invitee_id: user_invite.invitee_id, + inviter: user_invite.inviter.username, + inviter_id: user_invite.inviter_id, + name: user_invite.name, + project: user_invite |> Map.get(:project)|> (&(&1 || %{})).() |> Map.get(:title), + project_id: user_invite.project_id, + role: user_invite.role + } + end defp traits(%CodeCorps.UserRole{} = user_role) do user_role = user_role |> Repo.preload(:role) %{ diff --git a/lib/code_corps/validators/slug_validator.ex b/lib/code_corps/validators/slug_validator.ex index 4121215f2..7353a3d8c 100644 --- a/lib/code_corps/validators/slug_validator.ex +++ b/lib/code_corps/validators/slug_validator.ex @@ -50,8 +50,8 @@ defmodule CodeCorps.Validators.SlugValidator do stripe-platform-customers tags tasks task-images task-likes task-lists task-skills teams token tokens - user-categories user-roles user-skills user-tasks user username_available - users + user-categories user-invites user-roles user-skills user-tasks user + username_available users webhooks ) diff --git a/lib/code_corps_web/controllers/user_invite_controller.ex b/lib/code_corps_web/controllers/user_invite_controller.ex new file mode 100644 index 000000000..b8169076b --- /dev/null +++ b/lib/code_corps_web/controllers/user_invite_controller.ex @@ -0,0 +1,21 @@ +defmodule CodeCorpsWeb.UserInviteController do + @moduledoc false + use CodeCorpsWeb, :controller + + alias CodeCorps.{Accounts, Analytics.SegmentTracker, User, UserInvite} + + action_fallback CodeCorpsWeb.FallbackController + plug CodeCorpsWeb.Plug.DataToAttributes + plug CodeCorpsWeb.Plug.IdsToIntegers + + @spec create(Plug.Conn.t, map) :: Conn.t + def create(%Conn{} = conn, %{} = params) do + with %User{} = current_user <- conn |> CodeCorps.Guardian.Plug.current_resource, + {:ok, :authorized} <- current_user |> Policy.authorize(:create, %UserInvite{}, params), + {:ok, %UserInvite{} = user_invite} <- params |> Accounts.create_invite() do + + current_user.id |> SegmentTracker.track("Created User Invite", user_invite) + conn |> put_status(:created) |> render("show.json-api", data: user_invite) + end + end +end diff --git a/lib/code_corps_web/router.ex b/lib/code_corps_web/router.ex index 9f8761238..40d9b5313 100644 --- a/lib/code_corps_web/router.ex +++ b/lib/code_corps_web/router.ex @@ -98,6 +98,7 @@ defmodule CodeCorpsWeb.Router do resources "/task-skills", TaskSkillController, only: [:create, :delete] resources "/tasks", TaskController, only: [:create, :update] resources "/user-categories", UserCategoryController, only: [:create, :delete] + resources "/user-invites", UserInviteController, only: [:create] resources "/user-roles", UserRoleController, only: [:create, :delete] resources "/user-skills", UserSkillController, only: [:create, :delete] resources "/user-tasks", UserTaskController, only: [:create, :update, :delete] diff --git a/test/lib/code_corps/analytics/segment_traits_builder_test.exs b/test/lib/code_corps/analytics/segment_traits_builder_test.exs index 85e387db6..602e2177c 100644 --- a/test/lib/code_corps/analytics/segment_traits_builder_test.exs +++ b/test/lib/code_corps/analytics/segment_traits_builder_test.exs @@ -35,6 +35,7 @@ defmodule CodeCorps.Analytics.SegmentTraitsBuilderTest do assert :user |> insert |> SegmentTraitsBuilder.build assert :user_category |> insert |> SegmentTraitsBuilder.build + assert :user_invite |> insert |> SegmentTraitsBuilder.build assert :user_role |> insert |> SegmentTraitsBuilder.build assert :user_skill |> insert |> SegmentTraitsBuilder.build assert :user_task |> insert |> SegmentTraitsBuilder.build diff --git a/test/lib/code_corps_web/controllers/user_invite_controller_test.exs b/test/lib/code_corps_web/controllers/user_invite_controller_test.exs new file mode 100644 index 000000000..ad0d79123 --- /dev/null +++ b/test/lib/code_corps_web/controllers/user_invite_controller_test.exs @@ -0,0 +1,114 @@ +defmodule CodeCorpsWeb.UserInviteControllerTest do + @moduledoc false + + use CodeCorpsWeb.ApiCase + + alias CodeCorps.{Repo, UserInvite} + + describe "create" do + @tag :authenticated + test "creates an invite, renders 201", %{conn: conn, current_user: current_user} do + attrs = %{ + "email" => "foo@mail.com", + "inviter_id" => "#{current_user.id}", + "name" => "Foo" + } + + path = conn |> user_invite_path(:create) + json = conn |> post(path, attrs) |> json_response(201) + + created_invite = + Repo.get_by(UserInvite, email: "foo@mail.com", inviter_id: current_user.id, name: "Foo") + + assert created_invite + + json |> assert_id_from_response(created_invite.id) + end + + @tag :authenticated + test "creates an invite for a project, renders 201", %{conn: conn, current_user: current_user} do + project = insert(:project) + insert(:project_user, role: "admin", project: project, user: current_user) + + attrs = %{ + "email" => "foo@mail.com", + "inviter_id" => "#{current_user.id}", + "name" => "Foo", + "project_id" => "#{project.id}", + "role" => "contributor" + } + + path = conn |> user_invite_path(:create) + json = conn |> post(path, attrs) |> json_response(201) + + created_invite = + Repo.get_by( + UserInvite, + email: "foo@mail.com", + inviter_id: current_user.id, + name: "Foo", + project_id: project.id, + role: "contributor" + ) + + assert created_invite + + json |> assert_id_from_response(created_invite.id) + end + + @tag :authenticated + test "tracks creation of invite", %{conn: conn, current_user: %{id: user_id}} do + attrs = %{ + "email" => "foo@mail.com", + "inviter_id" => "#{user_id}", + "name" => "Foo" + } + + path = conn |> user_invite_path(:create) + conn |> post(path, attrs) + + created_invite = Repo.one(UserInvite) + traits = created_invite |> CodeCorps.Analytics.SegmentTraitsBuilder.build() + + assert_received({:track, ^user_id, "Created User Invite", ^traits}) + end + + test "renders 401 when unauthenticated", %{conn: conn} do + attrs = %{ + "email" => "foo@mail.com", + "name" => "Foo" + } + + path = conn |> user_invite_path(:create) + assert conn |> post(path, attrs) |> json_response(401) + refute Repo.one(UserInvite) + end + + @tag :authenticated + test "renders 403 on authorization error", %{conn: conn, current_user: current_user} do + project = insert(:project) + + attrs = %{ + "email" => "foo@mail.com", + "inviter_id" => "#{current_user.id}", + "name" => "Foo", + "project_id" => "#{project.id}", + "role" => "contributor" + } + + path = conn |> user_invite_path(:create) + assert conn |> post(path, attrs) |> json_response(403) + refute Repo.one(UserInvite) + end + + @tag :authenticated + test "renders 422 on validation error", %{conn: conn, current_user: current_user} do + attrs = %{"inviter_id" => "#{current_user.id}"} + + path = conn |> user_invite_path(:create) + assert conn |> post(path, attrs) |> json_response(422) + + refute Repo.one(UserInvite) + end + end +end From 776a9dde4f74a167e46e69d496bfefccc0ecb115 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 12:05:41 +0100 Subject: [PATCH 18/24] Track UserInvite claim - required refactoring of Accounts.create and Accounts.UserInvites.claim_invite --- lib/code_corps/accounts/accounts.ex | 6 +++++- lib/code_corps/accounts/user_invites.ex | 2 +- .../lib/code_corps/accounts/accounts_test.exs | 20 ++++++++++++++++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/code_corps/accounts/accounts.ex b/lib/code_corps/accounts/accounts.ex index c5c622d61..1f32d5fdd 100644 --- a/lib/code_corps/accounts/accounts.ex +++ b/lib/code_corps/accounts/accounts.ex @@ -8,6 +8,7 @@ defmodule CodeCorps.Accounts do alias CodeCorps.{ Accounts, Accounts.Changesets, + Analytics, Comment, GitHub.Adapters, GithubAppInstallation, @@ -31,7 +32,10 @@ defmodule CodeCorps.Accounts do """ @spec create(map) :: {:ok, User.t} | {:error, Changeset.t} | {:error, :invite_not_found} def create(%{"invite_id" => _} = params) do - params |> Accounts.UserInvites.claim_invite() + with {:ok, %User{} = user, %UserInvite{} = user_invite} <- params |> Accounts.UserInvites.claim_invite() do + user.id |> Analytics.SegmentTracker.track("Claimed User Invite", user_invite) + {:ok, user} + end end def create(%{} = params) do %User{} |> User.registration_changeset(params) |> Repo.insert() diff --git a/lib/code_corps/accounts/user_invites.ex b/lib/code_corps/accounts/user_invites.ex index 7920a8dc2..538c0a5e1 100644 --- a/lib/code_corps/accounts/user_invites.ex +++ b/lib/code_corps/accounts/user_invites.ex @@ -110,7 +110,7 @@ defmodule CodeCorps.Accounts.UserInvites do end @spec marshall_response(tuple) :: tuple - defp marshall_response({:ok, %{user: user}}), do: {:ok, user |> Repo.preload(:project_users)} + defp marshall_response({:ok, %{user: user, user_invite: user_invite}}), do: {:ok, user, user_invite} defp marshall_response({:error, :load_invite, :not_found, _}), do: {:error, :invite_not_found} defp marshall_response(other_tuple), do: other_tuple end diff --git a/test/lib/code_corps/accounts/accounts_test.exs b/test/lib/code_corps/accounts/accounts_test.exs index 8bd232254..f9d182911 100644 --- a/test/lib/code_corps/accounts/accounts_test.exs +++ b/test/lib/code_corps/accounts/accounts_test.exs @@ -50,6 +50,22 @@ defmodule CodeCorps.AccountsTest do assert Repo.get(User, user.id) end + test "tracks invite claim with segment" do + invite = insert(:user_invite, invitee: nil, project: nil) + + {:ok, %User{} = user} = + @valid_user_params + |> Map.put("invite_id", invite.id) + |> Accounts.create() + + %{id: created_user_id} = Repo.get(User, user.id) + + traits = + UserInvite |> Repo.get(invite.id) |> CodeCorps.Analytics.SegmentTraitsBuilder.build() + + assert_received({:track, ^created_user_id, "Claimed User Invite", ^traits}) + end + test "associates invite with user" do invite = insert(:user_invite, invitee: nil, project: nil) @@ -74,10 +90,12 @@ defmodule CodeCorps.AccountsTest do end test "returns :invite_not_found if bad invite id provided" do - assert {:error, :invite_not_found} = + response = @valid_user_params |> Map.put("invite_id", -1) |> Accounts.create() + + assert response == {:error, :invite_not_found} end end From bb1f4c2d89765e1bcdd06d2e2897f40ae66812f4 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 12:11:22 +0100 Subject: [PATCH 19/24] Format CodeCorps.UserInvite --- lib/code_corps/model/user_invite.ex | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/code_corps/model/user_invite.ex b/lib/code_corps/model/user_invite.ex index 391430b48..9f12687e7 100644 --- a/lib/code_corps/model/user_invite.ex +++ b/lib/code_corps/model/user_invite.ex @@ -4,13 +4,13 @@ defmodule CodeCorps.UserInvite do @type t :: %__MODULE__{} schema "user_invites" do - field :email, :string, null: false - field :role, :string - field :name, :string + field(:email, :string, null: false) + field(:role, :string) + field(:name, :string) - belongs_to :project, CodeCorps.Project - belongs_to :inviter, CodeCorps.User - belongs_to :invitee, CodeCorps.User + belongs_to(:project, CodeCorps.Project) + belongs_to(:inviter, CodeCorps.User) + belongs_to(:invitee, CodeCorps.User) timestamps() end From 561dfe2a276251c21c5baa98693d4dcbf8b90a66 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 13:19:22 +0100 Subject: [PATCH 20/24] Move invite claim tracking to controller --- lib/code_corps/accounts/accounts.ex | 6 +- lib/code_corps/accounts/user_invites.ex | 6 +- lib/code_corps/model/user.ex | 1 + .../controllers/user_controller.ex | 9 ++ .../lib/code_corps/accounts/accounts_test.exs | 16 --- .../controllers/user_controller_test.exs | 125 ++++++++++++------ 6 files changed, 98 insertions(+), 65 deletions(-) diff --git a/lib/code_corps/accounts/accounts.ex b/lib/code_corps/accounts/accounts.ex index 1f32d5fdd..c5c622d61 100644 --- a/lib/code_corps/accounts/accounts.ex +++ b/lib/code_corps/accounts/accounts.ex @@ -8,7 +8,6 @@ defmodule CodeCorps.Accounts do alias CodeCorps.{ Accounts, Accounts.Changesets, - Analytics, Comment, GitHub.Adapters, GithubAppInstallation, @@ -32,10 +31,7 @@ defmodule CodeCorps.Accounts do """ @spec create(map) :: {:ok, User.t} | {:error, Changeset.t} | {:error, :invite_not_found} def create(%{"invite_id" => _} = params) do - with {:ok, %User{} = user, %UserInvite{} = user_invite} <- params |> Accounts.UserInvites.claim_invite() do - user.id |> Analytics.SegmentTracker.track("Claimed User Invite", user_invite) - {:ok, user} - end + params |> Accounts.UserInvites.claim_invite() end def create(%{} = params) do %User{} |> User.registration_changeset(params) |> Repo.insert() diff --git a/lib/code_corps/accounts/user_invites.ex b/lib/code_corps/accounts/user_invites.ex index 538c0a5e1..4c06193a3 100644 --- a/lib/code_corps/accounts/user_invites.ex +++ b/lib/code_corps/accounts/user_invites.ex @@ -101,7 +101,7 @@ defmodule CodeCorps.Accounts.UserInvites do defp join_project(%User{}, %UserInvite{}), do: {:ok, nil} - @spec associate_invitee(UserInvite.t(), User.t()) :: ProjectUser.t() + @spec associate_invitee(UserInvite.t(), User.t()) :: {:ok, UserInvite.t()} defp associate_invitee(%UserInvite{invitee: nil} = invite, %User{} = user) do invite |> Changeset.change(%{}) @@ -110,7 +110,9 @@ defmodule CodeCorps.Accounts.UserInvites do end @spec marshall_response(tuple) :: tuple - defp marshall_response({:ok, %{user: user, user_invite: user_invite}}), do: {:ok, user, user_invite} + defp marshall_response({:ok, %{user: user, user_invite: user_invite}}) do + {:ok, user |> Map.put(:claimed_invite, user_invite)} + end defp marshall_response({:error, :load_invite, :not_found, _}), do: {:error, :invite_not_found} defp marshall_response(other_tuple), do: other_tuple end diff --git a/lib/code_corps/model/user.ex b/lib/code_corps/model/user.ex index 2c8be0cd3..f87a98964 100644 --- a/lib/code_corps/model/user.ex +++ b/lib/code_corps/model/user.ex @@ -40,6 +40,7 @@ defmodule CodeCorps.User do field :username, :string field :website, :string + has_one :claimed_invite, CodeCorps.UserInvite, foreign_key: :invitee_id has_one :github_app_installation, CodeCorps.GithubAppInstallation has_one :slugged_route, SluggedRoute diff --git a/lib/code_corps_web/controllers/user_controller.ex b/lib/code_corps_web/controllers/user_controller.ex index dbcbc8c64..94173ccd3 100644 --- a/lib/code_corps_web/controllers/user_controller.ex +++ b/lib/code_corps_web/controllers/user_controller.ex @@ -8,6 +8,7 @@ defmodule CodeCorpsWeb.UserController do Helpers.Query, Services.UserService, User, + UserInvite, Accounts } @@ -38,6 +39,14 @@ defmodule CodeCorpsWeb.UserController do @spec create(Conn.t(), map) :: Conn.t() def create(%Conn{} = conn, %{} = params) do with {:ok, %User{} = user} <- params |> Accounts.create() do + case user |> Map.get(:claimed_invite) do + %UserInvite{} = user_invite -> + user.id |> Analytics.SegmentTracker.track("Claimed User Invite", user_invite) + + _other -> + nil + end + conn |> put_status(:created) |> render("show.json-api", data: user |> preload()) diff --git a/test/lib/code_corps/accounts/accounts_test.exs b/test/lib/code_corps/accounts/accounts_test.exs index f9d182911..ec5bf397d 100644 --- a/test/lib/code_corps/accounts/accounts_test.exs +++ b/test/lib/code_corps/accounts/accounts_test.exs @@ -50,22 +50,6 @@ defmodule CodeCorps.AccountsTest do assert Repo.get(User, user.id) end - test "tracks invite claim with segment" do - invite = insert(:user_invite, invitee: nil, project: nil) - - {:ok, %User{} = user} = - @valid_user_params - |> Map.put("invite_id", invite.id) - |> Accounts.create() - - %{id: created_user_id} = Repo.get(User, user.id) - - traits = - UserInvite |> Repo.get(invite.id) |> CodeCorps.Analytics.SegmentTraitsBuilder.build() - - assert_received({:track, ^created_user_id, "Claimed User Invite", ^traits}) - end - test "associates invite with user" do invite = insert(:user_invite, invitee: nil, project: nil) 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 244020525..1e3e106b3 100644 --- a/test/lib/code_corps_web/controllers/user_controller_test.exs +++ b/test/lib/code_corps_web/controllers/user_controller_test.exs @@ -96,6 +96,7 @@ defmodule CodeCorpsWeb.UserControllerTest do describe "show" do test "shows chosen resource", %{conn: conn} do user = insert(:user) + conn |> request_show(user) |> json_response(200) @@ -115,41 +116,48 @@ defmodule CodeCorpsWeb.UserControllerTest do describe "create" do test "creates and renders resource when data is valid", %{conn: conn} do attrs = Map.put(@valid_attrs, :password, "password") - conn = post conn, user_path(conn, :create), %{ - "data" => %{ - "attributes" => attrs - } - } + + conn = + post(conn, user_path(conn, :create), %{ + "data" => %{ + "attributes" => attrs + } + }) assert conn |> json_response(201) end - test "calls segment tracking after user is created", %{conn: conn} do - conn = post conn, user_path(conn, :create), %{ - "meta" => %{}, - "data" => %{ - "type" => "user", - "attributes" => Map.put(@valid_attrs, :password, "password"), - "relationships" => @relationships - } - } - id = json_response(conn, 201)["data"]["id"] |> String.to_integer + test "tracks user creation with segment", %{conn: conn} do + conn = + post(conn, user_path(conn, :create), %{ + "meta" => %{}, + "data" => %{ + "type" => "user", + "attributes" => Map.put(@valid_attrs, :password, "password"), + "relationships" => @relationships + } + }) + + id = json_response(conn, 201)["data"]["id"] |> String.to_integer() assert_received {:track, ^id, "Signed Up", %{}} end test "does not create resource and renders errors when data is invalid", %{conn: conn} do attrs = Map.put(@invalid_attrs, :password, "password") - conn = post conn, user_path(conn, :create), %{ - "data" => %{ - "attributes" => attrs - } - } + + conn = + post(conn, user_path(conn, :create), %{ + "data" => %{ + "attributes" => attrs + } + }) assert conn |> json_response(422) end test "supports claiming a user invite if specified by an id parameter", %{conn: conn} do %{id: invite_id, email: email} = insert(:user_invite) + attrs = @valid_attrs |> Map.merge(%{email: email, invite_id: invite_id, password: "password"}) @@ -165,9 +173,31 @@ defmodule CodeCorpsWeb.UserControllerTest do assert updated_invite.invitee_id == created_user.id end - test "supports claiming a user invite to a project if specified by an id parameter", %{conn: conn} do + test "tracks invite claim with segment", %{conn: conn} do + %{id: invite_id, email: email} = invite = insert(:user_invite, invitee: nil, project: nil) + + attrs = + @valid_attrs + |> Map.merge(%{email: email, invite_id: invite_id, password: "password"}) + + path = conn |> user_path(:create) + conn |> post(path, attrs) + + %{invitee_id: invitee_id} = invite = UserInvite |> Repo.get(invite.id) + + traits = invite |> CodeCorps.Analytics.SegmentTraitsBuilder.build() + + assert_received({:track, ^invitee_id, "Claimed User Invite", ^traits}) + end + + test "supports claiming a user invite to a project if specified by an id parameter", %{ + conn: conn + } do project = insert(:project) - %{id: invite_id, email: email} = invite = insert(:user_invite, role: "contributor", project: project) + + %{id: invite_id, email: email} = + invite = insert(:user_invite, role: "contributor", project: project) + attrs = @valid_attrs |> Map.merge(%{email: email, invite_id: invite_id, password: "password"}) @@ -230,7 +260,7 @@ defmodule CodeCorpsWeb.UserControllerTest do |> authenticate(user) |> put(path, params) - id = json_response(conn, 200)["data"]["id"] |> String.to_integer + id = json_response(conn, 200)["data"]["id"] |> String.to_integer() assert_received {:identify, ^id, %{email: "original@mail.com"}} assert_received {:track, ^id, "Updated Profile", %{}} end @@ -274,24 +304,27 @@ defmodule CodeCorpsWeb.UserControllerTest do "relationships" => @relationships } } + conn = conn |> authenticate(user) |> put(path, params) - json = json_response(conn, 422) + json = json_response(conn, 422) assert json["errors"] != %{} end test "transitions from one state to the next", %{conn: conn} do user = insert(:user) - conn = put authenticate(conn, user), user_path(conn, :update, user), %{ - "data" => %{ - "type" => "user", - "id" => user.id, - "attributes" => %{"password" => "password", "state_transition" => "edit_profile"} - } - } + + conn = + put(authenticate(conn, user), user_path(conn, :update, user), %{ + "data" => %{ + "type" => "user", + "id" => user.id, + "attributes" => %{"password" => "password", "state_transition" => "edit_profile"} + } + }) %{"data" => %{"id" => id}} = json_response(conn, 200) user = Repo.get(User, id) @@ -306,24 +339,32 @@ defmodule CodeCorpsWeb.UserControllerTest do @attrs %{"code" => "foo", "state" => "bar"} @tag :authenticated - test "return the user when current user connects successfully", %{conn: conn, current_user: current_user} do + test "return the user when current user connects successfully", %{ + conn: conn, + current_user: current_user + } do path = user_path(conn, :github_oauth) json = conn |> post(path, @attrs) |> json_response(200) - assert json["data"]["id"] |> String.to_integer == current_user.id + assert json["data"]["id"] |> String.to_integer() == current_user.id assert json["data"]["attributes"]["github-id"] end @tag :authenticated - test "tracks event on segment when current user connects successfully", %{conn: conn, current_user: %{id: id}} do + test "tracks event on segment when current user connects successfully", %{ + conn: conn, + current_user: %{id: id} + } do path = user_path(conn, :github_oauth) assert conn |> post(path, @attrs) |> json_response(200) + expected_data = User |> Repo.get(id) - |> CodeCorps.Analytics.SegmentTraitsBuilder.build + |> CodeCorps.Analytics.SegmentTraitsBuilder.build() + assert_received {:track, ^id, "Connected to GitHub", ^expected_data} end @@ -336,7 +377,7 @@ defmodule CodeCorpsWeb.UserControllerTest do test "renders 500 if there's a GitHub API error", %{conn: conn} do path = user_path(conn, :github_oauth) - with_mock_api(CodeCorps.GitHub.FailureAPI) do + with_mock_api CodeCorps.GitHub.FailureAPI do assert conn |> post(path, @attrs) |> json_response(500) end end @@ -344,7 +385,7 @@ defmodule CodeCorpsWeb.UserControllerTest do describe "email_available" do test "returns valid and available when email is valid and available", %{conn: conn} do - resp = get conn, user_path(conn, :email_available, %{email: "available@mail.com"}) + resp = get(conn, user_path(conn, :email_available, %{email: "available@mail.com"})) json = json_response(resp, 200) assert json["available"] assert json["valid"] @@ -352,14 +393,14 @@ defmodule CodeCorpsWeb.UserControllerTest do test "returns valid but inavailable when email is valid but taken", %{conn: conn} do insert(:user, email: "used@mail.com") - resp = get conn, user_path(conn, :email_available, %{email: "used@mail.com"}) + resp = get(conn, user_path(conn, :email_available, %{email: "used@mail.com"})) json = json_response(resp, 200) refute json["available"] assert json["valid"] end test "returns as available but invalid when email is invalid", %{conn: conn} do - resp = get conn, user_path(conn, :email_available, %{email: "not_an_email"}) + resp = get(conn, user_path(conn, :email_available, %{email: "not_an_email"})) json = json_response(resp, 200) assert json["available"] refute json["valid"] @@ -368,7 +409,7 @@ defmodule CodeCorpsWeb.UserControllerTest do describe "username_available" do test "returns as valid and available when username is valid and available", %{conn: conn} do - resp = get conn, user_path(conn, :username_available, %{username: "available"}) + resp = get(conn, user_path(conn, :username_available, %{username: "available"})) json = json_response(resp, 200) assert json["available"] assert json["valid"] @@ -376,14 +417,14 @@ defmodule CodeCorpsWeb.UserControllerTest do test "returns as valid, but inavailable when username is valid but taken", %{conn: conn} do insert(:user, username: "used") - resp = get conn, user_path(conn, :username_available, %{username: "used"}) + resp = get(conn, user_path(conn, :username_available, %{username: "used"})) json = json_response(resp, 200) refute json["available"] assert json["valid"] end test "returns available but invalid when username is invalid", %{conn: conn} do - resp = get conn, user_path(conn, :username_available, %{username: ""}) + resp = get(conn, user_path(conn, :username_available, %{username: ""})) json = json_response(resp, 200) assert json["available"] refute json["valid"] From d639acba4b03dbc9180872f3dd096481bdc5047c Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 13:27:50 +0100 Subject: [PATCH 21/24] Assume new user when claiming invite --- lib/code_corps/accounts/user_invites.ex | 18 +++++++----------- test/lib/code_corps/accounts/accounts_test.exs | 12 ++++++++++++ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/lib/code_corps/accounts/user_invites.ex b/lib/code_corps/accounts/user_invites.ex index 4c06193a3..7b2f0d0e3 100644 --- a/lib/code_corps/accounts/user_invites.ex +++ b/lib/code_corps/accounts/user_invites.ex @@ -86,17 +86,12 @@ defmodule CodeCorps.Accounts.UserInvites do @spec join_project(User.t(), UserInvite.t()) :: {:ok, ProjectUser.t()} | {:error, Changeset.t()} defp join_project(%User{} = user, %UserInvite{role: role, project: %Project{} = project}) do - case ProjectUser |> Repo.get_by(user_id: user.id, project_id: project.id) do - %ProjectUser{} = project_user -> - {:ok, project_user} - - nil -> - %ProjectUser{} - |> Changeset.change(%{role: role}) - |> Changeset.put_assoc(:project, project) - |> Changeset.put_assoc(:user, user) - |> Repo.insert() - end + %ProjectUser{} + |> Changeset.change(%{role: role}) + |> Changeset.put_assoc(:project, project) + |> Changeset.put_assoc(:user, user) + |> Changeset.unique_constraint(:project, name: :project_users_user_id_project_id_index) + |> Repo.insert() end defp join_project(%User{}, %UserInvite{}), do: {:ok, nil} @@ -114,5 +109,6 @@ defmodule CodeCorps.Accounts.UserInvites do {:ok, user |> Map.put(:claimed_invite, user_invite)} end defp marshall_response({:error, :load_invite, :not_found, _}), do: {:error, :invite_not_found} + defp marshall_response({:error, :user, %Changeset{} = changeset, _}), do: {:error, changeset} defp marshall_response(other_tuple), do: other_tuple end diff --git a/test/lib/code_corps/accounts/accounts_test.exs b/test/lib/code_corps/accounts/accounts_test.exs index ec5bf397d..f04a0468d 100644 --- a/test/lib/code_corps/accounts/accounts_test.exs +++ b/test/lib/code_corps/accounts/accounts_test.exs @@ -81,6 +81,18 @@ defmodule CodeCorps.AccountsTest do assert response == {:error, :invite_not_found} end + + test "returns changeset if user validation errors on invite claim" do + invite = insert(:user_invite, invitee: nil, project: nil) + insert(:user, email: @valid_user_params["email"]) + + {:error, changeset} = + @valid_user_params + |> Map.put("invite_id", invite.id) + |> Accounts.create() + + refute changeset.valid? + end end describe "create_from_github/1" do From 1145d372c045ee4411e166e1a82835a44677fe83 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 13:33:59 +0100 Subject: [PATCH 22/24] Add helper to SegmentTraitsBuilder for improved clarity --- lib/code_corps/analytics/segment_traits_builder.ex | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/code_corps/analytics/segment_traits_builder.ex b/lib/code_corps/analytics/segment_traits_builder.ex index 6dc383154..c54c00bdb 100644 --- a/lib/code_corps/analytics/segment_traits_builder.ex +++ b/lib/code_corps/analytics/segment_traits_builder.ex @@ -42,7 +42,7 @@ defmodule CodeCorps.Analytics.SegmentTraitsBuilder do record |> Repo.preload([:project]) |> Map.get(:project) - |> (&(&1 || %{})).() + |> blank_map_if_nil() |> Map.get(:title, "") %{ @@ -206,12 +206,12 @@ defmodule CodeCorps.Analytics.SegmentTraitsBuilder do user_invite = user_invite |> Repo.preload([:project, :inviter, :invitee]) %{ email: user_invite.email, - invitee: user_invite |> Map.get(:invitee) |> (&(&1 || %{})).() |> Map.get(:username), + invitee: user_invite |> Map.get(:invitee) |> blank_map_if_nil() |> Map.get(:username), invitee_id: user_invite.invitee_id, inviter: user_invite.inviter.username, inviter_id: user_invite.inviter_id, name: user_invite.name, - project: user_invite |> Map.get(:project)|> (&(&1 || %{})).() |> Map.get(:title), + project: user_invite |> Map.get(:project)|> blank_map_if_nil() |> Map.get(:title), project_id: user_invite.project_id, role: user_invite.role } @@ -239,4 +239,8 @@ defmodule CodeCorps.Analytics.SegmentTraitsBuilder do } end defp traits(%{token: _, user_id: _}), do: %{} + + @spec blank_map_if_nil(struct | nil) :: struct | map + defp blank_map_if_nil(nil), do: %{} + defp blank_map_if_nil(struct), do: struct end From 965ea8381d2914c6513db34ef7cb9a891dffa4e2 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 13:40:22 +0100 Subject: [PATCH 23/24] Switch claimed_invite to has_many --- lib/code_corps/accounts/user_invites.ex | 2 +- lib/code_corps/model/user.ex | 2 +- lib/code_corps_web/controllers/user_controller.ex | 13 +++++++------ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/code_corps/accounts/user_invites.ex b/lib/code_corps/accounts/user_invites.ex index 7b2f0d0e3..334be9dec 100644 --- a/lib/code_corps/accounts/user_invites.ex +++ b/lib/code_corps/accounts/user_invites.ex @@ -106,7 +106,7 @@ defmodule CodeCorps.Accounts.UserInvites do @spec marshall_response(tuple) :: tuple defp marshall_response({:ok, %{user: user, user_invite: user_invite}}) do - {:ok, user |> Map.put(:claimed_invite, user_invite)} + {:ok, user |> Map.put(:claimed_invites, [user_invite])} end defp marshall_response({:error, :load_invite, :not_found, _}), do: {:error, :invite_not_found} defp marshall_response({:error, :user, %Changeset{} = changeset, _}), do: {:error, changeset} diff --git a/lib/code_corps/model/user.ex b/lib/code_corps/model/user.ex index f87a98964..e4f84a5aa 100644 --- a/lib/code_corps/model/user.ex +++ b/lib/code_corps/model/user.ex @@ -40,10 +40,10 @@ defmodule CodeCorps.User do field :username, :string field :website, :string - has_one :claimed_invite, CodeCorps.UserInvite, foreign_key: :invitee_id has_one :github_app_installation, CodeCorps.GithubAppInstallation has_one :slugged_route, SluggedRoute + has_many :claimed_invites, CodeCorps.UserInvite, foreign_key: :invitee_id has_many :github_app_installations, CodeCorps.GithubAppInstallation has_many :organizations, CodeCorps.Organization, foreign_key: :owner_id has_many :project_users, CodeCorps.ProjectUser diff --git a/lib/code_corps_web/controllers/user_controller.ex b/lib/code_corps_web/controllers/user_controller.ex index 94173ccd3..a238eeed1 100644 --- a/lib/code_corps_web/controllers/user_controller.ex +++ b/lib/code_corps_web/controllers/user_controller.ex @@ -3,13 +3,12 @@ defmodule CodeCorpsWeb.UserController do use CodeCorpsWeb, :controller alias CodeCorps.{ + Accounts, Analytics, GitHub, Helpers.Query, Services.UserService, - User, - UserInvite, - Accounts + User } action_fallback(CodeCorpsWeb.FallbackController) @@ -39,9 +38,11 @@ defmodule CodeCorpsWeb.UserController do @spec create(Conn.t(), map) :: Conn.t() def create(%Conn{} = conn, %{} = params) do with {:ok, %User{} = user} <- params |> Accounts.create() do - case user |> Map.get(:claimed_invite) do - %UserInvite{} = user_invite -> - user.id |> Analytics.SegmentTracker.track("Claimed User Invite", user_invite) + case user |> Map.get(:claimed_invites) do + user_invites when is_list(user_invites) -> + user_invites |> Enum.map(fn invite -> + user.id |> Analytics.SegmentTracker.track("Claimed User Invite", invite) + end) _other -> nil From b60e8a5480a5156c6caf1479667c03c585d1e438 Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 3 Jan 2018 14:05:13 +0100 Subject: [PATCH 24/24] Render 404 on user creation if invite specified but not found --- .../controllers/user_controller.ex | 28 +++++++++++++------ .../controllers/user_controller_test.exs | 11 ++++++-- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/lib/code_corps_web/controllers/user_controller.ex b/lib/code_corps_web/controllers/user_controller.ex index a238eeed1..5b57ecfd2 100644 --- a/lib/code_corps_web/controllers/user_controller.ex +++ b/lib/code_corps_web/controllers/user_controller.ex @@ -38,22 +38,32 @@ defmodule CodeCorpsWeb.UserController do @spec create(Conn.t(), map) :: Conn.t() def create(%Conn{} = conn, %{} = params) do with {:ok, %User{} = user} <- params |> Accounts.create() do - case user |> Map.get(:claimed_invites) do - user_invites when is_list(user_invites) -> - user_invites |> Enum.map(fn invite -> - user.id |> Analytics.SegmentTracker.track("Claimed User Invite", invite) - end) - - _other -> - nil - end + user |> maybe_track_claimed_invites conn |> put_status(:created) |> render("show.json-api", data: user |> preload()) + else + {:error, :invite_not_found} -> + conn + |> put_status(:not_found) + |> render(CodeCorpsWeb.ErrorView, "404.json", %{}) + + other -> + other end end + @spec maybe_track_claimed_invites(User.t()) :: list() | :ok + defp maybe_track_claimed_invites(%User{claimed_invites: invites} = user) when is_list(invites) do + invites + |> Enum.map(fn invite -> + user.id |> Analytics.SegmentTracker.track("Claimed User Invite", invite) + end) + end + + defp maybe_track_claimed_invites(%User{}), do: :ok + @spec update(Conn.t(), map) :: Conn.t() def update(%Conn{} = conn, %{"id" => id} = params) do with %User{} = user <- User |> Repo.get(id), 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 1e3e106b3..58e5a0337 100644 --- a/test/lib/code_corps_web/controllers/user_controller_test.exs +++ b/test/lib/code_corps_web/controllers/user_controller_test.exs @@ -190,9 +190,7 @@ defmodule CodeCorpsWeb.UserControllerTest do assert_received({:track, ^invitee_id, "Claimed User Invite", ^traits}) end - test "supports claiming a user invite to a project if specified by an id parameter", %{ - conn: conn - } do + test "supports claiming a user invite to a project if specified by an id parameter", %{conn: conn} do project = insert(:project) %{id: invite_id, email: email} = @@ -216,6 +214,13 @@ defmodule CodeCorpsWeb.UserControllerTest do assert membership.project_id == invite.project_id assert membership.role == invite.role end + + test "renders a 404 if attempting to claim an nonexistent invite", %{conn: conn} do + attrs = @valid_attrs |> Map.merge(%{invite_id: -1, password: "password"}) + + path = conn |> user_path(:create) + assert conn |> post(path, attrs) |> json_response(404) + end end describe "update" do