feat: Add privacy-first lending history and invite flow#24
Open
niklhut wants to merge 9 commits into
Open
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughImplements a full lending feature: DB schema and types, repositories/services with Effect layers, API endpoints for loans and invites, UI components/pages (lending modal, loans dashboard, invite acceptance), soft-delete removal with confirm flow, and tests. ChangesBook Lending Feature Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![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: 6
🧹 Nitpick comments (2)
test/unit/server/api/invite/[token]/accept.post.test.ts (1)
20-43: ⚡ Quick winAdd a missing-token validation test for the accept endpoint.
You already cover auth + success; adding a no-token case will lock in the 400 contract and prevent accidental service calls with undefined tokens.
Proposed test addition
describe('server/api/invite/[token]/accept.post', () => { beforeEach(setupApiRouteTest) afterEach(cleanupApiRouteTest) itRequiresAuth(route, { params: { token: 'token-1' } }) it('accepts a book invite for the signed-in user', async () => { @@ await expect(handler(makeEvent({ params: { token: 'token-1' } }))).resolves.toEqual(borrowed) expect(serviceMocks.acceptBookInvite).toHaveBeenCalledWith('token-1', 'user-1') }) + + it('rejects missing tokens', async () => { + mockLoggedInUser() + const handler = await importRoute(route) + + await expect(handler(makeEvent())).rejects.toMatchObject({ + statusCode: 400, + message: 'Invitation token is required' + }) + }) })🤖 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 `@test/unit/server/api/invite/`[token]/accept.post.test.ts around lines 20 - 43, Add a test that calls the accept endpoint with no token and asserts it returns a 400 and does not call the service; specifically, in test/unit/server/api/invite/[token]/accept.post.test.ts add an it block that imports the route via importRoute, invokes handler(makeEvent({ params: {} })) (or params omitted), expects the promise to reject or resolve with a 400 error/response, and asserts serviceMocks.acceptBookInvite was not called; reference the existing symbols route, importRoute, makeEvent, and serviceMocks.acceptBookInvite to mirror the other tests' structure.app/pages/i/[token].vue (1)
35-61: ⚡ Quick winRemove redundant state reset before navigation.
Line 48 sets
isAccepting.value = falsebeforenavigateTo, but thefinallyblock (line 59) always resets it afterward. Remove the redundant line 48 to simplify the control flow.♻️ Proposed fix
}) toast.add({ title: 'Added to borrowed books', color: 'success' }) - isAccepting.value = false await navigateTo({ path: '/library/loans', query: { view: 'borrowed' } }) } catch (err: unknown) {🤖 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/i/`[token].vue around lines 35 - 61, The acceptBook function contains a redundant state reset: remove the premature assignment to isAccepting.value = false that occurs before calling navigateTo, leaving the final cleanup in the finally block to always reset isAccepting; update the acceptBook function to rely on the existing finally block for resetting isAccepting and keep the toast and navigateTo calls intact.
🤖 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/components/BookLendingModal.vue`:
- Around line 24-28: The computed property tomorrowDate uses toISOString() which
is UTC-based and can produce the wrong calendar day for input type="date";
update the tomorrowDate computed (and any place using it as the min) to produce
a local "YYYY-MM-DD" string (e.g., build the value from date.getFullYear(),
date.getMonth()+1, date.getDate() with zero-padding or use
toLocaleDateString('en-CA')) after incrementing the day so the min date reflects
the local date instead of UTC.
In `@server/db/schema/domain.ts`:
- Around line 113-137: Add DB-level uniqueness by creating two unique partial
indexes on the loans table: a unique index on userBookId for active loans and a
unique index on acceptTokenHash when not null. In the table definition where
indexes are declared (refer to table, table.userBookId, table.status, and
table.acceptTokenHash), replace or add to the existing index(...) calls with
uniqueIndex(...) calls using a WHERE clause/partial predicate: one uniqueIndex
on table.userBookId with condition sql`${table.status} = 'active'`, and one
uniqueIndex on table.acceptTokenHash with condition sql`${table.acceptTokenHash}
IS NOT NULL` so concurrent writes cannot create multiple active loans or
duplicate non-null accept tokens.
In `@server/repositories/lending.repository.ts`:
- Around line 321-351: The invite preview and acceptance must verify the owner
copy still exists and isn't soft-deleted: in getInvitePreviewByHash (and the
sibling method around lines 354-379), extend the db query that reads from loans
to also confirm the owner's userBooks row for loan.ownerUserId and
loan.snapshotBookId is present and not soft-deleted (e.g., userBooks.removedAt
IS NULL or equivalent active flag), or alternatively mark unaccepted invites
cancelled during removeFromLibrary when confirmActiveLoan is true; if the owner
copy is missing treat the invite as invalid (return the same InvalidInviteError
or set canAccept=false and status='unavailable') so pending invites cannot be
accepted for removed owner copies.
- Around line 137-188: The createLoan flow currently uses a non-atomic preflight
check (snapshotForBook + existing query) and must be backed by a DB uniqueness
constraint on (userBookId, ownerUserId, status) for status='active' (partial
unique index) so the DB enforces a single active loan; then update createLoan to
handle uniqueness-violation errors from the insertion (the try/catch in the
Effect.tryPromise with operation 'createLoan.insert') by mapping the constraint
error to ActiveLoanExistsError (instead of only relying on the preflight), keep
or remove the preflight check as desired but ensure the insert error path
converts the DB constraint violation into new ActiveLoanExistsError and other
errors remain wrapped as DatabaseError.
- Around line 208-233: The returnLoan update allows marking invites that were
never accepted as returned; modify the where clause in returnLoan to also
require the invite was accepted by adding a check that the accept token is null
(e.g. include eq(loans.acceptTokenHash, null) alongside eq(loans.id, loanId),
eq(loans.ownerUserId, ownerUserId) and eq(loans.status, 'active')) so only truly
accepted/borrowed loans can be returned and pending invites cannot be bypassed.
In `@shared/utils/schemas.ts`:
- Around line 225-235: The refine() validation for dueAt currently compares a
Date parsed from nullableDateSchema against a local "today" midnight, causing
timezone-dependent failures for date-only inputs; update the validation to
compare calendar dates instead of absolute timestamps by either: (a) normalizing
value.dueAt to local date-only (extract year/month/day using the Date methods)
and compare those to today's local year/month/day, or (b) change
nullableDateSchema to parse date-only strings into a local Date (e.g., treat
'YYYY-MM-DD' as 'YYYY-MM-DDT00:00' in local time) before the refine check;
ensure you update the check within the .refine() for dueAt so it uses the
adjusted local-date comparison logic rather than direct >= Date comparisons.
---
Nitpick comments:
In `@app/pages/i/`[token].vue:
- Around line 35-61: The acceptBook function contains a redundant state reset:
remove the premature assignment to isAccepting.value = false that occurs before
calling navigateTo, leaving the final cleanup in the finally block to always
reset isAccepting; update the acceptBook function to rely on the existing
finally block for resetting isAccepting and keep the toast and navigateTo calls
intact.
In `@test/unit/server/api/invite/`[token]/accept.post.test.ts:
- Around line 20-43: Add a test that calls the accept endpoint with no token and
asserts it returns a 400 and does not call the service; specifically, in
test/unit/server/api/invite/[token]/accept.post.test.ts add an it block that
imports the route via importRoute, invokes handler(makeEvent({ params: {} }))
(or params omitted), expects the promise to reject or resolve with a 400
error/response, and asserts serviceMocks.acceptBookInvite was not called;
reference the existing symbols route, importRoute, makeEvent, and
serviceMocks.acceptBookInvite to mirror the other tests' structure.
🪄 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: 1801e69b-4d31-42e4-917f-88669663d4a8
⛔ Files ignored due to path filters (2)
server/db/migrations/sqlite/0010_lending_history.sqlis excluded by!server/db/migrations/**server/db/migrations/sqlite/meta/_journal.jsonis excluded by!server/db/migrations/**
📒 Files selected for processing (32)
app/components/AppHeader.vueapp/components/BookCard.vueapp/components/BookLendingModal.vueapp/pages/(auth)/login.vueapp/pages/(auth)/register.vueapp/pages/i/[token].vueapp/pages/library/[id].vueapp/pages/library/borrowed.vueapp/pages/library/index.vueapp/pages/library/loans.vueserver/api/books/[id]/index.delete.tsserver/api/books/[id]/loans/index.post.tsserver/api/borrowed/index.get.tsserver/api/invite/[token]/accept.post.tsserver/api/invite/[token]/cover.get.tsserver/api/invite/[token]/index.get.tsserver/api/loans/[id]/cancel.post.tsserver/api/loans/[id]/return.post.tsserver/api/loans/index.get.tsserver/db/schema/domain.tsserver/repositories/book.repository.tsserver/repositories/lending.repository.tsserver/services/book.service.tsserver/services/lending.service.tsserver/utils/effect.tsshared/types/book.tsshared/utils/schemas.tstest/unit/server/api/_helpers/api-route.tstest/unit/server/api/books/[id]/index.delete.test.tstest/unit/server/api/books/[id]/loans/index.post.test.tstest/unit/server/api/invite/[token]/accept.post.test.tstest/unit/server/api/invite/[token]/index.get.test.ts
Comment on lines
+208
to
+233
| returnLoan: (loanId, ownerUserId) => | ||
| Effect.gen(function* () { | ||
| const now = new Date() | ||
| const rows = yield* Effect.tryPromise({ | ||
| try: () => dbService.db | ||
| .update(loans) | ||
| .set({ | ||
| status: 'returned', | ||
| returnedAt: now, | ||
| acceptTokenHash: null, | ||
| updatedAt: now | ||
| }) | ||
| .where(and(eq(loans.id, loanId), eq(loans.ownerUserId, ownerUserId), eq(loans.status, 'active'))) | ||
| .returning(), | ||
| catch: error => new DatabaseError({ | ||
| message: `Failed to return loan: ${error}`, | ||
| operation: 'returnLoan' | ||
| }) | ||
| }) | ||
|
|
||
| if (!rows[0]) { | ||
| return yield* Effect.fail(new LoanNotFoundError({ loanId })) | ||
| } | ||
|
|
||
| return toOwnerLoan(rows[0]) | ||
| }), |
Contributor
There was a problem hiding this comment.
Block returnLoan for invites that were never accepted.
This path only checks status = 'active', so a pending invite can be marked as returned even though nobody ever borrowed it. That creates impossible history and bypasses the dedicated cancel flow.
Suggested guard
returnLoan: (loanId, ownerUserId) =>
Effect.gen(function* () {
const now = new Date()
const rows = yield* Effect.tryPromise({
try: () => dbService.db
.update(loans)
.set({
status: 'returned',
returnedAt: now,
acceptTokenHash: null,
updatedAt: now
})
- .where(and(eq(loans.id, loanId), eq(loans.ownerUserId, ownerUserId), eq(loans.status, 'active')))
+ .where(and(
+ eq(loans.id, loanId),
+ eq(loans.ownerUserId, ownerUserId),
+ eq(loans.status, 'active'),
+ isNotNull(loans.acceptedAt)
+ ))
.returning(),📝 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.
Suggested change
| returnLoan: (loanId, ownerUserId) => | |
| Effect.gen(function* () { | |
| const now = new Date() | |
| const rows = yield* Effect.tryPromise({ | |
| try: () => dbService.db | |
| .update(loans) | |
| .set({ | |
| status: 'returned', | |
| returnedAt: now, | |
| acceptTokenHash: null, | |
| updatedAt: now | |
| }) | |
| .where(and(eq(loans.id, loanId), eq(loans.ownerUserId, ownerUserId), eq(loans.status, 'active'))) | |
| .returning(), | |
| catch: error => new DatabaseError({ | |
| message: `Failed to return loan: ${error}`, | |
| operation: 'returnLoan' | |
| }) | |
| }) | |
| if (!rows[0]) { | |
| return yield* Effect.fail(new LoanNotFoundError({ loanId })) | |
| } | |
| return toOwnerLoan(rows[0]) | |
| }), | |
| returnLoan: (loanId, ownerUserId) => | |
| Effect.gen(function* () { | |
| const now = new Date() | |
| const rows = yield* Effect.tryPromise({ | |
| try: () => dbService.db | |
| .update(loans) | |
| .set({ | |
| status: 'returned', | |
| returnedAt: now, | |
| acceptTokenHash: null, | |
| updatedAt: now | |
| }) | |
| .where(and( | |
| eq(loans.id, loanId), | |
| eq(loans.ownerUserId, ownerUserId), | |
| eq(loans.status, 'active'), | |
| isNotNull(loans.acceptedAt) | |
| )) | |
| .returning(), | |
| catch: error => new DatabaseError({ | |
| message: `Failed to return loan: ${error}`, | |
| operation: 'returnLoan' | |
| }) | |
| }) | |
| if (!rows[0]) { | |
| return yield* Effect.fail(new LoanNotFoundError({ loanId })) | |
| } | |
| return toOwnerLoan(rows[0]) | |
| }), |
🤖 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 `@server/repositories/lending.repository.ts` around lines 208 - 233, The
returnLoan update allows marking invites that were never accepted as returned;
modify the where clause in returnLoan to also require the invite was accepted by
adding a check that the accept token is null (e.g. include
eq(loans.acceptTokenHash, null) alongside eq(loans.id, loanId),
eq(loans.ownerUserId, ownerUserId) and eq(loans.status, 'active')) so only truly
accepted/borrowed loans can be returned and pending invites cannot be bypassed.
niklhut
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/pages/library/loans.vue`:
- Around line 199-205: The card-level keydown handlers for Enter/Space are
allowing key events from nested controls (like the "Mark returned" button) to
bubble and trigger openOwnerBook(loan); update the card element's keydown
bindings to stop propagation by changing
`@keydown.enter.prevent`="openOwnerBook(loan)" and
`@keydown.space.prevent`="openOwnerBook(loan)" to
`@keydown.enter.prevent.stop`="openOwnerBook(loan)" and
`@keydown.space.prevent.stop`="openOwnerBook(loan)" so keyboard activation of the
inner button won't navigate away (note the button already has `@click.stop` but
keydown needs .stop too).
In `@shared/utils/schemas.ts`:
- Around line 181-193: The preprocessor for localNullableDateSchema currently
returns an empty string when the input is cleared, causing z.date() validation
to fail; update the preprocess function used by localNullableDateSchema so that
when val === '' it returns undefined (i.e., treat cleared due date as absent),
keep returning null for explicit null and continue converting valid date strings
to Date objects so the downstream z.date().nullable().optional() accepts unset
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 82bae3ee-b41c-4d47-a5bb-de28772d9411
⛔ Files ignored due to path filters (2)
server/db/migrations/sqlite/0011_lending_unique_indexes.sqlis excluded by!server/db/migrations/**server/db/migrations/sqlite/meta/_journal.jsonis excluded by!server/db/migrations/**
📒 Files selected for processing (7)
app/components/BookLendingModal.vueapp/pages/i/[token].vueapp/pages/library/loans.vueserver/db/schema/domain.tsserver/repositories/lending.repository.tsshared/utils/schemas.tstest/unit/server/api/invite/[token]/accept.post.test.ts
💤 Files with no reviewable changes (1)
- app/pages/i/[token].vue
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)
shared/utils/schemas.ts (1)
258-258:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winChange
.optional().default(false)to.default(false).optional()to match Zod v4 chaining guidelines.In Zod v4,
.optional()must be called after.default(), not before. The current chaining order (.optional().default(false)) is incorrect and can cause runtime mismatches. Change it to.default(false).optional().Current code
confirmActiveLoan: z.boolean().optional().default(false)🤖 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 `@shared/utils/schemas.ts` at line 258, The Zod schema property confirmActiveLoan uses the wrong chaining order (.optional().default(false)); change the chain to call .default(false) before .optional() so it becomes .default(false).optional() to comply with Zod v4 guidelines and avoid runtime mismatches; update the confirmActiveLoan property in the schema where it's defined to use this ordering.
♻️ Duplicate comments (1)
app/pages/library/loans.vue (1)
204-205:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
.selfon card keydown handlers to avoid keyboard-triggered misnavigation.Line 204 and Line 205 still fire when Enter/Space originates from nested controls (like “Mark returned”).
.stopon the parent handler does not filter by event target, so the card navigation can still run.💡 Minimal fix
- `@keydown.enter.prevent.stop`="openOwnerBook(loan)" - `@keydown.space.prevent.stop`="openOwnerBook(loan)" + `@keydown.enter.self.prevent`="openOwnerBook(loan)" + `@keydown.space.self.prevent`="openOwnerBook(loan)"In Vue 3, does `@keydown.enter.prevent.stop` on a parent still run when the keydown starts on a focused child button? Is `@keydown.enter.self.prevent` the correct way to restrict handling to the parent element itself?🤖 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/library/loans.vue` around lines 204 - 205, The card-level keydown handlers for navigation call openOwnerBook(loan) even when Enter/Space originate from focused child controls; update the attributes on the card element to use the .self modifier so the handler only runs when the card itself is the event target (e.g., change `@keydown.enter.prevent.stop`="openOwnerBook(loan)" and `@keydown.space.prevent.stop`="openOwnerBook(loan)" to `@keydown.enter.self.prevent.stop`="openOwnerBook(loan)" and `@keydown.space.self.prevent.stop`="openOwnerBook(loan)"), ensuring nested buttons like “Mark returned” won’t trigger openOwnerBook.
🤖 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.
Outside diff comments:
In `@shared/utils/schemas.ts`:
- Line 258: The Zod schema property confirmActiveLoan uses the wrong chaining
order (.optional().default(false)); change the chain to call .default(false)
before .optional() so it becomes .default(false).optional() to comply with Zod
v4 guidelines and avoid runtime mismatches; update the confirmActiveLoan
property in the schema where it's defined to use this ordering.
---
Duplicate comments:
In `@app/pages/library/loans.vue`:
- Around line 204-205: The card-level keydown handlers for navigation call
openOwnerBook(loan) even when Enter/Space originate from focused child controls;
update the attributes on the card element to use the .self modifier so the
handler only runs when the card itself is the event target (e.g., change
`@keydown.enter.prevent.stop`="openOwnerBook(loan)" and
`@keydown.space.prevent.stop`="openOwnerBook(loan)" to
`@keydown.enter.self.prevent.stop`="openOwnerBook(loan)" and
`@keydown.space.self.prevent.stop`="openOwnerBook(loan)"), ensuring nested buttons
like “Mark returned” won’t trigger openOwnerBook.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a955d221-6d85-4d32-9e84-7f388e86b9ee
📒 Files selected for processing (2)
app/pages/library/loans.vueshared/utils/schemas.ts
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
niklhut
commented
May 23, 2026
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/components/BorrowedLoansSection.vue`:
- Around line 149-151: The template currently renders "Returned {{
formatDate(book.returnedAt) }}" even when book.returnedAt is null, causing
"Returned null"; update the conditional in BorrowedLoansSection.vue so you only
call formatDate(book.returnedAt) when returnedAt is present/valid (e.g.,
book.returnedAt && formatDate(book.returnedAt)), and provide a fallback label
such as "Returned (date unknown)" or "Returned — unknown" when returnedAt is
missing; keep the check tied to the existing book.status === 'returned' branch
and reuse the existing formatDate helper.
In `@app/components/LoanedOutLoansSection.vue`:
- Around line 195-197: The template in LoanedOutLoansSection.vue can render
"Returned null" when formatDate(loan.returnedAt) returns null; update the
expression that builds the returned label (the line using loan.status and
formatDate) to provide a fallback when formatDate returns a falsy value (e.g.,
use a default like "date unknown" or "—") so that when loan.status ===
'returned' you render "Returned <fallback>" instead of "Returned null"; adjust
the inline ternary or move this logic into a small computed/helper near the
component (referencing loan.status, loan.returnedAt, and formatDate) to ensure a
safe display.
🪄 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: 5022264d-f7ef-43ed-a2fe-0c5fc6254d20
⛔ Files ignored due to path filters (2)
server/db/migrations/sqlite/0012_remove_loan_owner_note.sqlis excluded by!server/db/migrations/**server/db/migrations/sqlite/meta/_journal.jsonis excluded by!server/db/migrations/**
📒 Files selected for processing (8)
app/components/BorrowedLoansSection.vueapp/components/LoanedOutLoansSection.vueapp/pages/library/loans.vueserver/db/schema/domain.tsserver/repositories/lending.repository.tsserver/services/lending.service.tsshared/utils/schemas.tstest/unit/server/api/books/[id]/loans/index.post.test.ts
💤 Files with no reviewable changes (3)
- server/db/schema/domain.ts
- server/services/lending.service.ts
- server/repositories/lending.repository.ts
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/components/LoanedOutLoansSection.vue (1)
20-34: ⚡ Quick winExtract shared loan-display date helpers to avoid drift.
formatDateandreturnedLabelare duplicated here and inBorrowedLoansSection.vue; this already caused mirrored fixes once. Moving them into a shared helper/composable will keep labels consistent and reduce regression risk.🤖 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/components/LoanedOutLoansSection.vue` around lines 20 - 34, Extract the duplicated formatDate and returnedLabel functions into a single shared helper/composable that exports those same-named functions (e.g., create a useLoanDateHelpers module exporting formatDate and returnedLabel); keep the exact behavior and TypeScript signatures (formatDate(value: Date | string | null): string | null and returnedLabel(value: Date | string | null): string) and then remove the local definitions from LoanedOutLoansSection.vue and BorrowedLoansSection.vue and replace them with imports from the new helper (use the exported function names formatDate and returnedLabel in both components so existing call sites remain unchanged).
🤖 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.
Nitpick comments:
In `@app/components/LoanedOutLoansSection.vue`:
- Around line 20-34: Extract the duplicated formatDate and returnedLabel
functions into a single shared helper/composable that exports those same-named
functions (e.g., create a useLoanDateHelpers module exporting formatDate and
returnedLabel); keep the exact behavior and TypeScript signatures
(formatDate(value: Date | string | null): string | null and returnedLabel(value:
Date | string | null): string) and then remove the local definitions from
LoanedOutLoansSection.vue and BorrowedLoansSection.vue and replace them with
imports from the new helper (use the exported function names formatDate and
returnedLabel in both components so existing call sites remain unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b2b9f1b5-c5cc-40a4-963d-c6a7e3198aac
⛔ Files ignored due to path filters (2)
server/db/migrations/sqlite/0012_remove_loan_owner_note.sqlis excluded by!server/db/migrations/**server/db/migrations/sqlite/meta/_journal.jsonis excluded by!server/db/migrations/**
📒 Files selected for processing (8)
app/components/BorrowedLoansSection.vueapp/components/LoanedOutLoansSection.vueapp/pages/library/loans.vueserver/db/schema/domain.tsserver/repositories/lending.repository.tsserver/services/lending.service.tsshared/utils/schemas.tstest/unit/server/api/books/[id]/loans/index.post.test.ts
💤 Files with no reviewable changes (3)
- server/db/schema/domain.ts
- server/services/lending.service.ts
- server/repositories/lending.repository.ts
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
1 participant
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.
Summary
/library/loanswith a dedicated switcher for books loaned out vs books lent to you, plus public invite pages and accept flows.Testing
pnpm lint,pnpm typecheck, andpnpm test:unitpassed.Summary by CodeRabbit
New Features
Improvements