Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
102 changes: 90 additions & 12 deletions lib/logflare_web/controllers/api/team_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,30 @@ defmodule LogflareWeb.Api.TeamController do
use LogflareWeb, :controller
use OpenApiSpex.ControllerSpecs

alias Logflare.Teams
require Logger

alias Logflare.Backends.Adaptor.BigQueryAdaptor
alias Logflare.Teams
alias Logflare.Teams.Team
alias Logflare.TeamUsers
alias Logflare.TeamUsers.TeamUser
alias Logflare.User
alias Logflare.Users
alias LogflareWeb.OpenApi.Accepted
alias LogflareWeb.OpenApi.Created
alias LogflareWeb.OpenApi.List
alias LogflareWeb.OpenApi.NotFound
alias LogflareWeb.OpenApi.UnprocessableEntity

alias LogflareWeb.OpenApiSchemas.Team
alias LogflareWeb.OpenApiSchemas.Team, as: TeamSchema
alias LogflareWeb.OpenApiSchemas.TeamUser, as: TeamUserSchema

action_fallback(LogflareWeb.Api.FallbackController)

tags(["management"])

operation(:index,
summary: "List teams",
responses: %{200 => List.response(Team)}
responses: %{200 => List.response(TeamSchema)}
)

def index(%{assigns: %{user: user}} = conn, _) do
Expand All @@ -30,23 +37,23 @@ defmodule LogflareWeb.Api.TeamController do
summary: "Fetch team",
parameters: [token: [in: :path, description: "Team Token", type: :string]],
responses: %{
200 => Team.response(),
200 => TeamSchema.response(),
404 => NotFound.response()
}
)

def show(%{assigns: %{user: user}} = conn, %{"token" => token}) do
with team when not is_nil(team) <- Teams.get_team_by_user_access(user, token),
with %Team{} = team <- Teams.get_team_by_user_access(user, token),
team <- Teams.preload_fields(team, [:user, :team_users]) do
json(conn, team)
end
end

operation(:create,
summary: "Create Team",
request_body: Team.params(),
request_body: TeamSchema.params(),
responses: %{
201 => Created.response(Team),
201 => Created.response(TeamSchema),
404 => NotFound.response(),
422 => UnprocessableEntity.response()
}
Expand All @@ -64,17 +71,17 @@ defmodule LogflareWeb.Api.TeamController do
operation(:update,
summary: "Update team",
parameters: [token: [in: :path, description: "Team Token", type: :string]],
request_body: Team.params(),
request_body: TeamSchema.params(),
responses: %{
201 => Created.response(Team),
201 => Created.response(TeamSchema),
204 => Accepted.response(),
404 => NotFound.response(),
422 => UnprocessableEntity.response()
}
)

def update(%{assigns: %{user: user}} = conn, %{"token" => token} = params) do
with team when not is_nil(team) <- Teams.get_team_by(token: token, user_id: user.id),
with %Team{} = team <- Teams.get_team_by(token: token, user_id: user.id),
{:ok, team} <- Teams.update_team(team, params),
team <- Teams.preload_fields(team, [:user, :team_users]) do
conn
Expand All @@ -100,11 +107,82 @@ defmodule LogflareWeb.Api.TeamController do
)

def delete(%{assigns: %{user: user}} = conn, %{"token" => token}) do
with team when not is_nil(team) <- Teams.get_team_by(token: token, user_id: user.id),
with %Team{} = team <- Teams.get_team_by(token: token, user_id: user.id),
{:ok, _} <- Teams.delete_team(team) do
conn
|> Plug.Conn.send_resp(204, [])
|> Plug.Conn.halt()
end
end

operation(:add_member,
summary: "Add Team Member",
parameters: [
token: [in: :path, description: "Team Token", type: :string]
],
request_body: TeamUserSchema.params(),
responses: %{
204 => Accepted.response(),
404 => NotFound.response()
}
)

def add_member(%{assigns: %{user: user}} = conn, %{"team_token" => token, "email" => email}) do
auth_params =
case Users.get_by(email: email) do
nil ->
{:ok, new_user} = Users.insert_user(%{email: email, provider: user.provider})
Logger.info("Created new user #{email}")

%{
email: new_user.email,
provider_uid: new_user.provider_uid,
provider: new_user.provider
}

%User{} = existing_user ->
%{
email: existing_user.email,
provider_uid: existing_user.provider_uid,
provider: existing_user.provider
}
end

with %Team{} = team <- Teams.get_team_by(token: token, user_id: user.id),
team <- Teams.preload_user(team),
{:ok, _} <- TeamUsers.insert_or_update_team_user(team, auth_params) do
BigQueryAdaptor.update_iam_policy()
BigQueryAdaptor.patch_dataset_access(team.user)

conn
|> Plug.Conn.send_resp(204, [])
|> Plug.Conn.halt()
end
end

operation(:remove_member,
summary: "Remove Team Member",
parameters: [
token: [in: :path, description: "Team Token", type: :string],
id: [in: :path, description: "User ID as an email", type: :string]
],
responses: %{
204 => Accepted.response(),
404 => NotFound.response()
}
)

def remove_member(%{assigns: %{user: user}} = conn, %{"team_token" => token, "id" => id}) do
with %TeamUser{} = team_user <- TeamUsers.get_team_user_by(email: id),
%Team{} = team <- Teams.get_team_by(token: token, user_id: user.id),
team <- Teams.preload_user(team),
{:ok, _} <- TeamUsers.delete_team_user(team_user) do
BigQueryAdaptor.update_iam_policy()
BigQueryAdaptor.patch_dataset_access(team.user)

conn
|> Plug.Conn.send_resp(204, [])
|> Plug.Conn.halt()
end
end
end
5 changes: 4 additions & 1 deletion lib/logflare_web/router.ex
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,10 @@ defmodule LogflareWeb.Router do
resources("/teams", Api.TeamController,
param: "token",
only: [:index, :show, :create, :update, :delete]
)
) do
post "/members", Api.TeamController, :add_member
delete "/members/:id", Api.TeamController, :remove_member
end

resources("/backends", Api.BackendController,
param: "token",
Expand Down
98 changes: 98 additions & 0 deletions test/logflare_web/controllers/api/team_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ defmodule LogflareWeb.Api.TeamControllerTest do
@moduledoc false
use LogflareWeb.ConnCase

alias Logflare.TeamUsers

setup do
insert(:plan)
user = insert(:user)
Expand Down Expand Up @@ -201,4 +203,100 @@ defmodule LogflareWeb.Api.TeamControllerTest do
|> assert_schema("NotFoundResponse") == %{error: "Not Found"}
end
end

describe "add_member/2" do
test "adds an existing user to a team", %{
Copy link
Contributor

Choose a reason for hiding this comment

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

also needs case where there is no existing user record and we only create the record on team_users table

conn: conn,
user: user,
main_team: main_team
} do
new_member = insert(:user)

assert conn
|> add_access_token(user, "private")
|> post(~p"/api/teams/#{main_team.token}/members", %{email: new_member.email})
|> response(204)
|> assert_schema("AcceptedResponse") == ""
Copy link
Contributor

Choose a reason for hiding this comment

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

should return created user, should also be a 201 since we're creating a team_user


team_users = TeamUsers.list_team_users_by(team_id: main_team.id)

assert Enum.any?(team_users, fn tu ->
tu.email == String.downcase(new_member.email)
end)
end

test "creates a new team member when adding with non-existent email", %{
conn: conn,
user: user,
main_team: main_team
} do
new_email = "newuser@example.com"

assert conn
|> add_access_token(user, "private")
|> post(~p"/api/teams/#{main_team.token}/members", %{email: new_email})
|> response(204)
|> assert_schema("AcceptedResponse") == ""

team_users = TeamUsers.list_team_users_by(team_id: main_team.id)
assert Enum.any?(team_users, fn tu -> tu.email == String.downcase(new_email) end)
end

test "returns not found if doesn't own the team", %{conn: conn, main_team: main_team} do
invalid_user = insert(:user)
new_member = insert(:user)

assert conn
|> add_access_token(invalid_user, "private")
|> post(~p"/api/teams/#{main_team.token}/members", %{email: new_member.email})
|> json_response(404)
|> assert_schema("NotFoundResponse") == %{error: "Not Found"}
end
Comment on lines +245 to +254
Copy link
Contributor

Choose a reason for hiding this comment

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

401 would be more appropriate.

would also need a 400 test for bad team_user attrs

end

describe "remove_member/2" do
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
describe "remove_member/2" do
describe "remove_member/2 for team_user with user record" do

Also needs a test for team_user with when there is no user record, which is when they are invited without having a prior account.

test "removes a member from a team", %{
conn: conn,
user: user,
main_team: main_team
} do
member_to_remove = insert(:user)
insert(:team_user, team: main_team, email: member_to_remove.email)

assert conn
|> add_access_token(user, "private")
|> delete(~p"/api/teams/#{main_team.token}/members/#{member_to_remove.email}")
|> response(204)
|> assert_schema("AcceptedResponse") == ""

team_users = TeamUsers.list_team_users_by(team_id: main_team.id)
refute Enum.any?(team_users, fn tu -> tu.email == member_to_remove.email end)
end

test "returns not found if doesn't own the team", %{conn: conn, main_team: main_team} do
invalid_user = insert(:user)
member = insert(:user)
insert(:team_user, team: main_team, email: member.email)

assert conn
|> add_access_token(invalid_user, "private")
|> delete(~p"/api/teams/#{main_team.token}/members/#{member.email}")
|> json_response(404)
|> assert_schema("NotFoundResponse") == %{error: "Not Found"}
end

test "returns not found if team member doesn't exist", %{
conn: conn,
user: user,
main_team: main_team
} do
non_existent_email = "nonexistent@example.com"

assert conn
|> add_access_token(user, "private")
|> delete(~p"/api/teams/#{main_team.token}/members/#{non_existent_email}")
|> json_response(404)
|> assert_schema("NotFoundResponse") == %{error: "Not Found"}
end
end
end
Loading