neiam
/diogramos
diogramos
public · Issues · Pulls · Labels · Forks · Compare · Actions success · Badges · Packages
⭐
Log in to mark this repository.
Manage folders, and stop the folder list vanishing when you filter
5ff2909 · Gabriel Morell · 2026-09-16 04:07
Message
{commit_body(@commit)}
Files changed
modified
lib/diogramos/diagrams/folders.ex
+18
−1
@@ -70,9 +70,26 @@ defmodule Diogramos.Diagrams.Folders do
| 70 | 70 | |
| 71 | 71 | @spec delete(Scope.t(), Folder.t()) :: |
| 72 | 72 | {:ok, Folder.t()} | {:error, Ecto.Changeset.t() | :forbidden} |
| 73 | + @doc """ | |
| 74 | + Deletes a folder without taking anything else with it. | |
| 75 | + | |
| 76 | + Canvases inside fall back to "all canvases" -- `canvases.folder_id` is | |
| 77 | + `nilify_all`, so the database does that part. Child folders are moved | |
| 78 | + up to this folder's own parent first: `folders.parent_id` cascades, so | |
| 79 | + without this, deleting a folder silently destroys every folder nested | |
| 80 | + under it, which is not what "delete this folder" means to the person | |
| 81 | + clicking it. | |
| 82 | + """ | |
| 73 | 83 | def delete(%Scope{} = scope, %Folder{} = folder) do |
| 74 | 84 | with :ok <- enforce(scope, :admin, folder) do |
| 75 | − Repo.delete(folder) | |
| 85 | + Repo.transact(fn -> | |
| 86 | + from(f in Folder, where: f.parent_id == ^folder.id) | |
| 87 | + |> Repo.update_all( | |
| 88 | + set: [parent_id: folder.parent_id, updated_at: DateTime.utc_now(:second)] | |
| 89 | + ) | |
| 90 | + | |
| 91 | + Repo.delete(folder) | |
| 92 | + end) | |
| 76 | 93 | end |
| 77 | 94 | end |
| 78 | 95 |
modified
lib/diogramos_web/live/canvas_live/index.ex
+158
−2
@@ -22,6 +22,8 @@ defmodule DiogramosWeb.CanvasLive.Index do
| 22 | 22 | |> assign(:manage_grants, []) |
| 23 | 23 | |> assign(:manage_error, nil) |
| 24 | 24 | |> assign(:delete_confirm, "") |
| 25 | + |> assign(:manage_folder, nil) | |
| 26 | + |> assign(:folder_error, nil) | |
| 25 | 27 | |> allow_upload(:canvas_import, |
| 26 | 28 | accept: ~w(.json application/json), |
| 27 | 29 | max_entries: 1, |
@@ -254,6 +256,79 @@ defmodule DiogramosWeb.CanvasLive.Index do
| 254 | 256 | end |
| 255 | 257 | end |
| 256 | 258 | |
| 259 | + def handle_event("open_folder_manage", %{"id" => id}, socket) do | |
| 260 | + scope = socket.assigns.current_scope | |
| 261 | + {folder_id, _} = Integer.parse(id) | |
| 262 | + folder = Diagrams.get_folder!(scope, folder_id) | |
| 263 | + | |
| 264 | + {:noreply, | |
| 265 | + socket | |
| 266 | + |> assign(:manage_folder, folder) | |
| 267 | + |> assign(:folder_error, nil)} | |
| 268 | + end | |
| 269 | + | |
| 270 | + def handle_event("close_folder_manage", _, socket) do | |
| 271 | + {:noreply, | |
| 272 | + socket | |
| 273 | + |> assign(:manage_folder, nil) | |
| 274 | + |> assign(:folder_error, nil)} | |
| 275 | + end | |
| 276 | + | |
| 277 | + def handle_event("rename_folder", %{"name" => name}, socket) do | |
| 278 | + folder = socket.assigns.manage_folder | |
| 279 | + scope = socket.assigns.current_scope | |
| 280 | + | |
| 281 | + case Diagrams.rename_folder(scope, folder, String.trim(name)) do | |
| 282 | + {:ok, renamed} -> | |
| 283 | + {:noreply, | |
| 284 | + socket | |
| 285 | + |> stream_insert(:folders, renamed) | |
| 286 | + |> assign(:manage_folder, nil) | |
| 287 | + |> assign(:folder_error, nil) | |
| 288 | + |> put_flash(:info, "Renamed to #{renamed.name}.")} | |
| 289 | + | |
| 290 | + {:error, %Ecto.Changeset{} = changeset} -> | |
| 291 | + {:noreply, assign(socket, :folder_error, changeset_message(changeset))} | |
| 292 | + | |
| 293 | + {:error, :forbidden} -> | |
| 294 | + {:noreply, assign(socket, :folder_error, "Only the owner can rename this folder.")} | |
| 295 | + end | |
| 296 | + end | |
| 297 | + | |
| 298 | + def handle_event("delete_folder", _params, socket) do | |
| 299 | + scope = socket.assigns.current_scope | |
| 300 | + | |
| 301 | + case socket.assigns.manage_folder do | |
| 302 | + nil -> | |
| 303 | + {:noreply, socket} | |
| 304 | + | |
| 305 | + folder -> | |
| 306 | + case Diagrams.delete_folder(scope, folder) do | |
| 307 | + {:ok, _deleted} -> | |
| 308 | + {:noreply, | |
| 309 | + socket | |
| 310 | + |> stream_delete(:folders, folder) | |
| 311 | + |> stream(:folders, Diagrams.list_folders(scope), reset: true) | |
| 312 | + |> assign(:manage_folder, nil) | |
| 313 | + |> assign(:folder_error, nil) | |
| 314 | + # The canvases that were inside now belong to no folder, and | |
| 315 | + # the filter they were being viewed under is gone with it. | |
| 316 | + |> assign(:selected_folder_id, nil) | |
| 317 | + |> stream(:canvases, Diagrams.list_canvases(scope), reset: true) | |
| 318 | + |> put_flash( | |
| 319 | + :info, | |
| 320 | + "Deleted #{folder.name}. Its canvases are in All canvases." | |
| 321 | + )} | |
| 322 | + | |
| 323 | + {:error, :forbidden} -> | |
| 324 | + {:noreply, assign(socket, :folder_error, "Only the owner can delete this folder.")} | |
| 325 | + | |
| 326 | + {:error, _reason} -> | |
| 327 | + {:noreply, assign(socket, :folder_error, "Couldn't delete that folder.")} | |
| 328 | + end | |
| 329 | + end | |
| 330 | + end | |
| 331 | + | |
| 257 | 332 | def handle_event("create_folder", %{"folder" => params}, socket) do |
| 258 | 333 | scope = socket.assigns.current_scope |
| 259 | 334 |
@@ -318,16 +393,37 @@ defmodule DiogramosWeb.CanvasLive.Index do
| 318 | 393 | <.icon name="hero-inbox-micro" class="size-4" /> All canvases |
| 319 | 394 | </button> |
| 320 | 395 | </li> |
| 321 | − <li :for={{dom_id, folder} <- @streams.folders} id={dom_id}> | |
| 396 | + </ul> | |
| 397 | + | |
| 398 | + <%!-- Its own list: a phx-update="stream" container may hold | |
| 399 | + nothing but stream items, and without the attribute the | |
| 400 | + comprehension renders only what is in the stream buffer | |
| 401 | + -- which is empty after the first diff, so the folders | |
| 402 | + vanished on the next update until a reload. --%> | |
| 403 | + <ul class="menu menu-sm w-full p-0" id="folder-list" phx-update="stream"> | |
| 404 | + <li | |
| 405 | + :for={{dom_id, folder} <- @streams.folders} | |
| 406 | + id={dom_id} | |
| 407 | + class="flex flex-row items-center" | |
| 408 | + > | |
| 322 | 409 | <button |
| 323 | 410 | type="button" |
| 324 | 411 | phx-click="filter_folder" |
| 325 | 412 | phx-value-folder_id={folder.id} |
| 326 | − class={[@selected_folder_id == folder.id && "menu-active"]} | |
| 413 | + class={["flex-1 min-w-0", @selected_folder_id == folder.id && "menu-active"]} | |
| 327 | 414 | > |
| 328 | 415 | <.icon name="hero-folder-micro" class="size-4" /> |
| 329 | 416 | <span class="truncate">{folder.name}</span> |
| 330 | 417 | </button> |
| 418 | + <button | |
| 419 | + type="button" | |
| 420 | + phx-click="open_folder_manage" | |
| 421 | + phx-value-id={folder.id} | |
| 422 | + class="btn btn-ghost btn-xs" | |
| 423 | + aria-label={"Manage #{folder.name}"} | |
| 424 | + > | |
| 425 | + <.icon name="hero-cog-6-tooth-micro" class="size-4" /> | |
| 426 | + </button> | |
| 331 | 427 | </li> |
| 332 | 428 | </ul> |
| 333 | 429 | </aside> |
@@ -515,6 +611,59 @@ defmodule DiogramosWeb.CanvasLive.Index do
| 515 | 611 | </div> |
| 516 | 612 | <% end %> |
| 517 | 613 | |
| 614 | + <%= if @manage_folder do %> | |
| 615 | + <div | |
| 616 | + class="modal modal-open" | |
| 617 | + id="manage-folder-modal" | |
| 618 | + phx-window-keydown="close_folder_manage" | |
| 619 | + phx-key="escape" | |
| 620 | + > | |
| 621 | + <div class="modal-box max-w-md"> | |
| 622 | + <h3 class="font-bold text-lg mb-1">Manage folder</h3> | |
| 623 | + <p class="text-sm opacity-70 mb-4">{@manage_folder.name}</p> | |
| 624 | + | |
| 625 | + <form phx-submit="rename_folder" id="folder-rename-form" class="flex flex-col gap-2"> | |
| 626 | + <label class="font-mono text-xs uppercase tracking-wider text-accent">Name</label> | |
| 627 | + <div class="flex gap-2"> | |
| 628 | + <input | |
| 629 | + type="text" | |
| 630 | + name="name" | |
| 631 | + value={@manage_folder.name} | |
| 632 | + class="input input-bordered input-sm flex-1" | |
| 633 | + required | |
| 634 | + /> | |
| 635 | + <button type="submit" class="btn btn-primary btn-sm">Rename</button> | |
| 636 | + </div> | |
| 637 | + <p :if={@folder_error} class="text-error text-xs">{@folder_error}</p> | |
| 638 | + </form> | |
| 639 | + | |
| 640 | + <div class="divider mt-6 mb-2" /> | |
| 641 | + | |
| 642 | + <h4 class="font-mono text-xs uppercase tracking-wider text-error mb-2"> | |
| 643 | + Danger zone | |
| 644 | + </h4> | |
| 645 | + <p class="text-sm opacity-80 mb-2"> | |
| 646 | + Deleting this folder keeps everything in it: its canvases move to | |
| 647 | + All canvases, and any folders nested inside move up a level. | |
| 648 | + </p> | |
| 649 | + <button | |
| 650 | + type="button" | |
| 651 | + phx-click="delete_folder" | |
| 652 | + data-confirm={"Delete #{@manage_folder.name}? Its canvases move to All canvases."} | |
| 653 | + class="btn btn-error btn-sm" | |
| 654 | + > | |
| 655 | + Delete folder | |
| 656 | + </button> | |
| 657 | + | |
| 658 | + <div class="modal-action"> | |
| 659 | + <button type="button" phx-click="close_folder_manage" class="btn btn-ghost"> | |
| 660 | + Done | |
| 661 | + </button> | |
| 662 | + </div> | |
| 663 | + </div> | |
| 664 | + </div> | |
| 665 | + <% end %> | |
| 666 | + | |
| 518 | 667 | <%= if @manage_canvas do %> |
| 519 | 668 | <div |
| 520 | 669 | class="modal modal-open" |
@@ -687,6 +836,13 @@ defmodule DiogramosWeb.CanvasLive.Index do
| 687 | 836 | end |
| 688 | 837 | end |
| 689 | 838 | |
| 839 | + defp changeset_message(changeset) do | |
| 840 | + changeset | |
| 841 | + |> Ecto.Changeset.traverse_errors(fn {msg, _opts} -> msg end) | |
| 842 | + |> Enum.map(fn {field, messages} -> "#{field} #{Enum.join(messages, ", ")}" end) | |
| 843 | + |> Enum.join("; ") | |
| 844 | + end | |
| 845 | + | |
| 690 | 846 | defp error_to_string(:too_large), do: "File is too large." |
| 691 | 847 | defp error_to_string(:not_accepted), do: "File must be JSON." |
| 692 | 848 | defp error_to_string(:too_many_files), do: "Only one file at a time." |
modified
test/diogramos/diagrams_test.exs
+22
−2
@@ -239,12 +239,32 @@ defmodule Diogramos.DiagramsTest do
| 239 | 239 | assert {:error, :cycle} = Diagrams.move_folder(a, parent, child.id) |
| 240 | 240 | end |
| 241 | 241 | |
| 242 | − test "delete_folder/2 cascades to children via FK", %{scope: a} do | |
| 242 | + test "delete_folder/2 moves children up rather than cascading", %{scope: a} do | |
| 243 | 243 | parent = folder_fixture(a, %{name: "parent"}) |
| 244 | 244 | {:ok, child} = Diagrams.create_folder(a, %{name: "child", parent_id: parent.id}) |
| 245 | 245 | |
| 246 | 246 | assert {:ok, _} = Diagrams.delete_folder(a, parent) |
| 247 | − assert_raise Ecto.NoResultsError, fn -> Diagrams.get_folder!(a, child.id) end | |
| 247 | + | |
| 248 | + # This used to assert the FK cascade, which deleted every folder | |
| 249 | + # nested under the one being removed. That is not what "delete this | |
| 250 | + # folder" means to the person clicking it, and it destroyed folders | |
| 251 | + # they never named -- so delete/2 re-parents them first. | |
| 252 | + assert Diagrams.get_folder!(a, child.id).parent_id == nil | |
| 253 | + end | |
| 254 | + | |
| 255 | + test "delete_folder/2 re-parents children to the deleted folder's own parent", | |
| 256 | + %{scope: a} do | |
| 257 | + grandparent = folder_fixture(a, %{name: "grandparent"}) | |
| 258 | + | |
| 259 | + {:ok, parent} = | |
| 260 | + Diagrams.create_folder(a, %{name: "parent", parent_id: grandparent.id}) | |
| 261 | + | |
| 262 | + {:ok, child} = Diagrams.create_folder(a, %{name: "child", parent_id: parent.id}) | |
| 263 | + | |
| 264 | + assert {:ok, _} = Diagrams.delete_folder(a, parent) | |
| 265 | + | |
| 266 | + # Up one level, not all the way to the top. | |
| 267 | + assert Diagrams.get_folder!(a, child.id).parent_id == grandparent.id | |
| 248 | 268 | end |
| 249 | 269 | |
| 250 | 270 | test "effective_role returns owner for the owner and nil for outsiders", %{ |
added
test/diogramos_web/live/folder_manage_test.exs
+153
−0
@@ -0,0 +1,153 @@
| 1 | +defmodule DiogramosWeb.FolderManageTest do | |
| 2 | + use DiogramosWeb.ConnCase, async: true | |
| 3 | + | |
| 4 | + import Phoenix.LiveViewTest | |
| 5 | + import Diogramos.AccountsFixtures | |
| 6 | + import Diogramos.DiagramsFixtures | |
| 7 | + | |
| 8 | + alias Diogramos.Accounts.Scope | |
| 9 | + alias Diogramos.Diagrams | |
| 10 | + | |
| 11 | + setup %{conn: conn} do | |
| 12 | + owner = user_fixture() | |
| 13 | + scope = Scope.for_user(owner) | |
| 14 | + | |
| 15 | + %{conn: log_in_user(conn, owner), owner: owner, scope: scope} | |
| 16 | + end | |
| 17 | + | |
| 18 | + describe "the folder list" do | |
| 19 | + test "survives filtering, which used to wipe it until a reload", %{ | |
| 20 | + conn: conn, | |
| 21 | + scope: scope | |
| 22 | + } do | |
| 23 | + keep = folder_fixture(scope, %{name: "Keep"}) | |
| 24 | + other = folder_fixture(scope, %{name: "Other"}) | |
| 25 | + | |
| 26 | + {:ok, lv, html} = live(conn, ~p"/canvases") | |
| 27 | + assert html =~ "Keep" | |
| 28 | + assert html =~ "Other" | |
| 29 | + | |
| 30 | + # Filtering re-streams the canvases; the folders must not go with | |
| 31 | + # them. | |
| 32 | + html = lv |> element("button[phx-value-folder_id='#{keep.id}']") |> render_click() | |
| 33 | + assert html =~ "Keep" | |
| 34 | + assert html =~ "Other" | |
| 35 | + | |
| 36 | + html = lv |> element("button[phx-value-folder_id='all']") |> render_click() | |
| 37 | + assert html =~ "Keep" | |
| 38 | + assert html =~ "Other" | |
| 39 | + assert other.id | |
| 40 | + end | |
| 41 | + end | |
| 42 | + | |
| 43 | + describe "renaming" do | |
| 44 | + test "renames from the manage modal", %{conn: conn, scope: scope} do | |
| 45 | + folder = folder_fixture(scope, %{name: "Before"}) | |
| 46 | + | |
| 47 | + {:ok, lv, _html} = live(conn, ~p"/canvases") | |
| 48 | + lv |> element("button[phx-value-id='#{folder.id}']") |> render_click() | |
| 49 | + | |
| 50 | + html = | |
| 51 | + lv |> form("#folder-rename-form", %{"name" => "After"}) |> render_submit() | |
| 52 | + | |
| 53 | + assert html =~ "Renamed to After." | |
| 54 | + assert html =~ "After" | |
| 55 | + assert Diagrams.get_folder!(scope, folder.id).name == "After" | |
| 56 | + end | |
| 57 | + | |
| 58 | + test "reports a rejected name instead of swallowing it", %{conn: conn, scope: scope} do | |
| 59 | + folder = folder_fixture(scope, %{name: "Before"}) | |
| 60 | + | |
| 61 | + {:ok, lv, _html} = live(conn, ~p"/canvases") | |
| 62 | + lv |> element("button[phx-value-id='#{folder.id}']") |> render_click() | |
| 63 | + | |
| 64 | + html = lv |> form("#folder-rename-form", %{"name" => ""}) |> render_submit() | |
| 65 | + | |
| 66 | + assert html =~ "can't be blank" or html =~ "can't be blank" | |
| 67 | + assert Diagrams.get_folder!(scope, folder.id).name == "Before" | |
| 68 | + end | |
| 69 | + end | |
| 70 | + | |
| 71 | + describe "deleting" do | |
| 72 | + test "empties its canvases into all canvases rather than deleting them", %{ | |
| 73 | + conn: conn, | |
| 74 | + scope: scope | |
| 75 | + } do | |
| 76 | + folder = folder_fixture(scope, %{name: "Doomed"}) | |
| 77 | + canvas = canvas_fixture(scope, %{title: "Survivor", folder_id: folder.id}) | |
| 78 | + | |
| 79 | + {:ok, lv, _html} = live(conn, ~p"/canvases") | |
| 80 | + lv |> element("button[phx-value-id='#{folder.id}']") |> render_click() | |
| 81 | + | |
| 82 | + html = lv |> element("button", "Delete folder") |> render_click() | |
| 83 | + | |
| 84 | + assert html =~ "Its canvases are in All canvases." | |
| 85 | + # The flash names the folder, so check the list, not the page. | |
| 86 | + refute has_element?(lv, "button[phx-value-folder_id='#{folder.id}']") | |
| 87 | + | |
| 88 | + kept = Diagrams.get_canvas_by_slug(scope, canvas.slug) | |
| 89 | + assert kept, "the canvas must outlive its folder" | |
| 90 | + assert kept.folder_id == nil | |
| 91 | + assert html =~ "Survivor" | |
| 92 | + end | |
| 93 | + | |
| 94 | + test "moves nested folders up instead of destroying them", %{conn: conn, scope: scope} do | |
| 95 | + parent = folder_fixture(scope, %{name: "Parent"}) | |
| 96 | + {:ok, child} = Diagrams.create_folder(scope, %{name: "Child", parent_id: parent.id}) | |
| 97 | + | |
| 98 | + {:ok, lv, _html} = live(conn, ~p"/canvases") | |
| 99 | + lv |> element("button[phx-value-id='#{parent.id}']") |> render_click() | |
| 100 | + | |
| 101 | + html = lv |> element("button", "Delete folder") |> render_click() | |
| 102 | + | |
| 103 | + # folders.parent_id cascades in the database, so without the | |
| 104 | + # re-parent this child would be gone too. | |
| 105 | + assert html =~ "Child" | |
| 106 | + assert Diagrams.get_folder!(scope, child.id).parent_id == nil | |
| 107 | + end | |
| 108 | + | |
| 109 | + test "a canvas in a nested folder survives its grandparent's delete", %{ | |
| 110 | + conn: conn, | |
| 111 | + scope: scope | |
| 112 | + } do | |
| 113 | + parent = folder_fixture(scope, %{name: "Parent"}) | |
| 114 | + {:ok, child} = Diagrams.create_folder(scope, %{name: "Child", parent_id: parent.id}) | |
| 115 | + canvas = canvas_fixture(scope, %{title: "Deep", folder_id: child.id}) | |
| 116 | + | |
| 117 | + {:ok, lv, _html} = live(conn, ~p"/canvases") | |
| 118 | + lv |> element("button[phx-value-id='#{parent.id}']") |> render_click() | |
| 119 | + lv |> element("button", "Delete folder") |> render_click() | |
| 120 | + | |
| 121 | + kept = Diagrams.get_canvas_by_slug(scope, canvas.slug) | |
| 122 | + assert kept | |
| 123 | + # It stays where it was: only the deleted folder's own contents move. | |
| 124 | + assert kept.folder_id == child.id | |
| 125 | + end | |
| 126 | + | |
| 127 | + test "clears the filter when the folder being viewed is deleted", %{ | |
| 128 | + conn: conn, | |
| 129 | + scope: scope | |
| 130 | + } do | |
| 131 | + folder = folder_fixture(scope, %{name: "Viewing"}) | |
| 132 | + _elsewhere = canvas_fixture(scope, %{title: "Elsewhere"}) | |
| 133 | + | |
| 134 | + {:ok, lv, _html} = live(conn, ~p"/canvases") | |
| 135 | + lv |> element("button[phx-value-folder_id='#{folder.id}']") |> render_click() | |
| 136 | + lv |> element("button[phx-value-id='#{folder.id}']") |> render_click() | |
| 137 | + | |
| 138 | + html = lv |> element("button", "Delete folder") |> render_click() | |
| 139 | + | |
| 140 | + # Otherwise the list stays filtered by a folder that no longer | |
| 141 | + # exists, and looks empty for no visible reason. | |
| 142 | + assert html =~ "Elsewhere" | |
| 143 | + end | |
| 144 | + | |
| 145 | + test "someone else's folder is refused by the context", %{scope: scope} do | |
| 146 | + folder = folder_fixture(scope, %{name: "Mine"}) | |
| 147 | + stranger = user_fixture() | |
| 148 | + | |
| 149 | + assert {:error, :forbidden} = | |
| 150 | + Diagrams.delete_folder(Scope.for_user(stranger), folder) | |
| 151 | + end | |
| 152 | + end | |
| 153 | +end |
Parents: 5602f02