Feature/poly 16 listing tags backend#35
Merged
SamanSP1386 merged 4 commits intodevfrom Feb 24, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdded tag-based filtering and tag handling to listings: search now accepts optional tags and applies tag intersection filtering; createListing accepts and normalizes optional tags; tag normalization enforces min/max lengths, trimming, deduplication, and a maximum tag count. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as Backend API
participant DB as Database
Client->>API: searchAndFilterListings(filters, tags?)
API->>API: normalize/validate tags (trim, dedupe, length, limit)
API->>DB: query listings by filters (price/other)
DB-->>API: listings[]
API->>API: if tags present -> filter listings by tag intersection
API-->>Client: filtered listings[]
Client->>API: createListing(data, tags?)
API->>API: normalize/validate tags (trim, dedupe, length, limit)
API->>DB: insert listing with normalized tags
DB-->>API: created listing
API-->>Client: created listing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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.
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)
75-84: 🛠️ Refactor suggestion | 🟠 Major
normalizeTagsuses hardcoded magic numbers instead ofTAG_CONSTRAINTS.Lines 81 and 83 hardcode
1,20, and5rather than referencingTAG_CONSTRAINTS.MIN_TAG_LENGTH,TAG_CONSTRAINTS.MAX_TAG_LENGTH, andTAG_CONSTRAINTS.MAX_TAGSdefined at lines 13–17. If the constants are ever updated, this function will silently diverge.Also, minor typo on line 75: "tages" → "tags".
Proposed fix
-// Normalize tages to lowercase and within 1-20 characters exclusive +// Normalize tags to lowercase, deduplicate, and enforce length constraints function normalizeTags(tags: string[]): string[] { return [ ...new Set( tags .map((tag) => tag.trim().toLowerCase()) - .filter((tag) => tag.length >= 1 && tag.length <= 20) + .filter((tag) => tag.length >= TAG_CONSTRAINTS.MIN_TAG_LENGTH && tag.length <= TAG_CONSTRAINTS.MAX_TAG_LENGTH) ), - ].slice(0, 5); + ].slice(0, TAG_CONSTRAINTS.MAX_TAGS); }
🧹 Nitpick comments (2)
backend/convex/listings.ts (2)
232-245: Tag validation logic is duplicated betweencreateListingandupdateListing.Lines 232–245 here and lines 283–297 in
updateListingare identical. Extract a sharedvalidateTags(tags: string[])helper (similar tovalidateTitle/validateImages) to keep this DRY.Proposed refactor
+function validateTags(tags: string[]) { + if (tags.length > TAG_CONSTRAINTS.MAX_TAGS) { + throw new Error(`Maximum ${TAG_CONSTRAINTS.MAX_TAGS} tags allowed`); + } + for (const tag of tags) { + const trimmed = tag.trim(); + if (!trimmed) { + throw new Error('Empty tags are not allowed'); + } + if (trimmed.length > TAG_CONSTRAINTS.MAX_TAG_LENGTH) { + throw new Error(`Tags must be ${TAG_CONSTRAINTS.MAX_TAG_LENGTH} characters or less`); + } + } +}Then in both
createListingandupdateListing:- if (args.tags) { - if (args.tags.length > TAG_CONSTRAINTS.MAX_TAGS) { - throw new Error(`Maximum ${TAG_CONSTRAINTS.MAX_TAGS} tags allowed`); - } - for (const tag of args.tags) { - const trimmed = tag.trim(); - if (!trimmed) { - throw new Error('Empty tags are not allowed'); - } - if (trimmed.length > TAG_CONSTRAINTS.MAX_TAG_LENGTH) { - throw new Error(`Tags must be ${TAG_CONSTRAINTS.MAX_TAG_LENGTH} characters or less`); - } - } - } + if (args.tags) { + validateTags(args.tags); + }
298-332: Nit:normalizedTagsis computed unconditionally but only used whenargs.tags !== undefined.Consider moving the normalization inside the conditional block at line 330 to avoid the unnecessary call when tags aren't provided.
SamanSP1386
reviewed
Feb 10, 2026
Collaborator
SamanSP1386
left a comment
SamanSP1386
left a comment
There was a problem hiding this comment.
There is some few warnings, check the ai bot and try to fix them.
5 tasks
Contributor
There was a problem hiding this comment.
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)
163-258:⚠️ Potential issue | 🟠 MajorApply tag filtering in
searchAndFilterListings.
filters.tagsis accepted but never used, so tag filters are ignored and results can be broader than requested. This breaks the expected behavior and the PR’s objective.✅ Suggested fix (normalize tags + filter in both paths)
const filters = args.filters ?? {}; const searchTerm = filters.searchTerm?.trim(); const sortBy = filters.sortBy ?? 'newest'; + const normalizedTags = filters.tags ? normalizeSearchTags(filters.tags) : []; + const hasTags = normalizedTags.length > 0; @@ // Filter out hidden content results = results.filter((l) => l.isHidden !== true); + + // Apply tag filter (any-match) + if (hasTags) { + results = results.filter((l) => + (l.tags ?? []).some((tag) => normalizedTags.includes(tag)) + ); + } @@ - const needsPostFiltering = + const needsPostFiltering = + hasTags || // maxPrice needs post-filtering for non-price-sorted queries (can use db-level filter for price-sorted) (filters.maxPrice !== undefined && sortBy !== 'price_asc' && sortBy !== 'price_desc') || (filters.minPrice !== undefined && sortBy !== 'price_asc' && sortBy !== 'price_desc') || // Condition filter needs post-filtering when using price-based indexes (which don't include condition) (filters.condition !== undefined && (sortBy === 'price_asc' || sortBy === 'price_desc')); @@ const filtered = allResults.filter((l) => { + if (hasTags && !(l.tags ?? []).some((tag) => normalizedTags.includes(tag))) return false; if (filters.maxPrice !== undefined && l.price > filters.maxPrice) return false; if ( sortBy !== 'price_asc' && sortBy !== 'price_desc' && filters.minPrice !== undefined && l.price < filters.minPrice ) return false; if (filters.condition && l.condition !== filters.condition) return false; return true; });Also applies to: 260-382
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/convex/listings.ts` around lines 163 - 258, searchAndFilterListings accepts filters.tags but never applies it; normalize requested tags (e.g., const normalizedTags = (filters.tags ?? []).map(t => t.trim().toLowerCase()).filter(Boolean)) and then filter listings to only include items whose listing.tags (normalize each listing tag to lowercase/trim) contain every requested tag. In the search branch (inside the if (searchTerm) block) add this filter after price filtering (before hidden filtering/sorting) using results = results.filter(l => normalizedTags.every(t => (l.tags ?? []).map(x=>x.trim().toLowerCase()).includes(t))). In the non-search code path (the other query/paginate branch) apply the same normalization and either add an equivalent DB-level constraint if supported or perform the same post-query in-memory filter before pagination so both branches respect filters.tags.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@backend/convex/listings.ts`:
- Around line 163-258: searchAndFilterListings accepts filters.tags but never
applies it; normalize requested tags (e.g., const normalizedTags = (filters.tags
?? []).map(t => t.trim().toLowerCase()).filter(Boolean)) and then filter
listings to only include items whose listing.tags (normalize each listing tag to
lowercase/trim) contain every requested tag. In the search branch (inside the if
(searchTerm) block) add this filter after price filtering (before hidden
filtering/sorting) using results = results.filter(l => normalizedTags.every(t =>
(l.tags ?? []).map(x=>x.trim().toLowerCase()).includes(t))). In the non-search
code path (the other query/paginate branch) apply the same normalization and
either add an equivalent DB-level constraint if supported or perform the same
post-query in-memory filter before pagination so both branches respect
filters.tags.
SamanSP1386
approved these changes
Feb 24, 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
2 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.
Linked Issues
Closes #16
Linear: POLY-16 (e.g., POLY-123)
Summary
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