Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User invites #1351

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
df090d7
Add Accounts.create_invite
begedin Dec 28, 2017
6c6d22e
Add Accounts.claim_invite/1
begedin Dec 28, 2017
c9ce8fc
Switch Accounts.claim_invite to accept a params map
begedin Dec 28, 2017
bf72b97
Use ProjectUser as provider of valid roles for UserInvite validation
begedin Dec 29, 2017
3aa1a0f
Remove default "contributor" role from UserInvite
begedin Dec 29, 2017
9e137ca
Add tests for project user invite
begedin Dec 29, 2017
21fe4d4
Update user requirement on create to prevent invite of existing users
begedin Jan 2, 2018
00d2646
Add invite claim support to user controller
begedin Jan 2, 2018
64246b9
Refactor account creation by extracting into Accounts context
begedin Jan 2, 2018
c56eee7
Add policy and tests
begedin Jan 2, 2018
2969154
Clean up formatting in UserController
begedin Jan 3, 2018
93a6d67
Fix moduledoc in Policy.ConversationPart
begedin Jan 3, 2018
36eacd2
Add UserInviteView and tests
begedin Jan 3, 2018
cba0b7a
Correct Policy.UserInvite behavior
begedin Jan 3, 2018
88b103b
Add inviter_id support to Plugs.IdsToInteger
begedin Jan 3, 2018
d7a7b1d
Add UserInvite support to CodeCorps.Policy.can?
begedin Jan 3, 2018
461c8a4
Add UserInviteController with create endpoint and associated tests
begedin Jan 3, 2018
776a9dd
Track UserInvite claim
begedin Jan 3, 2018
bb1f4c2
Format CodeCorps.UserInvite
begedin Jan 3, 2018
561dfe2
Move invite claim tracking to controller
begedin Jan 3, 2018
d639acb
Assume new user when claiming invite
begedin Jan 3, 2018
1145d37
Add helper to SegmentTraitsBuilder for improved clarity
begedin Jan 3, 2018
965ea83
Switch claimed_invite to has_many
begedin Jan 3, 2018
b60e8a5
Render 404 on user creation if invite specified but not found
begedin Jan 3, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions lib/code_corps/accounts/accounts.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ defmodule CodeCorps.Accounts do
"""

alias CodeCorps.{
Accounts,
Accounts.Changesets,
Comment,
GitHub.Adapters,
Expand All @@ -14,12 +15,28 @@ defmodule CodeCorps.Accounts do
Processor,
Task,
User,
UserInvite,
Repo
}
alias Ecto.{Changeset, Multi}

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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We needed to extract creation into a context. I didn't have a better idea other than this multiple clause approach to differentiate between plain account creation and an invite claim.


@doc ~S"""
Creates a user record using attributes from a GitHub payload.
"""
Expand Down Expand Up @@ -184,4 +201,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
114 changes: 114 additions & 0 deletions lib/code_corps/accounts/user_invites.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
defmodule CodeCorps.Accounts.UserInvites do
@moduledoc ~S"""
Subcontext for managing of `UserInvite` records
"""

alias CodeCorps.{Project, ProjectUser, Repo, User, UserInvite}
alias Ecto.{Changeset, Multi}

@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, ProjectUser.roles())
|> Changeset.assoc_constraint(:inviter)
|> Changeset.assoc_constraint(:project)
|> ensure_email_not_owned_by_user()
|> ensure_role_and_project()
|> Repo.insert()
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 = [
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hoping there's a shorter way to write this, but couldn't think of anything.


@spec claim_invite(map) :: {:ok, User.t()}
def claim_invite(%{} = params) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to accommodate invites for projects and general invites to Code Corps.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, the code sort of does that. It will

  • create a user
  • join that user with the project, provided the project and role are specified
  • assign that user to the assignee

I'm guessing an email will also be involved, which should be pluggable into this process

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} ->
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()
|> marshall_response()
end

@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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed, we assume new user.

Existing users will eventually be invitable into the project by creating a special type of ProjectUser.

Also, as a user registers for an account directly, not via invite, there should be a step where any existing invites are claimed.


@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
%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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, validation can fail due to the unique_constraint. However, since the only case that can fail is if the user already exists, and the multi will fail on the previous step in that case, this can never happen.


defp join_project(%User{}, %UserInvite{}), do: {:ok, nil}

@spec associate_invitee(UserInvite.t(), User.t()) :: {:ok, UserInvite.t()}
defp associate_invitee(%UserInvite{invitee: nil} = invite, %User{} = user) do
invite
|> Changeset.change(%{})
|> Changeset.put_assoc(:invitee, user)
|> Repo.update()
end

@spec marshall_response(tuple) :: tuple
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({:error, :user, %Changeset{} = changeset, _}), do: {:error, changeset}
defp marshall_response(other_tuple), do: other_tuple
end
20 changes: 19 additions & 1 deletion lib/code_corps/analytics/segment_traits_builder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ defmodule CodeCorps.Analytics.SegmentTraitsBuilder do
record
|> Repo.preload([:project])
|> Map.get(:project)
|> (&(&1 || %{})).()
|> blank_map_if_nil()
|> Map.get(:title, "")

%{
Expand Down Expand Up @@ -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) |> 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)|> blank_map_if_nil() |> 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)
%{
Expand All @@ -225,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
15 changes: 9 additions & 6 deletions lib/code_corps/model/project_user.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""
Expand Down Expand Up @@ -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}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did an auto-formatting of the file as I was making the roles function public (to be used when validating a UserInvite

end
end
1 change: 1 addition & 0 deletions lib/code_corps/model/user.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
17 changes: 17 additions & 0 deletions lib/code_corps/model/user_invite.ex
Original file line number Diff line number Diff line change
@@ -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)
field(:name, :string)

belongs_to(:project, CodeCorps.Project)
belongs_to(:inviter, CodeCorps.User)
belongs_to(:invitee, CodeCorps.User)

timestamps()
end
end
4 changes: 2 additions & 2 deletions lib/code_corps/policy/conversation_part.ex
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
4 changes: 4 additions & 0 deletions lib/code_corps/policy/policy.ex
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ defmodule CodeCorps.Policy do
TaskSkill,
User,
UserCategory,
UserInvite,
UserRole,
UserSkill,
UserTask
Expand Down Expand Up @@ -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)
Expand Down
42 changes: 42 additions & 0 deletions lib/code_corps/policy/user_invite.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
defmodule CodeCorps.Policy.UserInvite do
@moduledoc ~S"""
Handles `CodeCorps.User` authorization of actions on `CodeCorps.UserInvite`
records.
"""

alias CodeCorps.{Policy.Helpers, User}

@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()
|> Helpers.get_membership(user)
|> Helpers.get_role()

new_role = Map.get(params, "role")
do_create?(user_role, new_role)
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two conditions for the case of a project invite

  • user is the inviter
  • user has a role with authority to invite a user with the specified role


def create?(%User{id: user_id}, %{"inviter_id" => inviter_id})
when user_id == inviter_id,
do: true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One condition for a plain invite

  • user is the inviter


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
defp do_create?("owner", "contributor"), do: true
defp do_create?("owner", "admin"), do: true
defp do_create?(_, _), do: false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could a contributor also invite a "pending" user, or even another contributor?

end
4 changes: 2 additions & 2 deletions lib/code_corps/validators/slug_validator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down
Loading