From 36e2d2b2775a143b217959edbebd50456985d7d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Thu, 23 Feb 2023 19:00:03 +0100 Subject: [PATCH] Don't validate changesets on mount (#1724) --- lib/livebook/hubs/enterprise.ex | 8 ++++++++ lib/livebook/hubs/fly.ex | 8 ++++++++ lib/livebook/hubs/personal.ex | 8 ++++++++ lib/livebook/settings.ex | 7 ++----- lib/livebook/users.ex | 10 +++++----- lib/livebook_web/live/env_var_component.ex | 9 +++++++-- lib/livebook_web/live/hooks/user_hook.ex | 4 +--- .../live/hub/edit/enterprise_component.ex | 2 +- .../live/hub/edit/fly_component.ex | 2 +- .../live/hub/edit/personal_component.ex | 2 +- .../live/hub/new/enterprise_component.ex | 4 ++-- .../live/hub/new/fly_component.ex | 6 +++--- lib/livebook_web/live/user_component.ex | 19 ++++++++++--------- 13 files changed, 57 insertions(+), 32 deletions(-) diff --git a/lib/livebook/hubs/enterprise.ex b/lib/livebook/hubs/enterprise.ex index 166366e23..581a05f86 100644 --- a/lib/livebook/hubs/enterprise.ex +++ b/lib/livebook/hubs/enterprise.ex @@ -36,6 +36,14 @@ defmodule Livebook.Hubs.Enterprise do """ @spec change_hub(t(), map()) :: Ecto.Changeset.t() def change_hub(%__MODULE__{} = enterprise, attrs \\ %{}) do + changeset(enterprise, attrs) + end + + @doc """ + Returns changeset with applied validations. + """ + @spec validate_hub(t(), map()) :: Ecto.Changeset.t() + def validate_hub(%__MODULE__{} = enterprise, attrs \\ %{}) do enterprise |> changeset(attrs) |> Map.put(:action, :validate) diff --git a/lib/livebook/hubs/fly.ex b/lib/livebook/hubs/fly.ex index 538bf12d6..a78097dea 100644 --- a/lib/livebook/hubs/fly.ex +++ b/lib/livebook/hubs/fly.ex @@ -42,6 +42,14 @@ defmodule Livebook.Hubs.Fly do """ @spec change_hub(t(), map()) :: Ecto.Changeset.t() def change_hub(%__MODULE__{} = fly, attrs \\ %{}) do + changeset(fly, attrs) + end + + @doc """ + Returns changeset with applied validations. + """ + @spec validate_hub(t(), map()) :: Ecto.Changeset.t() + def validate_hub(%__MODULE__{} = fly, attrs \\ %{}) do fly |> changeset(attrs) |> Map.put(:action, :validate) diff --git a/lib/livebook/hubs/personal.ex b/lib/livebook/hubs/personal.ex index d3fd9db97..075407505 100644 --- a/lib/livebook/hubs/personal.ex +++ b/lib/livebook/hubs/personal.ex @@ -24,6 +24,14 @@ defmodule Livebook.Hubs.Personal do """ @spec change_hub(t(), map()) :: Ecto.Changeset.t() def change_hub(%__MODULE__{} = personal, attrs \\ %{}) do + changeset(personal, attrs) + end + + @doc """ + Returns changeset with applied validations. + """ + @spec validate_hub(t(), map()) :: Ecto.Changeset.t() + def validate_hub(%__MODULE__{} = personal, attrs \\ %{}) do personal |> changeset(attrs) |> Map.put(:action, :validate) diff --git a/lib/livebook/settings.ex b/lib/livebook/settings.ex index 26a396d47..cf68625fb 100644 --- a/lib/livebook/settings.ex +++ b/lib/livebook/settings.ex @@ -161,8 +161,7 @@ defmodule Livebook.Settings do With success, notifies interested processes about environment variable data change. Otherwise, it will return an error tuple with changeset. """ - @spec set_env_var(EnvVar.t(), map()) :: - {:ok, EnvVar.t()} | {:error, Ecto.Changeset.t()} + @spec set_env_var(EnvVar.t(), map()) :: {:ok, EnvVar.t()} | {:error, Ecto.Changeset.t()} def set_env_var(%EnvVar{} = env_var \\ %EnvVar{}, attrs) do changeset = EnvVar.changeset(env_var, attrs) @@ -200,9 +199,7 @@ defmodule Livebook.Settings do """ @spec change_env_var(EnvVar.t(), map()) :: Ecto.Changeset.t() def change_env_var(%EnvVar{} = env_var, attrs \\ %{}) do - env_var - |> EnvVar.changeset(attrs) - |> Map.put(:action, :validate) + EnvVar.changeset(env_var, attrs) end @doc """ diff --git a/lib/livebook/users.ex b/lib/livebook/users.ex index a96eee642..3f230cd0b 100644 --- a/lib/livebook/users.ex +++ b/lib/livebook/users.ex @@ -8,9 +8,7 @@ defmodule Livebook.Users do """ @spec change_user(User.t(), map()) :: Ecto.Changeset.t() def change_user(%User{} = user, attrs \\ %{}) do - user - |> User.changeset(attrs) - |> Map.put(:action, :validate) + User.changeset(user, attrs) end @doc """ @@ -19,8 +17,10 @@ defmodule Livebook.Users do With success, notifies interested processes about user data change. Otherwise, it will return an error tuple with changeset. """ - @spec update_user(Ecto.Changeset.t()) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()} - def update_user(%Ecto.Changeset{} = changeset) do + @spec update_user(User.t(), map()) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()} + def update_user(%User{} = user, attrs \\ %{}) do + changeset = User.changeset(user, attrs) + with {:ok, user} <- Ecto.Changeset.apply_action(changeset, :update) do broadcast_change(user) {:ok, user} diff --git a/lib/livebook_web/live/env_var_component.ex b/lib/livebook_web/live/env_var_component.ex index e51313e4c..9400413b4 100644 --- a/lib/livebook_web/live/env_var_component.ex +++ b/lib/livebook_web/live/env_var_component.ex @@ -11,7 +11,7 @@ defmodule LivebookWeb.EnvVarComponent do do: {assigns.env_var, :edit}, else: {%EnvVar{}, :new} - changeset = EnvVar.changeset(env_var) + changeset = Settings.change_env_var(env_var) {:ok, socket @@ -69,6 +69,11 @@ defmodule LivebookWeb.EnvVarComponent do @impl true def handle_event("validate", %{"env_var" => attrs}, socket) do - {:noreply, assign(socket, changeset: Settings.change_env_var(socket.assigns.env_var, attrs))} + changeset = + socket.assigns.env_var + |> Settings.change_env_var(attrs) + |> Map.put(:action, :validate) + + {:noreply, assign(socket, changeset: changeset)} end end diff --git a/lib/livebook_web/live/hooks/user_hook.ex b/lib/livebook_web/live/hooks/user_hook.ex index 541bbe91a..ba3b65c18 100644 --- a/lib/livebook_web/live/hooks/user_hook.ex +++ b/lib/livebook_web/live/hooks/user_hook.ex @@ -39,9 +39,7 @@ defmodule LivebookWeb.UserHook do connect_params = get_connect_params(socket) || %{} attrs = connect_params["user_data"] || session["user_data"] || %{} - changeset = User.changeset(user, attrs) - - case Livebook.Users.update_user(changeset) do + case Livebook.Users.update_user(user, attrs) do {:ok, user} -> user {:error, _changeset} -> user end diff --git a/lib/livebook_web/live/hub/edit/enterprise_component.ex b/lib/livebook_web/live/hub/edit/enterprise_component.ex index 106a7b82c..8e4ccb711 100644 --- a/lib/livebook_web/live/hub/edit/enterprise_component.ex +++ b/lib/livebook_web/live/hub/edit/enterprise_component.ex @@ -68,6 +68,6 @@ defmodule LivebookWeb.Hub.Edit.EnterpriseComponent do end def handle_event("validate", %{"enterprise" => attrs}, socket) do - {:noreply, assign(socket, changeset: Enterprise.change_hub(socket.assigns.hub, attrs))} + {:noreply, assign(socket, changeset: Enterprise.validate_hub(socket.assigns.hub, attrs))} end end diff --git a/lib/livebook_web/live/hub/edit/fly_component.ex b/lib/livebook_web/live/hub/edit/fly_component.ex index 830bf8cc3..b87fa202a 100644 --- a/lib/livebook_web/live/hub/edit/fly_component.ex +++ b/lib/livebook_web/live/hub/edit/fly_component.ex @@ -138,7 +138,7 @@ defmodule LivebookWeb.Hub.Edit.FlyComponent do end def handle_event("validate", %{"fly" => attrs}, socket) do - {:noreply, assign(socket, changeset: Fly.change_hub(socket.assigns.hub, attrs))} + {:noreply, assign(socket, changeset: Fly.validate_hub(socket.assigns.hub, attrs))} end # EnvVar component callbacks diff --git a/lib/livebook_web/live/hub/edit/personal_component.ex b/lib/livebook_web/live/hub/edit/personal_component.ex index 44342fe0b..c04307a0b 100644 --- a/lib/livebook_web/live/hub/edit/personal_component.ex +++ b/lib/livebook_web/live/hub/edit/personal_component.ex @@ -69,6 +69,6 @@ defmodule LivebookWeb.Hub.Edit.PersonalComponent do end def handle_event("validate", %{"personal" => attrs}, socket) do - {:noreply, assign(socket, changeset: Personal.change_hub(socket.assigns.hub, attrs))} + {:noreply, assign(socket, changeset: Personal.validate_hub(socket.assigns.hub, attrs))} end end diff --git a/lib/livebook_web/live/hub/new/enterprise_component.ex b/lib/livebook_web/live/hub/new/enterprise_component.ex index 4bebbe168..8976c0062 100644 --- a/lib/livebook_web/live/hub/new/enterprise_component.ex +++ b/lib/livebook_web/live/hub/new/enterprise_component.ex @@ -123,7 +123,7 @@ defmodule LivebookWeb.Hub.New.EnterpriseComponent do case EnterpriseClient.send_request(pid, session_request) do {:session, session_response} -> base = %{base | external_id: session_response.id} - changeset = Enterprise.change_hub(base) + changeset = Enterprise.validate_hub(base) {:noreply, assign(socket, pid: pid, changeset: changeset, base: base)} @@ -160,6 +160,6 @@ defmodule LivebookWeb.Hub.New.EnterpriseComponent do end def handle_event("validate", %{"enterprise" => attrs}, socket) do - {:noreply, assign(socket, changeset: Enterprise.change_hub(socket.assigns.base, attrs))} + {:noreply, assign(socket, changeset: Enterprise.validate_hub(socket.assigns.base, attrs))} end end diff --git a/lib/livebook_web/live/hub/new/fly_component.ex b/lib/livebook_web/live/hub/new/fly_component.ex index f1b4933ee..cd8d58435 100644 --- a/lib/livebook_web/live/hub/new/fly_component.ex +++ b/lib/livebook_web/live/hub/new/fly_component.ex @@ -79,7 +79,7 @@ defmodule LivebookWeb.Hub.New.FlyComponent do {:ok, apps} -> opts = select_options(apps) base = %Fly{access_token: token, hub_emoji: "🚀"} - changeset = Fly.change_hub(base) + changeset = Fly.validate_hub(base) {:noreply, assign(socket, changeset: changeset, base: base, select_options: opts, apps: apps)} @@ -87,7 +87,7 @@ defmodule LivebookWeb.Hub.New.FlyComponent do {:error, _} -> changeset = %Fly{} - |> Fly.change_hub(%{access_token: token}) + |> Fly.validate_hub(%{access_token: token}) |> add_error(:access_token, "is invalid") {:noreply, @@ -118,7 +118,7 @@ defmodule LivebookWeb.Hub.New.FlyComponent do application_id = params["application_id"] selected_app = Enum.find(socket.assigns.apps, &(&1.application_id == application_id)) opts = select_options(socket.assigns.apps) - changeset = Fly.change_hub(selected_app || socket.assigns.base, params) + changeset = Fly.validate_hub(selected_app || socket.assigns.base, params) {:noreply, assign(socket, changeset: changeset, selected_app: selected_app, select_options: opts)} diff --git a/lib/livebook_web/live/user_component.ex b/lib/livebook_web/live/user_component.ex index d2f8d9983..9248f2432 100644 --- a/lib/livebook_web/live/user_component.ex +++ b/lib/livebook_web/live/user_component.ex @@ -12,7 +12,7 @@ defmodule LivebookWeb.UserComponent do user = socket.assigns.user changeset = Users.change_user(user) - {:ok, assign(socket, changeset: changeset, valid?: changeset.valid?, user: user)} + {:ok, assign(socket, changeset: changeset, user: user)} end @impl true @@ -44,7 +44,7 @@ defmodule LivebookWeb.UserComponent do