fix: wrap doubt like toggle in transaction with floor guard (#472)#497
fix: wrap doubt like toggle in transaction with floor guard (#472)#497vedant7007 wants to merge 1 commit into
Conversation
|
@vedant7007 is attempting to deploy a commit to the Karan Mani Tripathi 's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe doubt like action handler is refactored from a non-atomic, race-condition-prone implementation to an atomic transactional flow. The entire toggle is now wrapped in a single database transaction that locks the doubt row, atomically checks for existing likes, performs conditional insert/delete, updates the count with a floor guard, and returns a 404 on missing rows. ChangesAtomic Like Toggle with Transaction Lock
Sequence DiagramsequenceDiagram
participant Handler
participant Transaction
participant DoubtsTable
participant LikesTable
Handler->>Transaction: start transaction
Transaction->>DoubtsTable: lock doubt row (SELECT FOR UPDATE)
DoubtsTable-->>Transaction: doubt row
alt doubt not found
Transaction-->>Handler: return 404
else doubt found
Transaction->>LikesTable: check if user already liked
LikesTable-->>Transaction: existingLike
alt already liked
Transaction->>LikesTable: delete like
Transaction->>DoubtsTable: decrement count with GREATEST floor
Transaction-->>Handler: return {hasLiked: false}
else not liked
Transaction->>LikesTable: insert like
Transaction->>DoubtsTable: increment count
Transaction-->>Handler: return {hasLiked: true}
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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 review |
1 similar comment
|
@coderabbitai review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/app/api/doubts/action/`[id]/route.ts:
- Around line 55-56: The like/unlike flow is inconsistent because
action/[id]/route.ts uses secureUserIdentifier = email while the client and
src/app/api/doubts/route.ts use the anonymous_user value as userName, causing
mismatched lookups; fix this by normalizing the identifier server-side: in
action/[id]/route.ts set secureUserIdentifier to the incoming userName (from the
request body/query) when it exists and fall back to email (Clerk) only if
userName is absent, and ensure the same normalization is applied in the doubts
route that computes hasLiked (matching likesTable.userName) so reads, inserts
and deletes use the same key (references: secureUserIdentifier,
likesTable.userName, hasLiked logic in src/app/api/doubts/route.ts).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5a877b3a-92c4-4cb8-8e21-e2b948482a55
📒 Files selected for processing (1)
src/app/api/doubts/action/[id]/route.ts
|
|
||
| const secureUserIdentifier = email; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the format of existing userName values in likes table
rg -n "likesTable" --type ts -C 5 | head -100
# Check if there's any migration or data cleanup related to likes
fd -i migration -e ts -e sql --exec grep -l "likes" {} \;Repository: knoxiboy/DoubtDesk
Length of output: 7014
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Inspect secureUserIdentifier assignment in action route =="
sed -n '1,140p' src/app/api/doubts/action/[id]/route.ts | nl -ba | sed -n '40,120p'
echo
echo "== Locate anonymous_user usage on the client =="
rg -n 'anonymous_user' --hidden --glob '!.next/*' --glob '!.turbo/*' --glob '!.vercel/*' --glob '!*node_modules*'
echo
echo "== Find where like/unlike request sends userName =="
rg -n "userName" --hidden --glob '!.next/*' --glob '!.turbo/*' --glob '!.vercel/*' --glob '!*node_modules*' src
echo
echo "== Inspect likes table schema definition =="
sed -n '200,290p' src/configs/schema.ts | nl -baRepository: knoxiboy/DoubtDesk
Length of output: 185
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Inspect secureUserIdentifier assignment in action route =="
sed -n '1,120p' src/app/api/doubts/action/[id]/route.ts
echo
echo "== Locate anonymous_user usage on the client =="
rg -n 'anonymous_user' src
echo
echo "== Find where like/unlike request sends userName =="
rg -n "userName" src/app src/components src/lib src/pages src
echo
echo "== Inspect likes table schema definition =="
sed -n '210,270p' src/configs/schema.tsRepository: knoxiboy/DoubtDesk
Length of output: 25667
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Inspect client like/unlike payload in DoubtCard.tsx =="
sed -n '1,120p' src/components/DoubtCard.tsx
echo
echo "== Inspect InfiniteDoubtFeed.tsx passing userName query param =="
sed -n '60,120p' src/components/InfiniteDoubtFeed.tsx
echo
echo "== Inspect doubts list endpoint using likesTable.userName (src/app/api/doubts/route.ts) =="
sed -n '120,190p' src/app/api/doubts/route.ts
echo
echo "== Inspect likesTable schema for unique constraint =="
sed -n '220,280p' src/configs/schema.tsRepository: knoxiboy/DoubtDesk
Length of output: 12416
Fix inconsistent like/unlike identifiers after switching to Clerk email
src/app/api/doubts/action/[id]/route.ts stores/reads/deletes likes using secureUserIdentifier = email against likesTable.userName. But the client still sends userName = localStorage.getItem("anonymous_user") for like/unlike, and src/app/api/doubts/route.ts computes hasLiked by matching likesTable.userName to the userName query param (also from anonymous_user). If existing rows were keyed by anonymous_user, unlike will not find them; and new likes for the same doubt can be inserted again because likesTable enforces uniqueness on (userName, doubtId)—treating email and anonymous_user as different users—so the UI and doubtsTable.likes can diverge.
🤖 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/app/api/doubts/action/`[id]/route.ts around lines 55 - 56, The
like/unlike flow is inconsistent because action/[id]/route.ts uses
secureUserIdentifier = email while the client and src/app/api/doubts/route.ts
use the anonymous_user value as userName, causing mismatched lookups; fix this
by normalizing the identifier server-side: in action/[id]/route.ts set
secureUserIdentifier to the incoming userName (from the request body/query) when
it exists and fall back to email (Clerk) only if userName is absent, and ensure
the same normalization is applied in the doubts route that computes hasLiked
(matching likesTable.userName) so reads, inserts and deletes use the same key
(references: secureUserIdentifier, likesTable.userName, hasLiked logic in
src/app/api/doubts/route.ts).
Closes #472
the doubt like toggle used a non-atomic check-then-act pattern while the reply vote endpoint was already fixed for the same bug in PR #335. concurrent likes could cause duplicate entries and negative counts.
what i fixed:
response shape unchanged. typecheck clean, build passes.
for labels i think gssoc:approved + level:intermediate + type:bug + quality:clean fits here. happy to make changes if needed!
Summary by CodeRabbit