Add cancellation feedback feature for subscription management#938
Merged
Add cancellation feedback feature for subscription management#938
Conversation
- Introduced a new cancellation feedback table in the database to collect user insights on subscription cancellations, aimed at reducing churn. - Updated server API to handle cancellation feedback submissions and integrated it into the Stripe routes. - Enhanced the PaidPlan component to include a CancellationDialog for users to provide feedback when canceling their subscriptions. - Added the @calcom/embed-react dependency to the client for improved integration with the cancellation feature.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a subscription cancellation feedback flow: a 3-step client dialog to collect reasons and retention choices, a client hook to submit feedback, a server endpoint and route to persist entries, a new database table to store feedback records, and one client dependency. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Dialog as "CancellationDialog (Client)"
participant Hook as "useSubmitCancellationFeedback (Client)"
participant Server as "Server / POST /stripe/cancellation-feedback"
participant DB as "Database (cancellation_feedback)"
User->>Dialog: Open dialog, select reason & details
Dialog->>Dialog: Show retention offers (step 2)
User->>Dialog: Accept/decline or proceed to confirm
Dialog->>Hook: Submit feedback payload
Hook->>Server: POST /stripe/cancellation-feedback
Server->>Server: Authenticate & authorize owner
Server->>Server: Validate body (organizationId, reason, outcome)
Server->>DB: Insert cancellation_feedback record
DB-->>Server: Insert result
Server-->>Hook: 200 OK { success: true }
Hook-->>Dialog: Report success
Dialog-->>User: Close / continue to portal or plan change
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/components/subscription/PaidPlain/CancellationDialog.tsx`:
- Around line 82-125: The retain handlers (handleKeepPlan, handleSwitchToAnnual,
handleDowngrade) call submitFeedback.mutate() then immediately
toast/close/navigate which can swallow failures, and the "Keep My Plan" branch
currently skips submission; change these to wait for the submission to complete
(use submitFeedback.mutate with onSuccess/onError callbacks or await the
mutation promise) before calling toast/resetAndClose/onChangePlan, and make the
"Keep My Plan" branch perform the same submitFeedback.mutate call (with the
appropriate retentionOfferShown value and retentionOfferAccepted true) so
recovered users are reliably written to cancellation_feedback.
- Around line 127-145: The cancellation flow currently waits for
submitFeedback.mutate to succeed before calling onProceedToStripe inside
handleFinalCancel; instead ensure the Stripe portal is opened regardless of
feedback persistence by invoking onProceedToStripe outside of the mutate
onSuccess handler (either call it immediately after submitFeedback.mutate or use
mutate's onSettled/onError to always call it). Update handleFinalCancel so
submitFeedback.mutate still attempts to send the feedback (using the existing
payload) but does not gate or conditionally call onProceedToStripe based on
mutate success; keep references to submitFeedback.mutate, handleFinalCancel, and
onProceedToStripe to locate the change.
- Around line 31-38: The component currently hardcodes all user-facing strings
(e.g., the REASONS array and dialog/offer labels used in CancellationDialog,
plus text in functions/render blocks around lines referenced) instead of using
next-intl; import and call useTranslations() at the top of CancellationDialog,
replace every literal string (notably the REASONS entries and all visible
labels/buttons/headers in the dialog and retention offers) with t('key')
lookups, and add corresponding keys to messages/en.json (and other locales)
under a sensible namespace (e.g., "cancellation"). Ensure the REASONS array uses
t for each label (or maps values to translation keys) and update any JSX text,
button labels, and help copy between the noted ranges to use those translation
keys.
In `@client/src/components/subscription/PaidPlain/PaidPlan.tsx`:
- Around line 93-94: handleCancelSubscription currently only sets
showCancellationDialog and becomes a silent no-op when organizationId is
missing; restore the previous behavior by making handleCancelSubscription check
organizationId and if absent call createPortalSession() (which will surface the
toast) otherwise setShowCancellationDialog(true); update the rendering gating
(the showCancellationDialog/organizationId logic used when rendering the
cancellation dialog) to remain unchanged so the dialog only appears when org
exists.
In `@client/src/lib/subscription/useCancellationFeedback.ts`:
- Around line 15-31: The mutation function useSubmitCancellationFeedback's
mutationFn currently calls response.json() before checking response.ok, which
can throw on non-JSON error responses; update mutationFn to first check
response.ok and only attempt to parse JSON for successful responses, and for
error responses either try/catch response.json() to safely extract an error
message (falling back to response.text() or a default message) or read text when
JSON parsing fails, then throw a new Error with that extracted message so errors
from BACKEND_URL/stripe/cancellation-feedback are handled predictably.
In `@server/src/api/stripe/submitCancellationFeedback.ts`:
- Around line 71-76: The catch block in submitCancellationFeedback currently
returns error.message to the client; instead keep the detailed exception logged
server-side (e.g., console.error("Cancellation Feedback Error:", error)) but
change the response body to a generic message without error details: return
reply.status(500).send({ error: "Failed to submit cancellation feedback" }); —
remove or avoid sending error.message or other error internals to the client.
- Around line 6-15: Create a Zod schema that matches the
CancellationFeedbackBody shape and call schema.parse(request.body) at the start
of the submitCancellationFeedback handler before any destructuring; use the
parsed result for the rest of the function so malformed/null/non-object bodies
return a 400 with validation errors instead of throwing. Update the manual
validation branch (previously at the "validation at line 38") to use the parsed
data and remove trust in raw request.body. In the catch block of
submitCancellationFeedback, stop returning details: error.message to the
client—log the error internally (e.g., processLogger.error or console.error) and
return a generic error response/status to avoid leaking ORM/database internals.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8f2f0e34-7a62-4559-832e-e805593a7f89
⛔ Files ignored due to path filters (1)
client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
client/package.jsonclient/src/components/subscription/PaidPlain/CancellationDialog.tsxclient/src/components/subscription/PaidPlain/PaidPlan.tsxclient/src/lib/subscription/useCancellationFeedback.tsserver/src/api/stripe/index.tsserver/src/api/stripe/submitCancellationFeedback.tsserver/src/db/postgres/schema.tsserver/src/index.ts
Comment on lines
+31
to
+38
| const REASONS: { value: CancellationReason; label: string }[] = [ | ||
| { value: "too_expensive", label: "It's too expensive" }, | ||
| { value: "missing_features", label: "Missing features I need" }, | ||
| { value: "not_using", label: "I'm not using it enough" }, | ||
| { value: "too_complex", label: "It's too hard to set up or use" }, | ||
| { value: "switching_competitor", label: "Switching to another tool" }, | ||
| { value: "other", label: "Other reason" }, | ||
| ]; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file exists and get its size
wc -l client/src/components/subscription/PaidPlain/CancellationDialog.tsxRepository: rybbit-io/rybbit
Length of output: 131
🏁 Script executed:
# Read the entire file to understand its structure
cat -n client/src/components/subscription/PaidPlain/CancellationDialog.tsxRepository: rybbit-io/rybbit
Length of output: 17953
🏁 Script executed:
# Check for next-intl imports in the file
rg "next-intl|useTranslations" client/src/components/subscription/PaidPlain/CancellationDialog.tsxRepository: rybbit-io/rybbit
Length of output: 42
🏁 Script executed:
# Check if there are translation files for this component
fd -t f messagesRepository: rybbit-io/rybbit
Length of output: 129
Localize all user-facing text in this cancellation workflow.
This component has hardcoded English strings throughout the entire flow instead of using useTranslations(). The dialog and all retention offers will remain untranslated for non-English locales.
Add useTranslations() from next-intl and move all user-facing strings to the translation files in the messages/ directory (lines 31–38, 159–205, 216–318, 342–425).
This is required by the coding guideline: "client/src/**/*.{ts,tsx}: Use next-intl's 'useTranslations()' hook for i18n; maintain translation files in messages/ directory (en.json, de.json, etc.)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/components/subscription/PaidPlain/CancellationDialog.tsx` around
lines 31 - 38, The component currently hardcodes all user-facing strings (e.g.,
the REASONS array and dialog/offer labels used in CancellationDialog, plus text
in functions/render blocks around lines referenced) instead of using next-intl;
import and call useTranslations() at the top of CancellationDialog, replace
every literal string (notably the REASONS entries and all visible
labels/buttons/headers in the dialog and retention offers) with t('key')
lookups, and add corresponding keys to messages/en.json (and other locales)
under a sensible namespace (e.g., "cancellation"). Ensure the REASONS array uses
t for each label (or maps values to translation keys) and update any JSX text,
button labels, and help copy between the noted ranges to use those translation
keys.
Comment on lines
+93
to
94
| const handleCancelSubscription = () => setShowCancellationDialog(true); | ||
|
|
Contributor
There was a problem hiding this comment.
Avoid the new silent no-op when there is no active organization.
Before this change, canceling always flowed through createPortalSession(), which surfaces a toast when organizationId is missing. Now handleCancelSubscription only flips local state, and the dialog is not rendered unless organizationId exists at Lines 129-138, so the button does nothing in that state.
Also applies to: 129-138
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/components/subscription/PaidPlain/PaidPlan.tsx` around lines 93 -
94, handleCancelSubscription currently only sets showCancellationDialog and
becomes a silent no-op when organizationId is missing; restore the previous
behavior by making handleCancelSubscription check organizationId and if absent
call createPortalSession() (which will surface the toast) otherwise
setShowCancellationDialog(true); update the rendering gating (the
showCancellationDialog/organizationId logic used when rendering the cancellation
dialog) to remain unchanged so the dialog only appears when org exists.
Comment on lines
+15
to
+31
| export function useSubmitCancellationFeedback() { | ||
| return useMutation<{ success: boolean }, Error, SubmitCancellationFeedbackParams>({ | ||
| mutationFn: async (params) => { | ||
| const response = await fetch(`${BACKEND_URL}/stripe/cancellation-feedback`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| credentials: "include", | ||
| body: JSON.stringify(params), | ||
| }); | ||
|
|
||
| const data = await response.json(); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(data.error || "Failed to submit cancellation feedback"); | ||
| } | ||
|
|
||
| return data; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n client/src/lib/subscription/useCancellationFeedback.tsRepository: rybbit-io/rybbit
Length of output: 1286
🏁 Script executed:
# Check for Zod usage patterns in similar files
fd --type f -e ts -e tsx 'use.*\.ts' client/src/lib/ | head -20 | xargs grep -l "zod\|z\." 2>/dev/null | head -10Repository: rybbit-io/rybbit
Length of output: 42
🏁 Script executed:
# Check mutation patterns in React Query hooks
fd --type f -e ts -e tsx 'use.*Mutation' client/src/ | head -10 | xargs grep -A 10 "mutationFn:" 2>/dev/null | head -50Repository: rybbit-io/rybbit
Length of output: 800
🏁 Script executed:
# Look for existing response validation patterns
rg "response\.json\(\)" client/src --type ts --type tsx -B 2 -A 2 | head -60Repository: rybbit-io/rybbit
Length of output: 87
🏁 Script executed:
# Search for Zod imports and usage
rg "from.*zod\|import.*zod" client/src --type ts | head -20Repository: rybbit-io/rybbit
Length of output: 42
🏁 Script executed:
# Search for try/catch patterns in mutations more broadly
rg "try\s*{" client/src/lib --type ts -A 15 | head -80Repository: rybbit-io/rybbit
Length of output: 137
🏁 Script executed:
# Check complete mutation examples with error handling
fd . client/src/lib -name "use*.ts" | xargs cat | grep -A 20 "mutationFn:" | head -100Repository: rybbit-io/rybbit
Length of output: 290
🏁 Script executed:
# Look at API response validation patterns
rg "response\.ok|response\.json" client/src/api --type ts -B 3 -A 3 | head -60Repository: rybbit-io/rybbit
Length of output: 762
🏁 Script executed:
# Search for try/catch with different pattern
rg "try" client/src/lib --type ts -A 10 | head -60Repository: rybbit-io/rybbit
Length of output: 3228
🏁 Script executed:
# Check more complete hook examples for comparison
rg "useMutation" client/src/lib -A 20 --type ts | head -80Repository: rybbit-io/rybbit
Length of output: 6565
🏁 Script executed:
# Check if response.json is guarded elsewhere
rg "response.ok" client/src -B 5 -A 5 --type ts | head -80Repository: rybbit-io/rybbit
Length of output: 5636
🏁 Script executed:
# Verify Zod is actually in dependencies
cat package.json | grep -i zodRepository: rybbit-io/rybbit
Length of output: 104
Wrap response.json() in a try/catch block to handle non-JSON responses.
response.json() at line 25 runs before the response.ok check at line 27. If a proxy/CDN returns non-JSON content (HTML, plain text) on error responses, this throws a SyntaxError that bypasses the intended error handling. Wrap the JSON parsing in try/catch or reorder to check response.ok first.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/lib/subscription/useCancellationFeedback.ts` around lines 15 - 31,
The mutation function useSubmitCancellationFeedback's mutationFn currently calls
response.json() before checking response.ok, which can throw on non-JSON error
responses; update mutationFn to first check response.ok and only attempt to
parse JSON for successful responses, and for error responses either try/catch
response.json() to safely extract an error message (falling back to
response.text() or a default message) or read text when JSON parsing fails, then
throw a new Error with that extracted message so errors from
BACKEND_URL/stripe/cancellation-feedback are handled predictably.
Comment on lines
+6
to
+15
| interface CancellationFeedbackBody { | ||
| organizationId: string; | ||
| reason: string; | ||
| reasonDetails?: string; | ||
| retentionOfferShown?: string; | ||
| retentionOfferAccepted?: boolean; | ||
| outcome: string; | ||
| planNameAtCancellation?: string; | ||
| monthlyEventCountAtCancellation?: number; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -path "*/server/src/api/stripe/submitCancellationFeedback.ts" -type fRepository: rybbit-io/rybbit
Length of output: 113
🏁 Script executed:
cat -n server/src/api/stripe/submitCancellationFeedback.ts | head -100Repository: rybbit-io/rybbit
Length of output: 2892
Add Zod schema validation before destructuring the request body.
request.body is only validated at compile time by TypeScript. At runtime, if the request body is null, undefined, or not an object, the destructuring at line 27 will throw an error before reaching the manual validation at line 38, turning a client error into a 500. Per the backend guidelines, use Zod to parse and validate the payload first.
Additionally, the catch block at line 75 returns details: error.message, which exposes internal error details. Remove this to avoid leaking database or ORM error information.
🧩 Suggested hardening
+import { z } from "zod";
+
+const cancellationFeedbackBodySchema = z.object({
+ organizationId: z.string().min(1),
+ reason: z.string().min(1),
+ reasonDetails: z.string().trim().min(1).optional(),
+ retentionOfferShown: z.string().min(1).optional(),
+ retentionOfferAccepted: z.boolean().optional(),
+ outcome: z.enum(["cancelled", "retained"]),
+ planNameAtCancellation: z.string().min(1).optional(),
+ monthlyEventCountAtCancellation: z.number().int().nonnegative().optional(),
+});
...
- const {
+ const parsedBody = cancellationFeedbackBodySchema.safeParse(request.body);
+ if (!parsedBody.success) {
+ return reply.status(400).send({ error: "Invalid cancellation feedback payload" });
+ }
+
+ const {
organizationId,
reason,
reasonDetails,
retentionOfferShown,
retentionOfferAccepted,
outcome,
planNameAtCancellation,
monthlyEventCountAtCancellation,
- } = request.body;
-
- if (!organizationId || !reason || !outcome) {
- return reply.status(400).send({
- error: "Missing required parameters: organizationId, reason, outcome",
- });
- }
+ } = parsedBody.data; } catch (error: any) {
console.error("Cancellation Feedback Error:", error);
return reply.status(500).send({
error: "Failed to submit cancellation feedback",
- details: error.message,
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/api/stripe/submitCancellationFeedback.ts` around lines 6 - 15,
Create a Zod schema that matches the CancellationFeedbackBody shape and call
schema.parse(request.body) at the start of the submitCancellationFeedback
handler before any destructuring; use the parsed result for the rest of the
function so malformed/null/non-object bodies return a 400 with validation errors
instead of throwing. Update the manual validation branch (previously at the
"validation at line 38") to use the parsed data and remove trust in raw
request.body. In the catch block of submitCancellationFeedback, stop returning
details: error.message to the client—log the error internally (e.g.,
processLogger.error or console.error) and return a generic error response/status
to avoid leaking ORM/database internals.
…nent - Simplified the feedback submission process by creating a reusable feedbackPayload function to streamline data handling for various cancellation scenarios. - Updated the mutation calls in handleKeepPlan, handleSwitchToAnnual, handleDowngrade, and handleFinalCancel to utilize the new feedbackPayload function, improving code maintainability. - Removed redundant code in the CancellationDialog component for better clarity and efficiency.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
server/src/api/stripe/submitCancellationFeedback.ts (1)
6-42:⚠️ Potential issue | 🟠 MajorValidate and parse the payload with Zod before destructuring.
request.bodyis destructured before runtime validation, so malformed/null bodies can fail before your 400 branch. Parse first, then use parsed data.Suggested hardening
+import { z } from "zod"; + +const cancellationFeedbackBodySchema = z.object({ + organizationId: z.string().min(1), + reason: z.string().min(1), + reasonDetails: z.string().trim().min(1).optional(), + retentionOfferShown: z.string().min(1).optional(), + retentionOfferAccepted: z.boolean().optional(), + outcome: z.enum(["cancelled", "retained"]), + planNameAtCancellation: z.string().min(1).optional(), + monthlyEventCountAtCancellation: z.number().int().nonnegative().optional(), +}); ... - const { + const parsedBody = cancellationFeedbackBodySchema.safeParse(request.body); + if (!parsedBody.success) { + return reply.status(400).send({ error: "Invalid cancellation feedback payload" }); + } + + const { organizationId, reason, reasonDetails, retentionOfferShown, retentionOfferAccepted, outcome, planNameAtCancellation, monthlyEventCountAtCancellation, - } = request.body; + } = parsedBody.data;Based on learnings: Backend: Use Fastify, Drizzle ORM with Postgres, ClickHouse, and Zod.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/api/stripe/submitCancellationFeedback.ts` around lines 6 - 42, In submitCancellationFeedback, avoid destructuring request.body before runtime validation—create a Zod schema (e.g., CancellationFeedbackSchema) matching CancellationFeedbackBody, call CancellationFeedbackSchema.parse or safeParse on request.body at the start of the function, handle validation errors by returning reply.status(400).send(...) for malformed/null payloads, then destructure/use the validated/parsed object (not raw request.body) for organizationId, reason, outcome, etc.; ensure optional fields have proper Zod .optional() types and use the parsed result throughout the function.client/src/components/subscription/PaidPlain/CancellationDialog.tsx (3)
372-375:⚠️ Potential issue | 🟠 MajorStep-3 “Keep My Plan” still skips feedback submission.
This retained path closes the dialog without writing a retained outcome, so recovered users from the final confirmation step can be undercounted.
Record retained outcome on final-step keep
- <Button variant="success" onClick={resetAndClose}> + <Button variant="success" onClick={() => handleKeepPlan(getRetentionOfferType())}> Keep My Plan </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/subscription/PaidPlain/CancellationDialog.tsx` around lines 372 - 375, The "Keep My Plan" button currently calls resetAndClose and skips writing a retained outcome; update CancellationDialog so that clicking "Keep My Plan" records a retained outcome before closing. Add or reuse a handler (e.g., handleKeepPlan or modify resetAndClose) to call the existing feedback/telemetry function (e.g., submitFeedback, submitSurveyOutcome, recordRetentionOutcome—whichever is used elsewhere in this component) with outcome "retained" and any required metadata, await/handle its result, then call resetAndClose to close the dialog; finally wire the Button's onClick to this new handler instead of resetAndClose.
31-38:⚠️ Potential issue | 🟠 MajorLocalize all user-facing copy with
useTranslations()instead of hardcoded English.This dialog still hardcodes visible strings, so non-English locales won’t translate this flow.
As per coding guidelines:
client/src/**/*.{ts,tsx}: Use next-intl'suseTranslations()hook for i18n; maintain translation files inmessages/directory.Also applies to: 137-185, 188-334, 341-404
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/subscription/PaidPlain/CancellationDialog.tsx` around lines 31 - 38, Replace hardcoded labels in the REASONS array with translated strings using next-intl's useTranslations() in CancellationDialog.tsx: import and call useTranslations (e.g., const t = useTranslations('CancellationDialog')) and change each label to t('reason.too_expensive') / t('reason.missing_features') / etc.; ensure keys map to a nested "reason" namespace in your messages files and update other visible strings in the component (lines referenced: 137-185, 188-334, 341-404) to use t('...') as well so all user-facing copy is localized via the same translation keys.
93-118:⚠️ Potential issue | 🟠 MajorUse
onSuccessandonErrorcallbacks instead ofonSettledfor mutation side effects.All three handlers (
handleKeepPlan,handleSwitchToAnnual,handleDowngrade) misuseonSettled, causing success UX (toasts, state resets, plan changes) to execute even when mutations fail.handleKeepPlanshows a success toast on both success and failure cases, silently misreporting failed writes. The other handlers executeresetAndClose()andonChangePlan()regardless of success/failure.Safer mutation callbacks
const handleKeepPlan = (offerShown: string) => { submitFeedback.mutate(feedbackPayload(offerShown, true, "retained"), { - onSettled: () => { + onSuccess: () => { toast.success("Great, we're glad you're staying!"); resetAndClose(); }, + onError: () => { + toast.error("Could not save feedback. Please try again."); + }, }); }; const handleSwitchToAnnual = () => { submitFeedback.mutate(feedbackPayload("switch_annual", true, "retained"), { - onSettled: () => { + onSuccess: () => { resetAndClose(); onChangePlan(); }, }); }; const handleDowngrade = () => { submitFeedback.mutate(feedbackPayload("downgrade", true, "retained"), { - onSettled: () => { + onSuccess: () => { resetAndClose(); onChangePlan(); }, }); };Per React Query best practices and coding guidelines: handle errors via
onErrorcallbacks; let React Query surface API errors appropriately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/subscription/PaidPlain/CancellationDialog.tsx` around lines 93 - 118, The three handlers (handleKeepPlan, handleSwitchToAnnual, handleDowngrade) currently use submitFeedback.mutate with onSettled causing success UI to run even on failure; change them to use onSuccess for post-success actions (show toast, call resetAndClose(), and call onChangePlan() where applicable) and add onError to handle failures (e.g., show an error toast and avoid calling resetAndClose() or onChangePlan()). Specifically, move toast.success("Great, we're glad you're staying!") into the onSuccess of handleKeepPlan, move resetAndClose() and onChangePlan() into onSuccess for handleSwitchToAnnual and handleDowngrade, and implement onError callbacks for all three that surface the error (e.g., toast.error with the error message) without performing success-only state changes; keep submitFeedback.mutate and feedbackPayload usage unchanged.
🧹 Nitpick comments (1)
server/src/api/stripe/submitCancellationFeedback.ts (1)
71-72: Avoidanyincatch; narrow fromunknownand log structurally.Using
error: anyweakens strict typing and makes error handling less safe.Typed catch/logging example
- } catch (error: any) { - console.error("Cancellation Feedback Error:", error); + } catch (error: unknown) { + request.log.error({ err: error }, "Cancellation feedback submission failed"); return reply.status(500).send({ error: "Failed to submit cancellation feedback", }); }As per coding guidelines:
**/*.{ts,tsx}: Use TypeScript with strict typing throughout both client and server; Error handling: Use try/catch blocks with specific error types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/api/stripe/submitCancellationFeedback.ts` around lines 71 - 72, Change the catch clause to accept unknown (catch (error: unknown)) and then narrow it before logging: use an instanceof Error check to log structured fields like message and stack, and for non-Error values log the raw value under a well-named key; update the catch in submitCancellationFeedback.ts (the catch block currently using error: any) to perform these type-guards and structured console/processLogger calls so typing is strict and logs are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/components/subscription/PaidPlain/CancellationDialog.tsx`:
- Around line 125-133: The getRetentionOfferType() function returns "general"
for reason === "not_using" but the UI renders the setup-help offer for that
path, causing analytics mismatch; update getRetentionOfferType() so that when
reason === "not_using" it returns "setup_help" (keep other branches unchanged)
so the recorded retentionOfferShown matches the rendered offer in the
cancellation flow (e.g., when invoked from the step-3 cancellation handler).
---
Duplicate comments:
In `@client/src/components/subscription/PaidPlain/CancellationDialog.tsx`:
- Around line 372-375: The "Keep My Plan" button currently calls resetAndClose
and skips writing a retained outcome; update CancellationDialog so that clicking
"Keep My Plan" records a retained outcome before closing. Add or reuse a handler
(e.g., handleKeepPlan or modify resetAndClose) to call the existing
feedback/telemetry function (e.g., submitFeedback, submitSurveyOutcome,
recordRetentionOutcome—whichever is used elsewhere in this component) with
outcome "retained" and any required metadata, await/handle its result, then call
resetAndClose to close the dialog; finally wire the Button's onClick to this new
handler instead of resetAndClose.
- Around line 31-38: Replace hardcoded labels in the REASONS array with
translated strings using next-intl's useTranslations() in
CancellationDialog.tsx: import and call useTranslations (e.g., const t =
useTranslations('CancellationDialog')) and change each label to
t('reason.too_expensive') / t('reason.missing_features') / etc.; ensure keys map
to a nested "reason" namespace in your messages files and update other visible
strings in the component (lines referenced: 137-185, 188-334, 341-404) to use
t('...') as well so all user-facing copy is localized via the same translation
keys.
- Around line 93-118: The three handlers (handleKeepPlan, handleSwitchToAnnual,
handleDowngrade) currently use submitFeedback.mutate with onSettled causing
success UI to run even on failure; change them to use onSuccess for post-success
actions (show toast, call resetAndClose(), and call onChangePlan() where
applicable) and add onError to handle failures (e.g., show an error toast and
avoid calling resetAndClose() or onChangePlan()). Specifically, move
toast.success("Great, we're glad you're staying!") into the onSuccess of
handleKeepPlan, move resetAndClose() and onChangePlan() into onSuccess for
handleSwitchToAnnual and handleDowngrade, and implement onError callbacks for
all three that surface the error (e.g., toast.error with the error message)
without performing success-only state changes; keep submitFeedback.mutate and
feedbackPayload usage unchanged.
In `@server/src/api/stripe/submitCancellationFeedback.ts`:
- Around line 6-42: In submitCancellationFeedback, avoid destructuring
request.body before runtime validation—create a Zod schema (e.g.,
CancellationFeedbackSchema) matching CancellationFeedbackBody, call
CancellationFeedbackSchema.parse or safeParse on request.body at the start of
the function, handle validation errors by returning reply.status(400).send(...)
for malformed/null payloads, then destructure/use the validated/parsed object
(not raw request.body) for organizationId, reason, outcome, etc.; ensure
optional fields have proper Zod .optional() types and use the parsed result
throughout the function.
---
Nitpick comments:
In `@server/src/api/stripe/submitCancellationFeedback.ts`:
- Around line 71-72: Change the catch clause to accept unknown (catch (error:
unknown)) and then narrow it before logging: use an instanceof Error check to
log structured fields like message and stack, and for non-Error values log the
raw value under a well-named key; update the catch in
submitCancellationFeedback.ts (the catch block currently using error: any) to
perform these type-guards and structured console/processLogger calls so typing
is strict and logs are consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f7fab35-61b9-4748-968b-b9efe8df7305
📒 Files selected for processing (2)
client/src/components/subscription/PaidPlain/CancellationDialog.tsxserver/src/api/stripe/submitCancellationFeedback.ts
- Changed the return value for the "not_using" reason from "general" to "setup_help" to better reflect user feedback and improve guidance for users canceling their subscriptions.
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
1 participant
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 by CodeRabbit
New Features
Chores