Improve section management (#411)

* Allow inserting section anywhere in the notebook

* Improve section deletion

* Polishing

* Remove section insertion shortcuts

assets/css/components.css

@@ -95,6 +95,10 @@
     @apply relative inline-block w-14 h-7 mr-2 select-none transition;
   }
 
+  .switch-button--disabled {
+    @apply pointer-events-none opacity-50;
+  }
+
   .switch-button__checkbox {
     @apply appearance-none absolute block w-7 h-7 rounded-full bg-gray-400 border-[5px] border-gray-100 cursor-pointer transition-all duration-300;
   }

assets/js/session/index.js

@@ -316,8 +316,6 @@ function handleDocumentKeyDown(hook, event) {
       insertCellBelowFocused(hook, "markdown");
     } else if (keyBuffer.tryMatch(["M"])) {
       insertCellAboveFocused(hook, "markdown");
-    } else if (keyBuffer.tryMatch(["S"])) {
-      addSection(hook);
     }
   }
 }
@@ -642,10 +640,6 @@ function insertCellAboveFocused(hook, type) {
   }
 }
 
-function addSection(hook) {
-  hook.pushEvent("add_section", {});
-}
-
 function insertFirstCell(hook, type) {
   const sectionIds = getSectionIds();
 

lib/livebook/notebook.ex

@@ -94,6 +94,26 @@ defmodule Livebook.Notebook do
     %{notebook | sections: sections}
   end
 
+  @doc """
+  Inserts `section` below the parent section.
+
+  Cells below the given index are moved to the newly inserted section.
+  """
+  @spec insert_section_into(t(), Section.id(), non_neg_integer(), Section.t()) :: t()
+  def insert_section_into(notebook, section_id, index, section) do
+    {sections_above, [parent_section | sections_below]} =
+      Enum.split_while(notebook.sections, &(&1.id != section_id))
+
+    {cells_above, cells_below} = Enum.split(parent_section.cells, index)
+
+    sections =
+      sections_above ++
+        [%{parent_section | cells: cells_above}, %{section | cells: cells_below}] ++
+        sections_below
+
+    %{notebook | sections: sections}
+  end
+
   @doc """
   Inserts `cell` at the given `index` within section identified by `section_id`.
   """
@@ -106,11 +126,24 @@ defmodule Livebook.Notebook do
 
   @doc """
   Deletes section with the given id.
+
+  All cells are moved to the previous section if present.
   """
   @spec delete_section(t(), Section.id()) :: t()
   def delete_section(notebook, section_id) do
-    {_, notebook} = pop_in(notebook, [Access.key(:sections), access_by_id(section_id)])
-    notebook
+    sections =
+      case Enum.split_while(notebook.sections, &(&1.id != section_id)) do
+        {[], [_section | sections_below]} ->
+          sections_below
+
+        {sections_above, [section | sections_below]} ->
+          {prev_section, sections_above} = List.pop_at(sections_above, length(sections_above) - 1)
+
+          sections_above ++
+            [%{prev_section | cells: prev_section.cells ++ section.cells} | sections_below]
+      end
+
+    %{notebook | sections: sections}
   end
 
   @doc """

lib/livebook/session.ex

@@ -115,6 +115,14 @@ defmodule Livebook.Session do
     GenServer.cast(name(session_id), {:insert_section, self(), index})
   end
 
+  @doc """
+  Asynchronously sends section insertion request to the server.
+  """
+  @spec insert_section_into(id(), Section.id(), non_neg_integer()) :: :ok
+  def insert_section_into(session_id, section_id, index) do
+    GenServer.cast(name(session_id), {:insert_section_into, self(), section_id, index})
+  end
+
   @doc """
   Asynchronously sends cell insertion request to the server.
   """
@@ -127,9 +135,9 @@ defmodule Livebook.Session do
   @doc """
   Asynchronously sends section deletion request to the server.
   """
-  @spec delete_section(id(), Section.id()) :: :ok
-  def delete_section(session_id, section_id) do
-    GenServer.cast(name(session_id), {:delete_section, self(), section_id})
+  @spec delete_section(id(), Section.id(), boolean()) :: :ok
+  def delete_section(session_id, section_id, delete_cells) do
+    GenServer.cast(name(session_id), {:delete_section, self(), section_id, delete_cells})
   end
 
   @doc """
@@ -352,14 +360,20 @@ defmodule Livebook.Session do
     {:noreply, handle_operation(state, operation)}
   end
 
+  def handle_cast({:insert_section_into, client_pid, section_id, index}, state) do
+    # Include new id in the operation, so it's reproducible
+    operation = {:insert_section_into, client_pid, section_id, index, Utils.random_id()}
+    {:noreply, handle_operation(state, operation)}
+  end
+
   def handle_cast({:insert_cell, client_pid, section_id, index, type}, state) do
     # Include new id in the operation, so it's reproducible
     operation = {:insert_cell, client_pid, section_id, index, type, Utils.random_id()}
     {:noreply, handle_operation(state, operation)}
   end
 
-  def handle_cast({:delete_section, client_pid, section_id}, state) do
-    operation = {:delete_section, client_pid, section_id}
+  def handle_cast({:delete_section, client_pid, section_id, delete_cells}, state) do
+    operation = {:delete_section, client_pid, section_id, delete_cells}
     {:noreply, handle_operation(state, operation)}
   end
 

lib/livebook/session/data.ex

@@ -80,8 +80,9 @@ defmodule Livebook.Session.Data do
 
   @type operation ::
           {:insert_section, pid(), index(), Section.id()}
+          | {:insert_section_into, pid(), Section.id(), index(), Section.id()}
           | {:insert_cell, pid(), Section.id(), index(), Cell.type(), Cell.id()}
-          | {:delete_section, pid(), Section.id()}
+          | {:delete_section, pid(), Section.id(), delete_cells :: boolean()}
           | {:delete_cell, pid(), Cell.id()}
           | {:move_cell, pid(), Cell.id(), offset :: integer()}
           | {:move_section, pid(), Section.id(), offset :: integer()}
@@ -179,6 +180,18 @@ defmodule Livebook.Session.Data do
     |> wrap_ok()
   end
 
+  def apply_operation(data, {:insert_section_into, _client_pid, section_id, index, id}) do
+    with {:ok, _section} <- Notebook.fetch_section(data.notebook, section_id) do
+      section = %{Section.new() | id: id}
+
+      data
+      |> with_actions()
+      |> insert_section_into(section_id, index, section)
+      |> set_dirty()
+      |> wrap_ok()
+    end
+  end
+
   def apply_operation(data, {:insert_cell, _client_pid, section_id, index, type, id}) do
     with {:ok, _section} <- Notebook.fetch_section(data.notebook, section_id) do
       cell = %{Cell.new(type) | id: id}
@@ -191,39 +204,24 @@ defmodule Livebook.Session.Data do
     end
   end
 
-  def apply_operation(data, {:delete_section, _client_pid, id}) do
-    with {:ok, section} <- Notebook.fetch_section(data.notebook, id) do
+  def apply_operation(data, {:delete_section, _client_pid, id, delete_cells}) do
+    with {:ok, section} <- Notebook.fetch_section(data.notebook, id),
+         true <- section != hd(data.notebook.sections) or delete_cells do
       data
       |> with_actions()
-      |> delete_section(section)
+      |> delete_section(section, delete_cells)
       |> set_dirty()
       |> wrap_ok()
+    else
+      _ -> :error
     end
   end
 
   def apply_operation(data, {:delete_cell, _client_pid, id}) do
     with {:ok, cell, section} <- Notebook.fetch_cell_and_section(data.notebook, id) do
-      case data.cell_infos[cell.id].evaluation_status do
-        :evaluating ->
-          data
-          |> with_actions()
-          |> clear_evaluation()
-          |> add_action({:stop_evaluation, section})
-
-        :queued ->
-          data
-          |> with_actions()
-          |> unqueue_cell_evaluation(cell, section)
-          |> unqueue_dependent_cells_evaluation(cell)
-          |> mark_dependent_cells_as_stale(cell)
-
-        _ ->
-          data
-          |> with_actions()
-          |> mark_dependent_cells_as_stale(cell)
-      end
-      |> delete_cell(cell)
-      |> add_action({:forget_evaluation, cell, section})
+      data
+      |> with_actions()
+      |> delete_cell(cell, section)
       |> set_dirty()
       |> wrap_ok()
     end
@@ -332,26 +330,14 @@ defmodule Livebook.Session.Data do
   end
 
   def apply_operation(data, {:cancel_cell_evaluation, _client_pid, id}) do
-    with {:ok, cell, section} <- Notebook.fetch_cell_and_section(data.notebook, id) do
-      case data.cell_infos[cell.id].evaluation_status do
-        :evaluating ->
-          data
-          |> with_actions()
-          |> clear_evaluation()
-          |> add_action({:stop_evaluation, section})
-          |> wrap_ok()
-
-        :queued ->
-          data
-          |> with_actions()
-          |> unqueue_cell_evaluation(cell, section)
-          |> unqueue_dependent_cells_evaluation(cell)
-          |> mark_dependent_cells_as_stale(cell)
-          |> wrap_ok()
-
-        _ ->
-          :error
-      end
+    with {:ok, cell, section} <- Notebook.fetch_cell_and_section(data.notebook, id),
+         true <- data.cell_infos[cell.id].evaluation_status in [:evaluating, :queued] do
+      data
+      |> with_actions()
+      |> cancel_cell_evaluation(cell, section)
+      |> wrap_ok()
+    else
+      _ -> :error
     end
   end
 
@@ -491,6 +477,14 @@ defmodule Livebook.Session.Data do
     )
   end
 
+  defp insert_section_into({data, _} = data_actions, section_id, index, section) do
+    data_actions
+    |> set!(
+      notebook: Notebook.insert_section_into(data.notebook, section_id, index, section),
+      section_infos: Map.put(data.section_infos, section.id, new_section_info())
+    )
+  end
+
   defp insert_cell({data, _} = data_actions, section_id, index, cell) do
     data_actions
     |> set!(
@@ -499,18 +493,29 @@ defmodule Livebook.Session.Data do
     )
   end
 
-  defp delete_section({data, _} = data_actions, section) do
+  defp delete_section(data_actions, section, delete_cells) do
+    {data, _} =
+      data_actions =
+      if delete_cells do
+        data_actions
+        |> reduce(Enum.reverse(section.cells), &delete_cell(&1, &2, section))
+      else
+        data_actions
+      end
+
     data_actions
     |> set!(
       notebook: Notebook.delete_section(data.notebook, section.id),
       section_infos: Map.delete(data.section_infos, section.id),
-      deleted_sections: [section | data.deleted_sections]
+      deleted_sections: [%{section | cells: []} | data.deleted_sections]
     )
-    |> reduce(section.cells, &delete_cell_info/2)
   end
 
-  defp delete_cell({data, _} = data_actions, cell) do
+  defp delete_cell({data, _} = data_actions, cell, section) do
     data_actions
+    |> cancel_cell_evaluation(cell, section)
+    |> mark_dependent_cells_as_stale(cell)
+    |> add_action({:forget_evaluation, cell, section})
     |> set!(
       notebook: Notebook.delete_cell(data.notebook, cell.id),
       deleted_cells: [cell | data.deleted_cells]
@@ -630,6 +635,8 @@ defmodule Livebook.Session.Data do
     |> set_section_info!(section.id, evaluating_cell_id: nil)
   end
 
+  defp mark_dependent_cells_as_stale(data_actions, %Cell.Markdown{}), do: data_actions
+
   defp mark_dependent_cells_as_stale({data, _} = data_actions, cell) do
     dependent = dependent_cells_with_section(data, cell.id)
     mark_cells_as_stale(data_actions, dependent)
@@ -751,6 +758,24 @@ defmodule Livebook.Session.Data do
     end)
   end
 
+  defp cancel_cell_evaluation({data, _} = data_actions, cell, section) do
+    case data.cell_infos[cell.id].evaluation_status do
+      :evaluating ->
+        data_actions
+        |> clear_evaluation()
+        |> add_action({:stop_evaluation, section})
+
+      :queued ->
+        data_actions
+        |> unqueue_cell_evaluation(cell, section)
+        |> unqueue_dependent_cells_evaluation(cell)
+        |> mark_dependent_cells_as_stale(cell)
+
+      _ ->
+        data_actions
+    end
+  end
+
   defp unqueue_dependent_cells_evaluation({data, _} = data_actions, cell) do
     dependent = dependent_cells_with_section(data, cell.id)
     unqueue_cells_evaluation(data_actions, dependent)

lib/livebook_web/helpers.ex

@@ -135,13 +135,18 @@ defmodule LivebookWeb.Helpers do
   @doc """
   Renders a checkbox input styled as a switch.
   """
-  def render_switch(name, checked, label) do
-    assigns = %{name: name, checked: checked, label: label}
+  def render_switch(name, checked, label, opts \\ []) do
+    assigns = %{
+      name: name,
+      checked: checked,
+      label: label,
+      disabled: Keyword.get(opts, :disabled, false)
+    }
 
     ~L"""
     <div class="flex space-x-3 items-center justify-between">
       <span class="text-gray-700"><%= @label %></span>
-      <label class="switch-button">
+      <label class="switch-button <%= if(@disabled, do: "switch-button--disabled") %>">
         <%= tag :input, class: "switch-button__checkbox", type: "checkbox", name: @name, checked: @checked %>
         <div class="switch-button__bg"></div>
       </label>

lib/livebook_web/live/home_live/close_session_component.ex

@@ -16,8 +16,8 @@ defmodule LivebookWeb.HomeLive.CloseSessionComponent do
         This won't delete any persisted files.
       </p>
       <div class="mt-8 flex justify-end space-x-2">
-      <button class="button button-red" phx-click="close" phx-target="<%= @myself %>">
-        <%= remix_icon("close-circle-line", class: "align-middle mr-1") %>
+        <button class="button button-red" phx-click="close" phx-target="<%= @myself %>">
+          <%= remix_icon("close-circle-line", class: "align-middle mr-1") %>
           Close session
         </button>
         <%= live_patch "Cancel", to: @return_to, class: "button button-outlined-gray" %>

lib/livebook_web/live/session_live.ex

@@ -124,7 +124,7 @@ defmodule LivebookWeb.SessionLive do
               <% end %>
             </div>
             <button class="mt-8 p-8 py-1 text-gray-500 text-sm font-medium rounded-xl border border-gray-400 border-dashed hover:bg-gray-100 inline-flex items-center justify-center space-x-2"
-              phx-click="add_section" >
+              phx-click="append_section">
               <%= remix_icon("add-line", class: "text-lg align-center") %>
               <span>New section</span>
             </button>
@@ -211,9 +211,9 @@ defmodule LivebookWeb.SessionLive do
             <%= if @data_view.section_views == [] do %>
               <div class="flex justify-center">
                 <button class="button button-small"
-                  phx-click="insert_section"
-                  phx-value-index="0"
-                  >+ Section</button>
+                  phx-click="append_section">
+                  + Section
+                </button>
               </div>
             <% end %>
             <%= for {section_view, index} <- Enum.with_index(@data_view.section_views) do %>
@@ -287,6 +287,16 @@ defmodule LivebookWeb.SessionLive do
             uploads: @uploads,
             return_to: Routes.session_path(@socket, :page, @session_id) %>
     <% end %>
+
+    <%= if @live_action == :delete_section do %>
+      <%= live_modal LivebookWeb.SessionLive.DeleteSectionComponent,
+            id: :delete_section_modal,
+            modal_class: "w-full max-w-xl",
+            session_id: @session_id,
+            section: @section,
+            is_first: @section.id == @first_section_id,
+            return_to: Routes.session_path(@socket, :page, @session_id) %>
+    <% end %>
     """
   end
 
@@ -302,6 +312,12 @@ defmodule LivebookWeb.SessionLive do
     {:noreply, assign(socket, cell: cell)}
   end
 
+  def handle_params(%{"section_id" => section_id}, _url, socket) do
+    {:ok, section} = Notebook.fetch_section(socket.private.data.notebook, section_id)
+    first_section_id = hd(socket.private.data.notebook.sections).id
+    {:noreply, assign(socket, section: section, first_section_id: first_section_id)}
+  end
+
   def handle_params(_params, _url, socket) do
     {:noreply, socket}
   end
@@ -344,22 +360,16 @@ defmodule LivebookWeb.SessionLive do
     end
   end
 
-  def handle_event("add_section", _params, socket) do
-    end_index = length(socket.private.data.notebook.sections)
-    Session.insert_section(socket.assigns.session_id, end_index)
+  def handle_event("append_section", %{}, socket) do
+    idx = length(socket.private.data.notebook.sections)
+    Session.insert_section(socket.assigns.session_id, idx)
 
     {:noreply, socket}
   end
 
-  def handle_event("insert_section", %{"index" => index}, socket) do
+  def handle_event("insert_section_into", %{"section_id" => section_id, "index" => index}, socket) do
     index = ensure_integer(index) |> max(0)
-    Session.insert_section(socket.assigns.session_id, index)
-
-    {:noreply, socket}
-  end
-
-  def handle_event("delete_section", %{"section_id" => section_id}, socket) do
-    Session.delete_section(socket.assigns.session_id, section_id)
+    Session.insert_section_into(socket.assigns.session_id, section_id, index)
 
     {:noreply, socket}
   end
@@ -672,6 +682,18 @@ defmodule LivebookWeb.SessionLive do
     end
   end
 
+  defp after_operation(
+         socket,
+         _prev_socket,
+         {:insert_section_into, client_pid, _section_id, _index, section_id}
+       ) do
+    if client_pid == self() do
+      push_event(socket, "section_inserted", %{section_id: section_id})
+    else
+      socket
+    end
+  end
+
   defp after_operation(socket, _prev_socket, {:delete_section, _client_pid, section_id}) do
     push_event(socket, "section_deleted", %{section_id: section_id})
   end

lib/livebook_web/live/session_live/delete_section_component.ex

@@ -0,0 +1,45 @@
+defmodule LivebookWeb.SessionLive.DeleteSectionComponent do
+  use LivebookWeb, :live_component
+
+  @impl true
+  def render(assigns) do
+    ~L"""
+    <div class="p-6 pb-4 flex flex-col space-y-8">
+      <h3 class="text-2xl font-semibold text-gray-800">
+        Delete section
+      </h3>
+      <p class="text-gray-700">
+        Are you sure you want to delete this section -
+        <span class="font-semibold">“<%= @section.name %>”</span>?
+      </p>
+      <form phx-submit="delete" phx-target="<%= @myself %>">
+        <h3 class="mb-1 text-lg font-semibold text-gray-800">
+          Options
+        </h3>
+        <%# If there is no previous section, all cells need to be deleted %>
+        <%= render_switch("delete_cells", @is_first, "Delete all cells in this section", disabled: @is_first) %>
+        <div class="mt-8 flex justify-end space-x-2">
+          <button type="submit" class="button button-red">
+            <%= remix_icon("delete-bin-6-line", class: "align-middle mr-1") %>
+            Delete
+          </button>
+          <%= live_patch "Cancel", to: @return_to, class: "button button-outlined-gray" %>
+        </div>
+      </form>
+    </div>
+    """
+  end
+
+  @impl true
+  def handle_event("delete", params, socket) do
+    delete_cells? = Map.has_key?(params, "delete_cells")
+
+    Livebook.Session.delete_section(
+      socket.assigns.session_id,
+      socket.assigns.section.id,
+      delete_cells?
+    )
+
+    {:noreply, push_patch(socket, to: socket.assigns.return_to)}
+  end
+end

lib/livebook_web/live/session_live/insert_buttons_component.ex

@@ -23,12 +23,11 @@ defmodule LivebookWeb.SessionLive.InsertButtonsComponent do
           phx-value-section_id="<%= @section_id %>"
           phx-value-index="<%= @insert_cell_index %>"
           >+ Input</button>
-        <%= if @insert_section_index do %>
-          <button class="button button-small"
-            phx-click="insert_section"
-            phx-value-index="<%= @insert_section_index %>"
-            >+ Section</button>
-        <% end %>
+        <button class="button button-small"
+          phx-click="insert_section_into"
+          phx-value-section_id="<%= @section_id %>"
+          phx-value-index="<%= @insert_cell_index %>"
+          >+ Section</button>
       </div>
     </div>
     """

lib/livebook_web/live/session_live/section_component.ex

@@ -41,9 +41,10 @@ defmodule LivebookWeb.SessionLive.SectionComponent do
             </button>
           </span>
           <span class="tooltip top" aria-label="Delete">
-            <button class="icon-button" phx-click="delete_section" phx-value-section_id="<%= @section_view.id %>" tabindex="-1">
+            <%= live_patch to: Routes.session_path(@socket, :delete_section, @session_id, @section_view.id),
+                  class: "icon-button" do %>
               <%= remix_icon("delete-bin-6-line", class: "text-xl") %>
-            </button>
+            <% end %>
           </span>
         </div>
       </div>
@@ -54,8 +55,7 @@ defmodule LivebookWeb.SessionLive.SectionComponent do
                   id: "#{@section_view.id}:#{index}",
                   persistent: false,
                   section_id: @section_view.id,
-                  insert_cell_index: index,
-                  insert_section_index: nil %>
+                  insert_cell_index: index %>
             <%= live_component LivebookWeb.SessionLive.CellComponent,
                   id: cell_view.id,
                   session_id: @session_id,
@@ -65,8 +65,7 @@ defmodule LivebookWeb.SessionLive.SectionComponent do
                 id: "#{@section_view.id}:last",
                 persistent: @section_view.cell_views == [],
                 section_id: @section_view.id,
-                insert_cell_index: length(@section_view.cell_views),
-                insert_section_index: @index + 1 %>
+                insert_cell_index: length(@section_view.cell_views) %>
         </div>
       </div>
     </div>

lib/livebook_web/live/session_live/shortcuts_component.ex

@@ -24,7 +24,6 @@ defmodule LivebookWeb.SessionLive.ShortcutsComponent do
       %{seq: ["m"], desc: "Insert Markdown cell below"},
       %{seq: ["N"], desc: "Insert Elixir cell above"},
       %{seq: ["M"], desc: "Insert Markdown cell above"},
-      %{seq: ["S"], desc: "Add section"},
       %{seq: ["d", "d"], desc: "Delete cell"},
       %{seq: ["e", "e"], desc: "Evaluate cell"},
       %{seq: ["e", "s"], desc: "Evaluate section"},

lib/livebook_web/router.ex

@@ -33,6 +33,7 @@ defmodule LivebookWeb.Router do
     live "/sessions/:id/settings/file", SessionLive, :file_settings
     live "/sessions/:id/cell-settings/:cell_id", SessionLive, :cell_settings
     live "/sessions/:id/cell-upload/:cell_id", SessionLive, :cell_upload
+    live "/sessions/:id/delete-section/:section_id", SessionLive, :delete_section
     get "/sessions/:id/images/:image", SessionController, :show_image
 
     live_dashboard "/dashboard", metrics: LivebookWeb.Telemetry

test/livebook/session/data_test.exs

@@ -48,6 +48,49 @@ defmodule Livebook.Session.DataTest do
     end
   end
 
+  describe "apply_operation/2 given :insert_section_into" do
+    test "returns an error given invalid section id" do
+      data = Data.new()
+      operation = {:insert_section_into, self(), "nonexistent", 0, "s1"}
+      assert :error = Data.apply_operation(data, operation)
+    end
+
+    test "adds new section below the given one and section info" do
+      data =
+        data_after_operations!([
+          {:insert_section, self(), 0, "s1"}
+        ])
+
+      operation = {:insert_section_into, self(), "s1", 0, "s2"}
+
+      assert {:ok,
+              %{
+                notebook: %{
+                  sections: [%{id: "s1"}, %{id: "s2"}]
+                },
+                section_infos: %{"s2" => _}
+              }, []} = Data.apply_operation(data, operation)
+    end
+
+    test "moves cells below the given index to the newly inserted section" do
+      data =
+        data_after_operations!([
+          {:insert_section, self(), 0, "s1"},
+          {:insert_cell, self(), "s1", 0, :elixir, "c1"},
+          {:insert_cell, self(), "s1", 1, :elixir, "c2"}
+        ])
+
+      operation = {:insert_section_into, self(), "s1", 1, "s2"}
+
+      assert {:ok,
+              %{
+                notebook: %{
+                  sections: [%{cells: [%{id: "c1"}]}, %{cells: [%{id: "c2"}]}]
+                }
+              }, []} = Data.apply_operation(data, operation)
+    end
+  end
+
   describe "apply_operation/2 given :insert_cell" do
     test "returns an error given invalid section id" do
       data = Data.new()
@@ -95,7 +138,7 @@ defmodule Livebook.Session.DataTest do
   describe "apply_operation/2 given :delete_section" do
     test "returns an error given invalid section id" do
       data = Data.new()
-      operation = {:delete_section, self(), "nonexistent"}
+      operation = {:delete_section, self(), "nonexistent", true}
       assert :error = Data.apply_operation(data, operation)
     end
 
@@ -105,7 +148,7 @@ defmodule Livebook.Session.DataTest do
           {:insert_section, self(), 0, "s1"}
         ])
 
-      operation = {:delete_section, self(), "s1"}
+      operation = {:delete_section, self(), "s1", true}
       empty_map = %{}
 
       assert {:ok,
@@ -117,6 +160,82 @@ defmodule Livebook.Session.DataTest do
                 deleted_sections: [%{id: "s1"}]
               }, []} = Data.apply_operation(data, operation)
     end
+
+    test "returns error when cell deletion is disabled for the first cell" do
+      data =
+        data_after_operations!([
+          {:insert_section, self(), 0, "s1"},
+          {:insert_cell, self(), "s1", 0, :elixir, "c1"}
+        ])
+
+      operation = {:delete_section, self(), "s1", false}
+      assert :error = Data.apply_operation(data, operation)
+    end
+
+    test "keeps cells when cell deletion is disabled" do
+      data =
+        data_after_operations!([
+          {:insert_section, self(), 0, "s1"},
+          {:insert_cell, self(), "s1", 0, :elixir, "c1"},
+          {:insert_section, self(), 1, "s2"},
+          {:insert_cell, self(), "s2", 0, :elixir, "c2"}
+        ])
+
+      operation = {:delete_section, self(), "s2", false}
+
+      assert {:ok,
+              %{
+                notebook: %{
+                  sections: [%{id: "s1", cells: [%{id: "c1"}, %{id: "c2"}]}]
+                },
+                deleted_cells: []
+              }, []} = Data.apply_operation(data, operation)
+    end
+
+    test "deletes cells when cell deletion is enabled" do
+      data =
+        data_after_operations!([
+          {:insert_section, self(), 0, "s1"},
+          {:insert_cell, self(), "s1", 0, :elixir, "c1"},
+          {:insert_section, self(), 1, "s2"},
+          {:insert_cell, self(), "s2", 0, :elixir, "c2"}
+        ])
+
+      operation = {:delete_section, self(), "s2", true}
+
+      assert {:ok,
+              %{
+                notebook: %{
+                  sections: [%{id: "s1", cells: [%{id: "c1"}]}]
+                },
+                deleted_cells: [%{id: "c2"}]
+              },
+              [{:forget_evaluation, %{id: "c2"}, %{id: "s2"}}]} =
+               Data.apply_operation(data, operation)
+    end
+
+    test "marks evaluated child cells as stale when cells get deleted" do
+      data =
+        data_after_operations!([
+          {:insert_section, self(), 0, "s1"},
+          {:insert_cell, self(), "s1", 0, :elixir, "c1"},
+          {:insert_section, self(), 1, "s2"},
+          {:insert_cell, self(), "s2", 0, :elixir, "c2"},
+          # Evaluate both cells
+          {:set_runtime, self(), NoopRuntime.new()},
+          {:queue_cell_evaluation, self(), "c1"},
+          {:add_cell_evaluation_response, self(), "c1", @eval_resp, @eval_meta},
+          {:queue_cell_evaluation, self(), "c2"},
+          {:add_cell_evaluation_response, self(), "c2", @eval_resp, @eval_meta}
+        ])
+
+      operation = {:delete_section, self(), "s1", true}
+
+      assert {:ok,
+              %{
+                cell_infos: %{"c2" => %{validity_status: :stale}}
+              }, _actions} = Data.apply_operation(data, operation)
+    end
   end
 
   describe "apply_operation/2 given :delete_cell" do
@@ -207,6 +326,26 @@ defmodule Livebook.Session.DataTest do
               }, _actions} = Data.apply_operation(data, operation)
     end
 
+    test "deleting a markdown cell does not change child cell validity" do
+      data =
+        data_after_operations!([
+          {:insert_section, self(), 0, "s1"},
+          {:insert_cell, self(), "s1", 0, :markdown, "c1"},
+          {:insert_cell, self(), "s1", 1, :elixir, "c2"},
+          # Evaluate the elixir cell
+          {:set_runtime, self(), NoopRuntime.new()},
+          {:queue_cell_evaluation, self(), "c2"},
+          {:add_cell_evaluation_response, self(), "c2", @eval_resp, @eval_meta}
+        ])
+
+      operation = {:delete_cell, self(), "c1"}
+
+      assert {:ok,
+              %{
+                cell_infos: %{"c2" => %{validity_status: :evaluated}}
+              }, _actions} = Data.apply_operation(data, operation)
+    end
+
     test "returns forget evaluation action" do
       data =
         data_after_operations!([

test/livebook/session_test.exs

@@ -37,15 +37,15 @@ defmodule Livebook.SessionTest do
     end
   end
 
-  describe "delete_section/2" do
+  describe "delete_section/3" do
     test "sends a delete opreation to subscribers", %{session_id: session_id} do
       Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session_id}")
       pid = self()
 
       {section_id, _cell_id} = insert_section_and_cell(session_id)
 
-      Session.delete_section(session_id, section_id)
-      assert_receive {:operation, {:delete_section, ^pid, ^section_id}}
+      Session.delete_section(session_id, section_id, false)
+      assert_receive {:operation, {:delete_section, ^pid, ^section_id, false}}
     end
   end