From c699fbbd79af883992f8be7e625ab5465fd3a308 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@dashbit.co>
Date: Mon, 1 Apr 2024 10:25:33 +0200
Subject: [PATCH] Allow autosave to be reenable in the UI, closes #2534

---
 lib/livebook_web/live/open_live.ex            | 12 ++++++++--
 lib/livebook_web/live/settings_live.ex        | 24 +++++++++++++++----
 test/livebook_web/live/open_live_test.exs     |  5 ++++
 test/livebook_web/live/settings_live_test.exs | 21 +++++++++++++---
 4 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/lib/livebook_web/live/open_live.ex b/lib/livebook_web/live/open_live.ex
index 7d3870ccf..697ec7c9b 100644
--- a/lib/livebook_web/live/open_live.ex
+++ b/lib/livebook_web/live/open_live.ex
@@ -18,6 +18,12 @@ defmodule LivebookWeb.OpenLive do
     sessions = Sessions.list_sessions() |> Enum.filter(&(&1.mode == :default))
     recent_notebooks = Livebook.NotebookManager.recent_notebooks()
 
+    show_autosave_note? =
+      case Livebook.Settings.autosave_path() do
+        nil -> false
+        path -> match?({:ok, [_ | _]}, File.ls(path))
+      end
+
     {:ok,
      assign(socket,
        tab: "file",
@@ -25,7 +31,8 @@ defmodule LivebookWeb.OpenLive do
        url: params["url"],
        sessions: sessions,
        recent_notebooks: recent_notebooks,
-       page_title: "Open - Livebook"
+       page_title: "Open - Livebook",
+       show_autosave_note?: show_autosave_note?
      )}
   end
 
@@ -130,7 +137,8 @@ defmodule LivebookWeb.OpenLive do
               </:card_icon>
             </.live_component>
           <% end %>
-          <div class="mt-3 text-gray-600 text-sm">
+
+          <div :if={@show_autosave_note?} class="mt-3 text-gray-600 text-sm">
             Looking for unsaved notebooks? <.link
               class="font-semibold"
               navigate={~p"/open/storage?autosave=true"}
diff --git a/lib/livebook_web/live/settings_live.ex b/lib/livebook_web/live/settings_live.ex
index 1e473e232..35b6b28dc 100644
--- a/lib/livebook_web/live/settings_live.ex
+++ b/lib/livebook_web/live/settings_live.ex
@@ -202,7 +202,15 @@ defmodule LivebookWeb.SettingsLive do
     """
   end
 
-  defp autosave_path_select(%{state: %{file: nil}} = assigns), do: ~H""
+  defp autosave_path_select(%{state: %{file: nil}} = assigns) do
+    ~H"""
+    <div>
+      <.button color="gray" id="enable-autosave" phx-click="open_autosave_path_select">
+        Enable
+      </.button>
+    </div>
+    """
+  end
 
   defp autosave_path_select(%{state: %{dialog_opened?: true}} = assigns) do
     ~H"""
@@ -217,13 +225,14 @@ defmodule LivebookWeb.SettingsLive do
         file_system_select_disabled={true}
         target={self()}
       >
-        <.button color="gray" phx-click="cancel_autosave_path" tabindex="-1">
+        <.button id="cancel-autosave" color="gray" phx-click="cancel_autosave_path" tabindex="-1">
           Cancel
         </.button>
-        <.button color="gray" phx-click="reset_autosave_path" tabindex="-1">
+        <.button id="reset-autosave" color="gray" phx-click="reset_autosave_path" tabindex="-1">
           Reset
         </.button>
         <.button
+          id="save-autosave"
           phx-click="set_autosave_path"
           disabled={not Livebook.FileSystem.File.dir?(@state.file)}
           tabindex="-1"
@@ -241,7 +250,7 @@ defmodule LivebookWeb.SettingsLive do
       <div class="grow">
         <.text_field name={nil} readonly value={@state.file.path} />
       </div>
-      <.button color="gray" phx-click="open_autosave_path_select">
+      <.button color="gray" id="change-autosave" phx-click="open_autosave_path_select">
         Change
       </.button>
     </div>
@@ -290,7 +299,12 @@ defmodule LivebookWeb.SettingsLive do
   end
 
   def handle_event("open_autosave_path_select", %{}, socket) do
-    {:noreply, update(socket, :autosave_path_state, &%{&1 | dialog_opened?: true})}
+    {:noreply,
+     update(
+       socket,
+       :autosave_path_state,
+       &%{&1 | dialog_opened?: true, file: &1.file || default_autosave_dir()}
+     )}
   end
 
   def handle_event("save", %{"update_check_enabled" => enabled}, socket) do
diff --git a/test/livebook_web/live/open_live_test.exs b/test/livebook_web/live/open_live_test.exs
index 8bcb9d4d0..a0d0267f0 100644
--- a/test/livebook_web/live/open_live_test.exs
+++ b/test/livebook_web/live/open_live_test.exs
@@ -6,6 +6,11 @@ defmodule LivebookWeb.OpenLiveTest do
   alias Livebook.{Sessions, Session, FileSystem}
 
   describe "file selection" do
+    test "does not mention autosaving if disabled", %{conn: conn} do
+      refute conn |> get(~p"/open/storage") |> html_response(200) =~
+               "Looking for unsaved notebooks?"
+    end
+
     test "updates the list of files as the input changes", %{conn: conn} do
       {:ok, view, _} = live(conn, ~p"/open/storage")
 
diff --git a/test/livebook_web/live/settings_live_test.exs b/test/livebook_web/live/settings_live_test.exs
index 8bdc04f39..5033898e5 100644
--- a/test/livebook_web/live/settings_live_test.exs
+++ b/test/livebook_web/live/settings_live_test.exs
@@ -10,9 +10,7 @@ defmodule LivebookWeb.SettingsLiveTest do
   describe "environment variables configuration" do
     test "list persisted environment variables", %{conn: conn} do
       insert_env_var(:env_var, name: "MY_ENVIRONMENT_VAR")
-      {:ok, _view, html} = live(conn, ~p"/settings")
-
-      assert html =~ "MY_ENVIRONMENT_VAR"
+      assert conn |> get(~p"/settings") |> html_response(200) =~ "MY_ENVIRONMENT_VAR"
     end
 
     test "adds an environment variable", %{conn: conn} do
@@ -87,4 +85,21 @@ defmodule LivebookWeb.SettingsLiveTest do
              |> render() =~ env_var.name
     end
   end
+
+  describe "autosaving" do
+    # Autosaving can only be disable by directly changing storage
+    # but, given some users in the past had autosaving disabled,
+    # we take that into account.
+    test "can be enabled", %{conn: conn} do
+      {:ok, view, _html} = live(conn, ~p"/settings")
+
+      view
+      |> element("#enable-autosave")
+      |> render_click()
+
+      view
+      |> element("#cancel-autosave")
+      |> render_click()
+    end
+  end
 end