Merge pull request #577 from EdouardVanbelle/feat/users-perfs-and-filter-dotfiles

feat: users prefs server side + filter dotfiles + filter shares by resource type
2026-07-14 09:07:09 +02:00
parent 17bfb57af7 b9d6fa39c0
commit 5ae551a93d
43 changed files with 1873 additions and 94 deletions
@@ -146,6 +146,7 @@ log "Running Hurl tests..."
 hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test --jobs 1 \
  "$API_DIR/setup.hurl" \
  "$API_DIR/auth_login.hurl" \
+  "$API_DIR/user_ui_preferences.hurl" \
  "$API_DIR/auth_session_lifecycle.hurl" \
  "$API_DIR/auth_magic_link_login.hurl" \
  "$API_DIR/registration.hurl" \
@@ -0,0 +1,184 @@
+# =============================================================
+# OxiCloud — auth.users.ui_preferences round-trip
+# =============================================================
+# The `ui_preferences` JSONB column is the SPA's cross-device
+# backing store for pure UI toggles (hide dotfiles, view mode,
+# sidebar collapse, …). The server treats the contents as
+# opaque; this suite pins the semantics of the PATCH surface
+# so a future refactor can't silently break cross-device sync:
+#
+#   1. Fresh user starts with an empty object bag (`{}`), not
+#      `null` and not missing from the response body.
+#   2. PATCH does a SHALLOW merge — a partial write only
+#      touches the keys it mentions; siblings survive. Load-
+#      bearing invariant: without it, Device A's write would
+#      silently wipe preferences Device B just set.
+#   3. Sending `{key: null}` in the patch REMOVES that key
+#      server-side (jsonb_strip_nulls after the merge). This
+#      is the documented delete-a-key path.
+#   4. Non-object patch shape is rejected with 400. Prevents
+#      the endpoint from being a scratch scalar store and
+#      catches malformed clients early.
+#
+# Not covered here (intentional):
+#   • 16 KiB size cap — the CHECK is at the schema layer and
+#     is exercised by unit tests without needing an integration
+#     round-trip; constructing a 16 KiB JSON body in Hurl adds
+#     line noise without meaningful signal.
+#   • Concurrency safety of the shallow merge under two
+#     simultaneous PATCHes — postgres' `||` operator is atomic
+#     per row, so this is a DB-guarantee test rather than an
+#     API test.
+# =============================================================
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 1 — Admin login. All PATCH/GET below use this token so
+#          the same user's bag is under test.
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/auth/login
+Content-Type: application/json
+{ "username": "{{username}}", "password": "{{password}}" }
+
+HTTP 200
+[Captures]
+admin_token: jsonpath "$.access_token"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 2 — Fresh state: bag is present in the response and is
+#          an empty object.
+#
+#          Note: if a PRIOR test in the API suite has already
+#          PATCHed this user's ui_preferences, this step's
+#          `count == 0` check would fail. Currently no other
+#          test writes to `ui_preferences` — if a future test
+#          does, it MUST clean up its keys at teardown
+#          (`PATCH { key: null }`) to keep this baseline valid.
+# ─────────────────────────────────────────────────────────────
+GET {{base_url}}/api/auth/me
+Authorization: Bearer {{admin_token}}
+
+HTTP 200
+[Asserts]
+jsonpath "$.ui_preferences" exists
+jsonpath "$.ui_preferences" isCollection
+# Empty-object baseline. Neither `count == 0` on `.*` nor the
+# `== {}` object-literal predicate are supported by this Hurl
+# version. Fall back to a body-shape check on the serialised
+# response — serde_json emits `"ui_preferences":{}` without
+# whitespace inside the braces on Rust's default JSON writer,
+# so this pins the empty-object serialisation reliably.
+body contains "\"ui_preferences\":{}"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 3 — Write one key. Response echoes the merged bag with
+#          the new key. Bumps updated_at (not asserted — it's
+#          set by the repo unconditionally so no branch to pin).
+# ─────────────────────────────────────────────────────────────
+PATCH {{base_url}}/api/auth/me/profile
+Authorization: Bearer {{admin_token}}
+Content-Type: application/json
+{ "ui_preferences": { "hide_dotfiles": true } }
+
+HTTP 200
+[Asserts]
+jsonpath "$.ui_preferences.hide_dotfiles" == true
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 4 — Write a SECOND key. Shallow merge must preserve the
+#          first key. This is the load-bearing regression
+#          assertion: a full-replacement bug here would show
+#          `hide_dotfiles` missing from the response.
+# ─────────────────────────────────────────────────────────────
+PATCH {{base_url}}/api/auth/me/profile
+Authorization: Bearer {{admin_token}}
+Content-Type: application/json
+{ "ui_preferences": { "view_mode": "grid" } }
+
+HTTP 200
+[Asserts]
+jsonpath "$.ui_preferences.hide_dotfiles" == true
+jsonpath "$.ui_preferences.view_mode"     == "grid"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 5 — GET reflects the merged state after the round-trip
+#          (belt-and-braces — Step 4's PATCH response could
+#          have been returning a computed value while the DB
+#          state diverged; the fresh GET catches that).
+# ─────────────────────────────────────────────────────────────
+GET {{base_url}}/api/auth/me
+Authorization: Bearer {{admin_token}}
+
+HTTP 200
+[Asserts]
+jsonpath "$.ui_preferences.hide_dotfiles" == true
+jsonpath "$.ui_preferences.view_mode"     == "grid"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 6 — Null-value deletes the key. `hide_dotfiles` is
+#          removed; `view_mode` stays. This exercises the
+#          `jsonb_strip_nulls(bag || patch)` path in the repo.
+# ─────────────────────────────────────────────────────────────
+PATCH {{base_url}}/api/auth/me/profile
+Authorization: Bearer {{admin_token}}
+Content-Type: application/json
+{ "ui_preferences": { "hide_dotfiles": null } }
+
+HTTP 200
+[Asserts]
+jsonpath "$.ui_preferences.view_mode" == "grid"
+# Deleted key must not survive as `null` — it must be absent
+# (`jsonb_strip_nulls` in the repo strips it post-merge).
+jsonpath "$.ui_preferences.hide_dotfiles" not exists
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 7 — Non-object patch is rejected. Sending an array
+#          would be a client bug or an abuse attempt (the bag
+#          is documented as a JSON OBJECT). The schema CHECK
+#          `users_ui_preferences_is_object` enforces at the DB
+#          level; the service layer catches it earlier with a
+#          400.
+# ─────────────────────────────────────────────────────────────
+PATCH {{base_url}}/api/auth/me/profile
+Authorization: Bearer {{admin_token}}
+Content-Type: application/json
+{ "ui_preferences": [1, 2, 3] }
+
+HTTP 400
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 8 — Scalar patch is rejected (same class as array).
+#          Both cases route through the same `patch.is_object()`
+#          gate in `AuthApplicationService::update_profile`.
+# ─────────────────────────────────────────────────────────────
+PATCH {{base_url}}/api/auth/me/profile
+Authorization: Bearer {{admin_token}}
+Content-Type: application/json
+{ "ui_preferences": "not-a-bag" }
+
+HTTP 400
+
+
+# ─────────────────────────────────────────────────────────────
+# Teardown — restore the bag to empty so downstream tests
+# don't inherit `view_mode`. Sending each surviving key with
+# `null` deletes them via jsonb_strip_nulls, leaving `{}`.
+# ─────────────────────────────────────────────────────────────
+PATCH {{base_url}}/api/auth/me/profile
+Authorization: Bearer {{admin_token}}
+Content-Type: application/json
+{ "ui_preferences": { "view_mode": null } }
+
+HTTP 200
+[Asserts]
+# Same empty-object serialised shape as Step 2's baseline —
+# `body contains "\"ui_preferences\":{}"` is the tightest empty
+# check available on this Hurl version.
+body contains "\"ui_preferences\":{}"
@@ -224,6 +224,26 @@ export async function apiEmptyTrash(page: Page): Promise<void> {
  }
 }

+/**
+ * Flip the caller's `ui_preferences.hide_dotfiles` server-side. Used by the
+ * dotfile-filter e2e spec to establish a known state at test start and to
+ * clean up at teardown so sibling tests aren't polluted by a leftover
+ * "hidden" mode (the preference is persistent across sessions because it's
+ * stored on `auth.users.ui_preferences`, not in localStorage).
+ *
+ * PATCHes only `hide_dotfiles`; siblings in the bag (view_mode, future
+ * keys) survive the shallow-merge on the server side.
+ */
+export async function apiSetHideDotfiles(page: Page, hide: boolean): Promise<void> {
+  const res = await page.request.patch('/api/auth/me/profile', {
+    headers: await csrfHeaders(page),
+    data: { ui_preferences: { hide_dotfiles: hide } },
+  });
+  if (!res.ok()) {
+    throw new Error(`apiSetHideDotfiles(${hide}) failed: ${res.status()} ${await res.text()}`);
+  }
+}
+
 /** A file to seed: its name, MIME type, and raw bytes. */
 export type SeedFile = { name: string; mimeType: string; body: Buffer };

@@ -0,0 +1,186 @@
+import { test, expect } from './coverage-helpers';
+import {
+  apiCreateFolder,
+  apiLogin,
+  apiSetHideDotfiles,
+  apiTrashFolder,
+} from '../scenarios/helpers';
+
+/**
+ * Dotfile-hide filter — end-to-end coverage of the UI-only, per-user
+ * `hide_dotfiles` preference (JSONB `auth.users.ui_preferences`).
+ *
+ * Deliberately narrow scope:
+ *
+ * 1. Toggle: a `.hidden` folder in `/files` disappears when the
+ *    toolbar eye button is pressed and reappears when it's pressed
+ *    again. This is the "does the filter actually filter" test.
+ *
+ * 2. Empty state: a folder that contains ONLY dotfiles renders the
+ *    "N hidden items — Show hidden files" affordance rather than the
+ *    generic "This folder is empty" copy. Clicking the affordance
+ *    flips the preference back off and the rows reappear. Guards
+ *    against a mystery-empty-folder regression.
+ *
+ * 3. Trash safety: the hide preference is deliberately IGNORED on
+ *    `/trash`, so a dotfile-named item still shows up for recovery.
+ *    Pins the "safety-net surface always shows everything" rule
+ *    against a future refactor that might extend the filter to
+ *    trash by accident.
+ *
+ * Other surfaces (favorites, recent, photos, public share) all
+ * derive from the same `filterDotfiles` helper and the same
+ * `preferences.hideDotfiles` reactive read; unit tests cover the
+ * predicate, so we don't burn browser cycles verifying each list
+ * page renders one more filtered row correctly. The three tests
+ * above hit the three DIFFERENT semantics (filter, empty-state,
+ * exemption), which is what actually needs regression coverage.
+ *
+ * Isolation. `hide_dotfiles` is per-user and persists on the server,
+ * so it survives the login-per-test that other specs rely on for
+ * isolation. `beforeEach` explicitly resets it to `false` and
+ * `afterEach` restores it, otherwise a failed test would leave the
+ * whole suite running with the filter on.
+ */
+
+// Test-created folders. Tests push here in-flight; afterEach reaps.
+// Keeps /files root clean so unrelated specs' virtualised listings
+// don't lose their own fixtures to overflow.
+const scratchFolderIds: string[] = [];
+
+test.beforeEach(async ({ page }) => {
+  await apiLogin(page);
+  await apiSetHideDotfiles(page, false);
+});
+
+test.afterEach(async ({ page }) => {
+  // Belt-and-braces: even if a test forgot to reset, restore the
+  // default so the next spec file starts from a known state.
+  await apiSetHideDotfiles(page, false).catch(() => {});
+  // Reap this test's fixtures. `catch` per id so a stale reference
+  // (already trashed by the test body, e.g. Test 3) doesn't cascade
+  // a teardown error onto a real assertion failure.
+  while (scratchFolderIds.length) {
+    const id = scratchFolderIds.pop()!;
+    await apiTrashFolder(page, id).catch(() => {});
+  }
+});
+
+function uniq(prefix: string): string {
+  return `${prefix}-${Date.now()}-${Math.floor(Math.random() * 1e6)}`;
+}
+
+test('toolbar eye toggle hides and re-shows dotfiles in /files', async ({ page }) => {
+  // Scratch parent so we don't dump siblings into /files root — the
+  // root's virtualised list is shared with the rest of the suite and
+  // its DOM size caps out around a few dozen rows; every persistent
+  // fixture we leave there risks pushing an unrelated test's own
+  // folder out of view (see `files-extra.spec.ts` regressions).
+  // Trashing the parent in afterEach cascades to the children.
+  const parent = await apiCreateFolder(page, uniq('DotfileToggleScratch'));
+  scratchFolderIds.push(parent.id);
+  const visible = uniq('Visible');
+  const hidden = `.${uniq('hidden')}`;
+  await apiCreateFolder(page, visible, parent.id);
+  await apiCreateFolder(page, hidden, parent.id);
+
+  await page.goto(`/files/${parent.id}`);
+
+  // Baseline: both rows render. Row test-id = folder name (see
+  // ResourceList / +page.svelte's data-testid pattern used by the
+  // sibling files.spec.ts).
+  await expect(page.getByTestId(visible)).toBeVisible({ timeout: 15_000 });
+  await expect(page.getByTestId(hidden)).toBeVisible();
+
+  // Flip the filter on via the eye toggle in the ListToolbar. The
+  // click routes through `preferences.toggleHideDotfiles()` which
+  // does an optimistic local mutation, so the row update should be
+  // visible before the debounced PATCH lands.
+  await page.getByTestId('list-toolbar-dotfile-toggle-btn').click();
+
+  // Visible row stays; hidden row vanishes.
+  await expect(page.getByTestId(visible)).toBeVisible();
+  await expect(page.getByTestId(hidden)).toHaveCount(0);
+
+  // Flip it back off — the hidden row must reappear. Same button;
+  // its state flips atomically with `preferences.hideDotfiles`.
+  await page.getByTestId('list-toolbar-dotfile-toggle-btn').click();
+  await expect(page.getByTestId(hidden)).toBeVisible();
+});
+
+test('empty-state hint appears when a folder holds only dotfiles', async ({ page }) => {
+  // Isolate the folder: nest inside a fresh parent so the only
+  // children are our dotfiles. Root has accumulated cruft from the
+  // suite and would drown the empty-state case.
+  const parent = await apiCreateFolder(page, uniq('OnlyDotfilesParent'));
+  scratchFolderIds.push(parent.id);
+  const dot1 = `.${uniq('a')}`;
+  const dot2 = `.${uniq('b')}`;
+  await apiCreateFolder(page, dot1, parent.id);
+  await apiCreateFolder(page, dot2, parent.id);
+
+  // Navigate into the parent. `/files/[...path]` treats the path
+  // segments as folder ids in the deep-link form.
+  await page.goto(`/files/${parent.id}`);
+
+  // Baseline: both dotfiles are visible with hide off.
+  await expect(page.getByTestId(dot1)).toBeVisible({ timeout: 15_000 });
+  await expect(page.getByTestId(dot2)).toBeVisible();
+
+  // Turn hide on. Folder becomes visually empty — but not the
+  // generic empty state; the "N hidden items" affordance appears
+  // instead, offering a one-click "Show hidden files" escape.
+  await page.getByTestId('list-toolbar-dotfile-toggle-btn').click();
+
+  const showHiddenBtn = page.getByTestId('files-show-hidden-btn');
+  await expect(showHiddenBtn).toBeVisible({ timeout: 15_000 });
+  // Regression pin: the generic "This folder is empty" hint MUST NOT
+  // show — that would hide the fact that content exists.
+  await expect(page.getByText('This folder is empty')).toHaveCount(0);
+
+  // Click the "Show hidden files" button. It calls
+  // `preferences.setHideDotfiles(false)` and both dotfiles must
+  // reappear in the same view without a reload.
+  await showHiddenBtn.click();
+  await expect(page.getByTestId(dot1)).toBeVisible();
+  await expect(page.getByTestId(dot2)).toBeVisible();
+});
+
+test('trash always shows dotfiles even when hide is on', async ({ page }) => {
+  // Create a `.`-prefixed folder, trash it, then flip the hide
+  // preference on. Trash MUST still show the row: hiding a
+  // trashed dotfile would let it ride the retention timer to
+  // permanent deletion without being reviewable — a
+  // safety-net-defeating footgun.
+  const dotname = `.${uniq('TrashedHidden')}`;
+  const folder = await apiCreateFolder(page, dotname);
+  await apiTrashFolder(page, folder.id);
+
+  // Turn hide on server-side so the client picks it up on next
+  // session load (rather than driving it through the UI toggle
+  // and then navigating — same end state, one fewer moving part).
+  await apiSetHideDotfiles(page, true);
+
+  await page.goto('/trash');
+
+  // Row must be present. Trash entries render the resource name
+  // as plain text (no per-row test-id keyed by name in the current
+  // template); text lookup is the reliable selector.
+  //
+  // `exact: true` narrows to the name cell — the path cell (which
+  // renders as "Personal/{name}") would otherwise also match under
+  // Playwright's default substring semantics and trip strict mode.
+  await expect(page.getByText(dotname, { exact: true })).toBeVisible({ timeout: 15_000 });
+
+  // Belt-and-braces: also verify the hide preference IS on in the
+  // background — otherwise the assertion above passes trivially
+  // because nothing was being hidden in the first place. We check
+  // by visiting /files (where the filter IS supposed to apply) and
+  // asserting the OTHER dotfile from the earlier test class would
+  // be hidden. Actually — because tests are ordered arbitrarily,
+  // we just verify the toolbar toggle reflects the current server
+  // state via aria-pressed on /files.
+  await page.goto('/files');
+  const toggle = page.getByTestId('list-toolbar-dotfile-toggle-btn');
+  await expect(toggle).toHaveAttribute('aria-pressed', 'true');
+});