fix: Add validation for required body in CustomizePostDialog#46
fix: Add validation for required body in CustomizePostDialog#46amaldinesh7 wants to merge 4 commits into
Conversation
Show error message and disable Save button when a subreddit requires a description but the effective body is empty. This provides immediate feedback in the customize dialog rather than waiting for preflight validation. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds client-side body-required validation UI and disables Save when missing; removes runtime enforcement of subreddit body-required in preflight; tightens auth SWR revalidation; integrates Vercel Analytics; replaces checkout success with a polling-based payment confirmation flow; adds streaming mock updates and new end-to-end tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant App as Next.js App
participant API as /api/me
participant SWR as SWR Cache
participant Router
User->>Browser: Land on /checkout/success
Browser->>App: Render success page (usePaymentConfirmation)
App->>API: Poll /api/me every POLL_INTERVAL_MS
API-->>App: Respond with auth (entitlement)
alt entitlement === 'paid'
App->>SWR: mutate('/api/me') to refresh auth
App->>Router: navigate back to app
App-->>User: Show "You're Pro now!" UI
else still pending / timeout
App-->>User: Show "Confirming your upgrade…" loading UI
loop until timeout or paid
App->>API: poll again
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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)
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}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
components/subreddit-picker/CustomizePostDialog.tsx (2)
282-284: Apply same accessibility attributes here.For consistency and accessibility, apply the same
role="alert"attribute to this error message as well.♿ Proposed fix
{bodyError && ( - <p className="text-xs text-red-500 mt-1">{bodyError}</p> + <p id="body-error-global" role="alert" className="text-xs text-red-500 mt-1">{bodyError}</p> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/subreddit-picker/CustomizePostDialog.tsx` around lines 282 - 284, The body error message in CustomizePostDialog (the conditional render using bodyError) is missing accessibility attributes; update the <p> element that displays bodyError to include role="alert" (matching the other error messages) so screen readers announce it immediately; locate the conditional rendering that references the bodyError variable in the CustomizePostDialog component and add role="alert" to that paragraph element.
262-267: Add accessibility attributes for the error message.The error message is not accessible to screen readers. Add
role="alert"oraria-live="polite"to announce the error, and associate it with the textarea usingaria-describedby.♿ Proposed accessibility fix
<div className="flex justify-between items-center mt-1"> {bodyError ? ( - <p className="text-xs text-red-500">{bodyError}</p> + <p id="body-error" role="alert" className="text-xs text-red-500">{bodyError}</p> ) : ( <span /> )}Then add
aria-describedby="body-error"andaria-invalid={!!bodyError}to the Textarea at line 243:<Textarea ref={bodyRef} value={customBody} onChange={(e) => setCustomBody(e.target.value.slice(0, bodyMaxLength))} placeholder="Custom description for this community" className="resize-none min-h-[80px] overflow-hidden pr-14" rows={3} aria-describedby={bodyError ? "body-error" : undefined} aria-invalid={!!bodyError} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/subreddit-picker/CustomizePostDialog.tsx` around lines 262 - 267, The error paragraph for bodyError is not announced to screen readers and not linked to the textarea; update the error element rendered in CustomizePostDialog to include an id (e.g., "body-error") and an accessibility attribute such as role="alert" or aria-live="polite" (use the existing conditional that renders {bodyError} to add these). Then update the Textarea component (the one using bodyRef/customBody) to include aria-describedby={bodyError ? "body-error" : undefined} and aria-invalid={!!bodyError} so the textarea is associated with and reports the error state; keep the existing props and only add these accessibility attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@components/subreddit-picker/CustomizePostDialog.tsx`:
- Around line 282-284: The body error message in CustomizePostDialog (the
conditional render using bodyError) is missing accessibility attributes; update
the <p> element that displays bodyError to include role="alert" (matching the
other error messages) so screen readers announce it immediately; locate the
conditional rendering that references the bodyError variable in the
CustomizePostDialog component and add role="alert" to that paragraph element.
- Around line 262-267: The error paragraph for bodyError is not announced to
screen readers and not linked to the textarea; update the error element rendered
in CustomizePostDialog to include an id (e.g., "body-error") and an
accessibility attribute such as role="alert" or aria-live="polite" (use the
existing conditional that renders {bodyError} to add these). Then update the
Textarea component (the one using bodyRef/customBody) to include
aria-describedby={bodyError ? "body-error" : undefined} and
aria-invalid={!!bodyError} so the textarea is associated with and reports the
error state; keep the existing props and only add these accessibility
attributes.
Temporarily disable the body required validation logic in the validateBody function. This change allows for further testing and adjustments without enforcing the body restriction policy. The commented-out code can be revisited for future implementation. Made-with: Cursor
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dadf937903
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }); | ||
| continue; | ||
| } | ||
| // if (reqs.body_restriction_policy === 'required' && !body.trim()) { |
There was a problem hiding this comment.
Commenting out this branch removes the only global check that blocks submissions when a subreddit requires a description, so validatePreflight can now return canProceed=true for empty bodies and the queue/review flow will allow posting until Reddit rejects it downstream. The new inline check in CustomizePostDialog does not cover this because it only runs if that dialog is opened (and customization is gated by paid/trial in pages/index.tsx), so free users and any non-customized subreddit path lose required-body protection entirely.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/preflightValidation.ts`:
- Around line 275-286: Re-enable the server-side required-body validation in the
preflight routine by restoring the conditional that checks
reqs.body_restriction_policy === 'required' and empty body (body.trim() === ''),
and when triggered push the same issue object back onto issues (code:
'BODY_REQUIRED', severity: 'error', subreddit, message: `r/${subreddit} requires
a description`, suggestion: 'Add a description to your post', field: 'body',
expectedCategory: 'fixable_now') and then continue; locate this logic near the
validatePreflight / preflight validation loop that currently has the commented
block referencing reqs.body_restriction_policy and issues and restore it
unchanged so required-body policy is enforced server-side.
| // if (reqs.body_restriction_policy === 'required' && !body.trim()) { | ||
| // issues.push({ | ||
| // code: 'BODY_REQUIRED', | ||
| // severity: 'error', | ||
| // subreddit, | ||
| // message: `r/${subreddit} requires a description`, | ||
| // suggestion: 'Add a description to your post', | ||
| // field: 'body', | ||
| // expectedCategory: 'fixable_now', | ||
| // }); | ||
| // continue; | ||
| // } |
There was a problem hiding this comment.
Restore server-side/preflight BODY_REQUIRED enforcement.
Line 275-Line 286 currently bypasses required-body validation by commenting out the check. This regresses preflight guarantees and allows invalid payloads to pass validatePreflight when subreddit policy requires a body.
🔧 Proposed fix
- // Body required check
- // if (reqs.body_restriction_policy === 'required' && !body.trim()) {
- // issues.push({
- // code: 'BODY_REQUIRED',
- // severity: 'error',
- // subreddit,
- // message: `r/${subreddit} requires a description`,
- // suggestion: 'Add a description to your post',
- // field: 'body',
- // expectedCategory: 'fixable_now',
- // });
- // continue;
- // }
+ // Body required check
+ if (reqs.body_restriction_policy === 'required' && !body.trim()) {
+ issues.push({
+ code: 'BODY_REQUIRED',
+ severity: 'error',
+ subreddit,
+ message: `r/${subreddit} requires a description`,
+ suggestion: 'Add a description to your post',
+ field: 'body',
+ expectedCategory: 'fixable_now',
+ });
+ continue;
+ }As per coding guidelines, "Never bypass validation" and "Always validate inputs."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/preflightValidation.ts` around lines 275 - 286, Re-enable the server-side
required-body validation in the preflight routine by restoring the conditional
that checks reqs.body_restriction_policy === 'required' and empty body
(body.trim() === ''), and when triggered push the same issue object back onto
issues (code: 'BODY_REQUIRED', severity: 'error', subreddit, message:
`r/${subreddit} requires a description`, suggestion: 'Add a description to your
post', field: 'body', expectedCategory: 'fixable_now') and then continue; locate
this logic near the validatePreflight / preflight validation loop that currently
has the commented block referencing reqs.body_restriction_policy and issues and
restore it unchanged so required-body policy is enforced server-side.
- Install @vercel/analytics and add Analytics component to _app.tsx - Improve checkout success page with payment confirmation polling - Reduce auth revalidation interval to pick up post-payment webhook updates - Add full product demo test spec Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30d813727d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| try { | ||
| const { data } = await axios.get('/api/me'); |
There was a problem hiding this comment.
The new confirmation flow can block indefinitely when /api/me hangs, because the 30s max-duration check only runs before await axios.get('/api/me') and there is no request timeout; in that case the page stays on the spinner with no “Back to app” action exposed. This affects users on flaky/mobile networks where an XHR can stall without rejecting, so the poll loop never reaches the fallback path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pages/checkout/success.tsx (1)
61-61: Convert to const arrow function export for consistency with project rules.Switch
export default function CheckoutSuccess()toconst CheckoutSuccess = () => { ... }; export default CheckoutSuccess;to align with the project's style guidelines, which require const arrow functions instead of function declarations in TSX files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/checkout/success.tsx` at line 61, Change the default exported function declaration CheckoutSuccess to a const arrow function: replace the `export default function CheckoutSuccess()` declaration with `const CheckoutSuccess = () => { ... };` and then export it with `export default CheckoutSuccess;`, updating any references inside the component body as-is to match the new const arrow form.tests/demo/full-product-demo.spec.ts (1)
5-79: Replace fixedpause()sleeps with condition-based waits to improve reliability and reduce execution time.While this is a demo test where some delays may be intentional for visual/animation purposes, Playwright best practice is to replace arbitrary sleeps with UI-state waits. For example, instead of
pause(1500)after clicking the review button, wait for the confirmation dialog or next expected UI element to appear usingexpect().toBeVisible(),toBeEnabled(), orwaitForResponse(). This approach makes the test more reliable in CI and eliminates unnecessary slowdowns (the current pauses total ~10+ seconds).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/demo/full-product-demo.spec.ts` around lines 5 - 79, The test uses many fixed pause(ms) sleeps (pause, e.g., after goto, after expanding General, inside the demoSubreddits loop, after switching to URL mode, after typing title, after clicking the review button, and before assertions) which makes it flaky and slow; replace each arbitrary pause with condition-based waits: use expect(authenticatedPage.getByText(...)).toBeVisible()/toBeEnabled() or authenticatedPage.waitForSelector()/waitForResponse() to wait for the exact UI change or network response you expect (for example wait for the 'pics' text after clicking the General category, wait for toggle button states inside the demoSubreddits loop, wait for the URL/title input to be editable before typing, wait for the review dialog or confirmation button instead of pause(1500), and use waitForResponse or expect('All done!') visibility instead of the final pauses), keeping the calls and helpers like setupMockRoutes and setupQueueContractMock intact and only replacing pause(...) occurrences with the appropriate conditional wait for the related element or response.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pages/checkout/success.tsx`:
- Around line 28-30: The timeout branch in the polling logic currently sets
setReady(true) which shows a confirmed Pro state even when entitlement !==
'paid'; change it to not mark ready on timeout by removing setReady(true) and
instead set a distinct timeout state (e.g., setTimedOut(true) or
setPollingExpired(true)) so the UI can distinguish "timed out" vs "confirmed",
update the render logic to only show confirmed Pro when entitlement === 'paid'
(or isConfirmed flag) and show a timeout/error view when timedOut is true; apply
the same change to the duplicate polling block around the 105-136 area and
reference startRef, MAX_POLL_DURATION_MS, cancelled, setReady, and the
polling/effect function when making edits.
---
Nitpick comments:
In `@pages/checkout/success.tsx`:
- Line 61: Change the default exported function declaration CheckoutSuccess to a
const arrow function: replace the `export default function CheckoutSuccess()`
declaration with `const CheckoutSuccess = () => { ... };` and then export it
with `export default CheckoutSuccess;`, updating any references inside the
component body as-is to match the new const arrow form.
In `@tests/demo/full-product-demo.spec.ts`:
- Around line 5-79: The test uses many fixed pause(ms) sleeps (pause, e.g.,
after goto, after expanding General, inside the demoSubreddits loop, after
switching to URL mode, after typing title, after clicking the review button, and
before assertions) which makes it flaky and slow; replace each arbitrary pause
with condition-based waits: use
expect(authenticatedPage.getByText(...)).toBeVisible()/toBeEnabled() or
authenticatedPage.waitForSelector()/waitForResponse() to wait for the exact UI
change or network response you expect (for example wait for the 'pics' text
after clicking the General category, wait for toggle button states inside the
demoSubreddits loop, wait for the URL/title input to be editable before typing,
wait for the review dialog or confirmation button instead of pause(1500), and
use waitForResponse or expect('All done!') visibility instead of the final
pauses), keeping the calls and helpers like setupMockRoutes and
setupQueueContractMock intact and only replacing pause(...) occurrences with the
appropriate conditional wait for the related element or response.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
contexts/AuthContext.tsxpackage.jsonpages/_app.tsxpages/checkout/success.tsxtests/demo/full-product-demo.spec.ts
✅ Files skipped from review due to trivial changes (1)
- package.json
| if (Date.now() - startRef.current > MAX_POLL_DURATION_MS) { | ||
| if (!cancelled) setReady(true); | ||
| return; |
There was a problem hiding this comment.
Do not mark payment as confirmed on timeout.
When polling exceeds MAX_POLL_DURATION_MS, the code sets ready to true and renders a confirmed Pro success state even if /api/me never returned entitlement === 'paid'. This can produce false success UI.
Proposed fix (separate confirmed vs timeout states)
-const usePaymentConfirmation = () => {
- const [ready, setReady] = useState(false);
+const usePaymentConfirmation = () => {
+ const [status, setStatus] = useState<'checking' | 'confirmed' | 'timeout'>('checking');
const startRef = useRef(Date.now());
@@
- if (Date.now() - startRef.current > MAX_POLL_DURATION_MS) {
- if (!cancelled) setReady(true);
+ if (Date.now() - startRef.current > MAX_POLL_DURATION_MS) {
+ if (!cancelled) setStatus('timeout');
return;
}
@@
if (data?.entitlement === 'paid') {
if (!cancelled) {
mutate(SWR_KEYS.AUTH, data, { revalidate: false });
- setReady(true);
+ setStatus('confirmed');
}
return;
}
@@
- return ready;
+ return status;
};
@@
- const ready = usePaymentConfirmation();
+ const status = usePaymentConfirmation();
+ const ready = status === 'confirmed';Also applies to: 105-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pages/checkout/success.tsx` around lines 28 - 30, The timeout branch in the
polling logic currently sets setReady(true) which shows a confirmed Pro state
even when entitlement !== 'paid'; change it to not mark ready on timeout by
removing setReady(true) and instead set a distinct timeout state (e.g.,
setTimedOut(true) or setPollingExpired(true)) so the UI can distinguish "timed
out" vs "confirmed", update the render logic to only show confirmed Pro when
entitlement === 'paid' (or isConfirmed flag) and show a timeout/error view when
timedOut is true; apply the same change to the duplicate polling block around
the 105-136 area and reference startRef, MAX_POLL_DURATION_MS, cancelled,
setReady, and the polling/effect function when making edits.
Update the E2E flow tests to use the correct button selector pattern. The ReviewPanel button text changed from "Post now" to "Post to N communities" (e.g., "Post to 3 communities"). - Change selector regex from /post now/i to /post to \d+ communit/i - Add dialog visibility wait before clicking the post button - Fix mock streaming response format for queue processing - Update selectSubreddits helper to handle pre-checked state - Add entitlement mock data to bypass free user limits in tests Made-with: Cursor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/flows/helpers.ts (1)
125-180: Consider extracting shared streaming response builder.This
buildStreamResponsefunction duplicates the logic inbuildQueueProcessStreamResponsefromtests/mocks/handlers.ts(lines 166-238). Both construct identical streaming payloads with status, progress, result, waiting, and complete events.Consider extracting a shared utility to reduce duplication and ensure consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/flows/helpers.ts` around lines 125 - 180, The buildStreamResponse function duplicates logic already present in buildQueueProcessStreamResponse; extract a shared helper (e.g., createQueueStreamPayload or buildProcessStreamEvents) and replace both buildStreamResponse and buildQueueProcessStreamResponse to call that helper so status/progress/result/waiting/complete event construction is centralized; update tests to import/use the new utility and ensure it accepts parameters used in both places (mockJobId, subreddits, outcomes, waitSeconds) and returns the same newline-joined string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/flows/helpers.ts`:
- Around line 125-180: The buildStreamResponse function duplicates logic already
present in buildQueueProcessStreamResponse; extract a shared helper (e.g.,
createQueueStreamPayload or buildProcessStreamEvents) and replace both
buildStreamResponse and buildQueueProcessStreamResponse to call that helper so
status/progress/result/waiting/complete event construction is centralized;
update tests to import/use the new utility and ensure it accepts parameters used
in both places (mockJobId, subreddits, outcomes, waitSeconds) and returns the
same newline-joined string.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/flows/core/core-flow.spec.tstests/flows/edge/edge-flow.spec.tstests/flows/helpers.tstests/mocks/handlers.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53b6208d65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }) | ||
| ); | ||
|
|
||
| return lines.join('\n'); |
There was a problem hiding this comment.
The stream returned by setupQueueContractMock is built with lines.join('\n'), which leaves the final {"type":"complete"} record without a line terminator. useQueueJob parses updates by splitting on newlines and only consuming complete lines, so that last event stays buffered and never flips the queue to completed; flows that wait for completion UI (for example All done!) can stall or become flaky when this helper is used.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds inline validation in the CustomizePostDialog when a subreddit requires a description but the effective body content is empty. This provides immediate user feedback in the customize dialog rather than waiting for preflight validation to catch the issue.
Key Changes
body_restriction_policy === 'required') and displays an error message when the effective body is emptyMade with Cursor
Summary by CodeRabbit