Skip to content

fix(settings): wire Storage tab to runtime config#335

Open
ssorathia wants to merge 2 commits intoartifact-keeper:mainfrom
ssorathia:fix/settings-storage-tab-dynamic-values
Open

fix(settings): wire Storage tab to runtime config#335
ssorathia wants to merge 2 commits intoartifact-keeper:mainfrom
ssorathia:fix/settings-storage-tab-dynamic-values

Conversation

@ssorathia
Copy link
Copy Markdown

Summary

Partial fix for #334 — wires the admin Settings → Storage tab to the runtime config returned by /api/v1/admin/settings instead of the hardcoded placeholders. Same raw-cast + type-guard pattern that getPasswordPolicy and getSmtpConfig already use.

Scope is intentionally narrow: only the three Storage-tab fields whose source of truth already exists in the API response (Storage Backend, Storage Path, Max Upload Size). The other items called out in #334 (Auth token expiry fields, Deduplication, General → Environment badge) need backend changes first and are left out of this PR per the issue's "Out of scope" section.

Changes

  • src/lib/api/settings.ts — add StorageSettings type, DEFAULT_STORAGE_SETTINGS, and settingsApi.getStorageSettings().
  • src/app/(app)/(admin)/settings/page.tsx — wire the Storage tab's first three SettingRows to the new query; add formatStorageBackend (filesystem/s3/gcs/azure → friendly label, unknown values render as-is) and formatBytes helpers.
  • Leave the Deduplication row hardcoded with a comment, since the backend doesn't yet expose it on the settings endpoint.

Test plan

  • Five new tests for getStorageSettings: happy path, missing fields, wrong types, SDK error, SDK throw.
  • Four new tests for the rendered Storage tab: populated values, friendly backend labels (incl. filesystemLocal Filesystem), unknown backend fall-through, loading state.
  • Existing SMTP "populates from loaded config" test updated (queryCallIndex bumped from 3 to 4 since storage-settings now slots in between password-policy and smtp-config).
  • Lint, typecheck, unit tests, and build all pass locally.

Fixes #334 (partial — Storage tab only).

The admin Settings → Storage tab displayed hardcoded placeholder values
("Local Filesystem", "/data/artifacts", "5 GB") regardless of the actual
runtime configuration. The same banner above the tabs claims values are
"configured via environment variables and shown read-only", but for
Storage that wasn't true — the literals were baked into the React
component.

Replace the hardcoded SettingRow values with data pulled from
/api/v1/admin/settings via the existing SDK getSettings() call, using
the same raw-cast + type-guard pattern already established by
getPasswordPolicy and getSmtpConfig.

- Add `StorageSettings` type, `DEFAULT_STORAGE_SETTINGS`, and
  `settingsApi.getStorageSettings()` in src/lib/api/settings.ts.
- Add `formatStorageBackend` (filesystem/s3/gcs/azure → friendly label,
  unknown values render as-is) and `formatBytes` helpers in page.tsx.
- Wire the Storage tab's first three SettingRows to the new query.
- Leave the Deduplication row hardcoded with a comment, since the
  backend doesn't yet expose it on the settings endpoint.

Tests:
- Five new tests for getStorageSettings covering happy path, missing
  fields, wrong types, SDK error, and SDK throw.
- Four new tests for the rendered Storage tab covering populated
  values, friendly backend labels (incl. "filesystem" → "Local
  Filesystem"), unknown backend fall-through, and the loading state.
- Existing SMTP "populates from loaded config" test updated to bump
  the queryCallIndex from 3 to 4 (storage-settings now slots in
  between password-policy and smtp-config).

Out of scope (covered in the linked issue): Auth tab token expiry
fields and the General tab Environment badge — both need backend
changes to expose the underlying values via /api/v1/admin/settings.

Fixes artifact-keeper#334

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ssorathia ssorathia requested a review from a team as a code owner May 4, 2026 18:02
Address two findings from review of PR artifact-keeper#335:

1. The local `formatBytes` in page.tsx duplicated `@/lib/utils#formatBytes`
   and rendered the same byte counts differently ("1.0 GB" vs "1 GB" used
   everywhere else in the app). Drop the local copy and import the shared
   helper. Also drops the undocumented "0 bytes means Unlimited" semantic
   the local copy had invented; values now reflect what the backend
   actually returned.

2. The Storage tab's `getStorageSettings` silently fell back to
   `DEFAULT_STORAGE_SETTINGS` ("filesystem" / "/data/artifacts" / "5 GB")
   on SDK error or shape mismatch — the exact placeholder strings issue
   artifact-keeper#334 calls out as the bug. An admin couldn't tell "real config" from
   "API failed, you're seeing a fiction." Now `getStorageSettings` throws
   on SDK error or missing/wrong-typed required fields, and the page
   distinguishes loading ("Loading...") from error ("Unavailable") via
   `isLoading` / `isError` from useQuery. `DEFAULT_STORAGE_SETTINGS` is
   removed.

While here, refactored the page-test useQuery mocks from a fragile
queryCallIndex ordinal (which would silently break if a query were
added before/between storage and smtp) to a queryKey-keyed dispatch
helper. Adds a new test asserting that the placeholder strings
explicitly do NOT appear when the storage query errors.
@ssorathia
Copy link
Copy Markdown
Author

Pushed 44d3851 addressing two findings from a self-review:

  1. Reuse the existing formatBytes. The original commit shipped a local formatBytes in page.tsx that diverged from @/lib/utils#formatBytes (already used in 10+ places — repos, staging, builds, etc.), rendering "1.0 GB" where the rest of the app renders "1 GB". Switched to the shared helper. Side effect: dropped the undocumented "0 bytes → Unlimited" semantic that the local copy had invented; values now reflect exactly what the backend returns.

  2. Stop silently falling back to placeholder defaults on error. The original getStorageSettings returned DEFAULT_STORAGE_SETTINGS ("filesystem" / "/data/artifacts" / "5 GB") on SDK error or shape mismatch — i.e., the exact placeholder strings Admin Settings page shows hardcoded placeholder values, not runtime config #334 calls out as the bug. An admin couldn't distinguish "this is the real config" from "the API broke, you're looking at fiction." getStorageSettings now throws on SDK error / missing required fields, and the page distinguishes isLoading ("Loading...") from isError ("Unavailable") via the React Query state. DEFAULT_STORAGE_SETTINGS removed.

Also refactored the page-test useQuery mocks from an ordinal queryCallIndex to a queryKey-keyed dispatch helper (so adding a future query before storage/smtp won't silently break unrelated tests), and added a regression test asserting the placeholder strings explicitly do NOT appear when the storage query errors.

Lint / typecheck / unit tests / build all clean locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Admin Settings page shows hardcoded placeholder values, not runtime config

1 participant