[codex] add account enable scheduling toggle#108
Merged
james-6-23 merged 2 commits intoApr 30, 2026
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds account enable/disable support end-to-end: DB column and migration, admin API endpoint to toggle enabled, in-memory store flag to exclude disabled accounts from dispatch, scheduler integration, tests, frontend API, UI controls, and localization strings. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Frontend as Frontend UI
participant API as Admin Handler
participant DB as Database
participant Store as Account Store
participant Scheduler as Fast Scheduler
User->>Frontend: Click enable/disable
Frontend->>API: POST /api/admin/accounts/{id}/enable<br/>{ enabled }
API->>DB: SetAccountEnabled(id, enabled)
DB-->>API: OK / Err
API->>Store: ApplyAccountEnabled(id, enabled)
Store->>Store: Set DispatchPaused flag (atomic)
Store->>Scheduler: Update account state
Scheduler-->>Store: Acknowledged
API-->>Frontend: MessageResponse
Frontend->>Frontend: Show toast and reload accounts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 2/3 reviews remaining, refill in 20 minutes. Comment |
Cshvee
force-pushed
the
codex/account-enable-scheduling-toggle
branch
from
b41b203 to
6978bee
Compare
Cshvee
force-pushed
the
codex/account-enable-scheduling-toggle
branch
from
6978bee to
132ea2b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
database/sqlite_test.go (1)
54-60: ⚡ Quick winAdd a length assertion before
rows[0]access in the second read.After the second
ListActive, asserting length first avoids panic-style failures and keeps diagnostics clear.Suggested fix
rows, err = db.ListActive(ctx) if err != nil { t.Fatalf("ListActive 返回错误: %v", err) } + if len(rows) != 1 { + t.Fatalf("ListActive 返回 %d 条，want 1", len(rows)) + } if rows[0].Enabled { t.Fatal("disabled account Enabled = true, want false") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@database/sqlite_test.go` around lines 54 - 60, The test calls db.ListActive and immediately indexes rows[0] without verifying the slice length; add an assertion (e.g., require or t.Fatalf) that len(rows) > 0 (or equals the expected count) after the second db.ListActive call and before accessing rows[0] to prevent panics and produce clearer diagnostics; update the test around the ListActive result handling (the rows variable and the surrounding test function) to check length and fail with a helpful message if it's empty.frontend/src/pages/Accounts.tsx (1)
797-814: ⚡ Quick winHarden batch loading cleanup with
finally
setBatchLoading(false)should be in afinallyblock so the toolbar never gets stuck disabled if this handler grows or throws in future edits.Suggested diff
const handleBatchEnabled = async (enabled: boolean) => { if (selected.size === 0) return setBatchLoading(true) let success = 0 let fail = 0 - for (const id of selected) { - try { - await api.toggleAccountEnabled(id, enabled) - success++ - } catch { - fail++ - } - } - showToast(t(enabled ? 'accounts.batchEnableDone' : 'accounts.batchDisableDone', { success, fail })) - setBatchLoading(false) - setSelected(new Set()) - void reload() + try { + for (const id of selected) { + try { + await api.toggleAccountEnabled(id, enabled) + success++ + } catch { + fail++ + } + } + showToast(t(enabled ? 'accounts.batchEnableDone' : 'accounts.batchDisableDone', { success, fail })) + setSelected(new Set()) + void reload() + } finally { + setBatchLoading(false) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/Accounts.tsx` around lines 797 - 814, The handler function handleBatchEnabled can leave the UI stuck in loading if an unexpected error occurs before setBatchLoading(false); wrap the main work in a try/finally so setBatchLoading(false) always runs: move the for-loop and toast/show logic into a try block and call setBatchLoading(false) in a finally block; also ensure setSelected(new Set()) and void reload() remain in the finally (or after the finally) as appropriate so selection is cleared and reload always runs regardless of errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin/handler.go`:
- Around line 1688-1701: The request binding currently uses Enabled bool which
treats a missing field as false and can accidentally disable accounts; change
the inline req struct to use Enabled *bool `json:"enabled" binding:"required"`
(or otherwise validate presence) and return writeError(c, http.StatusBadRequest,
...) when req.Enabled == nil so the client must supply the field; additionally,
when calling h.db.SetAccountEnabled(ctx, id, req.Enabled) detect a not-found DB
error (e.g., sql.ErrNoRows or your repo's ErrNotFound) and return writeError(c,
http.StatusNotFound, ...) instead of treating it as a 500, otherwise continue to
return 500 for other errors.
In `@database/postgres.go`:
- Around line 2029-2033: In SetAccountEnabled on DB, check the ExecContext
result's RowsAffected and return sql.ErrNoRows when it is 0 instead of treating
it as success: change the call to capture the sql.Result (e.g., res, err :=
db.conn.ExecContext(...)), handle and return any err immediately, then call
res.RowsAffected(), and if that returns 0 return sql.ErrNoRows; otherwise return
nil. Ensure you import/use sql.ErrNoRows and keep the function signature
unchanged.
In `@frontend/src/pages/Accounts.tsx`:
- Around line 1341-1349: The row-level enable/disable button uses reversed
icons: swap the icons used in the Button where handleToggleEnabled is invoked so
that when account.enabled === false (the enable action) it renders the Power
icon and when enabled it renders PowerOff, matching the batch controls; update
the JSX inside the Button (the conditional that currently returns PowerOff for
the false case and Power for the true case) accordingly.
---
Nitpick comments:
In `@database/sqlite_test.go`:
- Around line 54-60: The test calls db.ListActive and immediately indexes
rows[0] without verifying the slice length; add an assertion (e.g., require or
t.Fatalf) that len(rows) > 0 (or equals the expected count) after the second
db.ListActive call and before accessing rows[0] to prevent panics and produce
clearer diagnostics; update the test around the ListActive result handling (the
rows variable and the surrounding test function) to check length and fail with a
helpful message if it's empty.
In `@frontend/src/pages/Accounts.tsx`:
- Around line 797-814: The handler function handleBatchEnabled can leave the UI
stuck in loading if an unexpected error occurs before setBatchLoading(false);
wrap the main work in a try/finally so setBatchLoading(false) always runs: move
the for-loop and toast/show logic into a try block and call
setBatchLoading(false) in a finally block; also ensure setSelected(new Set())
and void reload() remain in the finally (or after the finally) as appropriate so
selection is cleared and reload always runs regardless of errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ed176282-4f14-4ed8-9251-9ef417899e1b
📒 Files selected for processing (12)
admin/handler.goauth/fast_scheduler.goauth/fast_scheduler_test.goauth/store.godatabase/postgres.godatabase/sqlite.godatabase/sqlite_test.gofrontend/src/api.tsfrontend/src/locales/en.jsonfrontend/src/locales/zh.jsonfrontend/src/pages/Accounts.tsxfrontend/src/types.ts
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.
Summary
Validation
Summary by CodeRabbit
New Features
Tests