Show notifications for new app version and import warnings (#1955)

lib/livebook/app.ex

@@ -13,12 +13,22 @@ defmodule Livebook.App do
   # always taken from the most recently deployed notebook (e.g. access
   # type, automatic shutdown, deployment strategy).
 
-  defstruct [:slug, :pid, :version, :notebook_name, :public?, :multi_session, :sessions]
+  defstruct [
+    :slug,
+    :pid,
+    :version,
+    :warnings,
+    :notebook_name,
+    :public?,
+    :multi_session,
+    :sessions
+  ]
 
   @type t :: %{
           slug: slug(),
           pid: pid(),
           version: pos_integer(),
+          warnings: list(String.t()),
           notebook_name: String.t(),
           public?: boolean(),
           multi_session: boolean(),
@@ -46,12 +56,15 @@ defmodule Livebook.App do
 
     * `:notebook` (required) - the notebook for initial deployment
 
+    * `:warnings` - a list of warnings to show for the initial deployment
+
   """
   @spec start_link(keyword()) :: {:ok, pid} | {:error, any()}
   def start_link(opts) do
     notebook = Keyword.fetch!(opts, :notebook)
+    warnings = Keyword.get(opts, :warnings, [])
 
-    GenServer.start_link(__MODULE__, {notebook})
+    GenServer.start_link(__MODULE__, {notebook, warnings})
   end
 
   @doc """
@@ -100,9 +113,10 @@ defmodule Livebook.App do
   @doc """
   Deploys a new notebook into the app.
   """
-  @spec deploy(pid(), Livebook.Notebook.t()) :: :ok
-  def deploy(pid, notebook) do
-    GenServer.cast(pid, {:deploy, notebook})
+  @spec deploy(pid(), Livebook.Notebook.t(), keyword()) :: :ok
+  def deploy(pid, notebook, opts \\ []) do
+    warnings = Keyword.get(opts, :warnings, [])
+    GenServer.cast(pid, {:deploy, notebook, warnings})
   end
 
   @doc """
@@ -136,11 +150,12 @@ defmodule Livebook.App do
   end
 
   @impl true
-  def init({notebook}) do
+  def init({notebook, warnings}) do
     {:ok,
      %{
        version: 1,
        notebook: notebook,
+       warnings: warnings,
        sessions: [],
        users: %{}
      }
@@ -176,11 +191,11 @@ defmodule Livebook.App do
   end
 
   @impl true
-  def handle_cast({:deploy, notebook}, state) do
+  def handle_cast({:deploy, notebook, warnings}, state) do
     true = notebook.app_settings.slug == state.notebook.app_settings.slug
 
     {:noreply,
-     %{state | notebook: notebook, version: state.version + 1}
+     %{state | notebook: notebook, version: state.version + 1, warnings: warnings}
      |> start_eagerly()
      |> shutdown_old_versions()
      |> notify_update()}
@@ -221,6 +236,7 @@ defmodule Livebook.App do
       slug: state.notebook.app_settings.slug,
       pid: self(),
       version: state.version,
+      warnings: state.warnings,
       notebook_name: state.notebook.name,
       public?: state.notebook.app_settings.access_type == :public,
       multi_session: state.notebook.app_settings.multi_session,

lib/livebook/apps.ex

@@ -16,9 +16,16 @@ defmodule Livebook.Apps do
   If there is no app process under the corresponding slug, it is started.
   Otherwise the notebook is deployed as a new version into the existing
   app.
+
+  ## Options
+
+    * `:warnings` - a list of warnings to show for the new deployment
+
   """
-  @spec deploy(Livebook.Notebook.t()) :: {:ok, pid()} | {:error, term()}
-  def deploy(notebook) do
+  @spec deploy(Livebook.Notebook.t(), keyword()) :: {:ok, pid()} | {:error, term()}
+  def deploy(notebook, opts \\ []) do
+    opts = Keyword.validate!(opts, warnings: [])
+
     slug = notebook.app_settings.slug
     name = name(slug)
 
@@ -27,25 +34,25 @@ defmodule Livebook.Apps do
         :global.trans({{:app_registration, name}, node()}, fn ->
           case :global.whereis_name(name) do
             :undefined ->
-              with {:ok, pid} <- start_app(notebook) do
+              with {:ok, pid} <- start_app(notebook, opts[:warnings]) do
                 :yes = :global.register_name(name, pid)
                 {:ok, pid}
               end
 
             pid ->
-              App.deploy(pid, notebook)
+              App.deploy(pid, notebook, warnings: opts[:warnings])
               {:ok, pid}
           end
         end)
 
       pid ->
-        App.deploy(pid, notebook)
+        App.deploy(pid, notebook, warnings: opts[:warnings])
         {:ok, pid}
     end
   end
 
-  defp start_app(notebook) do
-    opts = [notebook: notebook]
+  defp start_app(notebook, warnings) do
+    opts = [notebook: notebook, warnings: warnings]
 
     case DynamicSupervisor.start_child(Livebook.AppSupervisor, {App, opts}) do
       {:ok, pid} ->
@@ -178,7 +185,8 @@ defmodule Livebook.Apps do
         end
 
       if Livebook.Notebook.AppSettings.valid?(notebook.app_settings) do
-        deploy(notebook)
+        warnings = Enum.map(warnings, &("Import: " <> &1))
+        deploy(notebook, warnings: warnings)
       else
         Logger.warning("Skipping app deployment at #{path} due to invalid settings")
       end

lib/livebook/live_markdown/import.ex

@@ -21,7 +21,7 @@ defmodule Livebook.LiveMarkdown.Import do
   end
 
   defp earmark_message_to_string({_severity, line_number, message}) do
-    "Line #{line_number}: #{message}"
+    "line #{line_number} - #{Livebook.Utils.downcase_first(message)}"
   end
 
   # Does initial pre-processing of the AST, so that it conforms to the expected form.
@@ -46,7 +46,7 @@ defmodule Livebook.LiveMarkdown.Import do
       ast = Enum.map(ast, &downgrade_heading/1)
 
       message =
-        "Downgrading all headings, because #{primary_headings} instances of heading 1 were found"
+        "downgrading all headings, because #{primary_headings} instances of heading 1 were found"
 
       {ast, [message]}
     else
@@ -75,7 +75,7 @@ defmodule Livebook.LiveMarkdown.Import do
           {ast, []}
         else
           ast = comments ++ [heading] ++ leading ++ rest
-          message = "Moving heading 1 to the top of the notebook"
+          message = "moving heading 1 to the top of the notebook"
           {ast, [message]}
         end
     end

lib/livebook/session.ex

@@ -2040,9 +2040,15 @@ defmodule Livebook.Session do
     locator = {container_ref_for_section(section), cell.id}
     parent_locators = parent_locators_for_cell(state.data, cell)
 
+    language =
+      case cell do
+        %Cell.Code{} -> cell.language
+        _ -> :elixir
+      end
+
     Runtime.evaluate_code(
       state.data.runtime,
-      cell.language,
+      language,
       cell.source,
       locator,
       parent_locators,

lib/livebook_web/components/core_components.ex

@@ -85,6 +85,34 @@ defmodule LivebookWeb.CoreComponents do
     """
   end
 
+  @doc """
+  Renders a message notice.
+
+  Similar to `flash/1`, but for permanent messages on the page.
+
+  ## Examples
+
+      <.message_box kind={:info} message="🦊 in a 📦" />
+
+  """
+
+  attr :message, :string, required: true
+  attr :kind, :atom, values: [:info, :success, :warning, :error]
+
+  def message_box(assigns) do
+    ~H"""
+    <div class={[
+      "shadow text-sm flex items-center space-x-3 rounded-lg px-4 py-2 border-l-4 rounded-l-none bg-white text-gray-700",
+      @kind == :info && "border-blue-500",
+      @kind == :success && "border-blue-500",
+      @kind == :warning && "border-yellow-300",
+      @kind == :error && "border-red-500"
+    ]}>
+      <div class="whitespace-pre-wrap pr-2 max-h-52 overflow-y-auto tiny-scrollbar" phx-no-format><%= @message %></div>
+    </div>
+    """
+  end
+
   @doc """
   Creates a live region with the given role.
 

lib/livebook_web/live/app_session_live.ex

@@ -251,6 +251,14 @@ defmodule LivebookWeb.AppSessionLive do
     redirect_on_closed(socket)
   end
 
+  defp after_operation(socket, _prev_socket, {:app_shutdown, _client_id}) do
+    put_flash(
+      socket,
+      :info,
+      "A new version has been deployed, this session will close once everybody leaves"
+    )
+  end
+
   defp after_operation(socket, _prev_socket, _operation), do: socket
 
   defp redirect_on_closed(socket) do

lib/livebook_web/live/apps_live.ex

@@ -53,6 +53,9 @@ defmodule LivebookWeb.AppsLive do
         <div class="mb-2 text-gray-800 font-medium text-xl">
           <%= "/" <> app.slug %>
         </div>
+        <div class="mt-4 flex flex-col gap-3">
+          <.message_box :for={warning <- app.warnings} kind={:warning} message={warning} />
+        </div>
         <div class="mt-4 mb-2 text-gray-600 font-medium text-sm">
           App info
         </div>

lib/livebook_web/live/session_live/app_info_component.ex

@@ -14,11 +14,12 @@ defmodule LivebookWeb.SessionLive.AppInfoComponent do
         <.app_info_icon />
       </div>
       <%= if @session.mode == :app do %>
-        <div class="mt-5 flex flex-col space-y-6">
-          <span class="text-gray-700 text-sm">
-            This session is a running app. To deploy a modified version, you can fork it.
-          </span>
-          <div>
+        <div class="mt-5 flex flex-col">
+          <.message_box
+            kind={:info}
+            message="This session is a running app. To deploy a modified version, you can fork it."
+          />
+          <div class="mt-6">
             <button class="button-base button-blue" phx-click="fork_session">
               <.remix_icon icon="git-branch-line" />
               <span>Fork</span>

test/livebook/apps_test.exs

@@ -72,5 +72,31 @@ defmodule Livebook.AppsTest do
 
       Livebook.App.close(app.pid)
     end
+
+    @tag :capture_log
+    @tag :tmp_dir
+    test "deploys with import warnings", %{tmp_dir: tmp_dir} do
+      app_path = Path.join(tmp_dir, "app.livemd")
+
+      File.write!(app_path, """
+      <!-- livebook:{"app_settings":{"slug":"app"}} -->
+
+      # App
+
+      ```elixir
+      """)
+
+      Livebook.Apps.subscribe()
+
+      Livebook.Apps.deploy_apps_in_dir(tmp_dir)
+
+      assert_receive {:app_created, %{slug: "app", warnings: warnings} = app}
+
+      assert warnings == [
+               "Import: line 5 - fenced Code Block opened with ``` not closed at end of input"
+             ]
+
+      Livebook.App.close(app.pid)
+    end
   end
 end

test/livebook/live_markdown/import_test.exs

@@ -241,7 +241,7 @@ defmodule Livebook.LiveMarkdown.ImportTest do
              ]
            } = notebook
 
-    assert ["Downgrading all headings, because 2 instances of heading 1 were found"] == messages
+    assert ["downgrading all headings, because 2 instances of heading 1 were found"] == messages
   end
 
   test "preserves markdown modifiers in notebok/section names" do
@@ -355,7 +355,7 @@ defmodule Livebook.LiveMarkdown.ImportTest do
              ]
            } = notebook
 
-    assert ["Moving heading 1 to the top of the notebook"] == messages
+    assert ["moving heading 1 to the top of the notebook"] == messages
   end
 
   test "includes parsing warnings in the returned message list" do
@@ -369,7 +369,7 @@ defmodule Livebook.LiveMarkdown.ImportTest do
 
     {_notebook, messages} = Import.notebook_from_livemd(markdown)
 
-    assert ["Line 3: Closing unclosed backquotes ` at end of input"] == messages
+    assert ["line 3 - closing unclosed backquotes ` at end of input"] == messages
   end
 
   test "imports non-elixir code snippets as part of markdown cells" do

test/livebook_web/live/open_live_test.exs

@@ -153,7 +153,7 @@ defmodule LivebookWeb.OpenLiveTest do
       {path, flash} = assert_redirect(view, 5000)
 
       assert flash["warning"] =~
-               "We found problems while importing the file and tried to autofix them:\n- Downgrading all headings, because 3 instances of heading 1 were found"
+               "We found problems while importing the file and tried to autofix them:\n- downgrading all headings, because 3 instances of heading 1 were found"
 
       close_session_by_path(path)
     end