Apply review comments

This commit is contained in:
Alexandre de Souza 2025-08-12 13:08:46 -03:00
parent 7a7dc7867c
commit 885437a4e5
No known key found for this signature in database
GPG key ID: E39228FFBA346545
4 changed files with 55 additions and 46 deletions

View file

@ -165,14 +165,11 @@ defmodule Livebook.Hubs.TeamClient do
end
@doc """
Returns if the given user has access to deploy apps to Teams.
Returns if the given user has access to deploy apps to given deployment group.
"""
@spec authorized_user_to_deploy?(String.t(), pos_integer() | nil, String.t()) :: boolean()
def authorized_user_to_deploy?(id, user_id, deployment_group_id) do
GenServer.call(
registry_name(id),
{:check_deployment_authorization, user_id, deployment_group_id}
)
@spec user_can_deploy?(String.t(), pos_integer() | nil, String.t()) :: boolean()
def user_can_deploy?(id, user_id, deployment_group_id) do
GenServer.call(registry_name(id), {:user_can_deploy?, user_id, deployment_group_id})
end
@doc """
@ -349,7 +346,7 @@ defmodule Livebook.Hubs.TeamClient do
end
end
def handle_call({:check_deployment_authorization, user_id, id}, _caller, state) do
def handle_call({:user_can_deploy?, user_id, id}, _caller, state) do
# App servers/Offline instances should not be able to deploy apps
if state.deployment_group_id || user_id == nil do
{:reply, false, state}
@ -746,22 +743,19 @@ defmodule Livebook.Hubs.TeamClient do
Teams.Broadcasts.deployment_group_updated(deployment_group)
with {:ok, current_deployment_group} <- fetch_deployment_group(deployment_group.id, state) do
cond do
state.deployment_group_id == deployment_group.id and
(current_deployment_group.authorization_groups !=
deployment_group.authorization_groups or
current_deployment_group.groups_auth != deployment_group.groups_auth or
current_deployment_group.teams_auth != deployment_group.teams_auth) ->
Teams.Broadcasts.server_authorization_updated(deployment_group)
if state.deployment_group_id == deployment_group.id and
(current_deployment_group.authorization_groups !=
deployment_group.authorization_groups or
current_deployment_group.groups_auth != deployment_group.groups_auth or
current_deployment_group.teams_auth != deployment_group.teams_auth) do
Teams.Broadcasts.server_authorization_updated(deployment_group)
end
state.deployment_group_id == nil and
(current_deployment_group.deployment_users !=
deployment_group.deployment_users or
current_deployment_group.deploy_auth != deployment_group.deploy_auth) ->
Teams.Broadcasts.deployment_authorization_updated(deployment_group)
true ->
:noop
if state.deployment_group_id == nil and
(current_deployment_group.deployment_users !=
deployment_group.deployment_users or
current_deployment_group.deploy_auth != deployment_group.deploy_auth) do
Teams.Broadcasts.deployment_users_updated(deployment_group)
end
end

View file

@ -296,10 +296,10 @@ defmodule Livebook.Teams do
end
@doc """
Deploys the given app deployment to given deployment group using a deploy key.
Checks if the given user has access to deploy apps to given deployment group.
"""
@spec authorized_user_to_deploy?(Team.t(), Teams.DeploymentGroup.t()) :: boolean()
def authorized_user_to_deploy?(%Team{} = team, %Teams.DeploymentGroup{} = deployment_group) do
TeamClient.authorized_user_to_deploy?(team.id, team.user_id, deployment_group.id)
@spec user_can_deploy?(Team.t(), Teams.DeploymentGroup.t()) :: boolean()
def user_can_deploy?(%Team{} = team, %Teams.DeploymentGroup{} = deployment_group) do
TeamClient.user_can_deploy?(team.id, team.user_id, deployment_group.id)
end
end

View file

@ -34,7 +34,7 @@ defmodule Livebook.Teams.Broadcasts do
* `{:deployment_group_created, DeploymentGroup.t()}`
* `{:deployment_group_updated, DeploymentGroup.t()}`
* `{:deployment_group_deleted, DeploymentGroup.t()}`
* `{:deployment_authorization_updated, DeploymentGroup.t()}`
* `{:deployment_users_updated, DeploymentGroup.t()}`
Topic `#{@app_server_topic}`:
@ -101,9 +101,9 @@ defmodule Livebook.Teams.Broadcasts do
@doc """
Broadcasts under `#{@deployment_groups_topic}` topic when hub received an updated deployment group that changed which users have access to deploy apps.
"""
@spec deployment_authorization_updated(Teams.DeploymentGroup.t()) :: broadcast()
def deployment_authorization_updated(%Teams.DeploymentGroup{} = deployment_group) do
broadcast(@deployment_groups_topic, {:deployment_authorization_updated, deployment_group})
@spec deployment_users_updated(Teams.DeploymentGroup.t()) :: broadcast()
def deployment_users_updated(%Teams.DeploymentGroup{} = deployment_group) do
broadcast(@deployment_groups_topic, {:deployment_users_updated, deployment_group})
end
@doc """

View file

@ -81,6 +81,7 @@ defmodule LivebookWeb.SessionLive.AppTeamsLive do
messages={@messages}
action={@action}
initial?={@initial?}
authorized={@authorized}
/>
</div>
"""
@ -162,7 +163,12 @@ defmodule LivebookWeb.SessionLive.AppTeamsLive do
<.remix_icon icon="rocket-line" /> Deploy
</.button>
<% else %>
<.button color="blue" outlined phx-click="deploy_app">
<.button
disabled={!@authorized[@deployment_group.id]}
color="blue"
outlined
phx-click="deploy_app"
>
<.remix_icon icon="rocket-line" /> Deploy anyway
</.button>
<% end %>
@ -198,6 +204,7 @@ defmodule LivebookWeb.SessionLive.AppTeamsLive do
deployment_group={@deployment_group}
num_agents={@num_agents}
num_app_deployments={@num_app_deployments}
authorized={@authorized[@deployment_group.id]}
active
/>
</div>
@ -224,7 +231,12 @@ defmodule LivebookWeb.SessionLive.AppTeamsLive do
<.button color="blue" phx-click="go_add_agent">
<.remix_icon icon="add-line" /> Add app server
</.button>
<.button color="blue" outlined phx-click="deploy_app">
<.button
disabled={!@authorized[@deployment_group.id]}
color="blue"
outlined
phx-click="deploy_app"
>
<.remix_icon icon="rocket-line" /> Deploy anyway
</.button>
</div>
@ -250,7 +262,7 @@ defmodule LivebookWeb.SessionLive.AppTeamsLive do
deployment_group={deployment_group}
num_agents={@num_agents}
num_app_deployments={@num_app_deployments}
authorized={Teams.authorized_user_to_deploy?(@hub, deployment_group)}
authorized={@authorized[deployment_group.id]}
selectable
/>
</div>
@ -313,16 +325,12 @@ defmodule LivebookWeb.SessionLive.AppTeamsLive do
<div
class={[
"border p-3 rounded-lg relative",
@selectable && @authorized && "cursor-pointer",
@selectable && !@authorized && "cursor-not-allowed",
!@authorized && "tooltip top",
if(@active,
do: "border-blue-600 bg-blue-50",
else: "border-gray-200"
),
!@authorized && "opacity-50 bg-gray-50"
cond do
!@authorized -> "!block cursor-not-allowed tooltip top opacity-50 bg-gray-50"
@selectable -> "cursor-pointer border-blue-600 bg-blue-50"
true -> "cursor-pointer border-gray-200"
end
]}
style={!@authorized && "display: block !important;"}
data-tooltip={!@authorized && "You are not authorized to deploy to this deployment group"}
phx-click={@selectable && @authorized && "select_deployment_group"}
phx-value-id={@deployment_group.id}
@ -462,7 +470,7 @@ defmodule LivebookWeb.SessionLive.AppTeamsLive do
{:noreply, socket |> assign_app_deployments() |> assign_app_deployment()}
end
def handle_info({:deployment_authorization_updated, deployment_group}, socket)
def handle_info({:deployment_users_updated, deployment_group}, socket)
when deployment_group.hub_id == socket.assigns.hub.id do
{:noreply, assign_deployment_groups(socket)}
end
@ -480,13 +488,20 @@ defmodule LivebookWeb.SessionLive.AppTeamsLive do
def handle_info(_message, socket), do: {:noreply, socket}
defp assign_deployment_groups(socket) do
hub = socket.assigns.hub
deployment_groups =
socket.assigns.hub
hub
|> Teams.get_deployment_groups()
|> Enum.filter(&(&1.mode == :online))
|> Enum.sort_by(& &1.name)
assign(socket, deployment_groups: deployment_groups)
authorized =
for deployment_group <- deployment_groups, into: %{} do
{deployment_group.id, Teams.user_can_deploy?(hub, deployment_group)}
end
assign(socket, deployment_groups: deployment_groups, authorized: authorized)
end
defp assign_app_deployments(socket) do