-
Notifications
You must be signed in to change notification settings - Fork 2
chore: tech debt — skeletons, error boundaries, toasts, validation, rate limit cleanup #257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e18399a
a27c637
26924c5
37373c5
52d508e
dc2d5b8
fbf3a91
033a2e5
6f871a3
b73452f
21af8df
df005f6
b1ab778
d7b31c9
578eccf
7026999
aa2cfb9
c444978
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| # TASK2: Remaining Issue #144 Items | ||
|
|
||
| Already done (skip these): | ||
| - Loading skeletons ✅ | ||
| - Error boundaries ✅ | ||
| - Toast notifications ✅ | ||
| - XP proxy validation ✅ | ||
| - Rate limit stale cleanup ✅ | ||
|
|
||
| ## Items to implement now | ||
|
|
||
| ### 1. Zustand store for member page | ||
| - File: `web/src/app/dashboard/[guildId]/members/page.tsx` (or similar) | ||
| - Find all useState/useEffect hooks managing member state | ||
| - Create `web/src/stores/members-store.ts` with Zustand | ||
| - Migrate: member list, loading state, search query, selected member, pagination | ||
| - Keep component using the store hooks | ||
|
|
||
| ### 2. Zustand store for moderation page | ||
| - File: `web/src/app/dashboard/[guildId]/moderation/page.tsx` (or similar) | ||
| - Same pattern — create `web/src/stores/moderation-store.ts` | ||
| - Migrate: cases list, filters, loading, pagination | ||
|
|
||
| ### 3. console.error cleanup in browser code | ||
| - `rg -rn "console\.error" web/src/` — find all occurrences in client components | ||
| - Replace with: toast.error() for user-facing errors, or keep console.error where it's truly logging-only | ||
| - Do NOT replace server-side console.error (only client components) | ||
|
|
||
| ### 4. events.js function extraction | ||
| - File: `src/modules/events/` — check if there's a monolithic events file or inline handlers in interactionCreate.js | ||
| - Run: `wc -l src/modules/events/interactionCreate.js` to check size | ||
| - If handlers are inline (ticket_open, ticket_close, poll, etc.), extract each to separate files in `src/modules/handlers/` | ||
| - Update interactionCreate.js to import from handlers | ||
|
|
||
| ### 5. Mobile responsiveness — critical fixes only | ||
| - `rg -rn "grid-cols-2\|grid-cols-3\|table\b" web/src/components/dashboard/ --include="*.tsx" | head -30` | ||
| - Add `sm:grid-cols-2` fallbacks where fixed grid-cols are used | ||
| - Add `overflow-x-auto` wrapper around data tables | ||
| - Focus on: member table, moderation cases table, config sections with grids | ||
|
|
||
| ### 6. Migration gap — add placeholder comment | ||
| - Create `migrations/012_placeholder.cjs` with a comment explaining the gap | ||
| - Content: migration that logs a warning about the gap, does nothing (no-op) | ||
|
|
||
| ### 7. Fix totalMessagesSent stat accuracy | ||
| - File: find where community page stats are calculated | ||
| - `rg -rn "totalMessagesSent\|messages_sent" web/src/ src/` | ||
| - Add a comment explaining the known limitation, or filter to last 30 days | ||
| - If it's a simple query change, fix it; if architectural, add a TODO comment | ||
|
|
||
| ### 8. Review bot consolidation (GitHub config) | ||
| - Disable Copilot and Greptile PR reviewers — keep Claude (coderabbitai style) + CodeRabbit | ||
| - Check `.github/` for reviewer config files | ||
| - Check `CODEOWNERS` or `.github/pull_request_review_protection.yml` | ||
| - If via GitHub API: `gh api repos/VolvoxLLC/volvox-bot/automated-security-fixes` | ||
| - Note: Greptile may be configured in `.greptile.yaml` or via webhook — find and disable | ||
|
|
||
| ## Architectural items (document only, don't implement) | ||
| These need separate planning — just add TODO comments: | ||
| - Server-side member search (needs Discord member cache or DB index) | ||
| - Conversation search pagination (needs full-text search index) | ||
| - Config patch deep validation (needs schema per config key) | ||
| - Logger browser shim (nice-to-have, low impact) | ||
|
|
||
| ## Rules | ||
| - Commit each fix separately with conventional commits | ||
| - Run `pnpm format && pnpm lint && pnpm test` | ||
| - Run `pnpm --prefix web lint && pnpm --prefix web typecheck` | ||
| - Do NOT push |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| /** | ||
| * Migration 012 — Placeholder (gap filler) | ||
| * | ||
| * There are two migrations numbered 004 in this project: | ||
| * - 004_performance_indexes.cjs | ||
| * - 004_voice_sessions.cjs | ||
| * | ||
| * This no-op migration occupies the 012 slot to make the numbering sequence | ||
| * explicit going forward. No schema changes are made here. | ||
| */ | ||
|
|
||
| /** @type {import('node-pg-migrate').MigrationBuilder} */ | ||
| exports.up = async (pgm) => { | ||
| // No-op — this migration exists only to document the sequence gap | ||
| pgm.noTransaction(); | ||
| }; | ||
|
|
||
| /** @type {import('node-pg-migrate').MigrationBuilder} */ | ||
| exports.down = async (_pgm) => { | ||
| // Nothing to undo | ||
| }; | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,5 +80,10 @@ export function validateConfigPatchBody(body, SAFE_CONFIG_KEYS) { | |
| return { error: 'Value validation failed', status: 400, details: valErrors }; | ||
| } | ||
|
|
||
| // TODO: Deep per-key schema validation — currently validateSingleValue only checks | ||
| // type/range for known paths. Unknown paths pass through without structural validation. | ||
| // For full coverage, add a per-key JSON schema registry (one schema per top-level config | ||
| // section) and run deep validation against it here before accepting the patch. | ||
|
|
||
|
Comment on lines
+83
to
+87
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial TODO acknowledged — schema validation gap documented. The comment clearly describes the current limitation (unknown paths bypass structural validation) and proposes a reasonable solution (per-key JSON schema registry). This is good documentation of technical debt. Would you like me to help by:
🤖 Prompt for AI Agents |
||
| return { path, value, topLevelKey }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This placeholder does not fix the duplicate migration IDs.
The header documents that the repo already has two
004_*migrations. Filling the012slot leaves that conflict in place, so node-pg-migrate can still hit out-of-order or duplicate-ID problems. The fix needs to renumber the conflicting migration file(s), not add a no-op gap marker.As per coding guidelines, "Database migrations must be sequentially numbered with non-conflicting IDs; rename conflicting migration files to resolve out-of-order execution errors."
🤖 Prompt for AI Agents