Add collection management features including CRUD operations, slug va… - #16
Conversation
…lidation, and internationalization support
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds authenticated collection APIs, create/edit form workflows, collection pages, slug validation and availability checks, uniqueness error handling, localized English/Russian UI text, and comprehensive composable tests. ChangesCollections backend
Collection form and pages
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CollectionForm
participant CollectionAPI
participant Session
participant Database
User->>CollectionForm: enter collection fields
CollectionForm->>CollectionAPI: check slug availability
CollectionAPI->>Session: resolve authenticated user
Session-->>CollectionAPI: return user id
CollectionAPI->>Database: query slug conflict
Database-->>CollectionAPI: return availability
CollectionForm->>CollectionAPI: submit collection
CollectionAPI->>Database: insert or update owned collection
Database-->>CollectionAPI: return collection or validation error
CollectionAPI-->>CollectionForm: return submission result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
…ssertions Wraps composable calls in a local effectScope() to silence a Vue onScopeDispose warning from testing outside a component context, and tightens submit-mapping assertions to check the actual payload sent (including description's "" -> null boundary mapping) rather than just call counts. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
app/features/collection-form/model/__tests__/use-collection-form.test.ts (2)
151-228: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the availability-check rejection path.
scheduleSlugCheck's.catchresetsslugStatusto"idle"(re-enabling submit) and is currently untested; a regression there silently changes submit gating. Same foronScopeDisposeclearing a pending debounce timer on scope stop.🤖 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 `@app/features/collection-form/model/__tests__/use-collection-form.test.ts` around lines 151 - 228, The debounced slug availability tests cover successful and stale responses but not rejection or scope cleanup. Extend the “useCollectionForm - debounced slug availability check” suite to verify a rejected check resets slugStatus to “idle” and to verify disposing the effect scope clears a pending debounce so checkSlugAvailabilityMock is not called.
51-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid duplicating the i18n output in the test helper
GENERIC_ERRORmirrorstMock’serrors.genericvalue, so if~/shared/testing/mocks/i18nchanges how it resolves keys the test expectation drifts. Either derive the helper from the mock, import it from the mock utilities, or assert thattMockwas called with"errors.generic"when handling the mapped error.🤖 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 `@app/features/collection-form/model/__tests__/use-collection-form.test.ts` at line 51, The test helper’s GENERIC_ERROR duplicates the i18n mock output and can drift from tMock. Update the error-handling assertions in the collection form test to derive the expected value from tMock or assert the "errors.generic" translation key, reusing the existing i18n mock behavior instead of hardcoding the message.app/features/collection-form/model/use-collection-form.ts (1)
149-156: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winTrim text fields in
toPayload.
nameis sent verbatim while the server's uniqueness index compareslower(trim(name))(server/utils/collection-errors.ts), so" My List "persists with padding yet collides with"My List". Whitespace-only descriptions likewise survive the|| nullcheck.♻️ Proposed fix
function toPayload(): CollectionInput { + const description = form.description.trim(); return { - name: form.name, - description: form.description || null, + name: form.name.trim(), + description: description || null, shared: form.shared, slug: form.slug }; }🤖 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 `@app/features/collection-form/model/use-collection-form.ts` around lines 149 - 156, Update toPayload to trim the name and description fields before submission. Ensure whitespace-only descriptions become null after trimming, while preserving the existing payload fields and behavior for non-text values.
🤖 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 `@app/features/collection-form/model/use-collection-form.ts`:
- Around line 18-28: Update slugify to limit the generated slug to the
validator’s 64-character maximum, trimming any trailing hyphen after truncation.
Keep the existing lowercase, normalization, and edge-hyphen removal behavior,
and ensure isValidSlugFormat continues validating the resulting slug.
- Around line 158-193: Add an early return at the start of performSubmit when
pending.value is already true, before resetting form state or issuing
createCollection/updateCollection calls. Preserve the existing submission flow
for non-pending requests and ensure concurrent confirmAndSubmit invocations
cannot duplicate the request.
In `@app/features/collection-form/ui/collection-form.vue`:
- Line 25: Type the formRef declaration used by onSubmit so its value exposes
the VForm validate method instead of remaining unknown. Update the
useTemplateRef call with the VForm instance type, or use a nullable ref<VForm>
type, while preserving the existing template reference and validation flow.
In `@app/pages/collections/`[id]/edit.vue:
- Line 20: Update the error alert condition in the collection edit view to
distinguish a 404 response from other GET failures: display
collections.errors.notFound only when the error status is 404, and display
collections.errors.loadFailed for 5xx or network failures while preserving the
existing error visibility behavior.
In `@app/pages/collections/`[id]/index.vue:
- Around line 1-11: Update the collection detail page around the setup block and
template to read the route parameter id and fetch the corresponding collection
through the authenticated /api/collections/:id endpoint. Add loading, not-found,
and error states, and render the loaded collection’s fields instead of the
generic pages.collectionDetail.title heading while preserving the auth
middleware.
In `@app/pages/collections/index.vue`:
- Around line 29-46: The routed v-list-item and its append NuxtLink create
nested interactive links. Update the collection list markup around the v-for so
the edit action is outside the :to-enabled v-list-item, or make the row
non-routed while preserving navigation and the existing edit destination.
- Around line 9-13: Make both authenticated collection async-data keys
user-scoped: update useAsyncData in app/pages/collections/index.vue around lines
9-13 to include a stable session/user discriminator in "collections", and update
useAsyncData in app/pages/collections/[id]/edit.vue around lines 12-15 to
include the same discriminator in `collection-${id}`. Keep the request behavior
unchanged while ensuring logout/login or account switching cannot reuse another
user’s cached data.
In `@server/api/collections/index.post.ts`:
- Around line 18-26: Authenticate before request parsing in both
server/api/collections/index.post.ts lines 18-26 and
server/api/collections/[id].patch.ts lines 24-32: move requireUserId(event)
ahead of readBody and collectionSchema.safeParse, while preserving the existing
validation and mutation behavior for authenticated callers.
In `@server/utils/session.ts`:
- Around line 23-37: Update requireUserId to use a trusted fixed or
configuration-derived auth origin instead of url.origin from
getRequestURL(event), while preserving the forwarded cookie behavior. Configure
the $fetch call with a timeout and appropriate abort handling so an unavailable
auth endpoint cannot hang the request, while retaining the existing 401 response
for missing sessions.
---
Nitpick comments:
In `@app/features/collection-form/model/__tests__/use-collection-form.test.ts`:
- Around line 151-228: The debounced slug availability tests cover successful
and stale responses but not rejection or scope cleanup. Extend the
“useCollectionForm - debounced slug availability check” suite to verify a
rejected check resets slugStatus to “idle” and to verify disposing the effect
scope clears a pending debounce so checkSlugAvailabilityMock is not called.
- Line 51: The test helper’s GENERIC_ERROR duplicates the i18n mock output and
can drift from tMock. Update the error-handling assertions in the collection
form test to derive the expected value from tMock or assert the "errors.generic"
translation key, reusing the existing i18n mock behavior instead of hardcoding
the message.
In `@app/features/collection-form/model/use-collection-form.ts`:
- Around line 149-156: Update toPayload to trim the name and description fields
before submission. Ensure whitespace-only descriptions become null after
trimming, while preserving the existing payload fields and behavior for non-text
values.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d3167cae-df08-4050-b9ff-dbca9eee0761
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
app/features/collection-form/index.tsapp/features/collection-form/model/__tests__/use-collection-form.test.tsapp/features/collection-form/model/use-collection-form.tsapp/features/collection-form/ui/collection-form.vueapp/middleware/auth.tsapp/pages/collections/[id]/edit.vueapp/pages/collections/[id]/index.vueapp/pages/collections/index.vueapp/pages/new-collection.vueapp/shared/api/collections.tsapp/shared/api/index.tsapp/shared/lib/index.tsapp/shared/lib/validators.tsapp/shared/testing/mocks/collections-api.tsi18n/locales/en.jsoni18n/locales/ru.jsonpackage.jsonserver/api/collections/[id].get.tsserver/api/collections/[id].patch.tsserver/api/collections/check-slug.get.tsserver/api/collections/index.get.tsserver/api/collections/index.post.tsserver/db/schema.tsserver/utils/collection-errors.tsserver/utils/session.tsserver/utils/validation.ts
| <script lang="ts" setup> | ||
| definePageMeta({ middleware: "auth" }); | ||
|
|
||
| const { t } = useI18n(); | ||
| </script> | ||
|
|
||
| <template> | ||
| <v-container class="fill-height d-flex align-center justify-center"> | ||
| <h1>{{ t("pages.collectionDetail.title") }}</h1> | ||
| </v-container> | ||
| </template> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Implement the collection detail data flow.
This page never reads route.params.id or calls /api/collections/:id, so every authenticated ID renders the same generic heading. Load the collection through the authenticated API and render its fields with loading, 404, and error states.
🤖 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 `@app/pages/collections/`[id]/index.vue around lines 1 - 11, Update the
collection detail page around the setup block and template to read the route
parameter id and fetch the corresponding collection through the authenticated
/api/collections/:id endpoint. Add loading, not-found, and error states, and
render the loaded collection’s fields instead of the generic
pages.collectionDetail.title heading while preserving the auth middleware.
Fixes the security-relevant items (spoofable Host header driving requireUserId's self-fetch origin, auth check running after body validation on mutating routes, non-user-scoped useAsyncData cache keys, nested interactive links in the collections list) plus the smaller correctness issues (double-submit guard, slug auto-sync overflowing the 64-char limit, untrimmed name/description, 404 vs. generic load-failure messaging, unresolved formRef typing) and the two test nitpicks (slug-check rejection/scope-dispose coverage, GENERIC_ERROR derived from the i18n mock instead of duplicated). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lidation, and internationalization support
Summary by CodeRabbit
New Features
Bug Fixes