web: add in-app feedback widget and usage events - #527
Conversation
…h per-user hourly email guard, dialog with bug/idea/other categories and Sentry replay id on bug reports, navbar dropdown and early-access banner entry points, track() util wrapping plausible with Project/Checklist/Collaborator/Feedback events Closes #525
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
📝 WalkthroughWalkthroughThis PR introduces an in-app feedback feature: a ChangesIn-app Feedback Feature
Analytics Tracking Pass
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant FeedbackDialog
participant submitFeedback
participant Database
participant EmailQueue
User->>FeedbackDialog: select category, enter message, submit
FeedbackDialog->>submitFeedback: submit(category, message, context)
submitFeedback->>Database: count recent submissions (last hour)
Database-->>submitFeedback: submission count
alt limit exceeded
submitFeedback-->>FeedbackDialog: throw rate-limit error
FeedbackDialog-->>User: show error state
else within limit
submitFeedback->>Database: insert feedback row
submitFeedback->>EmailQueue: send notification email
submitFeedback-->>FeedbackDialog: success response
FeedbackDialog-->>User: show success state, auto-close
end
Estimated code review effortEstimated code review effort: 3 (Moderate) | ~30 minutes PoemA rabbit hops with feedback in tow, 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/web/src/server/functions/feedback.server.ts (1)
70-71: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType-unsafe env cast.
(env as unknown as Record<string, string | undefined>).CONTACT_EMAILbypasses the CloudflareEnvtyping entirely. Consider extending the generatedEnvtype withCONTACT_EMAILso this reads asenv.CONTACT_EMAILwith proper typing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/server/functions/feedback.server.ts` around lines 70 - 71, The contact email lookup in feedback.server.ts is using a type-unsafe double cast on env, bypassing the Cloudflare Env typing. Update the generated Env type to include CONTACT_EMAIL and then read it directly through the feedback.server.ts logic in the contactEmail assignment, so the function uses env.CONTACT_EMAIL with proper typing and keeps the fallback value.packages/web/src/__tests__/server/migration-sql.js (1)
263-276: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFeedback table/index design looks solid.
FK cascade on
userIdand the composite(userId, createdAt)index correctly support the per-user hourly rate-limit COUNT query described in the PR objectives.Separately: this hunk also folds in an unrelated
subscription_referenceId_incomplete_uidxpartial index into the same migration as the feedback table. Not blocking, but worth confirming that was intentional (e.g., a hotfix bundled in) rather than accidental migration-file conflation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/__tests__/server/migration-sql.js` around lines 263 - 276, The feedback table and index changes look fine, but this migration hunk also includes the unrelated subscription_referenceId_incomplete_uidx partial index. Review the migration SQL assembly in migration-sql.js and confirm whether that subscription index belongs in the same migration as the feedback table; if not, move it to the correct migration or separate hunk so the feedback schema change stays isolated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/web/src/components/feedback/FeedbackDialog.tsx`:
- Around line 45-76: The pending submission in FeedbackDialog can still update
state after the dialog has been dismissed, causing stale sent state to leak into
a later session. Add a submission/session guard in handleSubmit and
handleOpenChange so closing the dialog invalidates any in-flight request before
it resolves. Use a ref-based submission id in FeedbackDialog, increment it when
starting submit and again inside handleOpenChange(false), then only run the
success path (setFormState('sent') and the auto-close timeout) if the id still
matches.
In `@packages/web/src/server/functions/feedback.server.ts`:
- Around line 35-52: The rate-limit logic in feedback.server.ts is non-atomic
because the existing count() query and db.insert(feedback) run as separate
steps, so concurrent requests can both pass MAX_SUBMISSIONS_PER_HOUR. Update the
submit flow in the feedback server function to use an atomic per-user guard or
serialization approach around the recent submission check and insert, keeping
the existing symbols db, feedback, recentCount, MAX_SUBMISSIONS_PER_HOUR, and
throwDomainError in view while changing the implementation so only one
submission decision can happen at a time for a user.
---
Nitpick comments:
In `@packages/web/src/__tests__/server/migration-sql.js`:
- Around line 263-276: The feedback table and index changes look fine, but this
migration hunk also includes the unrelated
subscription_referenceId_incomplete_uidx partial index. Review the migration SQL
assembly in migration-sql.js and confirm whether that subscription index belongs
in the same migration as the feedback table; if not, move it to the correct
migration or separate hunk so the feedback schema change stays isolated.
In `@packages/web/src/server/functions/feedback.server.ts`:
- Around line 70-71: The contact email lookup in feedback.server.ts is using a
type-unsafe double cast on env, bypassing the Cloudflare Env typing. Update the
generated Env type to include CONTACT_EMAIL and then read it directly through
the feedback.server.ts logic in the contactEmail assignment, so the function
uses env.CONTACT_EMAIL with proper typing and keeps the fallback value.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8fd351e3-5ec7-4cdb-8cad-af03606d4b6b
📒 Files selected for processing (28)
packages/db/src/schema.tspackages/shared/src/ids.tspackages/web/migrations/0004_lying_frog_thor.sqlpackages/web/migrations/meta/0004_snapshot.jsonpackages/web/migrations/meta/_journal.jsonpackages/web/src/__tests__/server/helpers.tspackages/web/src/__tests__/server/migration-sql.jspackages/web/src/components/EarlyAccessBanner.tsxpackages/web/src/components/checklist/CreateLocalChecklist.tsxpackages/web/src/components/checklist/LocalChecklistView.tsxpackages/web/src/components/checklist/ROB2Checklist/ScoringSummary.tsxpackages/web/src/components/checklist/ROBINSIChecklist/ScoringSummary.tsxpackages/web/src/components/dashboard/ContactPrompt.tsxpackages/web/src/components/feedback/FeedbackDialog.tsxpackages/web/src/components/layout/AppNavbar.tsxpackages/web/src/components/project/CreateProjectModal.tsxpackages/web/src/components/project/overview-tab/AddMemberModal.tsxpackages/web/src/components/project/reconcile-tab/ReconciliationWrapper.tsxpackages/web/src/config/sentry.tspackages/web/src/lib/analytics.tspackages/web/src/lib/devPdfPool.tspackages/web/src/project/actions.tspackages/web/src/routes/__root.tsxpackages/web/src/server/functions/__tests__/feedback.server.test.tspackages/web/src/server/functions/feedback.functions.tspackages/web/src/server/functions/feedback.server.tspackages/web/src/stores/feedbackStore.tspackages/workers/src/lib/mock-templates.ts
💤 Files with no reviewable changes (2)
- packages/web/src/components/checklist/ROB2Checklist/ScoringSummary.tsx
- packages/web/src/components/checklist/ROBINSIChecklist/ScoringSummary.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: checks
🧰 Additional context used
📓 Path-based instructions (9)
**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*: NEVER use emojis anywhere - not in code, comments, documentation, plan files, commit messages, or examples. This includes unicode symbols.
Never use emojis or unicode symbols - not in code, comments, docs, or commits
Files:
packages/web/src/routes/__root.tsxpackages/web/src/__tests__/server/migration-sql.jspackages/web/src/server/functions/feedback.functions.tspackages/shared/src/ids.tspackages/workers/src/lib/mock-templates.tspackages/web/src/components/project/reconcile-tab/ReconciliationWrapper.tsxpackages/web/src/components/project/overview-tab/AddMemberModal.tsxpackages/web/src/stores/feedbackStore.tspackages/web/src/project/actions.tspackages/web/src/components/dashboard/ContactPrompt.tsxpackages/web/src/config/sentry.tspackages/web/src/lib/devPdfPool.tspackages/web/migrations/0004_lying_frog_thor.sqlpackages/web/migrations/meta/_journal.jsonpackages/web/src/__tests__/server/helpers.tspackages/web/src/components/project/CreateProjectModal.tsxpackages/web/src/server/functions/__tests__/feedback.server.test.tspackages/web/src/components/feedback/FeedbackDialog.tsxpackages/web/src/components/checklist/LocalChecklistView.tsxpackages/web/src/components/checklist/CreateLocalChecklist.tsxpackages/web/src/lib/analytics.tspackages/db/src/schema.tspackages/web/src/server/functions/feedback.server.tspackages/web/src/components/layout/AppNavbar.tsxpackages/web/src/components/EarlyAccessBanner.tsxpackages/web/migrations/meta/0004_snapshot.json
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For UI icons, use
lucide-reactlibrary or SVGs only (never emojis)
Files:
packages/web/src/routes/__root.tsxpackages/web/src/server/functions/feedback.functions.tspackages/shared/src/ids.tspackages/workers/src/lib/mock-templates.tspackages/web/src/components/project/reconcile-tab/ReconciliationWrapper.tsxpackages/web/src/components/project/overview-tab/AddMemberModal.tsxpackages/web/src/stores/feedbackStore.tspackages/web/src/project/actions.tspackages/web/src/components/dashboard/ContactPrompt.tsxpackages/web/src/config/sentry.tspackages/web/src/lib/devPdfPool.tspackages/web/src/__tests__/server/helpers.tspackages/web/src/components/project/CreateProjectModal.tsxpackages/web/src/server/functions/__tests__/feedback.server.test.tspackages/web/src/components/feedback/FeedbackDialog.tsxpackages/web/src/components/checklist/LocalChecklistView.tsxpackages/web/src/components/checklist/CreateLocalChecklist.tsxpackages/web/src/lib/analytics.tspackages/db/src/schema.tspackages/web/src/server/functions/feedback.server.tspackages/web/src/components/layout/AppNavbar.tsxpackages/web/src/components/EarlyAccessBanner.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Use import aliases from tsconfig.json
Code comments should explain why something is being done or provide context, not repeat what the code is saying
Use TODO(agent) pattern for incomplete work or flagging items for future attention, with brief description and optional doc reference
Files:
packages/web/src/routes/__root.tsxpackages/web/src/__tests__/server/migration-sql.jspackages/web/src/server/functions/feedback.functions.tspackages/shared/src/ids.tspackages/workers/src/lib/mock-templates.tspackages/web/src/components/project/reconcile-tab/ReconciliationWrapper.tsxpackages/web/src/components/project/overview-tab/AddMemberModal.tsxpackages/web/src/stores/feedbackStore.tspackages/web/src/project/actions.tspackages/web/src/components/dashboard/ContactPrompt.tsxpackages/web/src/config/sentry.tspackages/web/src/lib/devPdfPool.tspackages/web/src/__tests__/server/helpers.tspackages/web/src/components/project/CreateProjectModal.tsxpackages/web/src/server/functions/__tests__/feedback.server.test.tspackages/web/src/components/feedback/FeedbackDialog.tsxpackages/web/src/components/checklist/LocalChecklistView.tsxpackages/web/src/components/checklist/CreateLocalChecklist.tsxpackages/web/src/lib/analytics.tspackages/db/src/schema.tspackages/web/src/server/functions/feedback.server.tspackages/web/src/components/layout/AppNavbar.tsxpackages/web/src/components/EarlyAccessBanner.tsx
packages/web/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/**/*.{tsx,ts}: Use lucide-react for the icon library
Use TanStack Query for server state management (useQuery,useMutation)
Import Zustand stores directly from@/stores/instead of prop-drilling shared state
AvoiduseMemooruseCallback- let the React Compiler handle memoization
Prefer newer React primitives where possible, always on the latest version
UseuseEffectEventfor stable event handler references inside effects
UseuseLayoutEffectfor DOM measurements before paint
UsestartTransition/useTransitionfor non-urgent state updates
UseuseDeferredValuefor deferring expensive re-renders
UseuseSyncExternalStorefor external store subscriptions (e.g., Yjs awareness)
UseuseId()for unique IDs on form elements (radio buttons, checkboxes)
Never prop-drill shared state - import Zustand stores directly
Files:
packages/web/src/routes/__root.tsxpackages/web/src/server/functions/feedback.functions.tspackages/web/src/components/project/reconcile-tab/ReconciliationWrapper.tsxpackages/web/src/components/project/overview-tab/AddMemberModal.tsxpackages/web/src/stores/feedbackStore.tspackages/web/src/project/actions.tspackages/web/src/components/dashboard/ContactPrompt.tsxpackages/web/src/config/sentry.tspackages/web/src/lib/devPdfPool.tspackages/web/src/__tests__/server/helpers.tspackages/web/src/components/project/CreateProjectModal.tsxpackages/web/src/server/functions/__tests__/feedback.server.test.tspackages/web/src/components/feedback/FeedbackDialog.tsxpackages/web/src/components/checklist/LocalChecklistView.tsxpackages/web/src/components/checklist/CreateLocalChecklist.tsxpackages/web/src/lib/analytics.tspackages/web/src/server/functions/feedback.server.tspackages/web/src/components/layout/AppNavbar.tsxpackages/web/src/components/EarlyAccessBanner.tsx
packages/web/src/routes/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use TanStack Router with file-based routing (
createFileRoute)
Files:
packages/web/src/routes/__root.tsx
packages/web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Path aliases:
@/maps topackages/web/src/
Files:
packages/web/src/routes/__root.tsxpackages/web/src/server/functions/feedback.functions.tspackages/web/src/components/project/reconcile-tab/ReconciliationWrapper.tsxpackages/web/src/components/project/overview-tab/AddMemberModal.tsxpackages/web/src/stores/feedbackStore.tspackages/web/src/project/actions.tspackages/web/src/components/dashboard/ContactPrompt.tsxpackages/web/src/config/sentry.tspackages/web/src/lib/devPdfPool.tspackages/web/src/__tests__/server/helpers.tspackages/web/src/components/project/CreateProjectModal.tsxpackages/web/src/server/functions/__tests__/feedback.server.test.tspackages/web/src/components/feedback/FeedbackDialog.tsxpackages/web/src/components/checklist/LocalChecklistView.tsxpackages/web/src/components/checklist/CreateLocalChecklist.tsxpackages/web/src/lib/analytics.tspackages/web/src/server/functions/feedback.server.tspackages/web/src/components/layout/AppNavbar.tsxpackages/web/src/components/EarlyAccessBanner.tsx
packages/workers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/workers/**/*.{ts,tsx}: Use Zod for schema and input validation (backend)
Use Drizzle ORM for ALL database interactions and migrations
Use Better-Auth for authentication and user management
Never bypass Drizzle for database access
Files:
packages/workers/src/lib/mock-templates.ts
packages/web/src/components/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use shadcn/ui for UI components (Radix-based, in
@/components/ui/)
Files:
packages/web/src/components/project/reconcile-tab/ReconciliationWrapper.tsxpackages/web/src/components/project/overview-tab/AddMemberModal.tsxpackages/web/src/components/dashboard/ContactPrompt.tsxpackages/web/src/components/project/CreateProjectModal.tsxpackages/web/src/components/feedback/FeedbackDialog.tsxpackages/web/src/components/checklist/LocalChecklistView.tsxpackages/web/src/components/checklist/CreateLocalChecklist.tsxpackages/web/src/components/layout/AppNavbar.tsxpackages/web/src/components/EarlyAccessBanner.tsx
packages/web/src/stores/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/stores/**/*.{tsx,ts}: Use Zustand for client state management (stores in@/stores/)
Shared state lives in Zustand stores underpackages/web/src/stores/
Files:
packages/web/src/stores/feedbackStore.ts
🔇 Additional comments (25)
packages/workers/src/lib/mock-templates.ts (1)
1612-1613: LGTM!packages/web/src/components/dashboard/ContactPrompt.tsx (1)
11-15: LGTM!packages/web/src/lib/devPdfPool.ts (1)
41-41: LGTM!packages/web/src/lib/analytics.ts (1)
1-16: LGTM!packages/web/src/components/checklist/CreateLocalChecklist.tsx (1)
25-25: LGTM!Also applies to: 83-86
packages/web/src/components/checklist/LocalChecklistView.tsx (1)
24-24: LGTM!Also applies to: 106-106
packages/web/src/components/project/CreateProjectModal.tsx (1)
37-37: LGTM!Also applies to: 101-101
packages/web/src/components/project/overview-tab/AddMemberModal.tsx (1)
35-35: LGTM!Also applies to: 139-139
packages/web/src/components/project/reconcile-tab/ReconciliationWrapper.tsx (1)
26-26: LGTM!Also applies to: 407-414
packages/web/src/project/actions.ts (1)
7-7: LGTM!Also applies to: 48-48
packages/db/src/schema.ts (1)
15-15: LGTM!Also applies to: 369-387, 412-412, 464-466
packages/shared/src/ids.ts (1)
64-66: LGTM!packages/web/migrations/0004_lying_frog_thor.sql (1)
1-12: LGTM!packages/web/migrations/meta/0004_snapshot.json (1)
126-202: LGTM!packages/web/src/__tests__/server/helpers.ts (1)
125-126: LGTM!packages/web/src/server/functions/feedback.functions.ts (1)
6-22: LGTM!packages/web/src/server/functions/__tests__/feedback.server.test.ts (1)
1-153: LGTM!packages/web/src/server/functions/feedback.server.ts (1)
90-94: 🔒 Security & PrivacyStrip CR/LF from the feedback email subject.
data.message.slice(0, 60)still preserves embedded\r/\n, so a raw-header transport could turn this into extraCc/Bccheaders. Normalize control characters before buildingsubject, or have the mail layer reject newline-bearing values.packages/web/migrations/meta/_journal.json (1)
32-39: LGTM!packages/web/src/stores/feedbackStore.ts (1)
1-21: LGTM!packages/web/src/components/feedback/FeedbackDialog.tsx (1)
9-149: LGTM on the rest of the component (category chips, textarea, bug-report disclosure text, success state) — matches PR objectives closely.packages/web/src/components/EarlyAccessBanner.tsx (1)
2-29: LGTM!packages/web/src/components/layout/AppNavbar.tsx (1)
10-10: LGTM!Also applies to: 35-35, 76-76, 165-165
packages/web/src/routes/__root.tsx (1)
9-9: LGTM!Also applies to: 224-228
packages/web/src/config/sentry.ts (1)
76-79: 🎯 Functional CorrectnessNo schema change needed for
replayId
getReplayId()returnsstring | undefinedin the JS SDK, andz.string().max(100).optional()already matches that shape.> Likely an incorrect or invalid review comment.
| const handleOpenChange = (open: boolean) => { | ||
| if (!open) { | ||
| close(); | ||
| setMessage(''); | ||
| setFormState('idle'); | ||
| } | ||
| }; | ||
|
|
||
| const handleSubmit = async () => { | ||
| if (!message.trim() || formState === 'sending') return; | ||
| setFormState('sending'); | ||
|
|
||
| try { | ||
| await submitFeedback({ | ||
| data: { | ||
| category, | ||
| message: message.trim(), | ||
| context: { | ||
| route: window.location.pathname, | ||
| userAgent: navigator.userAgent, | ||
| viewport: `${window.innerWidth}x${window.innerHeight}`, | ||
| replayId: category === 'bug' ? getSentryReplayId() : undefined, | ||
| }, | ||
| }, | ||
| }); | ||
| track('Feedback:Submitted', { category }); | ||
| setFormState('sent'); | ||
| setTimeout(() => handleOpenChange(false), CLOSE_AFTER_SENT_MS); | ||
| } catch { | ||
| setFormState('error'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Stale response can leak into a later dialog session.
If the dialog is closed (Escape/click-outside) while submitFeedback is still in flight, handleOpenChange(false) resets state to idle. When the pending request later resolves, its .then unconditionally calls setFormState('sent') and schedules a new auto-close timeout — even though that submission attempt was already abandoned. Reopening the dialog in that window shows a stale "Thanks for the feedback" screen and then auto-closes on its own.
🔧 Proposed fix using a submission-id guard
+ const submitIdRef = useRef(0);
+
const handleSubmit = async () => {
if (!message.trim() || formState === 'sending') return;
setFormState('sending');
+ const submitId = ++submitIdRef.current;
try {
await submitFeedback({
data: {
category,
message: message.trim(),
context: {
route: window.location.pathname,
userAgent: navigator.userAgent,
viewport: `${window.innerWidth}x${window.innerHeight}`,
replayId: category === 'bug' ? getSentryReplayId() : undefined,
},
},
});
+ if (submitId !== submitIdRef.current) return; // superseded by a later open/close
track('Feedback:Submitted', { category });
setFormState('sent');
setTimeout(() => handleOpenChange(false), CLOSE_AFTER_SENT_MS);
} catch {
+ if (submitId !== submitIdRef.current) return;
setFormState('error');
}
};And bump submitIdRef.current inside handleOpenChange when closing, so any in-flight submission is invalidated:
const handleOpenChange = (open: boolean) => {
if (!open) {
close();
setMessage('');
setFormState('idle');
+ submitIdRef.current++;
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleOpenChange = (open: boolean) => { | |
| if (!open) { | |
| close(); | |
| setMessage(''); | |
| setFormState('idle'); | |
| } | |
| }; | |
| const handleSubmit = async () => { | |
| if (!message.trim() || formState === 'sending') return; | |
| setFormState('sending'); | |
| try { | |
| await submitFeedback({ | |
| data: { | |
| category, | |
| message: message.trim(), | |
| context: { | |
| route: window.location.pathname, | |
| userAgent: navigator.userAgent, | |
| viewport: `${window.innerWidth}x${window.innerHeight}`, | |
| replayId: category === 'bug' ? getSentryReplayId() : undefined, | |
| }, | |
| }, | |
| }); | |
| track('Feedback:Submitted', { category }); | |
| setFormState('sent'); | |
| setTimeout(() => handleOpenChange(false), CLOSE_AFTER_SENT_MS); | |
| } catch { | |
| setFormState('error'); | |
| } | |
| }; | |
| const submitIdRef = useRef(0); | |
| const handleOpenChange = (open: boolean) => { | |
| if (!open) { | |
| close(); | |
| setMessage(''); | |
| setFormState('idle'); | |
| submitIdRef.current++; | |
| } | |
| }; | |
| const handleSubmit = async () => { | |
| if (!message.trim() || formState === 'sending') return; | |
| setFormState('sending'); | |
| const submitId = ++submitIdRef.current; | |
| try { | |
| await submitFeedback({ | |
| data: { | |
| category, | |
| message: message.trim(), | |
| context: { | |
| route: window.location.pathname, | |
| userAgent: navigator.userAgent, | |
| viewport: `${window.innerWidth}x${window.innerHeight}`, | |
| replayId: category === 'bug' ? getSentryReplayId() : undefined, | |
| }, | |
| }, | |
| }); | |
| if (submitId !== submitIdRef.current) return; // superseded by a later open/close | |
| track('Feedback:Submitted', { category }); | |
| setFormState('sent'); | |
| setTimeout(() => handleOpenChange(false), CLOSE_AFTER_SENT_MS); | |
| } catch { | |
| if (submitId !== submitIdRef.current) return; | |
| setFormState('error'); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/web/src/components/feedback/FeedbackDialog.tsx` around lines 45 -
76, The pending submission in FeedbackDialog can still update state after the
dialog has been dismissed, causing stale sent state to leak into a later
session. Add a submission/session guard in handleSubmit and handleOpenChange so
closing the dialog invalidates any in-flight request before it resolves. Use a
ref-based submission id in FeedbackDialog, increment it when starting submit and
again inside handleOpenChange(false), then only run the success path
(setFormState('sent') and the auto-close timeout) if the id still matches.
| const [{ count: recentCount }] = await db | ||
| .select({ count: count() }) | ||
| .from(feedback) | ||
| .where( | ||
| and(eq(feedback.userId, userId), gt(feedback.createdAt, new Date(Date.now() - 3600_000))), | ||
| ); | ||
| if (recentCount >= MAX_SUBMISSIONS_PER_HOUR) { | ||
| throwDomainError(SYSTEM_ERRORS.RATE_LIMITED); | ||
| } | ||
|
|
||
| const id = crypto.randomUUID() as FeedbackId; | ||
| await db.insert(feedback).values({ | ||
| id, | ||
| userId, | ||
| category: data.category, | ||
| message: data.message, | ||
| context: data.context ? JSON.stringify(data.context) : null, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
Does Cloudflare D1 support db.transaction() with drizzle-orm, or is a workaround needed for check-then-insert patterns?
💡 Result:
Cloudflare D1 does not support standard SQL transactions (e.g., BEGIN TRANSACTION or SAVEPOINT), and attempting to use db.transaction() with Drizzle ORM on D1 will typically fail or result in errors [1][2][3][4]. Because D1 operates in auto-commit mode, it does not provide the ACID guarantees associated with traditional database transactions [5][3][6]. To handle atomic operations or check-then-insert patterns, you must use D1's native db.batch() API instead of db.transaction() [3][4][6][7]. Key Workarounds and Patterns: 1. Use db.batch: The batch API allows you to send multiple SQL statements in a single request [3][6][7]. D1 executes these sequentially and atomistically; if one statement fails, the entire batch sequence is aborted [3][4]. However, because batch is not a traditional transaction, you generally cannot use the results of one statement to influence the logic of subsequent statements within the same batch [4][6]. 2. Pre-generate IDs: Since you cannot rely on the result of a previous insert (e.g., a generated ID) within the same db.batch() call, the recommended practice is to generate IDs (such as UUIDs) on the client side before executing the batch [6][7]. 3. Leverage.returning: For single-statement operations, you can use .returning() (supported by D1) to retrieve inserted data immediately, which helps minimize the need for multi-step checks in some scenarios [6][7]. In summary, db.transaction() is not supported, and db.batch() is the required mechanism for grouping operations in Cloudflare D1 [5][3][4][6].
Citations:
- 1: [BUG]: Cloudflare D1 transaction not supported drizzle-team/drizzle-orm#2463
- 2: [BUG]: Cloudflare D1 transactions are made incorrectly drizzle-team/drizzle-orm#4212
- 3: [BUG]: Transactions don't work with D1 drizzle-team/drizzle-orm#758
- 4: https://www.answeroverflow.com/m/1402580672937459803
- 5: https://registry.npmjs.org/%40drizzle-adapter%2Fd1
- 6: https://firdausng.com/posts/d1-cloudflare-with-drizzle
- 7: https://dev.to/whoffagents/cloudflare-d1-drizzle-orm-sqlite-at-the-edge-without-the-pain-255i
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and surrounding server functions.
git ls-files 'packages/web/src/server/functions/feedback.server.ts' \
'packages/web/src/server/functions/*.ts' \
'packages/web/src/server/**/*.ts'
echo
echo "---- outline ----"
ast-grep outline packages/web/src/server/functions/feedback.server.ts --view expanded
echo
echo "---- targeted read ----"
sed -n '1,220p' packages/web/src/server/functions/feedback.server.tsRepository: InfinityBowman/corates
Length of output: 9799
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- feedback test outline ----"
ast-grep outline packages/web/src/server/functions/__tests__/feedback.server.test.ts --view expanded
echo
echo "---- feedback tests ----"
sed -n '1,260p' packages/web/src/server/functions/__tests__/feedback.server.test.ts
echo
echo "---- feedback schema ----"
sed -n '1,260p' packages/web/src/server/lib/dbTables.tsRepository: InfinityBowman/corates
Length of output: 6346
Rate-limit check needs atomicity
count() and insert() are separate statements, so two concurrent submissions from the same user can both pass the cap before either row is written. That can exceed the 5/hour limit and spam the inbox; on D1 this likely needs per-user serialization or a different atomic counter pattern.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/web/src/server/functions/feedback.server.ts` around lines 35 - 52,
The rate-limit logic in feedback.server.ts is non-atomic because the existing
count() query and db.insert(feedback) run as separate steps, so concurrent
requests can both pass MAX_SUBMISSIONS_PER_HOUR. Update the submit flow in the
feedback server function to use an atomic per-user guard or serialization
approach around the recent submission check and insert, keeping the existing
symbols db, feedback, recentCount, MAX_SUBMISSIONS_PER_HOUR, and
throwDomainError in view while changing the implementation so only one
submission decision can happen at a time for a user.
Closes #525
Summary
Adds an in-app feedback widget for early-access users plus a small usage-analytics pass. No new vendors: feedback is stored in D1 and emailed via the existing contact queue, bug reports link to existing Sentry session replays, and usage tracking extends the self-hosted Plausible custom events.
Feedback widget
feedbacktable (migration 0004) withFeedbackIdbrand;statuscolumn (new/reviewed) is dormant until a triage view is needed.submitFeedbackserver function following the contact pattern: auth middleware, Zod validation, insert, notification email. No rate-limiting infra - a single COUNT on the feedback table caps submissions at 5/user/hour purely to protect the notification inbox. Email failure after insert is logged, not surfaced, since the feedback is already saved.FeedbackDialogmounted once in the root layout, opened via a newfeedbackStore: Bug/Idea/Other chips, textarea, success state with auto-close. Auto-captures route, user agent, viewport; bug reports silently attach the Sentry replay id (disclosed in small print)./contactwhen signed out on public pages).Analytics
track(event, props)util wrappingwindow.plausible; theWindowtype declaration moved out ofCreateLocalChecklist.tsxand existing call sites migrated.Project:Created,Checklist:Created(project checklist action funnel),Checklist:Completed(reconciled checklist finalized),Collaborator:Invited(email/direct prop),Feedback:Submitted(category prop).Signupevent: user counts are already in the admin dashboard and a client-side event would miss OAuth signups.Testing
submitFeedback(storage, context handling, email-failure tolerance, hourly cap, cap window, per-user isolation); full suite green (329 tests).Summary by CodeRabbit
New Features
Bug Fixes