From 885437a4e5f5369717558fbd3766e6f0bb80e201 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Tue, 12 Aug 2025 13:08:46 -0300 Subject: [PATCH] Apply review comments --- lib/livebook/hubs/team_client.ex | 40 +++++++---------- lib/livebook/teams.ex | 8 ++-- lib/livebook/teams/broadcasts.ex | 8 ++-- .../live/session_live/app_teams_live.ex | 45 ++++++++++++------- 4 files changed, 55 insertions(+), 46 deletions(-) diff --git a/lib/livebook/hubs/team_client.ex b/lib/livebook/hubs/team_client.ex index 5c2f1f5c9..9250680e6 100644 --- a/lib/livebook/hubs/team_client.ex +++ b/lib/livebook/hubs/team_client.ex @@ -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 diff --git a/lib/livebook/teams.ex b/lib/livebook/teams.ex index 5b31f991a..8e688b54d 100644 --- a/lib/livebook/teams.ex +++ b/lib/livebook/teams.ex @@ -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 diff --git a/lib/livebook/teams/broadcasts.ex b/lib/livebook/teams/broadcasts.ex index 340617e6b..d94d17eb1 100644 --- a/lib/livebook/teams/broadcasts.ex +++ b/lib/livebook/teams/broadcasts.ex @@ -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 """ diff --git a/lib/livebook_web/live/session_live/app_teams_live.ex b/lib/livebook_web/live/session_live/app_teams_live.ex index 4197e7262..9e4933d9e 100644 --- a/lib/livebook_web/live/session_live/app_teams_live.ex +++ b/lib/livebook_web/live/session_live/app_teams_live.ex @@ -81,6 +81,7 @@ defmodule LivebookWeb.SessionLive.AppTeamsLive do messages={@messages} action={@action} initial?={@initial?} + authorized={@authorized} /> """ @@ -162,7 +163,12 @@ defmodule LivebookWeb.SessionLive.AppTeamsLive do <.remix_icon icon="rocket-line" /> Deploy <% 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 <% 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 /> @@ -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 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 @@ -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 /> @@ -313,16 +325,12 @@ defmodule LivebookWeb.SessionLive.AppTeamsLive do
"!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