Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds hidden listing visibility control, message type tracking with participant metadata, and introduces short URL redirects for listings. The backend schema is extended with participantIds, lastMessageId, and message type fields, while the frontend introduces filtering for hidden listings, a sharing capability, and new components for unavailable and hidden listing states. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/convex/listings.ts (1)
249-250:⚠️ Potential issue | 🟠 Major
createListingis missing non-negative price validation.Line 249 accepts any number, while update path already rejects negative values. This allows invalid data on create.
💵 Proposed fix
handler: async (ctx, args) => { const userId = await getAuthUserId(ctx); if (userId === null) { throw new Error('You must be logged in to create a listing'); } + if (args.price < 0) { + throw new Error('Price must be non-negative'); + } if (args.tags) {Also applies to: 257-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/convex/listings.ts` around lines 249 - 250, The create path accepts any number for price; update already rejects negatives, so update the validation schemas used in createListing (and the analogous schema around lines 257-260) to enforce non-negative prices by adding a min(0) or equivalent check to the price field (e.g., change v.number() to v.number().min(0)) so create and update behavior are consistent; update the validation for the price field in the schema referenced by createListing and the other schema mentioned so negative prices are rejected on create and update.
🧹 Nitpick comments (2)
frontend/hooks/useAuth.ts (1)
27-33: Prefer explicit mapping over...userin normalized output.This keeps the hook contract tightly scoped to
Userand avoids carrying unintended fields from backend responses.♻️ Proposed refactor
- const normalizedUser: User | null = user - ? { - ...user, - name: user.name ?? null, - createdAt: user.createdAt ?? user._creationTime, - } - : null; + const normalizedUser: User | null = user + ? { + _id: user._id, + _creationTime: user._creationTime, + email: user.email, + name: user.name ?? null, + createdAt: user.createdAt ?? user._creationTime, + } + : null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/hooks/useAuth.ts` around lines 27 - 33, Replace the spread-based normalization in useAuth.ts (normalizedUser) with an explicit mapping of only the User interface fields so unintended backend fields aren’t carried forward; remove the "...user" spread and return an object that sets each known User property (e.g., id, email, name, createdAt, roles — whatever is declared on the User type in your codebase) using user.field ?? default, and keep null/default fallbacks for missing values (preserve the existing name and createdAt logic but assign them explicitly).backend/convex/listings.ts (1)
81-105: Pagination is applied after loading the full dataset.Line 81 collects all rows, then Lines 101-105 slice in memory. This removes most pagination benefits and will degrade latency/memory as listing count grows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/convex/listings.ts` around lines 81 - 105, The code currently calls query.order('desc').collect() into listings then applies pagination by slicing the in-memory array (listings.slice...), which wastes memory and CPU; update the logic to apply pagination to the Convex query before calling collect: use the pagination inputs (args.paginationOpts.cursor and args.paginationOpts.numItems) to compute an offset/limit and call the query's server-side pagination methods (e.g., query.limit(...) and query.skip(...) or the equivalent Convex cursor-based approach) on the query returned by query.order('desc') so only the needed rows are collected, and remove the in-memory slice logic that operates on listings. Ensure you still apply the minPrice/maxPrice/tags filters either on the query (preferred) or before calling collect so they are included in the server-side pagination.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/convex/listings.ts`:
- Around line 32-34: The authorization check in verifyOwnership is inverted: it
currently throws when listing.sellerId === userId which blocks owners and allows
non-owners; update verifyOwnership to throw only when listing.sellerId !==
userId so owners can proceed and non-owners are blocked, then ensure
updateListing, updateListingStatus, and deleteListing call verifyOwnership
before performing mutations and rely on the corrected check (referencing
verifyOwnership, listing.sellerId, and userId).
In `@frontend/app/auth/login.tsx`:
- Around line 69-71: The redirect logic allows protocol-relative targets like
"//evil.example"; tighten validation in the block that computes nextRoute: when
evaluating redirectTo (the variable) ensure it's a string that startsWith('/')
but NOT startsWith('//') (e.g., add a check !redirectTo.startsWith('//')), and
only then pass that safe nextRoute to router.replace(nextRoute as Href); keep
the fallback to '/' otherwise. Use the existing symbols redirectTo, nextRoute,
and router.replace to locate and update the code.
In `@frontend/app/l/`[id].tsx:
- Around line 6-10: The redirect uses the raw id in the path which can create
malformed URLs for IDs with special characters; update the component to use an
encoded id by calling encodeURIComponent(id) when constructing the redirect href
for the valid branch (the Redirect that currently uses `/listings/${id}`) so the
dynamic segment is safely URL-encoded; keep the existing validation (typeof id
=== 'string' && id) and only encode the id in the constructed href.
In `@frontend/app/listings/`[id].tsx:
- Around line 39-40: The auth gating currently uses useAuth() but ignores its
loading state, causing the sign-in prompt (triggered in the buy/contact actions
around the listing page handlers) to open while auth is still resolving; update
the code to destructure isLoading from useAuth (e.g., const { user: currentUser,
isLoading } = useAuth()) and change the action checks that currently do if
(!currentUser) { openSignIn(); } to first guard for loading (if (isLoading)
return or disable the action) and only call openSignIn() when !isLoading &&
!currentUser, ensuring signed-in users don't get prompted while auth is
resolving.
- Around line 232-236: The ReportModal is being passed a potentially invalid
route param via String(id); instead pass the canonical resolved ID from the
listing object. Update the ReportModal call to use listing._id (e.g.,
targetId={String(listing._id)} or targetId={listing._id} depending on prop type)
and keep the existing props (isVisible={reportOpen}, onClose={() =>
setReportOpen(false)}, targetType="listing") so the modal always receives the
resolved listing._id rather than the raw route id.
In `@frontend/components/ReportModal.tsx`:
- Around line 45-58: The submit flow in ReportModal.tsx currently simulates
submission (setTimeout) and logs sensitive report data; replace that with a real
Convex mutation call and remove the console.log. In the onSubmit handler (where
resetState(), props.onClose(), and Toast.show are used), call your Convex
mutation (e.g., await mutationClient.mutate('reports.createReport', { targetId:
props.targetId, targetType: props.targetType, reason, notes })) instead of the
Promise, handle errors with try/catch (show an error Toast on failure and do not
reset state or close the modal), and only on success call resetState(),
props.onClose(), and Toast.show for success; ensure no raw notes or sensitive
content are written to console or logs.
---
Outside diff comments:
In `@backend/convex/listings.ts`:
- Around line 249-250: The create path accepts any number for price; update
already rejects negatives, so update the validation schemas used in
createListing (and the analogous schema around lines 257-260) to enforce
non-negative prices by adding a min(0) or equivalent check to the price field
(e.g., change v.number() to v.number().min(0)) so create and update behavior are
consistent; update the validation for the price field in the schema referenced
by createListing and the other schema mentioned so negative prices are rejected
on create and update.
---
Nitpick comments:
In `@backend/convex/listings.ts`:
- Around line 81-105: The code currently calls query.order('desc').collect()
into listings then applies pagination by slicing the in-memory array
(listings.slice...), which wastes memory and CPU; update the logic to apply
pagination to the Convex query before calling collect: use the pagination inputs
(args.paginationOpts.cursor and args.paginationOpts.numItems) to compute an
offset/limit and call the query's server-side pagination methods (e.g.,
query.limit(...) and query.skip(...) or the equivalent Convex cursor-based
approach) on the query returned by query.order('desc') so only the needed rows
are collected, and remove the in-memory slice logic that operates on listings.
Ensure you still apply the minPrice/maxPrice/tags filters either on the query
(preferred) or before calling collect so they are included in the server-side
pagination.
In `@frontend/hooks/useAuth.ts`:
- Around line 27-33: Replace the spread-based normalization in useAuth.ts
(normalizedUser) with an explicit mapping of only the User interface fields so
unintended backend fields aren’t carried forward; remove the "...user" spread
and return an object that sets each known User property (e.g., id, email, name,
createdAt, roles — whatever is declared on the User type in your codebase) using
user.field ?? default, and keep null/default fallbacks for missing values
(preserve the existing name and createdAt logic but assign them explicitly).
ℹ️ 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 (11)
backend/convex/listings.tsbackend/convex/schema.tsfrontend/app.jsonfrontend/app/_layout.tsxfrontend/app/auth/login.tsxfrontend/app/index.tsxfrontend/app/l/[id].tsxfrontend/app/listings/[id].tsxfrontend/components/ReportModal.tsxfrontend/hooks/useAuth.tspackage.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/convex/listings.ts (1)
112-112:⚠️ Potential issue | 🟡 MinorTypo in comment:
tages→tags.-// Normalize tages to lowercase and within 1-20 characters exclusive +// Normalize tags to lowercase and within 1-20 characters exclusive🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/convex/listings.ts` at line 112, Fix the typo in the inline comment that currently reads "Normalize tages to lowercase and within 1-20 characters exclusive" by replacing "tages" with "tags" so the comment reads "Normalize tags to lowercase and within 1-20 characters exclusive"; update the comment near where tag normalization logic is implemented (look for the comment string "Normalize tages..." in backend/convex/listings.ts) to ensure clarity.
♻️ Duplicate comments (1)
backend/convex/listings.ts (1)
24-34:verifyOwnershipauth migration looks correct.
getAuthUserId(ctx)is used with a null guard, and the ownership condition is properly!==. The previously flagged inverted check is resolved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/convex/listings.ts` around lines 24 - 34, No change required: the auth migration in this block is correct—keep the null guard using getAuthUserId(ctx) and the ownership check using listing.sellerId !== userId as-is; if there are older/inverted checks elsewhere, replace them with this pattern (use getAuthUserId(ctx) with a null guard and compare listing.sellerId !== userId) to ensure consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/convex/listings.ts`:
- Around line 81-99: The current code calls query.order('desc').collect()
(assigning to listings) then filters in-memory using args.minPrice,
args.maxPrice and args.tags, which pulls the entire table; change the logic so
price bounds are applied at the DB layer before collect: add/ensure a composite
index (e.g., by_status_price) and use query.withIndex('by_status_price') plus
the appropriate range/greaterThanEqual/lessThanEqual calls to push args.minPrice
and args.maxPrice into the query before calling .collect(); keep tag filtering
(args.tags) in-memory after collect if necessary and retain the .order('desc')
behavior. Ensure any new index is declared in the Convex schema to support the
index name used.
- Line 73: The local variable named "query" in backend/convex/listings.ts
shadows the module-level import "query" from './_generated/server'; rename the
local variable (for example to "categoryQuery" or "selectedCategory") in the
handler where it's declared and update all its usages in that scope (the
declaration at the diff and any subsequent references) so the imported "query"
identifier is no longer shadowed and ESLint no-shadow errors are avoided.
- Line 70: getListings currently declares paginationOpts:
v.optional(paginationOptsValidator) but implements manual .collect(),
parseInt(cursor,10) and returns an array slice — breaking Convex's pagination
contract; either (A) convert getListings to use Convex native pagination by
replacing the .collect()/manual slicing with .paginate(paginationOpts) and
return the required PaginationResult{page,isDone,continueCursor} shape (ensure
all filters used are indexable), or (B) remove paginationOptsValidator from the
argument list, mirror searchAndFilterListings by accepting a manual cursor/limit
pattern (opaque string or offset scheme), stop using .paginate() in the
signature, and return the same manual result shape; update getListings, the
cursor parsing logic, and the return type accordingly.
In `@frontend/app/listings/`[id].tsx:
- Around line 207-210: The Seller Information section currently renders PII by
showing listing.sellerEmail directly (see listing.sellerEmail inside the JSX
that uses styles.section/styles.sectionText); remove that direct exposure and
instead render a non-PII fallback UI such as "Contact Seller" button or link
that triggers the existing auth-gated contact flow (e.g., call the contact
modal/navigation handler used elsewhere), and ensure any access to
listing.sellerEmail is only performed server-side or after authentication before
sending to the client.
- Around line 19-34: The copyLinkToClipboard function can throw if
navigator.clipboard.writeText rejects; wrap the await clipboard.writeText(url)
call in a try/catch so any rejection is caught and the function returns false
(preserving the existing boolean contract), and optionally log or ignore the
error; update the function (referenced by name copyLinkToClipboard) so callers
like handleCopyLink always receive false on failure instead of an unhandled
rejection.
---
Outside diff comments:
In `@backend/convex/listings.ts`:
- Line 112: Fix the typo in the inline comment that currently reads "Normalize
tages to lowercase and within 1-20 characters exclusive" by replacing "tages"
with "tags" so the comment reads "Normalize tags to lowercase and within 1-20
characters exclusive"; update the comment near where tag normalization logic is
implemented (look for the comment string "Normalize tages..." in
backend/convex/listings.ts) to ensure clarity.
---
Duplicate comments:
In `@backend/convex/listings.ts`:
- Around line 24-34: No change required: the auth migration in this block is
correct—keep the null guard using getAuthUserId(ctx) and the ownership check
using listing.sellerId !== userId as-is; if there are older/inverted checks
elsewhere, replace them with this pattern (use getAuthUserId(ctx) with a null
guard and compare listing.sellerId !== userId) to ensure consistency.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/convex/listings.tsfrontend/app/auth/login.tsxfrontend/app/l/[id].tsxfrontend/app/listings/[id].tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/app/l/[id].tsx
Comment on lines
+81
to
+99
| let listings = await query.order('desc').collect(); | ||
|
|
||
| if (args.minPrice !== undefined) { | ||
| listings = listings.filter((listing) => listing.price >= args.minPrice!); | ||
| } | ||
|
|
||
| if (args.maxPrice !== undefined) { | ||
| listings = listings.filter((listing) => listing.price <= args.maxPrice!); | ||
| } | ||
|
|
||
| if (args.tags && args.tags.length > 0) { | ||
| const normalizedTags = new Set( | ||
| args.tags.map((tag) => tag.trim().toLowerCase()).filter((tag) => tag.length > 0) | ||
| ); | ||
|
|
||
| listings = listings.filter((listing) => | ||
| (listing.tags ?? []).some((tag) => normalizedTags.has(tag.toLowerCase())) | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Full table collect before in-memory filtering — unbounded memory and latency.
collect() is called before any price or tag filtering, meaning every active listing is pulled into memory on every call. As the listings table grows, this becomes an O(n) read on each query regardless of how selective the filters are.
Consider adding a database-level index for price range (e.g., by_status_price) and using .withIndex to push price bounds down to the database tier. Tag filtering will likely have to remain in-memory given its multi-value nature, but at least the price range can prune the result set early.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/convex/listings.ts` around lines 81 - 99, The current code calls
query.order('desc').collect() (assigning to listings) then filters in-memory
using args.minPrice, args.maxPrice and args.tags, which pulls the entire table;
change the logic so price bounds are applied at the DB layer before collect:
add/ensure a composite index (e.g., by_status_price) and use
query.withIndex('by_status_price') plus the appropriate
range/greaterThanEqual/lessThanEqual calls to push args.minPrice and
args.maxPrice into the query before calling .collect(); keep tag filtering
(args.tags) in-memory after collect if necessary and retain the .order('desc')
behavior. Ensure any new index is declared in the Convex schema to support the
index name used.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/convex/listings.ts (1)
258-267:⚠️ Potential issue | 🟠 MajorDo not trust client-supplied
sellerEmailwhen creating listings.
sellerEmailis accepted from request args and persisted directly. A logged-in user can submit another person’s email.🔐 Suggested server-side source of truth
export const createListing = mutation({ args: { @@ - sellerEmail: v.string(), @@ handler: async (ctx, args) => { @@ + const userDoc = await ctx.db.get(userId as Id<'users'>); + if (!userDoc) { + throw new Error('Authenticated user not found'); + } @@ const listingId = await ctx.db.insert('listings', { ...args, + sellerEmail: userDoc.email, sellerId: userId,Also applies to: 294-297
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/convex/listings.ts` around lines 258 - 267, The listing creation accepts a client-supplied sellerEmail in the args schema — stop trusting that value and use the server-side authenticated user's email instead: remove/ignore the args.sellerEmail field in the validator (the args object with title, description, price, sellerEmail, ...), read the current user's email from the request/session context (e.g. ctx.auth.email or your session helper) in the create-listing mutation/handler, ensure it's present and canonicalized, and persist that value as the seller email; apply the same change to the other occurrence referenced at lines 294-297 so no code ever uses args.sellerEmail for persistence or authorization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/convex/listings.ts`:
- Around line 269-273: createListing currently allows negative prices while
updateListing rejects them; add the same validation in createListing by checking
the incoming price field (e.g., the price on the listing input) and throw an
error if price < 0 before inserting. Locate createListing in listings.ts (same
file as getAuthUserId usage) and mirror the negative-price check used in
updateListing so new listings with negative prices are rejected early.
- Around line 428-449: The submitReport mutation currently only validates
listings but allows 'profile' targets; add validation for args.targetType ===
'profile' in submitReport: fetch the profile via ctx.db.get(args.targetId as
Id<'profiles'>), throw 'Profile not found' if missing, and throw 'Cannot report
your own profile' if the profile belongs to the authenticated user (compare
userId to profile.userId or the appropriate owner field on the profile record).
Ensure this logic sits alongside the existing listing checks in the submitReport
handler and uses getAuthUserId, ctx.db.get, reportTargetTypeValidator and the
same error patterns.
In `@frontend/app/listings/`[id].tsx:
- Around line 12-17: Replace the local-dev fallback for WEB_BASE_URL so shared
links don't default to http://localhost:8081: change the WEB_BASE_URL constant
to use a production-safe default (e.g., 'https://polybuy.com') instead of
localhost, keep the trailing-slash removal (.replace(/\/$/, '')), and let
APP_DOWNLOAD_URL continue to derive from WEB_BASE_URL (APP_DOWNLOAD_URL
constant) so canonical/share links resolve to a public domain when
EXPO_PUBLIC_WEB_BASE_URL is not set; alternatively, make WEB_BASE_URL optional
(empty string) and add guards where WEB_BASE_URL or APP_DOWNLOAD_URL are used in
listings/[id].tsx to avoid emitting localhost links.
In `@frontend/components/ReportModal.tsx`:
- Around line 31-34: Prevent closing the modal while a report submission is in
flight by adding a guard that checks the local submitting state before calling
resetState() and props.onClose(); update the close handlers referenced
(handleClose and the other close triggers around the other noted locations) to
early-return when submitting is true (optionally show a brief "submission in
progress" toast or keep the modal open), ensuring the unique functions/methods
resetState(), props.onClose(), and handleClose are used to locate and update the
logic.
---
Outside diff comments:
In `@backend/convex/listings.ts`:
- Around line 258-267: The listing creation accepts a client-supplied
sellerEmail in the args schema — stop trusting that value and use the
server-side authenticated user's email instead: remove/ignore the
args.sellerEmail field in the validator (the args object with title,
description, price, sellerEmail, ...), read the current user's email from the
request/session context (e.g. ctx.auth.email or your session helper) in the
create-listing mutation/handler, ensure it's present and canonicalized, and
persist that value as the seller email; apply the same change to the other
occurrence referenced at lines 294-297 so no code ever uses args.sellerEmail for
persistence or authorization.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/convex/listings.tsbackend/convex/schema.tsfrontend/app/listings/[id].tsxfrontend/components/ReportModal.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
frontend/app/listings/[id].tsx (1)
12-21:⚠️ Potential issue | 🟠 Major
WEB_BASE_URLcan still resolve to localhost in non-web runtime.Line 15 falls back to
http://localhost:8081. On native (nowindow) with missing env config, shared/canonical links become invalid public URLs.💡 Suggested hardening (avoid localhost fallback in production paths)
-const fallbackWebBaseUrl = - typeof window !== 'undefined' && window.location?.origin - ? window.location.origin - : 'http://localhost:8081'; -const WEB_BASE_URL = (process.env.EXPO_PUBLIC_WEB_BASE_URL ?? fallbackWebBaseUrl).replace( - /\/$/, - '' -); +const envWebBaseUrl = process.env.EXPO_PUBLIC_WEB_BASE_URL?.replace(/\/$/, ''); +const runtimeWebBaseUrl = + typeof window !== 'undefined' && window.location?.origin + ? window.location.origin.replace(/\/$/, '') + : ''; +const WEB_BASE_URL = envWebBaseUrl ?? runtimeWebBaseUrl;- const shareUrl = `${WEB_BASE_URL}/listings/${listing._id}`; + const shareUrl = WEB_BASE_URL ? `${WEB_BASE_URL}/listings/${listing._id}` : '';+ if (!shareUrl) { + Toast.show({ type: 'error', text1: 'Share link unavailable' }); + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/listings/`[id].tsx around lines 12 - 21, The current fallbackWebBaseUrl allows WEB_BASE_URL to become "http://localhost:8081" when window is undefined, causing invalid public links on native builds; update the logic so that fallbackWebBaseUrl does not default to localhost in non-web runtimes (e.g., set fallbackWebBaseUrl to undefined or an empty string instead of 'http://localhost:8081'), keep WEB_BASE_URL derived from process.env.EXPO_PUBLIC_WEB_BASE_URL or that safe fallback, and ensure APP_DOWNLOAD_URL uses EXPO_PUBLIC_APP_DOWNLOAD_URL if provided or only constructs from WEB_BASE_URL when WEB_BASE_URL is truthy (so update references to fallbackWebBaseUrl, WEB_BASE_URL, and APP_DOWNLOAD_URL accordingly).backend/convex/listings.ts (1)
444-463:⚠️ Potential issue | 🔴 Critical
submitReportbranch nesting is wrong and breaks valid listing reports.Lines 451-462 are currently attached to the ownership check, not to
targetType. Result: non-owner listing reports hit profile lookup and fail; profile reports skip validation entirely.🐛 Suggested fix (move profile path to target-type branch)
if (args.targetType === 'listing') { const listing = await ctx.db.get(args.targetId as Id<'listings'>); if (!listing) { throw new Error('Listing not found'); } if (listing.sellerId === userId) { throw new Error('Cannot report your own listing'); - } else { - const profile = await ctx.db - .query('profiles') - .withIndex('by_userId', (q) => q.eq('userId', args.targetId)) - .unique(); - if (!profile) { - throw new Error('Profile not found'); - } - if (profile.userId === userId) { - throw new Error('Cannot report your own profile'); - } } + } else { + const profile = await ctx.db + .query('profiles') + .withIndex('by_userId', (q) => q.eq('userId', args.targetId)) + .unique(); + if (!profile) { + throw new Error('Profile not found'); + } + if (profile.userId === userId) { + throw new Error('Cannot report your own profile'); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/convex/listings.ts` around lines 444 - 463, The code in submitReport incorrectly nests the profile lookup and profile ownership check inside the "listing" branch's else, causing profile reports to skip validation and listing reports to run profile logic; fix by restructuring the conditional so that if args.targetType === 'listing' you only fetch the listing and check listing.sellerId !== userId (throwing 'Listing not found' or 'Cannot report your own listing' as appropriate), and add a separate else if branch for args.targetType === 'profile' that performs the profiles query (using the 'by_userId' index and .unique()), verifies the profile exists, and checks profile.userId !== userId (throwing 'Profile not found' or 'Cannot report your own profile'). Ensure you update the code around the listing/profile checks in submitReport to reflect these two distinct branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/listings/`[id].tsx:
- Around line 23-40: The copyLinkToClipboard function currently only checks
navigator.clipboard and returns false on native (iOS/Android); install and
import a cross-platform clipboard (expo-clipboard), detect platform using
Platform.OS === 'web' and use navigator.clipboard.writeText for web while
calling ExpoClipboard.setStringAsync for native; update copyLinkToClipboard to
branch on Platform.OS and return true/false based on the chosen API, and update
callers handleCopyLink and handleShare fallback to use the new
copyLinkToClipboard so mobile devices no longer trigger the "Clipboard
unavailable" path.
In `@frontend/components/ReportModal.tsx`:
- Around line 73-77: The unauthenticated error message in ReportModal always
says "report a listing" regardless of the component's targetType; update the
setError call inside the ReportModal (where isUnauth is handled) to pick the
message based on the targetType prop (targetType === 'profile' ? 'You must be
logged in to report a profile.' : 'You must be logged in to report a listing.').
Ensure the same targetType-aware wording is applied where setError is used for
unauthenticated cases so the message correctly reflects 'listing' or 'profile'.
---
Duplicate comments:
In `@backend/convex/listings.ts`:
- Around line 444-463: The code in submitReport incorrectly nests the profile
lookup and profile ownership check inside the "listing" branch's else, causing
profile reports to skip validation and listing reports to run profile logic; fix
by restructuring the conditional so that if args.targetType === 'listing' you
only fetch the listing and check listing.sellerId !== userId (throwing 'Listing
not found' or 'Cannot report your own listing' as appropriate), and add a
separate else if branch for args.targetType === 'profile' that performs the
profiles query (using the 'by_userId' index and .unique()), verifies the profile
exists, and checks profile.userId !== userId (throwing 'Profile not found' or
'Cannot report your own profile'). Ensure you update the code around the
listing/profile checks in submitReport to reflect these two distinct branches.
In `@frontend/app/listings/`[id].tsx:
- Around line 12-21: The current fallbackWebBaseUrl allows WEB_BASE_URL to
become "http://localhost:8081" when window is undefined, causing invalid public
links on native builds; update the logic so that fallbackWebBaseUrl does not
default to localhost in non-web runtimes (e.g., set fallbackWebBaseUrl to
undefined or an empty string instead of 'http://localhost:8081'), keep
WEB_BASE_URL derived from process.env.EXPO_PUBLIC_WEB_BASE_URL or that safe
fallback, and ensure APP_DOWNLOAD_URL uses EXPO_PUBLIC_APP_DOWNLOAD_URL if
provided or only constructs from WEB_BASE_URL when WEB_BASE_URL is truthy (so
update references to fallbackWebBaseUrl, WEB_BASE_URL, and APP_DOWNLOAD_URL
accordingly).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/convex/listings.tsfrontend/app/listings/[id].tsxfrontend/components/ReportModal.tsx
jaydonkc
force-pushed
the
feature/POLY-34-Shareable-Links
branch
from
a645e94 to
80e89cc
Compare
Collaborator
|
Created clean companion branch Feedback:
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/convex/schema.ts (1)
121-121: Consider using a union type for messagetypefield.Using
v.optional(v.string())allows any string value. If the message types are well-defined (e.g.,'text','image','system'), consider using a union type for better type safety and validation:- type: v.optional(v.string()), + type: v.optional(v.union(v.literal('text'), v.literal('image'), v.literal('system'))),This prevents invalid type values from being stored and improves developer experience with autocompletion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/convex/schema.ts` at line 121, The message schema's type field currently uses v.optional(v.string()), allowing any string; change it to a union/enum of the allowed message types (for example replace v.optional(v.string()) with v.optional(v.union(v.literal('text'), v.literal('image'), v.literal('system'))) or v.optional(v.enum(['text','image','system']))) in the schema definition (refer to the message schema's type field in schema.ts) so only valid types are accepted and developers get proper autocomplete and validation.backend/convex/messages.ts (1)
282-307: Backfill mutation may not scale for large datasets.The current implementation loads all conversations and messages into memory with
.collect()and patches them one by one. This could cause:
- Memory issues with large datasets
- Transaction timeouts if there are many records to patch
- Hitting Convex's mutation size limits
For production use, consider implementing pagination or running in batches:
♻️ Example batched approach
export const backfillMessagingFields = internalMutation({ - args: {}, - handler: async (ctx) => { - const conversations = await ctx.db.query('conversations').collect(); + args: { + cursor: v.optional(v.string()), + batchSize: v.optional(v.number()), + }, + handler: async (ctx, args) => { + const limit = args.batchSize ?? 100; + const conversationsQuery = ctx.db.query('conversations'); + const conversations = await (args.cursor + ? conversationsQuery.paginate({ cursor: args.cursor, numItems: limit }) + : conversationsQuery.paginate({ numItems: limit })); + let conversationPatches = 0; - - for (const convo of conversations) { + for (const convo of conversations.page) { if (!convo.participantIds || convo.participantIds.length !== 2) { await ctx.db.patch(convo._id, { participantIds: [convo.buyerId, convo.sellerId] }); conversationPatches += 1; } } - - // ... similar for messages - - return { conversationPatches, messagePatches }; + return { + conversationPatches, + isDone: conversations.isDone, + cursor: conversations.continueCursor, + }; }, });If this is intended as a one-time migration on a small dataset, the current approach is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/convex/messages.ts` around lines 282 - 307, The backfillMessagingFields internalMutation currently loads all records via ctx.db.query(...).collect() and patches each one, which won't scale; change backfillMessagingFields to iterate records in batches instead of using collect(): use paginated queries (e.g., loop using query with a limit/offset or a cursor) to fetch e.g. 100–1000 conversations/messages per page, apply patches for that page (either sequentially or with a bounded concurrency worker pool), await completion, then continue to the next page—replace direct collect() calls and single-item ctx.db.patch loops with this batched fetch-and-patch pattern for both conversations and messages to avoid memory bloat, long transactions, and mutation size limits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/convex/messages.ts`:
- Around line 282-307: The backfillMessagingFields internalMutation currently
loads all records via ctx.db.query(...).collect() and patches each one, which
won't scale; change backfillMessagingFields to iterate records in batches
instead of using collect(): use paginated queries (e.g., loop using query with a
limit/offset or a cursor) to fetch e.g. 100–1000 conversations/messages per
page, apply patches for that page (either sequentially or with a bounded
concurrency worker pool), await completion, then continue to the next
page—replace direct collect() calls and single-item ctx.db.patch loops with this
batched fetch-and-patch pattern for both conversations and messages to avoid
memory bloat, long transactions, and mutation size limits.
In `@backend/convex/schema.ts`:
- Line 121: The message schema's type field currently uses
v.optional(v.string()), allowing any string; change it to a union/enum of the
allowed message types (for example replace v.optional(v.string()) with
v.optional(v.union(v.literal('text'), v.literal('image'), v.literal('system')))
or v.optional(v.enum(['text','image','system']))) in the schema definition
(refer to the message schema's type field in schema.ts) so only valid types are
accepted and developers get proper autocomplete and validation.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
backend/convex/__tests__/messages.test.tsbackend/convex/__tests__/testUtils.tsbackend/convex/messages.tsbackend/convex/schema.tsfrontend/app/index.tsxfrontend/app/l/[id].tsxfrontend/app/listings/[id].tsxfrontend/app/listings/[id]/edit.tsxfrontend/components/HiddenBanner.tsxfrontend/components/ListingUnavailable.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/app/index.tsx
This was referenced Mar 2, 2026
This was referenced Mar 11, 2026
Closed
This was referenced Apr 14, 2026
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
4 participants
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 #42 & #34
Linear: POLY-42 & POLY-34
Summary
-Added shareable URL generation and copy to clipboard
How to Test
Steps to verify locally:
npm run lintnpm run typechecknpm testnpm run dev:backend(in terminal A)npm run dev(in terminal B)Checklist
npm run lint)devScreenshots / Demos
(if UI or visible behavior - attach images, videos, or GIFs)
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes