Send more fields to Teams API when deploying an app (#2563)

lib/livebook/teams.ex

@@ -4,6 +4,7 @@ defmodule Livebook.Teams do
   alias Livebook.Hubs
   alias Livebook.Hubs.Team
   alias Livebook.Hubs.TeamClient
+  alias Livebook.Notebook.AppSettings
   alias Livebook.Teams.{Agent, AppDeployment, DeploymentGroup, Org, Requests}
 
   import Ecto.Changeset,
@@ -49,13 +50,9 @@ defmodule Livebook.Teams do
       {:error, %Ecto.Changeset{} = changeset} ->
         {:error, changeset}
 
-      {:error, %{"errors" => errors_map}} ->
-        errors_map =
-          if errors = errors_map["key_hash"],
-            do: Map.put_new(errors_map, "teams_key", errors),
-            else: errors_map
-
-        {:error, add_org_errors(changeset, errors_map)}
+      {:error, %{"errors" => errors}} ->
+        errors = map_teams_field_to_livebook_field(errors, "key_hash", "teams_key")
+        {:error, add_org_errors(changeset, errors)}
 
       any ->
         any
@@ -194,19 +191,24 @@ defmodule Livebook.Teams do
   @doc """
   Creates a new app deployment.
   """
-  @spec deploy_app(Team.t(), AppDeployment.t()) ::
+  @spec deploy_app(Team.t(), AppDeployment.t(), AppSettings.t()) ::
           :ok
           | {:error, Ecto.Changeset.t()}
           | {:transport_error, String.t()}
-  def deploy_app(%Team{} = team, %AppDeployment{} = app_deployment) do
-    case Requests.deploy_app(team, app_deployment) do
+  def deploy_app(%Team{} = team, %AppDeployment{} = app_deployment, %AppSettings{} = app_settings) do
+    case Requests.deploy_app(team, app_deployment, app_settings) do
       {:ok, %{"id" => _id}} ->
         :ok
 
       {:error, %{"errors" => %{"detail" => error}}} ->
         {:error, Requests.add_errors(app_deployment, %{"file" => [error]})}
 
       {:error, %{"errors" => errors}} ->
+        errors =
+          errors
+          |> map_teams_field_to_livebook_field("multi_session", "file")
+          |> map_teams_field_to_livebook_field("access_type", "file")
+
         {:error, Requests.add_errors(app_deployment, errors)}
 
       any ->
@@ -229,4 +231,12 @@ defmodule Livebook.Teams do
   def get_agents(team) do
     TeamClient.get_agents(team.id)
   end
+
+  defp map_teams_field_to_livebook_field(map, teams_field, livebook_field) do
+    if value = map[teams_field] do
+      Map.put_new(map, livebook_field, value)
+    else
+      map
+    end
+  end
 end

lib/livebook/teams/requests.ex

@@ -2,6 +2,7 @@ defmodule Livebook.Teams.Requests do
   alias Livebook.FileSystem
   alias Livebook.FileSystems
   alias Livebook.Hubs.Team
+  alias Livebook.Notebook.AppSettings
   alias Livebook.Secrets.Secret
   alias Livebook.Teams
   alias Livebook.Teams.{AppDeployment, DeploymentGroup, Org}
@@ -183,14 +184,16 @@ defmodule Livebook.Teams.Requests do
   @doc """
   Send a request to Livebook Team API to deploy an app.
   """
-  @spec deploy_app(Team.t(), AppDeployment.t()) ::
+  @spec deploy_app(Team.t(), AppDeployment.t(), AppSettings.t()) ::
           {:ok, map()} | {:error, map() | String.t()} | {:transport_error, String.t()}
-  def deploy_app(team, app_deployment) do
+  def deploy_app(team, app_deployment, app_settings) do
     secret_key = Teams.derive_key(team.teams_key)
 
     params = %{
       title: app_deployment.title,
       slug: app_deployment.slug,
+      multi_session: app_settings.multi_session,
+      access_type: app_settings.access_type,
       deployment_group_id: app_deployment.deployment_group_id,
       sha: app_deployment.sha
     }

lib/livebook_web/live/session_live/app_teams_component.ex

@@ -165,17 +165,18 @@ defmodule LivebookWeb.SessionLive.AppTeamsComponent do
   end
 
   def handle_event("deploy_app", _, socket) do
-    with {:ok, app_deployment} <- pack_app(socket),
-         :ok <- deploy_app(socket, app_deployment) do
+    notebook = Livebook.Session.get_notebook(socket.assigns.session.pid)
+
+    with {:ok, app_deployment} <- pack_app(socket, notebook),
+         :ok <- deploy_app(socket, app_deployment, notebook.app_settings) do
       message =
         "App deployment for #{app_deployment.slug} with title #{app_deployment.title} created successfully"
 
       {:noreply, assign(socket, messages: [{:info, message}])}
     end
   end
 
-  defp pack_app(socket) do
-    notebook = Livebook.Session.get_notebook(socket.assigns.session.pid)
+  defp pack_app(socket, notebook) do
     files_dir = socket.assigns.session.files_dir
 
     case Livebook.Teams.AppDeployment.new(notebook, files_dir) do
@@ -192,8 +193,8 @@ defmodule LivebookWeb.SessionLive.AppTeamsComponent do
     end
   end
 
-  defp deploy_app(socket, app_deployment) do
-    case Livebook.Teams.deploy_app(socket.assigns.hub, app_deployment) do
+  defp deploy_app(socket, app_deployment, app_settings) do
+    case Livebook.Teams.deploy_app(socket.assigns.hub, app_deployment, app_settings) do
       :ok ->
         :ok
 

test/livebook_teams/hubs/team_client_test.exs

@@ -121,18 +121,19 @@ defmodule Livebook.Hubs.TeamClientTest do
       # creates the app deployment
       slug = Livebook.Utils.random_short_id()
       title = "MyNotebook-#{slug}"
+      app_settings = %{Livebook.Notebook.AppSettings.new() | slug: slug}
 
       notebook = %{
         Livebook.Notebook.new()
-        | app_settings: %{Livebook.Notebook.AppSettings.new() | slug: slug},
+        | app_settings: app_settings,
           name: title,
           hub_id: team.id,
           deployment_group_id: id
       }
 
       files_dir = Livebook.FileSystem.File.local(tmp_dir)
       {:ok, app_deployment} = Livebook.Teams.AppDeployment.new(notebook, files_dir)
-      :ok = Livebook.Teams.deploy_app(team, app_deployment)
+      :ok = Livebook.Teams.deploy_app(team, app_deployment, app_settings)
 
       sha = app_deployment.sha
 
@@ -749,7 +750,7 @@ defmodule Livebook.Hubs.TeamClientTest do
 
       # Since the app deployment struct generation is from Livebook side,
       # we don't have yet the information about who deployed the app,
-      # so we need to add it ourselves
+      # so we need to add it ourselves.
       app_deployment = %{
         app_deployment
         | id: to_string(teams_app_deployment.id),

test/livebook_teams/teams/connection_test.exs

@@ -119,10 +119,11 @@ defmodule Livebook.Teams.ConnectionTest do
       deployment_group_id = to_string(id)
       slug = Livebook.Utils.random_short_id()
       title = "MyNotebook3-#{slug}"
+      app_settings = %{Livebook.Notebook.AppSettings.new() | slug: slug}
 
       notebook = %{
         Livebook.Notebook.new()
-        | app_settings: %{Livebook.Notebook.AppSettings.new() | slug: slug},
+        | app_settings: app_settings,
           name: title,
           hub_id: hub.id,
           deployment_group_id: deployment_group_id
@@ -135,7 +136,7 @@ defmodule Livebook.Teams.ConnectionTest do
 
       # since we want to fetch the app deployment from connection event,
       # we need to persist it before we connect to the WebSocket
-      :ok = Livebook.Teams.deploy_app(hub, app_deployment)
+      :ok = Livebook.Teams.deploy_app(hub, app_deployment, app_settings)
 
       assert {:ok, _conn} = Connection.start_link(self(), headers)
       assert_receive :connected
@@ -163,10 +164,11 @@ defmodule Livebook.Teams.ConnectionTest do
       # creates a new app deployment
       slug = Livebook.Utils.random_short_id()
       title = "MyNotebook3-#{slug}"
+      app_settings = %{Livebook.Notebook.AppSettings.new() | slug: slug}
 
       notebook = %{
         Livebook.Notebook.new()
-        | app_settings: %{Livebook.Notebook.AppSettings.new() | slug: slug},
+        | app_settings: app_settings,
           name: title,
           hub_id: hub.id,
           deployment_group_id: to_string(id)
@@ -179,7 +181,7 @@ defmodule Livebook.Teams.ConnectionTest do
 
       # since we want to fetch the app deployment from connection event,
       # we need to persist it before we connect to the WebSocket
-      :ok = Livebook.Teams.deploy_app(hub, app_deployment)
+      :ok = Livebook.Teams.deploy_app(hub, app_deployment, app_settings)
 
       # As we need to be Agent to receive the app deployments list to be deployed,
       # we will create another connection here

test/livebook_teams/teams_test.exs

@@ -196,9 +196,11 @@ defmodule Livebook.TeamsTest do
 
       {:ok, id} = Teams.create_deployment_group(team, deployment_group)
 
+      app_settings = %{Notebook.AppSettings.new() | slug: Utils.random_short_id()}
+
       notebook = %{
         Notebook.new()
-        | app_settings: %{Notebook.AppSettings.new() | slug: Utils.random_short_id()},
+        | app_settings: app_settings,
           name: "MyNotebook",
           hub_id: team.id,
           deployment_group_id: to_string(id)
@@ -207,7 +209,21 @@ defmodule Livebook.TeamsTest do
       files_dir = Livebook.FileSystem.File.local(tmp_dir)
 
       assert {:ok, app_deployment} = Teams.AppDeployment.new(notebook, files_dir)
-      assert Teams.deploy_app(team, app_deployment) == :ok
+      assert Teams.deploy_app(team, app_deployment, app_settings) == :ok
+
+      assert {:error,
+              %{errors: [slug: {"should only contain alphanumeric characters and dashes", []}]}} =
+               Teams.deploy_app(team, %{app_deployment | slug: "@abc"}, app_settings)
+
+      # Since the fields below belongs to AppSettings, we're mapping the errors to `:file` field.
+      assert {:error, %{errors: [file: {"can't be blank", []}]}} =
+               Teams.deploy_app(team, app_deployment, %{app_settings | multi_session: nil})
+
+      assert {:error, %{errors: [file: {"can't be blank", []}]}} =
+               Teams.deploy_app(team, app_deployment, %{app_settings | access_type: nil})
+
+      assert {:error, %{errors: [file: {"is invalid", []}]}} =
+               Teams.deploy_app(team, app_deployment, %{app_settings | access_type: :abc})
     end
   end
 end

test/livebook_teams/web/hub/deployment_group_test.exs

@@ -267,12 +267,11 @@ defmodule LivebookWeb.Integration.Hub.DeploymentGroupTest do
            |> Floki.text()
            |> String.trim() == "0"
 
+    app_settings = %{Livebook.Notebook.AppSettings.new() | slug: Livebook.Utils.random_short_id()}
+
     notebook = %{
       Livebook.Notebook.new()
-      | app_settings: %{
-          Livebook.Notebook.AppSettings.new()
-          | slug: Livebook.Utils.random_short_id()
-        },
+      | app_settings: app_settings,
         name: "MyNotebook",
         hub_id: hub.id,
         deployment_group_id: to_string(id)
@@ -281,7 +280,7 @@ defmodule LivebookWeb.Integration.Hub.DeploymentGroupTest do
     files_dir = Livebook.FileSystem.File.local(tmp_dir)
 
     {:ok, app_deployment} = Livebook.Teams.AppDeployment.new(notebook, files_dir)
-    :ok = Livebook.Teams.deploy_app(hub, app_deployment)
+    :ok = Livebook.Teams.deploy_app(hub, app_deployment, app_settings)
 
     assert_receive {:app_deployment_started, _}