From 56df5a3802fe3cf9e9f11884d207bbcb283fd5f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonatan=20K=C5=82osko?= <jonatanklosko@gmail.com>
Date: Fri, 27 Oct 2023 18:33:32 +0200
Subject: [PATCH] Improve hub tests (#2308)

---
 lib/livebook/hubs/dockerfile.ex               |  2 +-
 .../live/file_select_component.ex             | 66 ++++++++++++-------
 .../live/hub/edit/team_component.ex           | 30 ++++-----
 .../livebook_teams/web/hub/edit_live_test.exs | 16 ++---
 test/livebook_teams/web/session_live_test.exs | 46 ++++++-------
 .../live/file_select_component_test.exs       |  2 +-
 test/livebook_web/live/session_live_test.exs  | 10 +--
 7 files changed, 88 insertions(+), 84 deletions(-)

diff --git a/lib/livebook/hubs/dockerfile.ex b/lib/livebook/hubs/dockerfile.ex
index 8f4c2bedd..5fd36f883 100644
--- a/lib/livebook/hubs/dockerfile.ex
+++ b/lib/livebook/hubs/dockerfile.ex
@@ -54,7 +54,7 @@ defmodule Livebook.Hubs.Dockerfile do
           list(Livebook.FileSystem.t()),
           Livebook.FileSystem.File.t() | nil,
           list(Livebook.Notebook.file_entry()),
-          list(Livebook.Session.Data.secrets())
+          Livebook.Session.Data.secrets()
         ) :: String.t()
   def build_dockerfile(config, hub, hub_secrets, hub_file_systems, file, file_entries, secrets) do
     base_image = Enum.find(Livebook.Config.docker_images(), &(&1.tag == config.docker_tag))
diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex
index ebce30cba..61701349c 100644
--- a/lib/livebook_web/live/file_select_component.ex
+++ b/lib/livebook_web/live/file_select_component.ex
@@ -87,11 +87,12 @@ defmodule LivebookWeb.FileSelectComponent do
   @impl true
   def render(assigns) do
     ~H"""
-    <div class="h-full flex flex-col">
+    <div id={@id} class="h-full flex flex-col">
       <h2 class="sr-only">File system</h2>
       <div class="flex space-x-3 items-center mb-4">
         <div class="grow flex space-x-1">
           <.file_system_menu_button
+            id={"#{@id}-file-system-menu"}
             file={@file}
             file_systems={@file_systems}
             configure_path={@configure_path}
@@ -107,7 +108,7 @@ defmodule LivebookWeb.FileSelectComponent do
           >
             <input
               class="input"
-              id="input-path"
+              id={"#{@id}-input-path"}
               aria-label="file path"
               phx-hook="FocusOnUpdate"
               type="text"
@@ -119,20 +120,27 @@ defmodule LivebookWeb.FileSelectComponent do
             />
           </form>
         </div>
-        <.menu id="new-item-menu" disabled={@file_system_select_disabled} position={:bottom_right}>
+        <.menu
+          id={"#{@id}-new-item-menu"}
+          disabled={@file_system_select_disabled}
+          position={:bottom_right}
+        >
           <:toggle>
             <button class="icon-button" tabindex="-1" aria-label="add">
               <.remix_icon icon="add-line" class="text-xl" />
             </button>
           </:toggle>
           <.menu_item>
-            <button role="menuitem" phx-click={js_show_new_item_section("dir")}>
+            <button role="menuitem" phx-click={js_show_new_item_section("#{@id}-new-dir-section")}>
               <.remix_icon icon="folder-add-line" />
               <span>New directory</span>
             </button>
           </.menu_item>
           <.menu_item>
-            <button role="menuitem" phx-click={js_show_new_item_section("notebook")}>
+            <button
+              role="menuitem"
+              phx-click={js_show_new_item_section("#{@id}-new-notebook-section")}
+            >
               <.remix_icon icon="file-add-line" />
               <span>New notebook</span>
             </button>
@@ -181,14 +189,24 @@ defmodule LivebookWeb.FileSelectComponent do
         </div>
       </div>
 
-      <.new_item_section type="dir" icon="folder-add-fill" myself={@myself} />
-      <.new_item_section type="notebook" icon="file-add-line" myself={@myself} />
+      <.new_item_section
+        id={"#{@id}-new-dir-section"}
+        type="dir"
+        icon="folder-add-fill"
+        myself={@myself}
+      />
+      <.new_item_section
+        id={"#{@id}-new-notebook-section"}
+        type="notebook"
+        icon="file-add-line"
+        myself={@myself}
+      />
 
       <div
         class="grow -m-1 p-1 h-full rounded-lg overflow-y-auto tiny-scrollbar"
         tabindex="-1"
         phx-hook="Dropzone"
-        id="file-select-upload-dropzone"
+        id={"#{@id}-file-select-upload-dropzone"}
       >
         <form phx-change="file_validate" phx-drop-target={@uploads.folder.ref} phx-target={@myself}>
           <.live_file_input
@@ -203,6 +221,7 @@ defmodule LivebookWeb.FileSelectComponent do
           >
             <%= for file_info <- @file_infos, file_info.highlighted != "" do %>
               <.file
+                id={"#{@id}-file-#{file_info.id}"}
                 file_info={file_info}
                 myself={@myself}
                 renaming_file={@renaming_file}
@@ -214,6 +233,7 @@ defmodule LivebookWeb.FileSelectComponent do
           <div class="grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-4 gap-2">
             <%= for file_info <- @file_infos, file_info.highlighted == "" do %>
               <.file
+                id={"#{@id}-file-#{file_info.id}"}
                 file_info={file_info}
                 myself={@myself}
                 renaming_file={@renaming_file}
@@ -243,25 +263,24 @@ defmodule LivebookWeb.FileSelectComponent do
     ~H"""
     <div
       class="hidden grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-4 gap-2 border-b border-dashed border-grey-200 mb-2 pb-2"
-      id={"new-#{@type}-section"}
+      id={@id}
     >
       <form
         class="flex space-x-2 items-center p-2 rounded-lg"
-        phx-submit={JS.push("create_#{@type}", target: @myself) |> js_hide_new_item_section(@type)}
+        phx-submit={JS.push("create_#{@type}", target: @myself) |> js_hide_new_item_section(@id)}
       >
         <span class="block">
           <.remix_icon icon={@icon} class="text-xl align-middle text-gray-400" />
         </span>
         <span class="flex font-medium text-gray-500">
-          <div phx-window-keydown={js_hide_new_item_section(@type)} phx-key="escape">
+          <div phx-window-keydown={js_hide_new_item_section(@id)} phx-key="escape">
             <input
-              id={"new-#{@type}-input"}
               type="text"
               name="name"
               aria-label="new directory"
               spellcheck="false"
               autocomplete="off"
-              phx-blur={js_hide_new_item_section(@type)}
+              phx-blur={js_hide_new_item_section(@id)}
             />
           </div>
         </span>
@@ -276,7 +295,7 @@ defmodule LivebookWeb.FileSelectComponent do
 
   defp file_system_menu_button(assigns) do
     ~H"""
-    <.menu id="file-system-menu" disabled={@file_system_select_disabled} position={:bottom_left}>
+    <.menu id={@id} disabled={@file_system_select_disabled} position={:bottom_left}>
       <:toggle>
         <button
           type="button"
@@ -293,7 +312,7 @@ defmodule LivebookWeb.FileSelectComponent do
       <%= for file_system <- @file_systems do %>
         <%= if file_system.id == @file.file_system_id do %>
           <.menu_item variant={:selected}>
-            <button id={"file-system-#{file_system.id}"} role="menuitem">
+            <button id={"#{@id}-file-system-#{file_system.id}"} role="menuitem">
               <.file_system_icon file_system={file_system} />
               <span><%= file_system_label(file_system) %></span>
             </button>
@@ -301,7 +320,7 @@ defmodule LivebookWeb.FileSelectComponent do
         <% else %>
           <.menu_item>
             <button
-              id={"file-system-#{file_system.id}"}
+              id={"#{@id}-file-system-#{file_system.id}"}
               role="menuitem"
               phx-target={@myself}
               phx-click="set_file_system"
@@ -360,7 +379,7 @@ defmodule LivebookWeb.FileSelectComponent do
     assigns = assign(assigns, :icon, icon)
 
     ~H"""
-    <.menu id={"file-#{Base.encode16(@file_info.file.path)}"} secondary_click>
+    <.menu id={@id} secondary_click>
       <:toggle>
         <button
           type="button"
@@ -428,16 +447,16 @@ defmodule LivebookWeb.FileSelectComponent do
     """
   end
 
-  defp js_show_new_item_section(js \\ %JS{}, type) do
+  defp js_show_new_item_section(js \\ %JS{}, id) do
     js
-    |> JS.show(to: "#new-#{type}-section")
-    |> JS.dispatch("lb:set_value", to: "#new-#{type}-input", detail: %{value: ""})
-    |> JS.dispatch("lb:focus", to: "#new-#{type}-input")
+    |> JS.show(to: "##{id}")
+    |> JS.dispatch("lb:set_value", to: "##{id} input", detail: %{value: ""})
+    |> JS.dispatch("lb:focus", to: "##{id} input")
   end
 
-  defp js_hide_new_item_section(js \\ %JS{}, type) do
+  defp js_hide_new_item_section(js \\ %JS{}, id) do
     js
-    |> JS.hide(to: "#new-#{type}-section")
+    |> JS.hide(to: "##{id}")
   end
 
   defp handle_progress(:folder, entry, socket) when entry.done? do
@@ -651,6 +670,7 @@ defmodule LivebookWeb.FileSelectComponent do
 
   defp file_info(file, name, running_files, opts \\ []) do
     %{
+      id: Base.url_encode64(file.path, padding: false),
       name: name,
       highlighted: "",
       unhighlighted: name,
diff --git a/lib/livebook_web/live/hub/edit/team_component.ex b/lib/livebook_web/live/hub/edit/team_component.ex
index b142193d6..d1dd1949f 100644
--- a/lib/livebook_web/live/hub/edit/team_component.ex
+++ b/lib/livebook_web/live/hub/edit/team_component.ex
@@ -16,7 +16,7 @@ defmodule LivebookWeb.Hub.Edit.TeamComponent do
     file_systems = Hubs.get_file_systems(assigns.hub, hub_only: true)
     secret_name = assigns.params["secret_name"]
     file_system_id = assigns.params["file_system_id"]
-    is_default? = is_default?(assigns.hub)
+    default? = default_hub?(assigns.hub)
 
     secret_value =
       if assigns.live_action == :edit_secret do
@@ -41,7 +41,7 @@ defmodule LivebookWeb.Hub.Edit.TeamComponent do
        secret_name: secret_name,
        secret_value: secret_value,
        hub_metadata: Provider.to_metadata(assigns.hub),
-       is_default: is_default?
+       default?: default?
      )
      |> assign_new(:config_changeset, fn -> Hubs.Dockerfile.config_changeset() end)
      |> update_dockerfile()
@@ -79,7 +79,7 @@ defmodule LivebookWeb.Hub.Edit.TeamComponent do
                   <span class="bg-green-100 text-green-800 text-xs px-2.5 py-0.5 rounded cursor-default">
                     Livebook Teams
                   </span>
-                  <%= if @is_default do %>
+                  <%= if @default? do %>
                     <span class="bg-blue-100 text-blue-800 text-xs px-2.5 py-0.5 rounded cursor-default">
                       Default
                     </span>
@@ -96,29 +96,29 @@ defmodule LivebookWeb.Hub.Edit.TeamComponent do
                   <.remix_icon icon="settings-line" /> Manage organization
                 </a>
 
-                <a
+                <button
                   phx-click={show_modal("show-key-modal")}
                   phx-target={@myself}
                   class="hover:text-blue-600 cursor-pointer"
                 >
                   <.remix_icon icon="key-2-fill" /> Display Teams key
-                </a>
-                <%= if @is_default do %>
-                  <a
-                    phx-click={JS.push("remove_as_default")}
+                </button>
+                <%= if @default? do %>
+                  <button
+                    phx-click="remove_as_default"
                     phx-target={@myself}
                     class="hover:text-blue-600 cursor-pointer"
                   >
                     <.remix_icon icon="star-fill" /> Remove as default
-                  </a>
+                  </button>
                 <% else %>
-                  <a
-                    phx-click={JS.push("mark_as_default")}
+                  <button
+                    phx-click="mark_as_default"
                     phx-target={@myself}
                     class="hover:text-blue-600 cursor-pointer"
                   >
                     <.remix_icon icon="star-line" /> Mark as default
-                  </a>
+                  </button>
                 <% end %>
               </p>
             </div>
@@ -429,15 +429,15 @@ defmodule LivebookWeb.Hub.Edit.TeamComponent do
 
   def handle_event("mark_as_default", _, socket) do
     Hubs.set_default_hub(socket.assigns.hub.id)
-    {:noreply, push_navigate(socket, to: ~p"/hub/#{socket.assigns.hub.id}")}
+    {:noreply, assign(socket, default?: true)}
   end
 
   def handle_event("remove_as_default", _, socket) do
     Hubs.unset_default_hub()
-    {:noreply, push_navigate(socket, to: ~p"/hub/#{socket.assigns.hub.id}")}
+    {:noreply, assign(socket, default?: false)}
   end
 
-  defp is_default?(hub) do
+  defp default_hub?(hub) do
     Hubs.get_default_hub().id == hub.id
   end
 
diff --git a/test/livebook_teams/web/hub/edit_live_test.exs b/test/livebook_teams/web/hub/edit_live_test.exs
index 5fe5d1f47..854aaf968 100644
--- a/test/livebook_teams/web/hub/edit_live_test.exs
+++ b/test/livebook_teams/web/hub/edit_live_test.exs
@@ -334,11 +334,9 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do
     test "marking and unmarking hub as default", %{conn: conn, hub: hub} do
       {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}")
 
-      assert {:ok, view, _html} =
-               view
-               |> element("a", "Mark as default")
-               |> render_click()
-               |> follow_redirect(conn)
+      view
+      |> element("button", "Mark as default")
+      |> render_click()
 
       assert view
              |> element("span", "Default")
@@ -346,11 +344,9 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do
 
       assert Hubs.get_default_hub().id == hub.id
 
-      assert {:ok, view, _html} =
-               view
-               |> element("a", "Remove as default")
-               |> render_click()
-               |> follow_redirect(conn)
+      view
+      |> element("button", "Remove as default")
+      |> render_click()
 
       refute view
              |> element("span", "Default")
diff --git a/test/livebook_teams/web/session_live_test.exs b/test/livebook_teams/web/session_live_test.exs
index f35434071..f1db5ed7d 100644
--- a/test/livebook_teams/web/session_live_test.exs
+++ b/test/livebook_teams/web/session_live_test.exs
@@ -51,7 +51,6 @@ defmodule LivebookWeb.Integration.SessionLiveTest do
 
       # clicks the button to add a new secret
       view
-      |> with_target("#secrets_list")
       |> element("#new-secret-button")
       |> render_click(%{})
 
@@ -76,8 +75,7 @@ defmodule LivebookWeb.Integration.SessionLiveTest do
 
       # fills and submits the secrets modal form
       # to create a new secret on team hub
-      secrets_modal = with_target(view, "#secrets")
-      form = element(secrets_modal, ~s{form[phx-submit="save"]})
+      form = element(view, ~s{#secrets-modal form[phx-submit="save"]})
 
       render_change(form, attrs)
       render_submit(form, attrs)
@@ -119,7 +117,6 @@ defmodule LivebookWeb.Integration.SessionLiveTest do
 
       # clicks the button to edit a secret
       view
-      |> with_target("#secrets_list")
       |> element("#hub-#{id}-secret-#{secret_name}-edit-button")
       |> render_click()
 
@@ -190,8 +187,7 @@ defmodule LivebookWeb.Integration.SessionLiveTest do
       # clicks the button and fills the form to create a new secret
       # that prefilled the name with the received from exception.
       render_click(add_secret_button)
-      secrets_component = with_target(view, "#secrets-modal")
-      form_element = element(secrets_component, "form[phx-submit='save']")
+      form_element = element(view, "#secrets-modal form[phx-submit='save']")
       assert has_element?(form_element)
       attrs = %{value: secret.value, hub_id: team.id}
       render_submit(form_element, %{secret: attrs})
@@ -257,13 +253,12 @@ defmodule LivebookWeb.Integration.SessionLiveTest do
       # is being shown, so clicks it's button to set the app secret
       # to the session, allowing the user to fetches the secret.
       render_click(add_secret_button)
-      secrets_component = with_target(view, "#secrets-modal")
 
-      assert render(secrets_component) =~
-               "in #{hub_label(team)}. Allow this session to access it?"
+      assert render(view) =~ "in #{hub_label(team)}. Allow this session to access it?"
 
-      grant_access_button = element(secrets_component, "button", "Grant access")
-      render_click(grant_access_button)
+      view
+      |> element("#secrets-modal button", "Grant access")
+      |> render_click()
 
       # checks if the secret exists and is inside the session,
       # then executes the code cell again and checks if the
@@ -294,15 +289,15 @@ defmodule LivebookWeb.Integration.SessionLiveTest do
 
       bucket_url = "https://my-own-bucket.s3.amazonaws.com"
 
-      file_system =
+      team_file_system =
         build(:fs_s3,
           id: Livebook.FileSystem.S3.id(team_id, bucket_url),
           bucket_url: bucket_url,
           hub_id: team_id
         )
 
-      Livebook.Hubs.create_file_system(team, file_system)
-      assert_receive {:file_system_created, team_file_system}
+      Livebook.Hubs.create_file_system(team, team_file_system)
+      assert_receive {:file_system_created, %{hub_id: ^team_id} = team_file_system}
 
       # loads the session page
       {:ok, view, _html} = live(conn, ~p"/sessions/#{session.id}/add-file/storage")
@@ -312,22 +307,21 @@ defmodule LivebookWeb.Integration.SessionLiveTest do
       Session.set_notebook_hub(session.pid, personal_id)
       assert_receive {:operation, {:set_notebook_hub, _client, ^personal_id}}
 
-      # targets the file system dropdown menu
-      file_system_menu = with_target(view, "#add-file-entry-modal #file-system-menu-content")
+      file_entry_select = element(view, "#add-file-entry-select")
 
       # checks the file systems from Personal
-      assert has_element?(file_system_menu, "#file-system-local")
-      assert has_element?(file_system_menu, "#file-system-#{personal_file_system.id}")
-      refute has_element?(file_system_menu, "#file-system-#{team_file_system.id}")
+      assert render(file_entry_select) =~ "local"
+      assert render(file_entry_select) =~ personal_file_system.id
+      refute render(file_entry_select) =~ team_file_system.id
 
       # change the hub to Team
       # and checks the file systems from Team
       Session.set_notebook_hub(session.pid, team.id)
       assert_receive {:operation, {:set_notebook_hub, _client, ^team_id}}
 
-      assert has_element?(file_system_menu, "#file-system-local")
-      refute has_element?(file_system_menu, "#file-system-#{personal_file_system.id}")
-      assert has_element?(file_system_menu, "#file-system-#{team_file_system.id}")
+      assert render(file_entry_select) =~ "local"
+      refute render(file_entry_select) =~ personal_file_system.id
+      assert render(file_entry_select) =~ team_file_system.id
     end
 
     test "shows file system from offline hub", %{conn: conn, session: session} do
@@ -357,12 +351,10 @@ defmodule LivebookWeb.Integration.SessionLiveTest do
       Session.set_notebook_hub(session.pid, hub_id)
       assert_receive {:operation, {:set_notebook_hub, _client, ^hub_id}}
 
-      # targets the file system dropdown menu
-      file_system_menu = with_target(view, "#add-file-entry-select #file-system-menu-content")
-
       # checks the file systems from Offline hub
-      assert has_element?(file_system_menu, "#file-system-local")
-      assert has_element?(file_system_menu, "#file-system-#{file_system.id}")
+      file_entry_select = element(view, "#add-file-entry-select")
+      assert render(file_entry_select) =~ "local"
+      assert render(file_entry_select) =~ file_system.id
 
       remove_offline_hub_file_system(file_system)
     end
diff --git a/test/livebook_web/live/file_select_component_test.exs b/test/livebook_web/live/file_select_component_test.exs
index dca197fc5..946e38330 100644
--- a/test/livebook_web/live/file_select_component_test.exs
+++ b/test/livebook_web/live/file_select_component_test.exs
@@ -26,7 +26,7 @@ defmodule LivebookWeb.FileSelectComponentTest do
   defp attrs(attrs) do
     Keyword.merge(
       [
-        id: 1,
+        id: "1",
         file: FileSystem.File.local(p("/")),
         extnames: [".livemd"],
         running_files: []
diff --git a/test/livebook_web/live/session_live_test.exs b/test/livebook_web/live/session_live_test.exs
index eadbf3f8d..ad75e41e0 100644
--- a/test/livebook_web/live/session_live_test.exs
+++ b/test/livebook_web/live/session_live_test.exs
@@ -1515,8 +1515,7 @@ defmodule LivebookWeb.SessionLiveTest do
       # Clicks the button and fills the form to create a new secret
       # that prefilled the name with the received from exception.
       render_click(add_secret_button)
-      secrets_component = with_target(view, "#secrets-modal")
-      form_element = element(secrets_component, "form[phx-submit='save']")
+      form_element = element(view, "#secrets-modal form[phx-submit='save']")
       assert has_element?(form_element)
       render_submit(form_element, %{secret: %{value: secret.value, hub_id: secret.hub_id}})
 
@@ -1562,12 +1561,10 @@ defmodule LivebookWeb.SessionLiveTest do
       # is being shown, so clicks it's button to set the app secret
       # to the session, allowing the user to fetches the secret.
       render_click(add_secret_button)
-      secrets_component = with_target(view, "#secrets-modal")
 
-      assert render(secrets_component) =~
-               "in #{hub_label(secret)}. Allow this session to access it?"
+      assert render(view) =~ "in #{hub_label(secret)}. Allow this session to access it?"
 
-      grant_access_button = element(secrets_component, "button", "Grant access")
+      grant_access_button = element(view, "#secrets-modal button", "Grant access")
       render_click(grant_access_button)
 
       # Checks if the secret exists and is inside the session,
@@ -1621,7 +1618,6 @@ defmodule LivebookWeb.SessionLiveTest do
 
       # clicks the button to edit a secret
       view
-      |> with_target("#secrets_list")
       |> element("#hub-#{hub.id}-secret-#{secret_name}-edit-button")
       |> render_click()