feat: enhance tag management and user library checks#14
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 14 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUser-scoped book lookup was implemented by threading Changes
Sequence DiagramsequenceDiagram
actor Client
participant API as lookup.post (endpoint)
participant Service as BookService.lookupBook
participant Repo as BookRepository.hasBookInUserLibrary
participant DB as Database
Client->>API: POST /books/lookup { isbn, auth }
API->>Service: lookupBook(userId, isbn)
Service->>Service: normalize ISBN
Service->>Repo: hasBookInUserLibrary(userId, normalizedISBN)
Repo->>DB: Query JOIN userBooks + books WHERE userId & isbn LIMIT 1
DB-->>Repo: row / no row
Repo-->>Service: boolean (user owns book)
alt user owns book
Service-->>API: BookLookupResult (existsLocally: true)
else not in user library
Service->>Service: fallback to OpenLibrary
Service-->>API: BookLookupResult (existsLocally: false)
end
API-->>Client: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/components/TagManagerModal.vue (1)
240-250: Make the remove action explicit for screen readers.On Line 249, the button text is only the tag name; the destructive intent is mostly conveyed by the icon. Consider adding hidden assistive text like “Remove tag …” for clearer accessibility.
♿ Suggested refinement
<UButton v-for="tag in workingUserTags" :key="tag.id" color="primary" variant="subtle" size="xs" trailing-icon="i-lucide-x" `@click`="removeUserTag(tag)" > - {{ tag.name }} + <span>{{ tag.name }}</span> + <span class="sr-only">Remove tag {{ tag.name }}</span> </UButton>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/TagManagerModal.vue` around lines 240 - 250, The UButton list renders only the tag name with a trailing "x" icon, which hides the destructive action from screen readers; update the UButton (rendered in the v-for over workingUserTags, using tag.id/tag.name and removeUserTag) to include explicit assistive text such as "Remove tag {tag.name}" for screen readers (either by adding a visually-hidden span inside the button or setting an appropriate aria-label on the button) and ensure the decorative icon is aria-hidden so screen readers hear only the remove text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/TagManagerModal.vue`:
- Around line 54-57: The combined array passed into getAvailableSuggestedTags
can contain duplicate tag.id values because allSuggestedTags.value and
releasedSuggestedTags.value may both include the same non-custom tag; to fix,
deduplicate by id before calling getAvailableSuggestedTags (or update
getAvailableSuggestedTags to dedupe internally) by building a map/Set keyed on
tag.id from the merged arrays and then passing the unique list along with
workingUserTags.value; apply the same dedupe change to every place that
currently does [...allSuggestedTags.value, ...releasedSuggestedTags.value] so
rendering with :key="tag.id" is stable.
---
Nitpick comments:
In `@app/components/TagManagerModal.vue`:
- Around line 240-250: The UButton list renders only the tag name with a
trailing "x" icon, which hides the destructive action from screen readers;
update the UButton (rendered in the v-for over workingUserTags, using
tag.id/tag.name and removeUserTag) to include explicit assistive text such as
"Remove tag {tag.name}" for screen readers (either by adding a visually-hidden
span inside the button or setting an appropriate aria-label on the button) and
ensure the decorative icon is aria-hidden so screen readers hear only the remove
text.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 15163bcb-66fd-4ced-a370-80559ddd19ca
⛔ Files ignored due to path filters (1)
server/db/migrations/sqlite/0005_add_normalized_name_to_tags.sqlis excluded by!server/db/migrations/**
📒 Files selected for processing (4)
app/components/TagManagerModal.vueserver/api/books/lookup.post.tsserver/repositories/book.repository.tsserver/services/book.service.ts
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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)
app/components/TagManagerModal.vue (1)
101-123:⚠️ Potential issue | 🟠 MajorNormalize the canonical tag value before matching or creating.
normalizeTagInputText(raw)now changes the stored/displayed value, but the duplicate/suggested-tag checks still compare the pre-normalizedrawstring. That lets inputs like"History!"bypass the existing-tag match and lets punctuation-only input collapse into an empty custom tag.🔧 Proposed fix
function addCustomTag() { const raw = customTagInput.value.trim() if (!raw) return - const normalized = normalizeTagName(raw) + const normalizedInput = normalizeTagInputText(raw) + if (!normalizedInput) { + customTagInput.value = '' + return + } + + const displayName = toSensibleTitleCase(normalizedInput) + const normalized = normalizeTagName(displayName) const alreadyInUserTags = workingUserTags.value.some(tag => normalizeTagName(tag.name) === normalized) if (alreadyInUserTags) { customTagInput.value = '' return } @@ - const displayName = toSensibleTitleCase(normalizeTagInputText(raw)) - workingUserTags.value.push({ id: `custom:${crypto.randomUUID()}`, name: displayName, isCustom: true })I’d also mirror the same canonical-empty check in the Add button disabled state so the UI doesn’t offer a submit that becomes a no-op.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/TagManagerModal.vue` around lines 101 - 123, In addCustomTag, normalize the input first by calling normalizeTagInputText(raw) (or a new const canonical = normalizeTagInputText(raw)) and use normalizeTagName(canonical) for all duplicate and suggested-tag checks instead of using raw; if the canonical string is empty after normalization, clear customTagInput.value and return to avoid creating empty tags; keep displayName = toSensibleTitleCase(normalizeTagInputText(raw)) but base its source on the canonical variable; also update the Add button disabled logic to consult the same canonical-empty check so the button is disabled when the normalized input is empty (referencing customTagInput, normalizeTagInputText and the Add button computed/prop used in the template).
🤖 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 `@app/components/TagManagerModal.vue`:
- Around line 101-123: In addCustomTag, normalize the input first by calling
normalizeTagInputText(raw) (or a new const canonical =
normalizeTagInputText(raw)) and use normalizeTagName(canonical) for all
duplicate and suggested-tag checks instead of using raw; if the canonical string
is empty after normalization, clear customTagInput.value and return to avoid
creating empty tags; keep displayName =
toSensibleTitleCase(normalizeTagInputText(raw)) but base its source on the
canonical variable; also update the Add button disabled logic to consult the
same canonical-empty check so the button is disabled when the normalized input
is empty (referencing customTagInput, normalizeTagInputText and the Add button
computed/prop used in the template).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06874ba8-a232-4fd4-949d-97be82abe153
📒 Files selected for processing (1)
app/components/TagManagerModal.vue
1 participant
Summary by CodeRabbit
New Features
UI Improvements
Bug Fixes