feat: implement home feed sorting, filtering#32
Conversation
- Update getListings query with category, minPrice, maxPrice filters - Use by_status_createdAt index for deterministic newest-first ordering - Add price validation (min >= 0, max >= min) - Add FilterBar component with filter chips - Add CategoryPicker bottom sheet - Add PriceRangePicker bottom sheet with presets - Integrate filters into home screen with state management - Add 'No results' state with clear filters option - Add 7 tests for filter combinations
|
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:
📝 WalkthroughWalkthroughAdds reporting infrastructure and auto-moderation (new reports table and createReport mutation), hidden-content fields on listings/profiles, server-side listing filters and visibility rules (including getMyHiddenListings), extensive backend tests, and new frontend filter UI components wired to getListings. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Server as Convex Server
participant DB as Database
Client->>Server: createReport(targetId, targetType, reason, notes)
Server->>Server: Authenticate & lookup reporter by email
Server->>DB: Query existing report for (targetId, reporterId)
alt duplicate
Server->>Client: Return error (duplicate)
else
Server->>DB: Count reporter's reports in last 24h
alt rate limit exceeded
Server->>Client: Return error (rate limit)
else
Server->>DB: Insert new report
Server->>DB: Count unique reporters for targetId
alt unique reporters >= 3
Server->>DB: Update target (listing/profile) set isHidden=true, hiddenAt, hiddenReason
end
Server->>Client: Return reportId
end
end
sequenceDiagram
participant User as User
participant Client as Client
participant Server as Convex Server
participant DB as Database
User->>Client: Select category/min/max filters
Client->>Server: getListings(category, minPrice, maxPrice, limit)
Server->>DB: Query by by_status_createdAt index
Server->>Server: Exclude listings where isHidden=true
Server->>Server: Apply category and price filters, enforce validations
Server->>Server: Paginate results
Server->>Client: Return filtered listings
Client->>User: Render listings and show Clear Filters option
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@frontend/components/FilterBar.tsx`:
- Around line 69-77: The clear button is nested inside a parent pressable which
can cause the parent’s onPress to fire; instead of attempting stopPropagation
(not supported), remove nesting by moving the clear TouchableOpacity (the
hasCategory block with onClearCategory and styles.clearButton) out of the parent
chip TouchableOpacity and position it alongside via styles (or absolutely
position it using the chip container) so taps target only the clear button; if
you cannot restructure, set the parent chip TouchableOpacity to
pointerEvents="box-none" and tighten hitSlop on the clear button to avoid
overlap, or alternatively implement onStartShouldSetResponder on the clear
button container to claim touches before the parent.
In `@frontend/components/PriceRangePicker.tsx`:
- Around line 47-59: The handler handleApply currently validates minVal but not
maxVal alone; add a check that when maxVal is defined it is not negative (e.g.,
if (maxVal !== undefined && maxVal < 0) { setError('Maximum price cannot be
negative'); return; }) before the min/max comparison, and ensure you still keep
the existing maxVal < minVal check; use the existing minVal, maxVal, and
setError symbols to locate and implement this validation.
🧹 Nitpick comments (6)
backend/convex/schema.ts (1)
64-73: Consider adding a composite index for rate limiting queries.The
by_reporterindex supports querying reports by reporter, but the rate limiting logic inreports.tsalso filters bycreatedAt. For optimal performance with many reports per user, consider a composite index:.index('by_target', ['targetId', 'targetType']) - .index('by_reporter', ['reporterId']), + .index('by_reporter', ['reporterId']) + .index('by_reporter_createdAt', ['reporterId', 'createdAt']),This would allow the rate limiting query to use an index range scan on
createdAtrather than in-memory filtering. However, this is optional given the expected low volume of reports per user per day.backend/convex/reports.ts (1)
87-97: Minor: Duplicate step number in comments and rate limit query optimization.
Line 87 comment says "// 6." but should be "// 7." (duplicate of line 74).
The rate limiting query uses
.collect()which loads all recent reports into memory. Consider using a count approach or early exit:♻️ Proposed optimization
// 7. Check rate limiting (max 10 reports per day) const oneDayAgo = Date.now() - ONE_DAY_MS; const recentReports = await ctx.db .query('reports') .withIndex('by_reporter', (q) => q.eq('reporterId', user._id)) .filter((q) => q.gt(q.field('createdAt'), oneDayAgo)) - .collect(); - - if (recentReports.length >= MAX_REPORTS_PER_DAY) { + .take(MAX_REPORTS_PER_DAY); + + if (recentReports.length >= MAX_REPORTS_PER_DAY) {Using
.take(MAX_REPORTS_PER_DAY)stops scanning once the limit is reached, avoiding unnecessary reads.backend/convex/listings.ts (1)
78-101: Consider usingby_status_categoryindex when filtering by category.The current implementation uses
by_status_createdAtfor all queries, then applies category filter in memory. Whenargs.categoryis provided, usingby_status_categorycould reduce the scanned dataset significantly:♻️ Proposed optimization
- // Use by_status_createdAt index for deterministic newest-first ordering - const query = ctx.db - .query('listings') - .withIndex('by_status_createdAt', (q) => q.eq('status', 'active')) - .order('desc'); - - // Apply filters - const listings = await query - .filter((q) => { - let conditions = q.neq(q.field('isHidden'), true); - - if (args.category) { - conditions = q.and(conditions, q.eq(q.field('category'), args.category)); - } + // Choose index based on filter: category filter uses by_status_category, otherwise by_status_createdAt + let baseQuery; + if (args.category) { + baseQuery = ctx.db + .query('listings') + .withIndex('by_status_category', (q) => + q.eq('status', 'active').eq('category', args.category) + ); + } else { + baseQuery = ctx.db + .query('listings') + .withIndex('by_status_createdAt', (q) => q.eq('status', 'active')) + .order('desc'); + } + + // Apply remaining filters + const listings = await baseQuery + .filter((q) => { + let conditions = q.neq(q.field('isHidden'), true); + if (args.minPrice !== undefined) {Note: When using
by_status_category, results need sorting bycreatedAtin memory afterward to maintain newest-first ordering. This trade-off may not be worth it for small datasets.backend/convex/__tests__/listings.test.ts (1)
173-182: Consider extracting sharedbaseArgsto reduce duplication.The
baseArgsobject is duplicated between the twodescribeblocks (lines 23-31 and 174-182). Consider extracting it to the module scope.♻️ Proposed refactor
const modules = { '../listings.ts': () => Promise.resolve(listingsModule), '../_generated/api.ts': () => Promise.resolve(apiModule), '../_generated/server.ts': () => Promise.resolve(serverModule), } as any; +// Shared test fixture for listing creation +const baseListingArgs = { + title: 'Great textbook for CSC 202', + description: 'Gently used, highlights in a few chapters.', + price: 50, + sellerEmail: 'test@example.com', + category: 'textbooks' as const, + images: ['https://example.com/book1.png'], + condition: 'used' as const, +}; describe('Listings mutations', () => { - const baseArgs = { - title: 'Great textbook for CSC 202', - ... - }; + const baseArgs = baseListingArgs;frontend/app/index.tsx (1)
48-54: Consider handling query error states.The loading state check (
listings === undefined) is correct for Convex, but there's no handling for potential query errors. While Convex may throw errors that bubble up, consider adding error boundary support or checking for error states to improve user feedback.frontend/components/PriceRangePicker.tsx (1)
96-115: Consider handling non-numeric input more robustly.
parseFloatcan produce unexpected results for certain inputs (e.g.,"12abc"parses to12). WhilekeyboardType="numeric"helps restrict input on mobile, consider sanitizing or validating that the entire string is a valid number before applying.💡 Optional: Stricter input validation
const handleApply = () => { - const minVal = min ? parseFloat(min) : undefined; - const maxVal = max ? parseFloat(max) : undefined; + const minVal = min ? Number(min) : undefined; + const maxVal = max ? Number(max) : undefined; + + // Check for invalid number input + if (min && (isNaN(minVal!) || !/^\d*\.?\d*$/.test(min))) { + setError('Please enter a valid minimum price'); + return; + } + if (max && (isNaN(maxVal!) || !/^\d*\.?\d*$/.test(max))) { + setError('Please enter a valid maximum price'); + return; + }
- Refactor FilterBar to avoid nested TouchableOpacity - Add negative max price validation to PriceRangePicker
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/components/PriceRangePicker.tsx`:
- Around line 47-63: In handleApply, parseFloat(min) / parseFloat(max) can
produce NaN for non-numeric input; before performing negative/compare checks and
calling onApply, verify minVal and maxVal are finite numbers (e.g., using
Number.isFinite) and treat empty/whitespace-only strings as undefined (trim
first or test the original strings), calling setError('Please enter a valid
number') when a non-finite value is provided; ensure the rest of the validations
(negative and max < min) only run when values are finite and only pass finite
numbers or undefined into onApply.
🧹 Nitpick comments (2)
frontend/components/FilterBar.tsx (2)
63-75: Consider adding accessibility props for screen reader support.The interactive elements lack
accessibilityLabelandaccessibilityRoleprops, which would improve the experience for users relying on screen readers.♿ Suggested accessibility improvement
- <TouchableOpacity style={styles.chipMainButton} onPress={onCategoryPress}> + <TouchableOpacity + style={styles.chipMainButton} + onPress={onCategoryPress} + accessibilityRole="button" + accessibilityLabel={hasCategory ? `Category filter: ${CATEGORY_LABELS[filters.category!]}` : 'Select category filter'} + > <Text style={[styles.chipText, hasCategory && styles.chipTextActive]}> {hasCategory ? CATEGORY_LABELS[filters.category!] : 'Category'} </Text> </TouchableOpacity> {hasCategory && ( <TouchableOpacity style={styles.clearButton} onPress={onClearCategory} hitSlop={{ top: 10, bottom: 10, left: 10, right: 10 }} + accessibilityRole="button" + accessibilityLabel="Clear category filter" >Apply similar changes to the price chip and "Clear All" button.
117-128: Remove redundant padding properties for clarity.The
padding: 0on line 126 overrides thepaddingHorizontalandpaddingVerticaldeclarations on lines 120-121. While the comment explains the intent, having overridden values is confusing.🧹 Suggested cleanup
chip: { flexDirection: 'row', alignItems: 'center', - paddingHorizontal: 14, - paddingVertical: 8, borderRadius: 20, backgroundColor: '#f0f0f0', borderWidth: 1, borderColor: '#e0e0e0', - padding: 0, // Reset padding as inner elements handle spacing overflow: 'hidden', },
- Trim whitespace from inputs before parsing - Treat empty/whitespace-only strings as undefined - Validate parsed values are finite with Number.isFinite - Show clear error message for invalid number input
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/convex/listings.ts`:
- Around line 72-82: The validation logic in listings.ts checks that minPrice is
non-negative but misses a standalone non-negative check for maxPrice, so add a
guard that throws an error when args.maxPrice !== undefined && args.maxPrice < 0
(similar to the minPrice check) and keep the existing cross-check (maxPrice >=
minPrice) intact; update the validation block that references args.minPrice and
args.maxPrice to include this new check so negative maxPrice values are rejected
consistently.
🧹 Nitpick comments (3)
backend/convex/listings.ts (2)
84-88: Variablequeryshadows the importedqueryfrom'./_generated/server'.Line 85 declares
const querywhich shadows the module-level import on line 2. This works because the import is only used at module scope (forquery({...})), but it's confusing. Consider renaming tolistingsQueryordbQuery.
428-443:getMyHiddenListingsperforms a full table scan without an index onsellerId.The
.filter()on line 439 scans every document in thelistingstable. The schema defines indexes onstatus,category, and their combinations, but lacks an index onsellerId. Add an index on['sellerId', 'isHidden']to efficiently filter by seller and visibility status, or at minimum add a single-column index onsellerId.backend/convex/schema.ts (1)
66-75: Consider a compound index for rate-limiting queries onreports.The rate-limiting logic in
backend/convex/reports.ts(lines 86-95) queries reports byreporterIdand then filters by time:.withIndex('by_reporter', ...).filter((q) => q.gt(q.field('createdAt'), oneDayAgo)). This causes the time filter to run in-memory over all of that reporter's reports. A compound index['reporterId', 'createdAt']would let you efficiently range-scan by both fields.Also,
targetIdis typed asv.string()— a reasonable trade-off for the polymorphic reference, but Convex won't enforce referential integrity. Worth a doc comment noting this.Suggested index improvement
.index('by_target', ['targetId', 'targetType']) - .index('by_reporter', ['reporterId']), + .index('by_reporter', ['reporterId', 'createdAt']),
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/convex/listings.ts`:
- Line 69: The limit arg currently defined as v.optional(v.number()) can be
non-positive or fractional and gets passed to .take(); update the handler that
reads the limit (the argument named "limit" used before calling .take()) to
coerce and clamp it to a positive integer with a sensible upper bound (e.g.,
default 20, min 1, max 100): if missing use 20, otherwise use Math.floor and
clamp between 1 and the max before passing to .take(); reference the "limit"
argument and the call to .take() in the listings query to locate where to apply
this change.
- Around line 72-86: The current price validation in the listings query only
checks for negativity and ordering but misses NaN/Infinity; update the checks
that reference args.minPrice and args.maxPrice to also ensure
Number.isFinite(value) is true and throw the same validation errors when it's
not; specifically, in the block that validates minPrice, maxPrice and the
comparison (the code using args.minPrice and args.maxPrice), add guards that
reject non-finite numbers so NaN/Infinity cannot pass into the filtering logic.
🧹 Nitpick comments (1)
backend/convex/listings.ts (1)
441-446: Add a composite index to optimize thesellerId + isHiddenquery.The query uses
.filter()without.withIndex(), causing a full table scan of the listings table. Adding a composite index likeby_sellerId_isHiddenwould improve performance as the listings table grows. Other queries in the codebase use explicit indexes (e.g.,by_status_createdAt,by_status_category); this should follow the same pattern.Regarding the status filter: whether to exclude
deletedlistings is a design choice. Since this returns a user's own hidden listings, excluding deleted listings may not be necessary—users may want visibility into their deleted hidden listings for management purposes.
| category: v.optional(categoryValidator), | ||
| minPrice: v.optional(v.number()), | ||
| maxPrice: v.optional(v.number()), | ||
| limit: v.optional(v.number()), // default 20 |
There was a problem hiding this comment.
limit parameter lacks validation — non-positive or non-integer values could cause unexpected behavior with .take().
The limit arg accepts any number with no guards. Values like 0, -1, or 1.5 will be forwarded directly to .take(). Consider clamping to a positive integer with an upper bound:
Proposed fix
+ const MAX_LIMIT = 100;
+ const take = Math.min(
+ Math.max(1, Math.floor(args.limit ?? ITEMS_PER_PAGE)),
+ MAX_LIMIT
+ );
+
// Use by_status_createdAt index for deterministic newest-first ordering
const query = ctx.db
.query('listings')
.withIndex('by_status_createdAt', (q) => q.eq('status', 'active'))
.order('desc');
// Apply filters
const listings = await query
.filter((q) => {
...
})
- .take(args.limit ?? ITEMS_PER_PAGE);
+ .take(take);Also applies to: 111-111
🤖 Prompt for AI Agents
In `@backend/convex/listings.ts` at line 69, The limit arg currently defined as
v.optional(v.number()) can be non-positive or fractional and gets passed to
.take(); update the handler that reads the limit (the argument named "limit"
used before calling .take()) to coerce and clamp it to a positive integer with a
sensible upper bound (e.g., default 20, min 1, max 100): if missing use 20,
otherwise use Math.floor and clamp between 1 and the max before passing to
.take(); reference the "limit" argument and the call to .take() in the listings
query to locate where to apply this change.
| // Validate price filters | ||
| if (args.minPrice !== undefined && args.minPrice < 0) { | ||
| throw new Error('minPrice must be non-negative'); | ||
| } | ||
| if (args.maxPrice !== undefined && args.maxPrice < 0) { | ||
| throw new Error('maxPrice must be non-negative'); | ||
| } | ||
|
|
||
| if ( | ||
| args.maxPrice !== undefined && | ||
| args.minPrice !== undefined && | ||
| args.maxPrice < args.minPrice | ||
| ) { | ||
| throw new Error('maxPrice must be greater than or equal to minPrice'); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Convex v.number() validator NaN Infinity
💡 Result:
v.number() in Convex validates Float64 (IEEE-754 double) numbers and does allow special values like NaN, Infinity, and -Infinity. In Convex’s JSON/export representation, Inf and NaN are serialized as strings (since JSON can’t represent them as numbers). [1] [2]
🏁 Script executed:
# Find and examine the listings.ts file to understand the function signature and validators
find . -type f -name "listings.ts" | head -5Repository: codebox-calpoly/PolyBuys
Length of output: 96
🏁 Script executed:
# Also search for how v.number() is used in the codebase
rg "v\.number\(\)" --type ts -A 2 -B 2 | head -50Repository: codebox-calpoly/PolyBuys
Length of output: 2498
🏁 Script executed:
# Read the listings.ts file to see the full function context and validators
cat -n backend/convex/listings.ts | head -100Repository: codebox-calpoly/PolyBuys
Length of output: 3766
🏁 Script executed:
# Search for the function that contains the validation at lines 72-86
rg "minPrice|maxPrice" backend/convex/listings.ts -B 10 -A 15Repository: codebox-calpoly/PolyBuys
Length of output: 3767
Add Number.isFinite() guards to price validation.
Convex's v.number() validator accepts NaN and Infinity as valid numbers. Since NaN < 0 and NaN < NaN both evaluate to false, these values silently bypass all validation checks and enter the filter logic, causing unexpected query behavior (NaN comparisons in database filters always return false, matching no results).
Proposed fix
// Validate price filters
+ if (args.minPrice !== undefined && !Number.isFinite(args.minPrice)) {
+ throw new Error('minPrice must be a finite number');
+ }
+ if (args.maxPrice !== undefined && !Number.isFinite(args.maxPrice)) {
+ throw new Error('maxPrice must be a finite number');
+ }
if (args.minPrice !== undefined && args.minPrice < 0) {
throw new Error('minPrice must be non-negative');
}📝 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.
| // Validate price filters | |
| if (args.minPrice !== undefined && args.minPrice < 0) { | |
| throw new Error('minPrice must be non-negative'); | |
| } | |
| if (args.maxPrice !== undefined && args.maxPrice < 0) { | |
| throw new Error('maxPrice must be non-negative'); | |
| } | |
| if ( | |
| args.maxPrice !== undefined && | |
| args.minPrice !== undefined && | |
| args.maxPrice < args.minPrice | |
| ) { | |
| throw new Error('maxPrice must be greater than or equal to minPrice'); | |
| } | |
| // Validate price filters | |
| if (args.minPrice !== undefined && !Number.isFinite(args.minPrice)) { | |
| throw new Error('minPrice must be a finite number'); | |
| } | |
| if (args.maxPrice !== undefined && !Number.isFinite(args.maxPrice)) { | |
| throw new Error('maxPrice must be a finite number'); | |
| } | |
| if (args.minPrice !== undefined && args.minPrice < 0) { | |
| throw new Error('minPrice must be non-negative'); | |
| } | |
| if (args.maxPrice !== undefined && args.maxPrice < 0) { | |
| throw new Error('maxPrice must be non-negative'); | |
| } | |
| if ( | |
| args.maxPrice !== undefined && | |
| args.minPrice !== undefined && | |
| args.maxPrice < args.minPrice | |
| ) { | |
| throw new Error('maxPrice must be greater than or equal to minPrice'); | |
| } |
🤖 Prompt for AI Agents
In `@backend/convex/listings.ts` around lines 72 - 86, The current price
validation in the listings query only checks for negativity and ordering but
misses NaN/Infinity; update the checks that reference args.minPrice and
args.maxPrice to also ensure Number.isFinite(value) is true and throw the same
validation errors when it's not; specifically, in the block that validates
minPrice, maxPrice and the comparison (the code using args.minPrice and
args.maxPrice), add guards that reject non-finite numbers so NaN/Infinity cannot
pass into the filtering logic.
3 participants
Linked Issues
Linear: POLY-15
Summary
Implement recency-based home feed sorting with category and price range filtering for the PolyBuys marketplace.
Backend changes:
getListingsquery with optionalcategory,minPrice,maxPrice,limitargsby_status_createdAtindex for deterministic newest-first orderingFrontend changes:
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)devSummary by CodeRabbit
New Features
Schema / Data
Tests