filter occupation#1979
Conversation
ignaciojimenezr
temporarily deployed
to
preview-pr-1979
GitHub Actions
Inactive
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
dosubot
Bot
added
the
size:L
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Internal previewPreview URL: https://mcp-inspector-pr-1979.up.railway.app |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe App now queries Convex ( 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mcpjam-inspector/client/src/App.tsx`:
- Around line 1800-1806: The current conditional treats currentUser === null the
same as loading, causing a permanent LoadingScreen; update the check so only
unresolved state (currentUser === undefined or isEnsuringUser) shows
<LoadingScreen /> and handle currentUser === null as a resolved-but-missing user
by rendering a recovery UI (e.g., a retry button that re-invokes the
ensure/user-fetch flow, a "User Missing" message with sign-out/retry options, or
a UserMissingScreen component). Modify the logic around isHostedChatRoute,
isAuthenticated, currentUser and isEnsuringUser to separate undefined (loading)
from null (missing) and wire the missing case to a retry/recovery handler that
calls the same ensureUser/refresh function used elsewhere.
In `@mcpjam-inspector/client/src/components/signup/OccupationGate.tsx`:
- Around line 72-100: The component currently only renders
OCCUPATION_SUGGESTIONS buttons and lacks the free-text path; update the UI in
OccupationGate to add an input field (controlled by a new state like freeText or
occupationInput) alongside the suggestion buttons, wire its onChange to update
that state and clear error (use setError(null)), and change handleSubmit to
prefer the free-text value when non-empty otherwise fall back to selected;
ensure the input and suggestion buttons remain mutually coherent (selecting a
suggestion sets selected via setSelected and clears the input, typing into the
input clears selected) and reuse existing identifiers (selected, setSelected,
error, setError, handleSubmit, OCCUPATION_SUGGESTIONS) so validation and
submission behave correctly.
In `@mcpjam-inspector/client/src/hooks/usePostHogIdentify.ts`:
- Around line 35-37: The occupation value from convexUser is assigned without
trimming, allowing whitespace-only strings to be sent; update the logic in
usePostHogIdentify to trim convexUser.occupation, verify the trimmed string is
non-empty (and a string) before setting personProperties.occupation, and assign
the trimmed value (not the original) to personProperties.occupation to avoid
polluted analytics.
🪄 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: f801c831-5627-4784-bc52-0ae026063eba
📒 Files selected for processing (6)
mcpjam-inspector/client/src/App.tsxmcpjam-inspector/client/src/__tests__/App.hosted-oauth.test.tsxmcpjam-inspector/client/src/components/signup/OccupationGate.tsxmcpjam-inspector/client/src/hooks/__tests__/usePostHogIdentify.test.tsmcpjam-inspector/client/src/hooks/useEnsureDbUser.tsmcpjam-inspector/client/src/hooks/usePostHogIdentify.ts
ignaciojimenezr
temporarily deployed
to
preview-pr-1979
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mcpjam-inspector/client/src/components/signup/OccupationGate.tsx`:
- Around line 72-83: The occupation input lacks an accessible name and its
validation message isn't programmatically associated; update the OccupationGate
form so the Input (the occupation state bound input used in handleSubmit) has a
proper label or aria-label and a stable id, mark it required (required or
aria-required), and render the error message with an id referenced by the
input's aria-describedby; additionally ensure the error element uses an
appropriate live region (e.g., role="alert" or aria-live="assertive") so screen
readers announce the validation failure when setError populates the error state.
- Around line 73-100: The input and suggestion buttons are still interactive
while a save is in flight, causing a mismatch between the shown role and the
value being saved; add a boolean saving state (if one exists, e.g., isSaving) or
wire the existing submit-in-flight flag into this component and use it to
disable the Input and the suggestion buttons and to short-circuit their
onClick/onChange handlers (update references in setOccupation, occupation,
error, and OCCUPATION_SUGGESTIONS usage) so that while handleSubmit is running
the user cannot change the role or clear errors until the save completes.
- Around line 43-55: The catch is treating any error (including PostHog
analytics failures) as a data save failure; change this by separating analytics
into a best-effort nested try-catch: keep the call to updateOccupation(...) as
the primary operation and only setError(...) when updateOccupation throws, then
immediately after a successful update call the posthog.* calls inside their own
try-catch so analytics failures are logged/ignored and do not trigger setError
or affect the user's save state; ensure setIsSubmitting(false) still runs in the
existing finally block.
🪄 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: 055803c9-0266-4176-9837-61b4d50800af
📒 Files selected for processing (2)
mcpjam-inspector/client/src/App.tsxmcpjam-inspector/client/src/components/signup/OccupationGate.tsx
ignaciojimenezr
temporarily deployed
to
preview-pr-1979
GitHub Actions
Inactive
ignaciojimenezr
temporarily deployed
to
preview-pr-1979
GitHub Actions
Inactive
2 participants
Added occupation to the user record in Convex.
New users get marked as occupationRequired: true.
Existing users are not blocked.
After signup, app waits for the Convex user row to exist.
If occupation is required, app shows the “What is your role?” screen.
User can type any role or pick from dropdown options.
Submit is blocked if empty.
On submit, we save occupation to Convex.
Then we mark occupationRequired: false.
We also send occupation to PostHog as a person property + event.
Added tests for new users, existing users, and saving occupation.
Build/tests pass.