[POLY-25] Add tags field to listing schema with validation#30
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds optional tags to listings: schema, types, mutation args; enforces constraints (max 5 tags, 1–20 chars), normalizes (trim, lowercase, dedupe), persists normalized tags, and expands tests to cover validation and normalization behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as Convex Mutation
participant DB as Convex DB
rect rgba(0,128,0,0.5)
Client->>API: call createListing/updateListing(tags?)
end
rect rgba(0,0,255,0.5)
API->>API: validate tags against TAG_CONSTRAINTS\n(normalize: trim, lowercase, dedupe, filter, cap)
API-->>Client: error if validation fails
end
rect rgba(255,165,0,0.5)
API->>DB: persist listing with normalizedTags
DB-->>API: confirmation
end
API-->>Client: return created/updated listing (with normalized tags)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/convex/__tests__/schema.test.ts (1)
44-55:⚠️ Potential issue | 🟡 MinorAdd coverage for the new
by_tagindex.
The schema addsby_tag, but the test doesn’t assert it yet.Suggested update
expect(indexNames).toContain('by_status'); expect(indexNames).toContain('by_category'); expect(indexNames).toContain('by_status_category'); expect(indexNames).toContain('by_status_createdAt'); + expect(indexNames).toContain('by_tag'); @@ const categoryIndex = listings.indexes.find((i: any) => i.indexDescriptor === 'by_category'); expect(categoryIndex.fields).toEqual(['category']); + + const tagIndex = listings.indexes.find((i: any) => i.indexDescriptor === 'by_tag'); + expect(tagIndex.fields).toEqual(['tags']);
🤖 Fix all issues with AI agents
In `@backend/convex/__tests__/listings.test.ts`:
- Around line 286-354: The tests named like "updateListing removes duplicate
tags" etc. currently call api.listings.createListing; change them to exercise
the update path by first creating a listing (using createListing) to obtain
listingId and then calling api.listings.updateListing with that listingId and
the test-specific tags; update the mutation calls in each affected test to
asOwner.mutation(api.listings.updateListing, { listingId, ... }) and keep
assertions on ctx.db.get(listingId) the same so the update validations are
actually executed.
🧹 Nitpick comments (2)
backend/convex/listings.ts (2)
75-84: UseTAG_CONSTRAINTSin normalization + fix typo.
This avoids drift between constraints and normalization, and fixes the comment typo.Suggested change
-// Normalize tages to lowercase and within 1-20 characters exclusive +// Normalize tags to lowercase and within allowed lengths 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); }
217-252: DRY tag validation across create/update.
The validation block is duplicated; consider extracting a helper to prevent drift and reduce maintenance.Suggested refactor
+function validateTags(tags?: string[]) { + if (!tags) return; + 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`); + } + } +}- 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`); - } - } - } + validateTags(args.tags);- 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`); - } - } - } + validateTags(args.tags);Also applies to: 266-324
This was referenced Feb 6, 2026
Merged
This was referenced Feb 22, 2026
Merged
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
Linear: POLY-25
Summary
How to Test
Steps to verify locally:
npm testto make sure that all test cases pass, especially test cases from listings.test.tsChecklist
npm run lint)devScreenshots / Demos
(if UI or visible behavior - attach images, videos, or GIFs)
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.