feat(config-editor): diff preview before saving (#133)#217
Conversation
- Add ConfigDiffModal component with scrollable diff view - Show changed sections as badges with per-section revert buttons - Save button opens diff modal for review instead of saving directly - Ctrl+S now triggers diff preview modal - Add unsaved-changes indicator dot on Save button - Add undo-last-save button after successful save (1 level deep) - Inline diff view remains below the form as a live preview
…133) - Add showDiffModal/prevSavedConfig state - Add changedSections computed from patches - openDiffModal() replaces direct save — Ctrl+S also opens modal - executeSave() is the actual save logic (called from modal confirm) - revertSection() reverts individual top-level sections - undoLastSave() restores config to pre-save state - Save button indicator dot when unsaved changes exist - Undo Last Save button shown after successful save - Fix a11y: switched fieldset for changed-sections group - Biome format applied
- 5 tests for ConfigDiff: no-changes, added/removed counts, custom title, default title, diff view aria region - 8 tests for ConfigDiffModal: renders, section badges, revert per section, confirm save, cancel, disabled while saving, saving text, empty sections
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe pull request adds a server-side API route for guild AI feedback statistics that proxies to the bot API, updates the dashboard component to call this internal endpoint, introduces a ConfigDiffModal component for previewing configuration changes before saving, and integrates this modal into the config editor with per-section patching and undo functionality. Supporting tests are included. Changes
Possibly related PRs
🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| web/src/components/dashboard/config-diff-modal.tsx | New component implementing diff preview modal with section badges, revert buttons, and save confirmation - well structured with good accessibility |
| web/src/components/dashboard/config-editor.tsx | Major refactor removing auto-save and adding explicit save flow with diff modal, undo functionality, and per-section revert - has useEffect dependency issue |
| web/src/components/dashboard/ai-feedback-stats.tsx | Refactored to use proxy route instead of direct bot API, fixed React key prop, moved type to shared file - minor type name inconsistency |
| web/src/app/api/guilds/[guildId]/ai-feedback/stats/route.ts | Added input validation and clamping for days parameter (1-90 range) with proper error handling |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User Edits Config] --> B{Has Changes?}
B -->|No| C[Save Button Disabled]
B -->|Yes| D[Save Button Active with Yellow Dot]
D --> E[User Clicks Save or Ctrl+S]
E --> F[ConfigDiffModal Opens]
F --> G[Show Changed Sections with Revert Buttons]
G --> H{User Action?}
H -->|Cancel| I[Close Modal, Keep Draft]
H -->|Revert Section| J[Restore Section from Saved]
J --> G
H -->|Confirm Save| K[Execute Save]
K --> L{Save Success?}
L -->|Partial Failure| M[Update Draft with Succeeded Sections]
L -->|Full Success| N[Store Previous Config for Undo]
N --> O[Reload from Server]
O --> P[Show Undo Last Save Button]
P --> Q{User Clicks Undo?}
Q -->|Yes| R[Restore Previous Config to Draft]
R --> D
Q -->|No| S[Continue Editing]
Last reviewed commit: e2e2de1
… route
- Replace incorrect useGuildSelection() destructuring ({selectedGuild, apiBase})
with correct string return value (guildId)
- Add missing /api/guilds/[guildId]/ai-feedback/stats Next.js proxy route
so the component fetches via server-side proxy instead of direct bot API
- Fix 'percent' possibly undefined in Pie chart label callback
- Fix noArrayIndexKey lint warning by using entry.name as Cell key
- Apply biome formatter fixes
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/app/api/guilds/`[guildId]/ai-feedback/stats/route.ts:
- Around line 12-52: Create a shared analytics response type in
web/src/types/analytics.ts (e.g., export type AiFeedbackStatsResponse = { ... })
that matches the current stats payload returned by the bot API, then update this
route's GET handler to import and use that type for its response contract
(reference GET in web/src/app/api/guilds/[guildId]/ai-feedback/stats/route.ts)
and ensure the dashboard consumer also imports the same type from
web/src/types/analytics.ts so both API flow (proxyToBotApi, buildUpstreamUrl)
and dashboard UI share the exact contract to avoid drift.
- Around line 38-44: Validate and clamp the 'days' query parameter before
proxying it: in the loop that reads request.nextUrl.searchParams.get('days')
(where you currently call upstreamUrl.searchParams.set), parse the value to an
integer (e.g., via Number.parseInt or Number(value)), check it's a finite
positive integer, then clamp it to an acceptable range (for example min 1, max
90) and only then set upstreamUrl.searchParams.set('days',
String(clampedValue)); if parsing fails or the value is out-of-range, either
omit the param or use a safe default (e.g., 7) instead of forwarding the raw
value.
In `@web/src/components/dashboard/config-editor.tsx`:
- Around line 127-129: prevSavedConfig is stored globally so switching guilds
lets "Undo Last Save" apply another guild's snapshot; change prevSavedConfig to
be guild-scoped by storing snapshots keyed by guild id (e.g.,
useState<Record<string, GuildConfig | null>> or a Map) and update all places
that read/write prevSavedConfig to instead use the current guildId key
(references: prevSavedConfig, setPrevSavedConfig, and any handlers like the
Undo/Save handlers around lines ~413-415, ~426-435, ~765-776). Ensure when
switching guilds you read the snapshot for that guildId (or default null) and
when saving/creating an undo snapshot you write it to the guildId key so
snapshots cannot be applied across guilds.
In `@web/tests/components/dashboard/config-diff-modal.test.tsx`:
- Around line 57-67: Tests for ConfigDiffModal check button disabling but miss
asserting that dialog-level close attempts are blocked while saving; add a
regression test that renders <ConfigDiffModal {...baseProps} saving={true} />
with baseProps.onOpenChange mocked, simulate a dialog-close attempt (e.g.,
Escape key press and/or outside click using fireEvent/userEvent against the
document) and assert baseProps.onOpenChange was not called; reference the
ConfigDiffModal component, the saving prop, and the onOpenChange handler to
locate where to add this test.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
web/src/app/api/guilds/[guildId]/ai-feedback/stats/route.tsweb/src/components/dashboard/ai-feedback-stats.tsxweb/src/components/dashboard/config-diff-modal.tsxweb/src/components/dashboard/config-editor.tsxweb/tests/components/dashboard/config-diff-modal.test.tsxweb/tests/components/dashboard/config-diff.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Greptile Review
- GitHub Check: Docker Build Validation
🧰 Additional context used
📓 Path-based instructions (2)
web/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with type safety. Share contracts between dashboard UI and API responses via
web/src/types/analytics.tsand similar type definition files
Files:
web/src/app/api/guilds/[guildId]/ai-feedback/stats/route.ts
web/src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Component files should integrate with Zustand stores for state management (e.g., discord-entities store for caching Discord channels and roles per guild)
Files:
web/src/components/dashboard/config-editor.tsxweb/src/components/dashboard/ai-feedback-stats.tsxweb/src/components/dashboard/config-diff-modal.tsx
🧠 Learnings (1)
📚 Learning: 2026-03-01T06:03:34.399Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-01T06:03:34.399Z
Learning: Applies to src/modules/*.js : Per-request modules (AI, spam, moderation) should call `getConfig(guildId)` on every invocation for automatic config changes. Stateful resources should use `onConfigChange` listeners for reactive updates
Applied to files:
web/src/components/dashboard/config-editor.tsxweb/src/components/dashboard/ai-feedback-stats.tsx
🧬 Code graph analysis (3)
web/tests/components/dashboard/config-diff.test.tsx (1)
web/src/components/dashboard/config-diff.tsx (1)
ConfigDiff(34-120)
web/src/app/api/guilds/[guildId]/ai-feedback/stats/route.ts (1)
web/src/lib/bot-api-proxy.ts (4)
authorizeGuildAdmin(37-69)getBotApiConfig(82-92)buildUpstreamUrl(100-113)proxyToBotApi(135-176)
web/src/components/dashboard/ai-feedback-stats.tsx (2)
src/api/routes/ai-feedback.js (3)
guildId(94-94)guildId(190-190)stats(104-107)web/src/hooks/use-guild-selection.ts (1)
useGuildSelection(12-54)
🔇 Additional comments (5)
web/src/components/dashboard/ai-feedback-stats.tsx (2)
40-76: Good API wiring update and guild guard.Switching to guildId-based internal API calls with
encodeURIComponentand early null return is clean and safe.
147-154: Nice defensive rendering improvement.The
percent ?? 0fallback and stableCellkey improve chart resilience.web/src/components/dashboard/config-diff-modal.tsx (1)
61-122: Modal interaction locking while saving is well implemented.Disabling close/revert/confirm paths during
savingis solid and prevents conflicting user actions.web/tests/components/dashboard/config-diff.test.tsx (1)
5-39: Test coverage for core ConfigDiff paths looks good.The suite hits both semantic output and accessibility-relevant rendering states.
web/src/components/dashboard/config-editor.tsx (1)
2327-2339: The modal wiring is clean and matches the new save workflow.Passing
changedSections, confirm, and per-section revert callbacks intoConfigDiffModalis well structured.
…ruption (resolves coderabbit thread)
…es coderabbit thread)
…es coderabbit thread)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| useEffect(() => { | ||
| // Don't auto-save during initial config load or if no changes | ||
| if (isLoadingConfigRef.current || !hasChanges || hasValidationErrors || saving) return; | ||
| setPrevSavedConfig(null); | ||
| }, []); |
There was a problem hiding this comment.
Effect should depend on guildId to actually clear when guild changes
| useEffect(() => { | |
| // Don't auto-save during initial config load or if no changes | |
| if (isLoadingConfigRef.current || !hasChanges || hasValidationErrors || saving) return; | |
| setPrevSavedConfig(null); | |
| }, []); | |
| useEffect(() => { | |
| setPrevSavedConfig(null); | |
| }, [guildId]); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/components/dashboard/config-editor.tsx
Line: 430-432
Comment:
Effect should depend on `guildId` to actually clear when guild changes
```suggestion
useEffect(() => {
setPrevSavedConfig(null);
}, [guildId]);
```
How can I resolve this? If you propose a fix, please make it concise.| } | ||
|
|
||
| const data = (await res.json()) as FeedbackStats; | ||
| const data = (await res.json()) as AiFeedbackStats; |
There was a problem hiding this comment.
Type name inconsistency - import renames to AiFeedbackStatsType but assertion uses AiFeedbackStats
| const data = (await res.json()) as AiFeedbackStats; | |
| const data = (await res.json()) as AiFeedbackStatsType; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/components/dashboard/ai-feedback-stats.tsx
Line: 53
Comment:
Type name inconsistency - import renames to `AiFeedbackStatsType` but assertion uses `AiFeedbackStats`
```suggestion
const data = (await res.json()) as AiFeedbackStatsType;
```
How can I resolve this? If you propose a fix, please make it concise.
Summary
Closes #133
Implements all subtasks from the issue: diff preview modal before saving, per-section revert, unsaved-changes indicator, Ctrl+S diff preview, and undo-after-save.
Changes
New Component:
ConfigDiffModalUpdated:
ConfigEditorTests
ConfigDiff: no-changes state, added/removed counts, titles, aria regionConfigDiffModal: render, section badges, revert, confirm, cancel, disabled statesChecklist