From bf17e0046344b0648e673fb16d3deb4a30c692ed Mon Sep 17 00:00:00 2001
From: Roman Priesol <roman@priesol.net>
Date: Mon, 7 Sep 2026 23:31:45 +0200
Subject: [PATCH] Hide write-action controls in the tree UI when logged out

Delete (file and folder), download, upload, and new-folder were all
rendered unconditionally and relied on the backend's 401 + a redirect
to /login on click -- fine for surfacing the reason something failed,
but showing controls that plainly won't do anything until you log in
was the actual complaint. isLoggedIn() checks for the topbar's own
"Log out" link (only rendered when logged_in) instead of threading a
new flag through -- already there, and login state can't change
without a full page reload anyway.

Caught a real gap while verifying in a real browser: the folder row's
own delete button wasn't gated (only the file row's was), so a
"logged out" check still showed a delete icon on every folder. Fixed
before it shipped -- confirmed clean in both states afterward.
---
 docs/architecture.adoc |  9 +++++++
 public/js/gpx.js       | 58 +++++++++++++++++++++++++++++-------------
 2 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/docs/architecture.adoc b/docs/architecture.adoc
index 27f33f6..8205d8f 100644
--- a/docs/architecture.adoc
+++ b/docs/architecture.adoc
@@ -691,6 +691,15 @@ require login, guarded by `require_login!` in `app.rb` — and so does
 exporting a file (`GET /api/gpx/file/*?download=...`), even though
 plain viewing of that same URL (no `?download`) doesn't; not a write
 exactly, but treated the same as the others rather than left open.
+The tree UI (`public/js/gpx.js`) mirrors this: every control for a
+gated action (delete, download, upload, new folder) is left out of
+the DOM entirely when logged out rather than shown and failing on
+click — `isLoggedIn()` checks for the topbar's own "Log out" link
+(`views/index.erb` only renders one when `logged_in`) instead of
+threading a separate flag through, since that's already there and
+login state can't change without a full page reload anyway. Delete
+exists on both file and folder rows — both are gated, easy to miss
+one and only catch it by testing an actually-logged-out session.
 
 Login delegates to the Gitea instance at `git.priesol.net` via OAuth2
 (`GET /login` redirects to Gitea's authorize endpoint, `GET
diff --git a/public/js/gpx.js b/public/js/gpx.js
index e443fc5..eaf8744 100644
--- a/public/js/gpx.js
+++ b/public/js/gpx.js
@@ -31,6 +31,17 @@ const BADGE_IMAGE_PREFIX = "badge-"; // one registered image per distinct badge
 // one — both land on the same drop targets, told apart by this.
 const MOVE_MIME = "application/x-gpx-path";
 
+// No server-rendered flag for this exists yet, and adding one would be more
+// plumbing than reusing what's already on the page: the topbar's own
+// logged_in check (views/index.erb) already renders a "Log out" link only
+// when logged in. Write-action controls (delete/download/upload/new
+// folder) check this to hide themselves entirely rather than being shown
+// and failing on click — login state never changes without a full page
+// reload, so this only needs checking once per render, not reactively.
+function isLoggedIn() {
+  return !!document.querySelector('a[href="/logout"]');
+}
+
 // A sidebar icon that opens a side panel for uploading GPX files (via
 // drag & drop, stored server-side in a folder hierarchy the user can add to
 // and reorganize by dragging), and toggling their tracks/waypoints on and
@@ -109,6 +120,10 @@ export class GpxPanelControl {
     this._uploadStatus = this._panel.querySelector(".gpx-upload__status");
     this._panel.querySelector('[data-action="new-folder"]').innerHTML = NEW_FOLDER_ICON;
     this._panel.querySelector('[data-action="upload"]').innerHTML = UPLOAD_ICON;
+    if (!isLoggedIn()) {
+      this._panel.querySelector('[data-action="new-folder"]').hidden = true;
+      this._panel.querySelector('[data-action="upload"]').hidden = true;
+    }
     this._panel
       .querySelector(".gpx-panel__close")
       .addEventListener("click", () => this._togglePanel(false));
@@ -249,7 +264,7 @@ export class GpxPanelControl {
     const name = document.createElement("span");
     name.textContent = folder.name;
     row.appendChild(name);
-    row.appendChild(this._deleteButton(() => this._deleteFolder(folder.path)));
+    if (isLoggedIn()) row.appendChild(this._deleteButton(() => this._deleteFolder(folder.path)));
     summary.appendChild(row);
     this._wireFolderDropTarget(summary, folder.path);
 
@@ -277,34 +292,43 @@ export class GpxPanelControl {
     // name reflects that (matching the real "<path>.geojson" cache file on
     // disk) rather than still reading as a plain, un-converted .gpx/.fit.
     // A direct .geojson upload already ends in .geojson, so it's left
-    // alone. It's an <a>, not a <span>: clicking it downloads that GeoJSON
-    // — the only way to get it out of the app, since the original .gpx/.fit
+    // alone.
+    const displayName = file.cached && !file.name.toLowerCase().endsWith(".geojson") ? `${file.name}.geojson` : file.name;
+    const loggedIn = isLoggedIn();
+    // Logged in: an <a>, not a <span> — clicking it downloads that GeoJSON,
+    // the only way to get it out of the app, since the original .gpx/.fit
     // is never deleted but also never re-shown once converted. An <a>
     // nested in a <label> doesn't also toggle the checkbox on click (unlike
     // a plain element), so this doesn't fight the row's own
     // toggle-on-map behavior. href kept pointing at the real endpoint (so
     // hover/right-click/middle-click still do something sensible) but the
-    // actual click goes through _download, which — like every other write
-    // action in this file — needs to redirect to /login on a 401 rather
-    // than navigate to a raw JSON error page (this endpoint now requires
-    // login the same as upload/move/delete, see app.rb).
-    const displayName = file.cached && !file.name.toLowerCase().endsWith(".geojson") ? `${file.name}.geojson` : file.name;
-    const name = document.createElement("a");
+    // actual click goes through _download, which redirects to /login on a
+    // 401 rather than navigating to a raw JSON error page (this endpoint
+    // requires login the same as upload/move/delete, see app.rb).
+    // Logged out: plain text — nothing here would actually work, so it
+    // isn't shown as if it would (same reasoning as the buttons below).
+    const name = document.createElement(loggedIn ? "a" : "span");
     name.className = "gpx-tree__file-name";
-    name.href = `${gpxFileUrl(file.path)}?download=geojson`;
     name.textContent = displayName;
-    name.title = `Download ${displayName}`;
-    name.addEventListener("click", (event) => {
-      event.preventDefault();
-      this._download(name.href, displayName);
-    });
+    if (loggedIn) {
+      name.href = `${gpxFileUrl(file.path)}?download=geojson`;
+      name.title = `Download ${displayName}`;
+      name.addEventListener("click", (event) => {
+        event.preventDefault();
+        this._download(name.href, displayName);
+      });
+    }
     label.appendChild(checkbox);
     (file.kinds || []).forEach((kind) => label.appendChild(this._kindIcon(kind)));
     if (!file.cached) label.appendChild(this._pendingIcon());
     label.appendChild(name);
     row.appendChild(label);
-    row.appendChild(this._downloadButton(file));
-    row.appendChild(this._deleteButton(() => this._deleteFile(file.path)));
+    // Delete/download require login (app.rb); shown only once logged in
+    // rather than shown-but-failing-on-click.
+    if (loggedIn) {
+      row.appendChild(this._downloadButton(file));
+      row.appendChild(this._deleteButton(() => this._deleteFile(file.path)));
+    }
 
     return row;
   }