Fix/peer card view profile navigation#995
Open
OmanshiRaj wants to merge 3 commits into
Open
Conversation
|
@OmanshiRaj is attempting to deploy a commit to the durdana3105's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
📝 WalkthroughWalkthroughThree independent UI improvements: ChangesUI Improvements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 4
🤖 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 `@src/components/PeerCard.tsx`:
- Line 183: The onClick handler in PeerCard.tsx navigates to
`/profile/${peer.id}`, but this route doesn't exist in the router configuration.
The router only defines `/profile` which expects the Profile page to read
identity from the session user. Add a new parameterized route `/profile/:id` to
your router configuration and update the Profile page component to read the id
parameter from the URL and fetch the corresponding user when the id parameter is
present, falling back to session-based loading when no parameter is provided.
In `@src/pages/AnonymousDoubts.tsx`:
- Around line 48-49: The character limit validation uses the raw text length but
the actual persisted content uses trimmedText, causing valid submissions with
trailing spaces to be rejected. At line 48 in the AnonymousDoubts.tsx file,
refactor the limit check to use trimmedText.length instead of text.length by
first trimming the text variable before performing the character limit
validation. Apply the same fix at line 111-112 in the same file to ensure
consistency across all limit checks. This ensures the validation matches what
will actually be stored in the database.
- Line 6: The MAX_CHARS constant in AnonymousDoubts.tsx is only enforced on the
client side and can be bypassed. Add a database-level CHECK constraint in the
migration file (supabase/migrations/20260608000001_anonymous_doubts.sql) on the
doubts.content column to enforce a maximum length of 500 characters. This
constraint should be added to the table definition in the CREATE TABLE statement
to ensure that the backend reliably enforces the length limit regardless of
client-side checks. The client-side MAX_CHARS definition and its usage in
AnonymousDoubts.tsx (referenced at lines 6 and 48-49) remain as UI validations
but will now be backed by database enforcement.
- Around line 85-95: The text counter UI spanning the relative div is present
but the input element is missing its onChange handler binding. Add an onChange
event handler to the input or textarea element that captures user input and
updates the text state by calling setText with the trimmed or raw input value.
This will ensure that as users type, the text state is updated, enabling the
"Post Doubt" button (currently disabled due to the !text.trim() condition at
line 111) to become clickable when text is entered.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 312b55b2-1801-4207-8ae8-767bf38b8206
📒 Files selected for processing (3)
src/components/PeerCard.tsxsrc/components/studyroom/LiveCodeRunner.tsxsrc/pages/AnonymousDoubts.tsx
| <Button | ||
| size="sm" | ||
| variant="outline" | ||
| onClick={() => navigate(`/profile/${peer.id}`)} |
Contributor
There was a problem hiding this comment.
Navigation target does not match current route contract.
Line 183 navigates to /profile/${peer.id}, but the router defines /profile and the Profile page resolves identity from session user id. This breaks the intended "view peer profile" flow.
Suggested fix direction
- onClick={() => navigate(`/profile/${peer.id}`)}
+ onClick={() => navigate(`/profile/${peer.id}`)}Keep this line only if you also add the matching route + param-based profile loading:
- Route:
/profile/:id(or a dedicated/users/:id) - Profile page: read
idfrom params and fetch that user when present
🤖 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 `@src/components/PeerCard.tsx` at line 183, The onClick handler in PeerCard.tsx
navigates to `/profile/${peer.id}`, but this route doesn't exist in the router
configuration. The router only defines `/profile` which expects the Profile page
to read identity from the session user. Add a new parameterized route
`/profile/:id` to your router configuration and update the Profile page
component to read the id parameter from the URL and fetch the corresponding user
when the id parameter is present, falling back to session-based loading when no
parameter is provided.
| import { toast } from "sonner"; | ||
| import { Loader2 } from "lucide-react"; | ||
|
|
||
| const MAX_CHARS = 500; |
Contributor
There was a problem hiding this comment.
MAX_CHARS is only enforced in UI; backend currently accepts unlimited content.
MAX_CHARS client checks are bypassable. Cross-file evidence shows doubts.content is unrestricted text and insert policy has no length predicate (supabase/migrations/20260608000001_anonymous_doubts.sql, Lines 1-9 and 20-29). Add a DB-level constraint/check to enforce this contract reliably.
Also applies to: 48-49
🤖 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 `@src/pages/AnonymousDoubts.tsx` at line 6, The MAX_CHARS constant in
AnonymousDoubts.tsx is only enforced on the client side and can be bypassed. Add
a database-level CHECK constraint in the migration file
(supabase/migrations/20260608000001_anonymous_doubts.sql) on the doubts.content
column to enforce a maximum length of 500 characters. This constraint should be
added to the table definition in the CREATE TABLE statement to ensure that the
backend reliably enforces the length limit regardless of client-side checks. The
client-side MAX_CHARS definition and its usage in AnonymousDoubts.tsx
(referenced at lines 6 and 48-49) remain as UI validations but will now be
backed by database enforcement.
Comment on lines
+48
to
49
| if (text.length > MAX_CHARS) { toast.error(`Doubt exceeds ${MAX_CHARS} character limit.`); return; } | ||
| if (!user) { toast.error("Please log in to post a doubt."); return; } |
Contributor
There was a problem hiding this comment.
Use trimmed length for limit checks to match persisted content.
At Line 48 and Line 111, the limit uses text.length, but insert uses trimmedText (Line 54). This can reject valid submissions with trailing spaces even when stored content is within limit. Use a single trimmedText.length-based check for consistency.
Also applies to: 111-112
🤖 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 `@src/pages/AnonymousDoubts.tsx` around lines 48 - 49, The character limit
validation uses the raw text length but the actual persisted content uses
trimmedText, causing valid submissions with trailing spaces to be rejected. At
line 48 in the AnonymousDoubts.tsx file, refactor the limit check to use
trimmedText.length instead of text.length by first trimming the text variable
before performing the character limit validation. Apply the same fix at line
111-112 in the same file to ensure consistency across all limit checks. This
ensures the validation matches what will actually be stored in the database.
Comment on lines
+85
to
95
| <div className="relative"> | ||
|
|
||
| <span className={`absolute bottom-2 right-2 text-xs select-none ${ | ||
| text.length > MAX_CHARS ? "text-red-500 font-semibold" | ||
| : text.length >= MAX_CHARS * 0.9 ? "text-amber-500" | ||
| : "text-slate-400"}`} | ||
| > | ||
| {text.length}/{MAX_CHARS} | ||
| </span> | ||
| </div> | ||
| <input |
Contributor
There was a problem hiding this comment.
Text entry control is missing, so posting is effectively broken.
The new counter UI at Line 85 appears without a bound text input/textarea, and setText is never invoked from this section. Combined with disabled={... || !text.trim() ...} at Line 111, “Post Doubt” stays disabled and users cannot submit.
Proposed fix
-<div className="relative">
- <span className={`absolute bottom-2 right-2 text-xs select-none ${
- text.length > MAX_CHARS ? "text-red-500 font-semibold"
- : text.length >= MAX_CHARS * 0.9 ? "text-amber-500"
- : "text-slate-400"}`}
- >
- {text.length}/{MAX_CHARS}
- </span>
- </div>
+<div className="relative">
+ <textarea
+ className="border p-2 w-full rounded min-h-28 pr-16"
+ placeholder="Write your doubt..."
+ value={text}
+ onChange={(e) => setText(e.target.value)}
+ />
+ <span
+ className={`absolute bottom-2 right-2 text-xs select-none ${
+ text.length > MAX_CHARS
+ ? "text-red-500 font-semibold"
+ : text.length >= MAX_CHARS * 0.9
+ ? "text-amber-500"
+ : "text-slate-400"
+ }`}
+ >
+ {text.length}/{MAX_CHARS}
+ </span>
+</div>📝 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.
Suggested change
| <div className="relative"> | |
| <span className={`absolute bottom-2 right-2 text-xs select-none ${ | |
| text.length > MAX_CHARS ? "text-red-500 font-semibold" | |
| : text.length >= MAX_CHARS * 0.9 ? "text-amber-500" | |
| : "text-slate-400"}`} | |
| > | |
| {text.length}/{MAX_CHARS} | |
| </span> | |
| </div> | |
| <input | |
| <div className="relative"> | |
| <textarea | |
| className="border p-2 w-full rounded min-h-28 pr-16" | |
| placeholder="Write your doubt..." | |
| value={text} | |
| onChange={(e) => setText(e.target.value)} | |
| /> | |
| <span | |
| className={`absolute bottom-2 right-2 text-xs select-none ${ | |
| text.length > MAX_CHARS | |
| ? "text-red-500 font-semibold" | |
| : text.length >= MAX_CHARS * 0.9 | |
| ? "text-amber-500" | |
| : "text-slate-400" | |
| }`} | |
| > | |
| {text.length}/{MAX_CHARS} | |
| </span> | |
| </div> |
🤖 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 `@src/pages/AnonymousDoubts.tsx` around lines 85 - 95, The text counter UI
spanning the relative div is present but the input element is missing its
onChange handler binding. Add an onChange event handler to the input or textarea
element that captures user input and updates the text state by calling setText
with the trimmed or raw input value. This will ensure that as users type, the
text state is updated, enabling the "Post Doubt" button (currently disabled due
to the !text.trim() condition at line 111) to become clickable when text is
entered.
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.
Closes #926
The "View Profile" button in PeerCard had no onClick handler, making it a dead no-op since the component was created. Fixed by importing useNavigate from react-router-dom and navigating to /profile/${peer.id} on click. The peer.id field already exists on the User type so no type changes were needed.
Summary by CodeRabbit