Minor Hub improvements (#1357)

config/dev.exs

@@ -25,7 +25,11 @@ config :livebook, LivebookWeb.Endpoint,
 
 config :livebook, :iframe_port, 4001
 config :livebook, :shutdown_enabled, true
-config :livebook, :feature_flags, hub: true
+
+# Feature flags
+config :livebook, :feature_flags,
+  hub: true,
+  localhost_hub: true
 
 # ## SSL Support
 #

config/test.exs

@@ -22,7 +22,11 @@ if File.exists?(data_path) do
 end
 
 config :livebook, :data_path, data_path
-config :livebook, :feature_flags, hub: true
+
+# Feature flags
+config :livebook, :feature_flags,
+  hub: true,
+  localhost_hub: true
 
 # Use longnames when running tests in CI, so that no host resolution is required,
 # see https://github.com/livebook-dev/livebook/pull/173#issuecomment-819468549

lib/livebook/application.ex

@@ -48,6 +48,7 @@ defmodule Livebook.Application do
       {:ok, _} = result ->
         clear_env_vars()
         display_startup_info()
+        insert_development_hub()
         result
 
       {:error, error} ->
@@ -179,6 +180,20 @@ defmodule Livebook.Application do
     defp app_specs, do: []
   end
 
+  if Livebook.Config.feature_flag_enabled?(:localhost_hub) do
+    defp insert_development_hub do
+      unless Livebook.Hubs.hub_exists?("local-host") do
+        Livebook.Hubs.save_hub(%Livebook.Hubs.Local{
+          id: "local-host",
+          hub_name: "Localhost",
+          hub_color: Livebook.EctoTypes.HexColor.random()
+        })
+      end
+    end
+  else
+    defp insert_development_hub, do: :ok
+  end
+
   defp iframe_server_specs() do
     server? = Phoenix.Endpoint.server?(:livebook, LivebookWeb.Endpoint)
     port = Livebook.Config.iframe_port()

lib/livebook/config.ex

@@ -138,6 +138,24 @@ defmodule Livebook.Config do
     Application.get_env(:livebook, :update_instructions_url)
   end
 
+  @feature_flags Application.compile_env(:livebook, :feature_flags)
+
+  @doc """
+  Returns the feature flag list.
+  """
+  @spec feature_flags() :: keyword(boolean()) | []
+  def feature_flags do
+    @feature_flags
+  end
+
+  @doc """
+  Return if the feature flag is enabled.
+  """
+  @spec feature_flag_enabled?(atom()) :: boolean()
+  def feature_flag_enabled?(key) do
+    @feature_flags[key]
+  end
+
   ## Parsing
 
   @doc """

lib/livebook/hubs.ex

@@ -2,7 +2,7 @@ defmodule Livebook.Hubs do
   @moduledoc false
 
   alias Livebook.Storage
-  alias Livebook.Hubs.{Fly, Metadata, Provider}
+  alias Livebook.Hubs.{Fly, Local, Metadata, Provider}
 
   defmodule NotFoundError do
     @moduledoc false
@@ -119,4 +119,8 @@ defmodule Livebook.Hubs do
   defp to_struct(%{id: "fly-" <> _} = fields) do
     Provider.load(%Fly{}, fields)
   end
+
+  defp to_struct(%{id: "local-" <> _} = fields) do
+    Provider.load(%Local{}, fields)
+  end
 end

lib/livebook/hubs/local.ex

@@ -0,0 +1,21 @@
+defmodule Livebook.Hubs.Local do
+  @moduledoc false
+  defstruct [:id, :hub_name, :hub_color]
+end
+
+defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Local do
+  def load(%Livebook.Hubs.Local{} = local, fields) do
+    %{local | id: fields.id, hub_name: fields.hub_name, hub_color: fields.hub_color}
+  end
+
+  def normalize(%Livebook.Hubs.Local{} = local) do
+    %Livebook.Hubs.Metadata{
+      id: local.id,
+      name: local.hub_name,
+      provider: local,
+      color: local.hub_color
+    }
+  end
+
+  def type(_), do: "local"
+end

lib/livebook_web/live/hub_live.ex

@@ -126,6 +126,11 @@ defmodule LivebookWeb.HubLive do
   defp card_item_bg_color(_id, _selected), do: ""
 
   @impl true
+  def handle_params(%{"id" => "local-host"}, _url, socket) do
+    {:noreply,
+     put_flash(socket, :warning, "This is a localhost Hub, you shouldn't be able to edit")}
+  end
+
   def handle_params(%{"id" => id}, _url, socket) do
     hub = Hubs.fetch_hub!(id)
     provider = Provider.type(hub)

lib/livebook_web/live/hub_live/fly_component.ex

@@ -138,11 +138,7 @@ defmodule LivebookWeb.HubLive.FlyComponent do
     case FlyClient.fetch_apps(token) do
       {:ok, apps} ->
         opts = select_options(apps)
-
+        changeset = Fly.change_hub(%Fly{}, %{access_token: token, hub_color: HexColor.random()})
-        changeset =
-          socket.assigns.changeset
-          |> Fly.changeset(%{access_token: token, hub_color: HexColor.random()})
-          |> clean_errors()
 
         {:noreply,
          assign(socket,
@@ -154,14 +150,10 @@ defmodule LivebookWeb.HubLive.FlyComponent do
 
       {:error, _} ->
         changeset =
-          socket.assigns.changeset
+          %Fly{}
-          |> Fly.changeset()
+          |> Fly.change_hub(%{access_token: token})
-          |> clean_errors()
-          |> put_action()
           |> add_error(:access_token, "is invalid")
 
-        send(self(), {:flash, :error, "Failed to fetch Applications"})
-
         {:noreply,
          assign(socket,
            changeset: changeset,
@@ -173,17 +165,15 @@ defmodule LivebookWeb.HubLive.FlyComponent do
   end
 
   def handle_event("randomize_color", _, socket) do
-    changeset =
+    handle_event("validate", %{"fly" => %{"hub_color" => HexColor.random()}}, socket)
-      socket.assigns.changeset
-      |> clean_errors()
-      |> Fly.change_hub(%{hub_color: HexColor.random()})
-      |> put_action()
-
-    {:noreply, assign(socket, changeset: changeset, valid?: changeset.valid?)}
   end
 
   def handle_event("save", %{"fly" => params}, socket) do
-    {:noreply, save_fly(socket, socket.assigns.operation, params)}
+    if socket.assigns.valid? do
+      {:noreply, save_fly(socket, socket.assigns.operation, params)}
+    else
+      {:noreply, socket}
+    end
   end
 
   def handle_event("validate", %{"fly" => attrs}, socket) do
@@ -197,7 +187,9 @@ defmodule LivebookWeb.HubLive.FlyComponent do
       if selected_app do
         Fly.change_hub(selected_app, params)
       else
-        Fly.changeset(socket.assigns.changeset, params)
+        socket.assigns.changeset
+        |> Fly.changeset(params)
+        |> Map.replace!(:action, :validate)
       end
 
     {:noreply,
@@ -226,19 +218,16 @@ defmodule LivebookWeb.HubLive.FlyComponent do
 
   defp save_fly(socket, :new, params) do
     case Fly.create_hub(socket.assigns.selected_app, params) do
-      {:ok, fly} ->
+      {:ok, hub} ->
-        changeset =
+        changeset = Fly.change_hub(hub, params)
-          fly
-          |> Fly.change_hub(params)
-          |> put_action()
 
         socket
-        |> assign(changeset: changeset, valid?: changeset.valid?)
+        |> assign(changeset: changeset, selected_app: hub, valid?: changeset.valid?)
         |> put_flash(:success, "Hub created successfully")
-        |> push_redirect(to: Routes.hub_path(socket, :edit, fly.id))
+        |> push_redirect(to: Routes.hub_path(socket, :edit, hub.id))
 
       {:error, changeset} ->
-        assign(socket, changeset: put_action(changeset), valid?: changeset.valid?)
+        assign(socket, changeset: %{changeset | action: :validate}, valid?: changeset.valid?)
     end
   end
 
@@ -246,25 +235,19 @@ defmodule LivebookWeb.HubLive.FlyComponent do
     id = socket.assigns.selected_app.id
 
     case Fly.update_hub(socket.assigns.selected_app, params) do
-      {:ok, fly} ->
+      {:ok, hub} ->
-        changeset =
+        changeset = Fly.change_hub(hub, params)
-          fly
-          |> Fly.change_hub(params)
-          |> put_action()
 
         socket
-        |> assign(changeset: changeset, selected_app: fly, valid?: changeset.valid?)
+        |> assign(changeset: changeset, selected_app: hub, valid?: changeset.valid?)
         |> put_flash(:success, "Hub updated successfully")
         |> push_redirect(to: Routes.hub_path(socket, :edit, id))
 
       {:error, changeset} ->
-        assign(socket, changeset: changeset, valid?: changeset.valid?)
+        assign(socket, changeset: %{changeset | action: :validate}, valid?: changeset.valid?)
     end
   end
 
-  defp clean_errors(changeset), do: %{changeset | errors: []}
-  defp put_action(changeset, action \\ :validate), do: %{changeset | action: action}
-
   defp hub_color(changeset), do: get_field(changeset, :hub_color)
   defp access_token(changeset), do: get_field(changeset, :access_token)
 end

lib/livebook_web/live/layout_helpers.ex

@@ -184,7 +184,7 @@ defmodule LivebookWeb.LayoutHelpers do
 
   defp hub_section(assigns) do
     ~H"""
-    <%= if Application.get_env(:livebook, :feature_flags)[:hub] do %>
+    <%= if Livebook.Config.feature_flag_enabled?(:hub) do %>
       <div id="hubs" class="flex flex-col mt-12">
         <div class="space-y-1">
           <div class="grid grid-cols-1 md:grid-cols-2 relative leading-6 mb-2">

lib/livebook_web/router.ex

@@ -55,7 +55,7 @@ defmodule LivebookWeb.Router do
       live "/explore", ExploreLive, :page
       live "/explore/notebooks/:slug", ExploreLive, :notebook
 
-      if Application.compile_env(:livebook, :feature_flags)[:hub] do
+      if Livebook.Config.feature_flag_enabled?(:hub) do
         live "/hub", HubLive, :new
         live "/hub/:id", HubLive, :edit
       end

test/livebook_web/live/home_live_test.exs

@@ -232,14 +232,6 @@ defmodule LivebookWeb.HomeLiveTest do
   end
 
   describe "hubs sidebar" do
-    test "doesn't show with disabled feature flag", %{conn: conn} do
-      Application.put_env(:livebook, :feature_flags, hub: false)
-      {:ok, _view, html} = live(conn, "/")
-      Application.put_env(:livebook, :feature_flags, hub: true)
-
-      refute html =~ "HUBS"
-    end
-
     test "render section", %{conn: conn} do
       {:ok, _view, html} = live(conn, "/")
       assert html =~ "HUBS"