Update Local hub to Personal hub and makes it editable (#1708)

lib/livebook/application.ex

@@ -53,7 +53,7 @@ defmodule Livebook.Application do
         load_lb_env_vars()
         clear_env_vars()
         display_startup_info()
-        insert_development_hub()
+        insert_personal_hub()
         Livebook.Hubs.connect_hubs()
         result
 
@@ -203,16 +203,14 @@ defmodule Livebook.Application do
     defp app_specs, do: []
   end
 
-  if Livebook.Config.feature_flag_enabled?(:localhost_hub) do
-    defp insert_development_hub do
-      Livebook.Hubs.save_hub(%Livebook.Hubs.Local{
-        id: "local-host",
-        hub_name: "Localhost",
+  defp insert_personal_hub do
+    unless Livebook.Hubs.hub_exists?("personal-hub") do
+      Livebook.Hubs.save_hub(%Livebook.Hubs.Personal{
+        id: "personal-hub",
+        hub_name: "My Hub",
         hub_emoji: "🏠"
       })
     end
-  else
-    defp insert_development_hub, do: :ok
   end
 
   defp iframe_server_specs() do

lib/livebook/hubs.ex

@@ -2,7 +2,7 @@ defmodule Livebook.Hubs do
   @moduledoc false
 
   alias Livebook.Storage
-  alias Livebook.Hubs.{Broadcasts, Enterprise, Fly, Local, Metadata, Provider}
+  alias Livebook.Hubs.{Broadcasts, Enterprise, Fly, Metadata, Personal, Provider}
   alias Livebook.Secrets
   alias Livebook.Secrets.Secret
 
@@ -94,6 +94,7 @@ defmodule Livebook.Hubs do
   @spec delete_hub(String.t()) :: :ok
   def delete_hub(id) do
     with {:ok, hub} <- get_hub(id) do
+      true = Provider.type(hub) != "personal"
       :ok = Broadcasts.hub_changed()
       :ok = Storage.delete(@namespace, id)
       :ok = disconnect_hub(hub)
@@ -111,13 +112,6 @@ defmodule Livebook.Hubs do
     :ok
   end
 
-  @doc false
-  def clean_hubs do
-    for hub <- get_hubs(), do: delete_hub(hub.id)
-
-    :ok
-  end
-
   @doc """
   Subscribes to one or more subtopics in `"hubs"`.
 
@@ -172,8 +166,8 @@ defmodule Livebook.Hubs do
     Provider.load(%Enterprise{}, fields)
   end
 
-  defp to_struct(%{id: "local-" <> _} = fields) do
-    Provider.load(%Local{}, fields)
+  defp to_struct(%{id: "personal-" <> _} = fields) do
+    Provider.load(%Personal{}, fields)
   end
 
   @doc """

lib/livebook/hubs/enterprise_client.ex

@@ -59,9 +59,11 @@ defmodule Livebook.Hubs.EnterpriseClient do
   @doc """
   Returns the latest error from connection.
   """
-  @spec get_connection_error(String.t()) :: Secret.t() | nil
+  @spec get_connection_error(String.t()) :: String.t() | nil
   def get_connection_error(id) do
     GenServer.call(registry_name(id), :get_connection_error)
+  catch
+    :exit, _ -> "connection refused"
   end
 
   @doc """

lib/livebook/hubs/local.ex

@@ -1,35 +0,0 @@
-defmodule Livebook.Hubs.Local do
-  @moduledoc false
-
-  defstruct [:id, :hub_name, :hub_emoji]
-end
-
-defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Local do
-  def load(local, fields) do
-    %{local | id: fields.id, hub_name: fields.hub_name, hub_emoji: fields.hub_emoji}
-  end
-
-  def to_metadata(local) do
-    %Livebook.Hubs.Metadata{
-      id: local.id,
-      name: local.hub_name,
-      provider: local,
-      emoji: local.hub_emoji,
-      connected?: false
-    }
-  end
-
-  def type(_local), do: "local"
-
-  def connection_spec(_local), do: nil
-
-  def disconnect(_local), do: :ok
-
-  def capabilities(_local), do: []
-
-  def get_secrets(_local), do: []
-
-  def create_secret(_local, _secret), do: :ok
-
-  def connection_error(_local), do: nil
-end

lib/livebook/hubs/personal.ex

@@ -0,0 +1,84 @@
+defmodule Livebook.Hubs.Personal do
+  @moduledoc false
+
+  use Ecto.Schema
+  import Ecto.Changeset
+
+  alias Livebook.Hubs
+
+  @type t :: %__MODULE__{
+          id: String.t() | nil,
+          hub_name: String.t() | nil,
+          hub_emoji: String.t() | nil
+        }
+
+  embedded_schema do
+    field :hub_name, :string
+    field :hub_emoji, :string
+  end
+
+  @fields ~w(hub_name hub_emoji)a
+
+  @doc """
+  Returns an `%Ecto.Changeset{}` for tracking hub changes.
+  """
+  @spec change_hub(t(), map()) :: Ecto.Changeset.t()
+  def change_hub(%__MODULE__{} = personal, attrs \\ %{}) do
+    personal
+    |> changeset(attrs)
+    |> Map.put(:action, :validate)
+  end
+
+  @doc """
+  Updates a Hub.
+
+  With success, notifies interested processes about hub metadatas data change.
+  Otherwise, it will return an error tuple with changeset.
+  """
+  @spec update_hub(t(), map()) :: {:ok, t()} | {:error, Ecto.Changeset.t()}
+  def update_hub(%__MODULE__{} = personal, attrs) do
+    changeset = changeset(personal, attrs)
+
+    with {:ok, struct} <- apply_action(changeset, :update) do
+      Hubs.save_hub(struct)
+      {:ok, struct}
+    end
+  end
+
+  defp changeset(personal, attrs) do
+    personal
+    |> cast(attrs, @fields)
+    |> validate_required(@fields)
+    |> put_change(:id, "personal-hub")
+  end
+end
+
+defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Personal do
+  def load(personal, fields) do
+    %{personal | id: fields.id, hub_name: fields.hub_name, hub_emoji: fields.hub_emoji}
+  end
+
+  def to_metadata(personal) do
+    %Livebook.Hubs.Metadata{
+      id: personal.id,
+      name: personal.hub_name,
+      provider: personal,
+      emoji: personal.hub_emoji,
+      connected?: false
+    }
+  end
+
+  def type(_personal), do: "personal"
+
+  def connection_spec(_personal), do: nil
+
+  def disconnect(_personal), do: :ok
+
+  def capabilities(_personal), do: []
+
+  def get_secrets(_personal), do: []
+
+  def create_secret(_personal, _secret), do: :ok
+
+  def connection_error(_personal), do: nil
+end

lib/livebook_web/live/hub/edit/personal_component.ex

@@ -0,0 +1,82 @@
+defmodule LivebookWeb.Hub.Edit.PersonalComponent do
+  use LivebookWeb, :live_component
+
+  alias Livebook.Hubs.Personal
+
+  @impl true
+  def update(assigns, socket) do
+    changeset = Personal.change_hub(assigns.hub)
+
+    {:ok,
+     socket
+     |> assign(assigns)
+     |> assign(changeset: changeset)}
+  end
+
+  @impl true
+  def render(assigns) do
+    ~H"""
+    <div id={"#{@id}-component"}>
+      <div class="flex flex-col space-y-10">
+        <div class="flex flex-col space-y-2">
+          <h2 class="text-xl text-gray-800 font-medium pb-2 border-b border-gray-200">
+            General
+          </h2>
+
+          <.form
+            :let={f}
+            id={@id}
+            class="flex flex-col mt-4 space-y-4"
+            for={@changeset}
+            phx-submit="save"
+            phx-change="validate"
+            phx-target={@myself}
+            phx-debounce="blur"
+          >
+            <div class="grid grid-cols-1 md:grid-cols-2 gap-3">
+              <.input_wrapper form={f} field={:hub_name} class="flex flex-col space-y-1">
+                <div class="input-label">Name</div>
+                <%= text_input(f, :hub_name, class: "input") %>
+              </.input_wrapper>
+
+              <.input_wrapper form={f} field={:hub_emoji} class="flex flex-col space-y-1">
+                <div class="input-label">Emoji</div>
+                <.emoji_input
+                  id="personal-emoji-input"
+                  form={f}
+                  field={:hub_emoji}
+                  container_class="mt-10"
+                />
+              </.input_wrapper>
+            </div>
+
+            <%= submit("Update Hub",
+              class: "button-base button-blue",
+              phx_disable_with: "Updating...",
+              disabled: not @changeset.valid?
+            ) %>
+          </.form>
+        </div>
+      </div>
+    </div>
+    """
+  end
+
+  @impl true
+  def handle_event("save", %{"personal" => params}, socket) do
+    case Personal.update_hub(socket.assigns.hub, params) do
+      {:ok, hub} ->
+        {:noreply,
+         socket
+         |> put_flash(:success, "Hub updated successfully")
+         |> push_redirect(to: Routes.hub_path(socket, :edit, hub.id))}
+
+      {:error, changeset} ->
+        {:noreply, assign(socket, changeset: changeset)}
+    end
+  end
+
+  def handle_event("validate", %{"personal" => attrs}, socket) do
+    {:noreply, assign(socket, changeset: Personal.change_hub(socket.assigns.hub, attrs))}
+  end
+end

lib/livebook_web/live/hub/edit_live.ex

@@ -17,21 +17,14 @@ defmodule LivebookWeb.Hub.EditLive do
     hub = Hubs.fetch_hub!(params["id"])
     type = Provider.type(hub)
 
-    if type == "local" do
-      {:noreply,
-       socket
-       |> redirect(to: "/")
-       |> put_flash(:warning, "You can't edit the localhost Hub")}
-    else
-      {:noreply,
-       assign(socket,
-         hub: hub,
-         type: type,
-         page_title: "Livebook - Hub",
-         params: params,
-         env_var_id: params["env_var_id"]
-       )}
-    end
+    {:noreply,
+     assign(socket,
+       hub: hub,
+       type: type,
+       page_title: "Livebook - Hub",
+       params: params,
+       env_var_id: params["env_var_id"]
+     )}
   end
 
   @impl true
@@ -47,38 +40,45 @@ defmodule LivebookWeb.Hub.EditLive do
         <div class="flex relative">
           <PageHelpers.title text="Edit Hub" socket={@socket} />
 
-          <button
-            phx-click={
-              with_confirm(
-                JS.push("delete_hub", value: %{id: @hub.id}),
-                title: "Delete hub",
-                description: "Are you sure you want to delete this hub?",
-                confirm_text: "Delete",
-                confirm_icon: "close-circle-line"
-              )
-            }
-            class="absolute right-0 button-base bg-red-500"
-          >
-            Delete hub
-          </button>
+          <%= if @type != "personal" do %>
+            <button
+              phx-click={
+                with_confirm(
+                  JS.push("delete_hub", value: %{id: @hub.id}),
+                  title: "Delete hub",
+                  description: "Are you sure you want to delete this hub?",
+                  confirm_text: "Delete",
+                  confirm_icon: "close-circle-line"
+                )
+              }
+              class="absolute right-0 button-base bg-red-500"
+            >
+              Delete hub
+            </button>
+          <% end %>
         </div>
 
-        <%= if @type == "fly" do %>
-          <.live_component
-            module={LivebookWeb.Hub.Edit.FlyComponent}
-            hub={@hub}
-            id="fly-form"
-            live_action={@live_action}
-            env_var_id={@env_var_id}
-          />
-        <% end %>
-
-        <%= if @type == "enterprise" do %>
-          <.live_component
-            module={LivebookWeb.Hub.Edit.EnterpriseComponent}
-            hub={@hub}
-            id="enterprise-form"
-          />
+        <%= case @type do %>
+          <% "fly" -> %>
+            <.live_component
+              module={LivebookWeb.Hub.Edit.FlyComponent}
+              hub={@hub}
+              id="fly-form"
+              live_action={@live_action}
+              env_var_id={@env_var_id}
+            />
+          <% "personal" -> %>
+            <.live_component
+              module={LivebookWeb.Hub.Edit.PersonalComponent}
+              hub={@hub}
+              id="personal-form"
+            />
+          <% "enterprise" -> %>
+            <.live_component
+              module={LivebookWeb.Hub.Edit.EnterpriseComponent}
+              hub={@hub}
+              id="enterprise-form"
+            />
         <% end %>
       </div>
     </LayoutHelpers.layout>

test/livebook/hubs_test.exs

@@ -3,46 +3,34 @@ defmodule Livebook.HubsTest do
 
   alias Livebook.Hubs
 
-  setup do
-    on_exit(&Hubs.clean_hubs/0)
-
-    :ok
-  end
-
   test "get_hubs/0 returns a list of persisted hubs" do
     fly = insert_hub(:fly, id: "fly-baz")
-    assert Hubs.get_hubs() == [fly]
+    assert fly in Hubs.get_hubs()
 
     Hubs.delete_hub("fly-baz")
-    assert Hubs.get_hubs() == []
+    refute fly in Hubs.get_hubs()
   end
 
   test "get_metadata/0 returns a list of persisted hubs normalized" do
     fly = insert_hub(:fly, id: "fly-livebook")
+    metadata = Hubs.Provider.to_metadata(fly)
 
-    assert Hubs.get_metadatas() == [
-             %Hubs.Metadata{
-               id: "fly-livebook",
-               emoji: fly.hub_emoji,
-               name: fly.hub_name,
-               provider: fly
-             }
-           ]
+    assert metadata in Hubs.get_metadatas()
 
     Hubs.delete_hub("fly-livebook")
-    assert Hubs.get_metadatas() == []
+    refute metadata in Hubs.get_metadatas()
   end
 
   test "fetch_hub!/1 returns one persisted fly" do
     assert_raise Livebook.Storage.NotFoundError,
-                 ~s/could not find entry in \"hubs\" with ID "fly-foo"/,
+                 ~s/could not find entry in \"hubs\" with ID "fly-exception-foo"/,
                  fn ->
-                   Hubs.fetch_hub!("fly-foo")
+                   Hubs.fetch_hub!("fly-exception-foo")
                  end
 
-    fly = insert_hub(:fly, id: "fly-foo")
+    fly = insert_hub(:fly, id: "fly-exception-foo")
 
-    assert Hubs.fetch_hub!("fly-foo") == fly
+    assert Hubs.fetch_hub!("fly-exception-foo") == fly
   end
 
   test "hub_exists?/1" do

test/livebook_web/live/hub/edit_live_test.exs

@@ -5,14 +5,6 @@ defmodule LivebookWeb.Hub.EditLiveTest do
 
   alias Livebook.Hubs
 
-  setup do
-    on_exit(fn ->
-      Hubs.clean_hubs()
-    end)
-
-    :ok
-  end
-
   describe "fly" do
     setup do
       bypass = Bypass.open()
@@ -68,10 +60,10 @@ defmodule LivebookWeb.Hub.EditLiveTest do
       app_id = Livebook.Utils.random_short_id()
       hub_id = "fly-#{app_id}"
 
-      hub = insert_hub(:fly, id: hub_id, application_id: app_id)
+      hub = insert_hub(:fly, id: hub_id, hub_name: "My Deletable Hub", application_id: app_id)
       fly_bypass(bypass, app_id, pid)
 
-      {:ok, view, html} = live(conn, Routes.hub_path(conn, :edit, hub.id))
+      {:ok, view, _html} = live(conn, Routes.hub_path(conn, :edit, hub.id))
 
       assert {:ok, view, _html} =
                view
@@ -80,7 +72,6 @@ defmodule LivebookWeb.Hub.EditLiveTest do
 
       hubs_html = view |> element("#hubs") |> render()
 
-      refute hubs_html =~ hub.hub_emoji
       refute hubs_html =~ Routes.hub_path(conn, :edit, hub.id)
       refute hubs_html =~ hub.hub_name
 

test/livebook_web/live/hub/new_live_test.exs

@@ -5,11 +5,6 @@ defmodule LivebookWeb.Hub.NewLiveTest do
 
   alias Livebook.Hubs
 
-  setup do
-    on_exit(&Hubs.clean_hubs/0)
-    :ok
-  end
-
   test "render hub selection cards", %{conn: conn} do
     {:ok, _view, html} = live(conn, Routes.hub_path(conn, :new))