From 602d852b31d33c587f5e073a7aea4376593d8b63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Thu, 16 May 2024 18:17:45 +0200 Subject: [PATCH] Update to Phoenix LV 1.0 (#2607) --- assets/tailwind.config.js | 1 - lib/livebook/hubs/provider.ex | 10 +-- lib/livebook/hubs/team.ex | 20 +++--- lib/livebook/teams.ex | 64 ++++++++++++------- lib/livebook/teams/requests.ex | 19 ++---- lib/livebook/utils.ex | 13 ++++ .../components/form_components.ex | 44 +++++++++---- .../controllers/auth_html/index.html.heex | 25 ++++++-- lib/livebook_web/live/app_auth_live.ex | 7 +- .../live/hub/file_system_form_component.ex | 18 ++++-- lib/livebook_web/live/hub/new_live.ex | 2 - .../live/hub/secret_form_component.ex | 44 +++++++------ .../teams/deployment_group_form_component.ex | 18 ++---- .../live/session_live/secrets_component.ex | 27 +++++--- mix.exs | 2 +- mix.lock | 2 +- test/livebook_teams/hubs_test.exs | 27 ++++---- test/livebook_teams/teams_test.exs | 23 +++---- test/support/factory.ex | 9 ++- 19 files changed, 217 insertions(+), 158 deletions(-) diff --git a/assets/tailwind.config.js b/assets/tailwind.config.js index 0b375a989..7622d43ad 100644 --- a/assets/tailwind.config.js +++ b/assets/tailwind.config.js @@ -122,7 +122,6 @@ module.exports = { addVariant("phx-loading", [".phx-loading&", ".phx-loading &"]); addVariant("phx-connected", [".phx-connected&", ".phx-connected &"]); addVariant("phx-error", [".phx-error&", ".phx-error &"]); - addVariant("phx-form-error", [":not(.phx-no-feedback).show-errors &"]); addVariant("phx-click-loading", [ ".phx-click-loading&", ".phx-click-loading &", diff --git a/lib/livebook/hubs/provider.ex b/lib/livebook/hubs/provider.ex index e2d60c249..a1ac56259 100644 --- a/lib/livebook/hubs/provider.ex +++ b/lib/livebook/hubs/provider.ex @@ -19,6 +19,8 @@ defprotocol Livebook.Hubs.Provider do """ @type notebook_stamp :: map() + @type field_errors :: list({atom(), list(String.t())}) + @doc """ Transforms given hub to `Livebook.Hubs.Metadata` struct. """ @@ -60,7 +62,7 @@ defprotocol Livebook.Hubs.Provider do """ @spec create_secret(t(), Secret.t()) :: :ok - | {:error, Ecto.Changeset.t()} + | {:error, field_errors()} | {:transport_error, String.t()} def create_secret(hub, secret) @@ -69,7 +71,7 @@ defprotocol Livebook.Hubs.Provider do """ @spec update_secret(t(), Secret.t()) :: :ok - | {:error, Ecto.Changeset.t()} + | {:error, field_errors()} | {:transport_error, String.t()} def update_secret(hub, secret) @@ -120,7 +122,7 @@ defprotocol Livebook.Hubs.Provider do """ @spec create_file_system(t(), FileSystem.t()) :: :ok - | {:error, Ecto.Changeset.t()} + | {:error, field_errors()} | {:transport_error, String.t()} def create_file_system(hub, file_system) @@ -129,7 +131,7 @@ defprotocol Livebook.Hubs.Provider do """ @spec update_file_system(t(), FileSystem.t()) :: :ok - | {:error, Ecto.Changeset.t()} + | {:error, field_errors()} | {:transport_error, String.t()} def update_file_system(hub, file_system) diff --git a/lib/livebook/hubs/team.ex b/lib/livebook/hubs/team.ex index 167ea3b54..f6bab7b1d 100644 --- a/lib/livebook/hubs/team.ex +++ b/lib/livebook/hubs/team.ex @@ -212,7 +212,7 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Team do def create_secret(%Team{} = team, %Secret{} = secret) do case Requests.create_secret(team, secret) do {:ok, %{"id" => _}} -> :ok - {:error, %{"errors" => errors}} -> {:error, add_secret_errors(secret, errors)} + {:error, %{"errors" => errors}} -> {:error, parse_secret_errors(errors)} any -> any end end @@ -220,7 +220,7 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Team do def update_secret(%Team{} = team, %Secret{} = secret) do case Requests.update_secret(team, secret) do {:ok, %{"id" => _}} -> :ok - {:error, %{"errors" => errors}} -> {:error, add_secret_errors(secret, errors)} + {:error, %{"errors" => errors}} -> {:error, parse_secret_errors(errors)} any -> any end end @@ -228,7 +228,7 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Team do def delete_secret(%Team{} = team, %Secret{} = secret) do case Requests.delete_secret(team, secret) do {:ok, _} -> :ok - {:error, %{"errors" => errors}} -> {:error, add_secret_errors(secret, errors)} + {:error, %{"errors" => errors}} -> {:error, parse_secret_errors(errors)} any -> any end end @@ -238,7 +238,7 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Team do def create_file_system(%Team{} = team, file_system) do case Requests.create_file_system(team, file_system) do {:ok, %{"id" => _}} -> :ok - {:error, %{"errors" => errors}} -> {:error, add_file_system_errors(file_system, errors)} + {:error, %{"errors" => errors}} -> {:error, parse_file_system_errors(file_system, errors)} any -> any end end @@ -246,7 +246,7 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Team do def update_file_system(%Team{} = team, file_system) do case Requests.update_file_system(team, file_system) do {:ok, %{"id" => _}} -> :ok - {:error, %{"errors" => errors}} -> {:error, add_file_system_errors(file_system, errors)} + {:error, %{"errors" => errors}} -> {:error, parse_file_system_errors(file_system, errors)} any -> any end end @@ -254,7 +254,7 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Team do def delete_file_system(%Team{} = team, file_system) do case Requests.delete_file_system(team, file_system) do {:ok, _} -> :ok - {:error, %{"errors" => errors}} -> {:error, add_file_system_errors(file_system, errors)} + {:error, %{"errors" => errors}} -> {:error, parse_file_system_errors(file_system, errors)} any -> any end end @@ -276,13 +276,13 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Team do end end - defp add_secret_errors(%Secret{} = secret, errors_map) do - Requests.add_errors(secret, errors_map) + defp parse_secret_errors(errors_map) do + Requests.to_error_list(Secret, errors_map) end - defp add_file_system_errors(file_system, errors_map) do + defp parse_file_system_errors(%struct{} = file_system, errors_map) do %{error_field: field} = FileSystem.external_metadata(file_system) errors_map = Map.new(errors_map, fn {_key, values} -> {field, values} end) - Requests.add_errors(file_system, errors_map) + Requests.to_error_list(struct, errors_map) end end diff --git a/lib/livebook/teams.ex b/lib/livebook/teams.ex index 3f7006072..d8268de50 100644 --- a/lib/livebook/teams.ex +++ b/lib/livebook/teams.ex @@ -42,19 +42,18 @@ defmodule Livebook.Teams do defp create_org_request(%Org{} = org, attrs, callback) when is_function(callback, 1) do changeset = Org.changeset(org, attrs) - with {:ok, %Org{} = org} <- apply_action(changeset, :insert), - {:ok, response} <- callback.(org) do - {:ok, response} - else - {:error, %Ecto.Changeset{} = changeset} -> - {:error, changeset} + with {:ok, %Org{} = org} <- apply_action(changeset, :insert) do + case callback.(org) do + {:ok, response} -> + {:ok, response} - {:error, %{"errors" => errors}} -> - errors = map_teams_field_to_livebook_field(errors, "key_hash", "teams_key") - {:error, add_org_errors(changeset, errors)} + {:error, %{"errors" => errors}} -> + errors = map_teams_field_to_livebook_field(errors, "key_hash", "teams_key") + {:error, changeset |> add_external_errors(errors) |> Map.replace!(:action, :insert)} - any -> - any + any -> + any + end end end @@ -152,10 +151,6 @@ defmodule Livebook.Teams do Plug.Crypto.KeyGenerator.generate(binary_key, "notebook secret", cache: Plug.Crypto.Keys) end - defp add_org_errors(%Ecto.Changeset{} = changeset, errors_map) do - Requests.add_errors(changeset, Org.__schema__(:fields), errors_map) - end - @doc """ Returns an `%Ecto.Changeset{}` for tracking deployment group changes. """ @@ -167,15 +162,27 @@ defmodule Livebook.Teams do @doc """ Creates a Deployment Group. """ - @spec create_deployment_group(Team.t(), DeploymentGroup.t()) :: - {:ok, pos_integer()} + @spec create_deployment_group(Team.t(), map()) :: + {:ok, DeploymentGroup.t()} | {:error, Ecto.Changeset.t()} | {:transport_error, String.t()} - def create_deployment_group(%Team{} = team, deployment_group) do - case Requests.create_deployment_group(team, deployment_group) do - {:ok, %{"id" => id}} -> {:ok, id} - {:error, %{"errors" => errors}} -> {:error, Requests.add_errors(deployment_group, errors)} - any -> any + def create_deployment_group(%Team{} = team, attrs) do + changeset = DeploymentGroup.changeset(%DeploymentGroup{}, attrs) + + with {:ok, %DeploymentGroup{} = deployment_group} <- apply_action(changeset, :insert) do + case Requests.create_deployment_group(team, deployment_group) do + {:ok, %{"id" => id}} -> + {:ok, %{deployment_group | id: to_string(id)}} + + {:error, %{"errors" => errors}} -> + {:error, + changeset + |> add_external_errors(errors) + |> Map.replace!(:action, :insert)} + + any -> + any + end end end @@ -200,10 +207,10 @@ defmodule Livebook.Teams do :ok {:error, %{"errors" => %{"detail" => error}}} -> - {:error, Requests.add_errors(app_deployment, %{"file" => [error]})} + {:error, add_external_errors(app_deployment, %{"file" => [error]})} {:error, %{"errors" => errors}} -> - {:error, Requests.add_errors(app_deployment, errors)} + {:error, add_external_errors(app_deployment, errors)} any -> any @@ -233,4 +240,13 @@ defmodule Livebook.Teams do map end end + + defp add_external_errors(%Ecto.Changeset{data: %struct{}} = changeset, errors_map) do + errors = Requests.to_error_list(struct, errors_map) + Livebook.Utils.put_changeset_errors(changeset, errors) + end + + defp add_external_errors(struct, errors_map) do + struct |> Ecto.Changeset.change() |> add_external_errors(errors_map) + end end diff --git a/lib/livebook/teams/requests.ex b/lib/livebook/teams/requests.ex index 77ad2d073..fbf5e2a5b 100644 --- a/lib/livebook/teams/requests.ex +++ b/lib/livebook/teams/requests.ex @@ -212,22 +212,17 @@ defmodule Livebook.Teams.Requests do end @doc """ - Add requests errors to a `changeset` for the given `fields`. + Normalizes errors map into errors for the given schema. """ - def add_errors(%Ecto.Changeset{} = changeset, fields, errors_map) do + @spec to_error_list(module(), %{String.t() => list(String.t())}) :: + list({atom(), list(String.t())}) + def to_error_list(struct, errors_map) do + fields = struct.__schema__(:fields) |> MapSet.new() + for {key, errors} <- errors_map, field = String.to_atom(key), field in fields, - error <- errors, - reduce: changeset, - do: (acc -> Ecto.Changeset.add_error(acc, field, error)) - end - - @doc """ - Add requests errors to a struct. - """ - def add_errors(%struct{} = value, errors_map) do - value |> Ecto.Changeset.change() |> add_errors(struct.__schema__(:fields), errors_map) + do: {field, errors} end @doc false diff --git a/lib/livebook/utils.ex b/lib/livebook/utils.ex index 604d515ab..11bb6d625 100644 --- a/lib/livebook/utils.ex +++ b/lib/livebook/utils.ex @@ -213,6 +213,19 @@ defmodule Livebook.Utils do end) end + @doc """ + Adds all the given errors to the changeset for the corresponding + fields. + """ + @spec put_changeset_errors(Ecto.Changeset.t(), list({atom(), list(String.t())})) :: + Ecto.Changeset.t() + def put_changeset_errors(changeset, errors) do + for {field, errors} <- errors, + error <- errors, + reduce: changeset, + do: (changeset -> Ecto.Changeset.add_error(changeset, field, error)) + end + @doc ~S""" Validates if the given string forms valid CLI flags. diff --git a/lib/livebook_web/components/form_components.ex b/lib/livebook_web/components/form_components.ex index 108ba994d..1caaa341c 100644 --- a/lib/livebook_web/components/form_components.ex +++ b/lib/livebook_web/components/form_components.ex @@ -30,15 +30,23 @@ defmodule LivebookWeb.FormComponents do name={@name} id={@id || @name} value={Phoenix.HTML.Form.normalize_value("text", @value)} - class={[input_classes(), @class]} + class={[input_classes(@errors), @class]} {@rest} /> """ end - defp input_classes() do - "w-full px-3 py-2 bg-gray-50 text-sm font-normal border border-gray-200 rounded-lg placeholder-gray-400 text-gray-600 disabled:opacity-70 disabled:cursor-not-allowed phx-form-error:bg-red-50 phx-form-error:border-red-600 phx-form-error:text-red-600 invalid:bg-red-50 invalid:border-red-600 invalid:text-red-600" + defp input_classes(errors) do + [ + "w-full px-3 py-2 text-sm font-normal border rounded-lg placeholder-gray-400 disabled:opacity-70 disabled:cursor-not-allowed", + if errors == [] do + "bg-gray-50 border-gray-200 text-gray-600" + else + "bg-red-50 border-red-600 text-red-600" + end, + "invalid:bg-red-50 invalid:border-red-600 invalid:text-red-600" + ] end @doc """ @@ -65,7 +73,12 @@ defmodule LivebookWeb.FormComponents do @@ -115,7 +128,7 @@ defmodule LivebookWeb.FormComponents do name={@name} id={@id || @name} value={Phoenix.HTML.Form.normalize_value("text", @value)} - class={[input_classes(), "pr-8", @class]} + class={[input_classes(@errors), "pr-8", @class]} {@rest} />
@@ -184,7 +197,7 @@ defmodule LivebookWeb.FormComponents do name={@name} id={@id || @name} value={@value} - class={input_classes()} + class={input_classes(@errors)} spellcheck="false" maxlength="7" {@rest} @@ -221,7 +234,7 @@ defmodule LivebookWeb.FormComponents do assigns = assigns_from_field(assigns) ~H""" -
+
<%= @label %> @@ -281,7 +294,7 @@ defmodule LivebookWeb.FormComponents do assigns = assigns_from_field(assigns) ~H""" -
+