feat: Wave 1 quick wins — 7 follow-ups from PR #523 review#527
Open
tayebmokni wants to merge 7 commits into
Open
feat: Wave 1 quick wins — 7 follow-ups from PR #523 review#527tayebmokni wants to merge 7 commits into
tayebmokni wants to merge 7 commits into
Conversation
…obs/dlq (#502) On a freshly booted system the \`default\` queue's Redis keys don't exist, so \`Inspector.ListArchivedTasks\` returns an error wrapping \`asynq.ErrQueueNotFound\`. The DLQ list handler bubbled that as a 500 with body \"failed to list archived tasks\" — the admin Jobs page rendered its FailureState UI on every clean install. Treat \`ErrQueueNotFound\` as the empty case: HTTP 200 with \`{data: [], pagination: {next_cursor: \"\"}}\`. Operationally identical to a queue that exists but has no archived tasks. Test: TestList_QueueNotFoundIsEmpty pins the contract with a fake inspector that returns a wrapped ErrQueueNotFound. Closes #502. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Tayeb Mokni <tayeb.mokni@gmail.com>
…orage (#503) \`PostgresStorage.List\` propagated \`SQLSTATE 42P01\` (relation \`plugins\` does not exist) up through \`lifecycle.Manager.List\` to the /api/v1/admin/plugin-pages handler, which surfaced it as 500 \"failed to list plugins\". The plugins table has no migration today — clean installs hit this on every admin sidebar render. Semantically \"no table\" and \"no rows\" are the same outcome for a read-only lister: there are zero plugins. Treat 42P01 as empty result rather than a runtime error. Tests: - TestPostgresStorage_List_UndefinedTableIsEmpty pins the storage contract. - TestList_EmptyOnCleanInstall pins the handler shape: 200 with a non-nil empty \`pages\` slice so JSON encodes \`[]\`, not \`null\`. Note: the /api/v1/admin/plugin-pages route is also currently returning 401 because the route mount in main.go is missing the authmw.RequireSession wrap. That's a separate wiring bug — fix follows in a sibling commit. Closes #503. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Tayeb Mokni <tayeb.mokni@gmail.com>
…ssing (#515) The PR #523 adapter set \`author.displayName: ''\` (empty) — every row rendered a blank Author cell. Now fall back to the last 8 chars of \`author_id\` (mirrors the \`shortId\` helper in posts/[id]/revisions/page.tsx). When/if the list API joins users and starts emitting \`author.display_name\`, the adapter prefers that. Refactored the inline map into an exported \`adaptApiPost\` plus \`ApiPost\` type so the conversion is unit-testable without spinning up the Server Component. Three new tests: - fallback to short id when no display name supplied - API-supplied display_name wins - commentsCount stays 0 (documented deferred behavior) \`commentsCount\` still hard-coded to 0 with a comment — the list endpoint doesn't emit the aggregate, and adding a per-row count needs a separate join the API doesn't ship today. Closes #515. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Tayeb Mokni <tayeb.mokni@gmail.com>
…rch (#516) Three handlers all parsed a \`status\` query param + the \`any\` → empty alias, two by hand and the third (admin/search) silently treating \`any\` as a literal status value (returning zero matches). Extract \`packages/go/util/queryparse.ParseStatus(raw, valid)\` with shared behavior: - "" and "any" both mean \"no filter\" → returns "" - otherwise must be in \`valid\`; returns ErrInvalidStatus - nil-map-safe Migrate the three call sites: - apps/api/internal/rest/posts/handlers.go (in parseListQuery) - apps/api/internal/admin/comments/list.go (uses AllStatuses) - apps/api/internal/admin/search/handler.go (was unchecked; now returns 400 invalid_status on bad input, like the other two) Behavior change observed: \`/api/v1/admin/search?status=any\` now maps to "no filter" (previously embedded verbatim in the SQL predicate, silently returning empty). Unknown statuses on search now return 400 instead of silently filtering to nothing. queryparse_test.go: 7 cases including \"any\" alias, valid pass- through, unknown → ErrInvalidStatus, nil-map handling, case sensitivity. Closes #516. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Tayeb Mokni <tayeb.mokni@gmail.com>
PR #523 mounted /api/v1/settings but the registry only knew \`core.site.*\` + \`core.permalinks.format\`. Admin Reading/Writing sub-pages were calling \`?group=core.reading\` and \`?group=core.writing\` which returned \`{}\` — the forms had no schema to render. Register 11 new Setting{} entries (6 reading, 5 writing) using core.site.name as the template (JSON-Schema string + Default + Autoload: true + RequiresCapability: CapManageOptions). Reading keys match the admin ReadingForm.tsx camelCase mapping: - homepage_type (enum latest_posts | static_page, default latest_posts) - homepage_page_id (string) - posts_per_page (integer 1..100, default 10) - show_summary (bool, default true) - rss_items (integer 1..100, default 10) - rss_full_text (bool, default false) Writing keys match WritingForm.tsx: - default_category (enum, default uncategorized) - default_format (enum, default standard) - default_editor (enum block | classic, default block) - post_by_email_enabled (bool, default false) - post_by_email_address (string) Verified live: \`GET /api/v1/settings?group=core.reading\` and \`?group=core.writing\` both return the seeded defaults instead of empty objects. core_test.go: 6 tests covering presence, group-locking, defaults-readability, integer round-trip, enum rejection. Closes #525. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Tayeb Mokni <tayeb.mokni@gmail.com>
…526) The themes/static ActiveResolver called \`themeActiveStore.Get(ctx)\` on every request to /themes/active/style.css. Postgres options is single-row + autoloaded so the query is microsecond-cheap, but it scales linearly with public traffic. Wrap with a 60s TTL cache: - RLock-only on the cache-hit path (the common case). - Refill takes the write lock and re-checks the deadline — single- flight across the inner resolver per expiry window. - Empty values are cached too. The production resolver returns \"\" on a DB error, and not caching that would degenerate into a hammering loop on a transient outage. - TTL <= 0 disables caching (test seam). - Injectable \`now func() time.Time\` for deterministic TTL tests. - \`Invalidate()\` forces the next call to hit the inner resolver (will be wired from the admin theme-activate handler in a follow-up; switches lag up to TTL until that lands). 8 tests, all pass under -race: cache hit memoization, TTL expiry, explicit Invalidate, fresh-resolver Invalidate, zero TTL, empty-value caching, concurrent access (64 goroutines × 500 iters), value-change pickup after Invalidate. Closes #526. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Tayeb Mokni <tayeb.mokni@gmail.com>
Themes used to require a manual \`docker run --rm -v api-themes:/themes alpine cp ...\` every time the volume got wiped (e.g. \`compose down -v\`). Without that, \`/themes/active/style.css\` 404s and the customizer 500s on \"failed to load theme gn-hello\". Migrate now seeds the volume idempotently on every run: - New \`cli/gonext/cmd/migrate/themes.go::seedThemes\` copies /themes/* from the bundled CLI image into the mounted volume when the destination is empty. Only directories with a theme.json are treated as themes (skips README.md and cruft). - Soft-warning + nil error if the source isn't present (operator runs CLI outside the image). - New --seed-themes flag (default true) on the migrate command. - Runs after pkgmigrate.Run succeeds, BEFORE the existing DB-side runThemeSeed step, so files are on disk before the core.active_theme options row is written. docker-compose.dev.yml: migrate service now mounts \`api-themes:/var/lib/gonext-themes\` and exports the path env vars. Both api and migrate services reference the same named volume, so what migrate writes at /var/lib/gonext-themes shows up to the api at /themes. themes_test.go: 8 cases — empty-destination seed, idempotence (sentinel preservation), non-theme dir filtering, cruft tolerance, missing-source soft-warning, file-as-source hard error, env override resolution, --seed-themes flag parsing. Closes #513. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Tayeb Mokni <tayeb.mokni@gmail.com>
This was referenced May 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on top of PR #523. Seven atomic fixes for the small/medium issues filed during the #522 review tracker.
Commits (each closes a separate issue)
fix(api): translate asynq ErrQueueNotFound into empty 200 on /admin/jobs/dlqfix(api): treat undefined plugins table as empty list in lifecycle Storagefix(admin): posts list shows shortened author_id when display name missingrefactor(api): shared queryparse.ParseStatus + propagate to admin/searchfeat(settings): register core.reading.* + core.writing.* keysperf(api): TTL-cache active-theme resolver in /themes static handlerchore(cli): seed themes into api-themes volume during migrateVerification
go build ./...clean in apps/api, apps/worker, cli/gonext, packages/gopnpm exec tsc --noEmit0 errors in apps/adminDepends on
feat/post-session-cleanup) — this PR's base. Some commits here reference packages introduced in fix: post-session regressions + mount /api/v1/settings + /themes static #523 (Register core.reading and core.writing setting keys #525 builds on Mount /api/v1/settings registry — admin Settings tab is a UI mockup until this lands #499's settings mount; Cache active-theme resolver in /themes static handler #526 builds on Mount /themes/active/style.css static handler — public site has zero theme CSS #501's themes static).Next
After this merges, Wave 2: 4 medium features (#507 dead admin routes, #508 wire Settings→public, #509 wire Menus→public nav, #510 show_on_front dispatch).
🤖 Generated with Claude Code