feat: add Rent Invoices + Payments modules (issues/012-014) - #139
Open
HadiSaaibi wants to merge 5 commits into
Open
feat: add Rent Invoices + Payments modules (issues/012-014)#139HadiSaaibi wants to merge 5 commits into
HadiSaaibi wants to merge 5 commits into
Conversation
Invoice/InvoiceLineItem schema, computeInvoiceSummary deep module, read-only InvoicesService/Controller, invoices permission area, and the filtered list page. Write path lands in issues/013. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
InvoicesService.create/update/remove with wholesale line-item replacement on update, LeasesService.remove guarded against referenced invoices, and the New/Edit Invoice dialog (cascading lease picker, repeatable line-item rows) plus delete confirmation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
InvoicePayment schema + InvoicePaymentsService (create/findAll/ findOne/remove), building-scoped via the parent Invoice, returning recomputed totalAmount/paidAmount/status on create/remove. InvoicesService.remove now also rejects when a payment references the invoice. Adds the Invoice detail page showing line items and payment history together, with Record Payment and delete actions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 50 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds invoice and invoice-payment database models, contracts, secured NestJS APIs, computed status summaries, RTK Query integration, authorization rules, and dashboard pages for invoice and payment management. ChangesInvoice Management
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant InvoicesPage
participant invoicesApi
participant NextAPI
participant InvoicesController
participant InvoicesService
participant PrismaService
User->>InvoicesPage: create or update invoice
InvoicesPage->>invoicesApi: submit invoice mutation
invoicesApi->>NextAPI: POST or PATCH /api/invoices
NextAPI->>InvoicesController: forward request
InvoicesController->>InvoicesService: create or update scoped invoice
InvoicesService->>PrismaService: persist invoice and line items
PrismaService-->>InvoicesService: invoice record
InvoicesService-->>InvoicesPage: formatted invoice response
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (18)
apps/api/src/modules/invoice-payments/invoice-payments.service.ts-32-38 (1)
32-38: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse the mandated service infrastructure.
Inject
DatabaseServiceinstead ofPrismaService, and add the required NestJSLoggerfield.As per coding guidelines, API services must access Prisma through
DatabaseServiceand declare a NestJSLogger.🤖 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 `@apps/api/src/modules/invoice-payments/invoice-payments.service.ts` around lines 32 - 38, Update InvoicePaymentsService to inject DatabaseService in place of PrismaService, and replace the corresponding field type and usage references accordingly. Add a NestJS Logger field to the service, following the existing service logging convention.Source: Coding guidelines
apps/api/src/modules/invoice-payments/invoice-payments.service.ts-165-185 (1)
165-185: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAvoid partially completed payment operations.
apps/api/src/modules/invoice-payments/invoice-payments.service.ts#L165-L185: atomically persist the payment and timeline event, or make creation idempotent.apps/api/src/modules/invoice-payments/invoice-payments.service.ts#L203-L214: atomically delete the payment and record its timeline event.🤖 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 `@apps/api/src/modules/invoice-payments/invoice-payments.service.ts` around lines 165 - 185, The invoice payment create and delete flows can leave database state and timeline events inconsistent. Update the relevant create flow around prisma.invoicePayment.create and the delete flow at the specified sibling site to use a shared Prisma transaction that atomically persists/deletes the payment and records the corresponding timeline event; preserve the existing event data and return behavior.apps/api/src/modules/invoice-payments/invoice-payments.service.ts-63-73 (1)
63-73: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winApply organization scope to every invoice-payment query.
apps/api/src/modules/invoice-payments/invoice-payments.service.ts#L63-L73: acceptorgIdand query the invoice with{ id: invoiceId, orgId }.apps/api/src/modules/invoice-payments/invoice-payments.service.ts#L203-L203: perform the deletion with bothidandorgId.apps/api/src/modules/invoice-payments/invoice-payments.service.spec.ts#L250-L252: assert the scoped deletion contract.As per coding guidelines, every Prisma query on tenant-scoped models must filter by organization ID.
🤖 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 `@apps/api/src/modules/invoice-payments/invoice-payments.service.ts` around lines 63 - 73, Scope all invoice Prisma operations by organization ID: update summarizeInvoice to accept orgId and include it with invoiceId in the invoice lookup, update the deletion query to filter by both id and orgId, and extend the corresponding service spec assertion to verify the scoped deletion contract. Apply changes at apps/api/src/modules/invoice-payments/invoice-payments.service.ts lines 63-73 and 203, and apps/api/src/modules/invoice-payments/invoice-payments.service.spec.ts lines 250-252.Source: Coding guidelines
apps/api/src/modules/invoice-payments/dto/create-invoice-payment.dto.ts-1-36 (1)
1-36: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftReplace the inline DTO with the shared Zod contract.
Define this request in
packages/contracts, export its inferred type, and validate it in the controller usingZodValidationPipe. ImportingInvoicePaymentMethodfrom@repo/dbalso couples the public API contract to persistence.As per coding guidelines, wire schemas must come from
@repo/contractsand API request inputs must useZodValidationPipe.🤖 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 `@apps/api/src/modules/invoice-payments/dto/create-invoice-payment.dto.ts` around lines 1 - 36, Replace CreateInvoicePaymentDto’s class-validator decorators with a shared Zod request schema defined and exported from packages/contracts, including the inferred request type and an API-safe payment-method definition rather than importing InvoicePaymentMethod from `@repo/db`. Update the corresponding controller endpoint to accept this contract through ZodValidationPipe and remove the inline DTO usage.Source: Coding guidelines
apps/web/src/components/dashboard/invoice-detail-page.tsx-154-155 (1)
154-155: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle payment-list failures instead of showing an empty history.
When the query fails,
paymentsis undefined, so the page incorrectly renders “No payments recorded yet.” CaptureisErrorand render an explicit retry/error state.Also applies to: 345-375
🤖 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 `@apps/web/src/components/dashboard/invoice-detail-page.tsx` around lines 154 - 155, Update the useListInvoicePaymentsQuery call to capture its isError state, then adjust the payment history rendering to show an explicit payment-loading error and retry action when the query fails instead of treating undefined payments as an empty history. Preserve the existing empty-state rendering for successful queries with no payments.apps/web/src/components/dashboard/invoices-page.tsx-210-251 (1)
210-251: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd accessible labels to every line-item control.
The category, description, and amount controls have no associated labels. Add unique
Labelelements or descriptivearia-labelattributes so screen-reader users can complete the required form.🤖 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 `@apps/web/src/components/dashboard/invoices-page.tsx` around lines 210 - 251, Add accessible labeling to the category Select, description Input, and amount Input within the line-item rendering block. Use unique per-row identifiers based on idPrefix and index, associate visible Label elements through matching htmlFor/id attributes or provide descriptive aria-labels, and preserve the existing form registration and validation behavior.apps/web/src/components/dashboard/invoice-detail-page.tsx-70-75 (1)
70-75: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReplace raw status palette colors with design tokens.
apps/web/src/components/dashboard/invoice-detail-page.tsx#L70-L75: use semantic tokens fromglobals.css.apps/web/src/components/dashboard/invoices-page.tsx#L84-L89: share the same token-based status styles.As per coding guidelines, Tailwind components must use project design tokens instead of arbitrary color values.
🤖 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 `@apps/web/src/components/dashboard/invoice-detail-page.tsx` around lines 70 - 75, Replace the raw Tailwind color classes in STATUS_STYLES within apps/web/src/components/dashboard/invoice-detail-page.tsx at lines 70-75 with the semantic design tokens defined in globals.css. Apply the same token-based status styling to the corresponding STATUS_STYLES mapping in apps/web/src/components/dashboard/invoices-page.tsx at lines 84-89, keeping status-specific variants consistent between both components.Source: Coding guidelines
apps/web/src/components/dashboard/invoice-detail-page.tsx-54-59 (1)
54-59: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winImport wire contract types directly from
@repo/contracts.Both components consume API response enums and shapes through
@/types/api, allowing the web definitions to drift from the shared contract.
apps/web/src/components/dashboard/invoice-detail-page.tsx#L54-L59: import invoice and payment response types from@repo/contracts.apps/web/src/components/dashboard/invoices-page.tsx#L67-L71: import invoice response types and enums from@repo/contracts.As per coding guidelines, “Both the API and web app must import from
@repo/contracts.”🤖 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 `@apps/web/src/components/dashboard/invoice-detail-page.tsx` around lines 54 - 59, Replace the invoice and payment API type imports in invoice-detail-page.tsx with their corresponding exports from `@repo/contracts`, preserving the existing type usage. Apply the same migration in invoices-page.tsx for its invoice response types and enums; both components must consume shared wire-contract definitions directly rather than importing from `@/types/api`.Source: Coding guidelines
apps/web/src/components/dashboard/invoice-detail-page.tsx-185-210 (1)
185-210: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve actionable
ApiErrormessages in mutation handlers.The generic catches hide validation, authorization, and conflict details—especially the invoice deletion guard for recorded payments.
apps/web/src/components/dashboard/invoice-detail-page.tsx#L185-L210: catchApiErrorand toast its message for payment mutations.apps/web/src/components/dashboard/invoices-page.tsx#L496-L560: do the same for create, edit, and delete mutations.As per coding guidelines, submit handlers must catch
ApiErrorand display it withtoast.🤖 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 `@apps/web/src/components/dashboard/invoice-detail-page.tsx` around lines 185 - 210, Update onRecordSubmit and handleDelete in apps/web/src/components/dashboard/invoice-detail-page.tsx (lines 185-210) to catch ApiError and display its message via toast, while retaining the generic fallback for unknown errors. Apply the same ApiError-aware handling to the create, edit, and delete mutation handlers in apps/web/src/components/dashboard/invoices-page.tsx (lines 496-560), ensuring actionable validation, authorization, and conflict messages are shown.Source: Coding guidelines
apps/web/src/components/dashboard/invoice-detail-page.tsx-63-110 (1)
63-110: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftUse the locale dictionaries for invoice labels and remaining UI copy.
The Arabic route currently renders English status, category, payment-method, and dialog text.
apps/web/src/components/dashboard/invoice-detail-page.tsx#L63-L110: derive labels and rendered copy from the active dictionary.apps/web/src/components/dashboard/invoices-page.tsx#L77-L117: localize labels and the remaining JSX literals through the same mechanism.🤖 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 `@apps/web/src/components/dashboard/invoice-detail-page.tsx` around lines 63 - 110, Replace the hard-coded invoice labels and UI copy with values from the active locale dictionary. In apps/web/src/components/dashboard/invoice-detail-page.tsx#L63-L110, update STATUS_LABELS, LINE_ITEM_CATEGORY_LABELS, PAYMENT_METHOD_LABELS, and related dialog text to use the dictionary; in apps/web/src/components/dashboard/invoices-page.tsx#L77-L117, localize its label constants and remaining JSX literals through the same mechanism, preserving the existing rendering behavior for both locales.apps/web/src/components/dashboard/invoice-detail-page.tsx-265-269 (1)
265-269: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFormat date-only due dates as local dates
new Date(invoice.dueDate).toLocaleDateString()will show the previous calendar day for users west of UTC. Parse theYYYY-MM-DDparts as a local date before formatting in bothapps/web/src/components/dashboard/invoice-detail-page.tsxandapps/web/src/components/dashboard/invoices-page.tsx.🤖 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 `@apps/web/src/components/dashboard/invoice-detail-page.tsx` around lines 265 - 269, Format date-only due dates as local calendar dates by splitting the YYYY-MM-DD value into numeric parts and constructing a local Date before calling toLocaleDateString. Apply this change at the due-date rendering in apps/web/src/components/dashboard/invoice-detail-page.tsx (265-269) and apps/web/src/components/dashboard/invoices-page.tsx (754-756), preserving the existing display behavior aside from preventing UTC day shifts.apps/web/src/components/dashboard/invoices-page.tsx-734-745 (1)
734-745: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPrevent nested action controls from triggering row navigation. The actions menu sits inside a keyboard-activatable row, so Space/Enter on the trigger can bubble into the row’s
onKeyDownand navigate instead of just opening the menu. Guard the row handler withe.target === e.currentTarget, or stop propagation on the trigger.🤖 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 `@apps/web/src/components/dashboard/invoices-page.tsx` around lines 734 - 745, Prevent nested action controls from triggering navigation in the TableRow keyboard handler: update the row’s onKeyDown around goToInvoice so it only handles Enter or Space when e.target equals e.currentTarget, preserving navigation for direct row activation while allowing the actions-menu trigger to handle its own keyboard events.packages/database/prisma/schema.prisma-344-360 (1)
344-360: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winSpecify the schema and use UUIDs for primary keys.
Multiple models are missing the
@@schema("public")declaration and usecuid()instead ofuuid()for the primary key. As per coding guidelines, every model must declare@@schema(...)and use UUID primary keys (@id@default(uuid())).
packages/database/prisma/schema.prisma#L344-L360: In theInvoicemodel, changecuid()touuid()and append@@schema("public").packages/database/prisma/schema.prisma#L362-L373: In theInvoiceLineItemmodel, changecuid()touuid()and append@@schema("public").packages/database/prisma/schema.prisma#L375-L389: In theInvoicePaymentmodel, changecuid()touuid()and append@@schema("public").🤖 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 `@packages/database/prisma/schema.prisma` around lines 344 - 360, Update the Invoice, InvoiceLineItem, and InvoicePayment models in packages/database/prisma/schema.prisma (lines 344-360, 362-373, and 375-389) to use `@default`(uuid()) for their primary key IDs and append @@schema("public") to each model.Source: Coding guidelines
packages/contracts/src/index.ts-646-779 (1)
646-779: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMove schema definitions to separate files.
The invoice and payment schemas are defined directly within
index.ts. As per coding guidelines, you must create one schema per file inpackages/contracts, naming them according to the pattern<resource>-<operation>.{request,response}.ts(e.g.,invoice-create.request.ts), and export them from the folderindex.tsand the mainindex.ts.🤖 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 `@packages/contracts/src/index.ts` around lines 646 - 779, Move the invoice and invoice-payment schema definitions currently in packages/contracts/src/index.ts into separate resource/operation files following the <resource>-<operation>.request.ts or .response.ts naming convention. Update the folder index and main index exports so all existing invoice symbols remain publicly available, including invoiceLineItemCategorySchema, invoiceStatusSchema, invoicePaymentMethodSchema, and the related request/response types.Source: Coding guidelines
apps/api/src/common/invoice-summary/compute-invoice-summary.ts-24-29 (1)
24-29: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAvoid floating-point precision errors in monetary comparisons.
JavaScript floating-point arithmetic can produce rounding errors (e.g.,
0.1 + 0.2 = 0.30000000000000004). IftotalAmountsuffers from this precision loss whilepaidAmountdoes not (or vice versa),paidAmount >= totalAmountmight evaluate tofalsefor a fully paid invoice, leaving it stuck inpartially_paidoroverduestatus.Consider rounding both amounts to the nearest cent before comparison, or use integer cents internally.
💻 Proposed fix
const totalAmount = lineItems.reduce((sum, item) => sum + item.amount, 0); const paidAmount = payments.reduce((sum, p) => sum + p.amount, 0); let status: InvoiceStatus; - if (paidAmount >= totalAmount) { + // Round sums to 2 decimal places to avoid floating-point comparison errors + const roundedTotal = Math.round(totalAmount * 100) / 100; + const roundedPaid = Math.round(paidAmount * 100) / 100; + + if (roundedPaid >= roundedTotal) { status = 'paid'; } else if (dueDate < now) {🤖 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 `@apps/api/src/common/invoice-summary/compute-invoice-summary.ts` around lines 24 - 29, Update the amount calculation and comparison in the invoice summary logic so totalAmount and paidAmount are normalized to integer cents (or rounded to the nearest cent) before the paid status check. Ensure the existing paid, partially_paid, and overdue status flow uses these normalized values and correctly treats fully paid invoices as paid.apps/api/src/modules/invoice-payments/invoice-payments.controller.ts-29-32 (1)
29-32: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate the query and request body with contract-backed Zod pipes.
Both
invoiceIdand the creation payload currently bypassZodValidationPipe. Apply schemas imported from@repo/contracts; otherwise malformed query values and bodies can reach service/Prisma logic.As per coding guidelines, every controller request body and query parameter must use
ZodValidationPipe, with schemas from@repo/contracts.Also applies to: 54-57
🤖 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 `@apps/api/src/modules/invoice-payments/invoice-payments.controller.ts` around lines 29 - 32, Update getInvoicePayments and the creation endpoint to apply ZodValidationPipe to the invoiceId query parameter and request body, using the corresponding schemas imported from `@repo/contracts`. Ensure both query values and creation payloads are validated before reaching the service layer, while preserving the existing controller behavior after validation.Source: Coding guidelines
apps/api/src/modules/invoices/invoices.service.ts-285-294 (1)
285-294: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake reference checks and deletions atomic and tenant-scoped.
Both deletion safeguards can race with creation of a new dependent record.
apps/api/src/modules/invoices/invoices.service.ts#L285-L294: atomically guard invoice deletion against payments, includeorgId, and translate the FK conflict.apps/api/src/modules/leases/leases.service.ts#L333-L342: atomically guard lease deletion against invoices, includeorgId, and translate the FK conflict.🤖 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 `@apps/api/src/modules/invoices/invoices.service.ts` around lines 285 - 294, Update invoice deletion in invoices.service.ts around the payment check and prisma.invoice.delete to use an atomic, tenant-scoped deletion guard including orgId; remove the race-prone pre-count or incorporate the dependency condition into the delete transaction, and translate any resulting foreign-key conflict into the existing ConflictException behavior. Apply the same atomic invoice-reference guard, orgId scoping, and FK-conflict translation to lease deletion in leases.service.ts around its dependent-invoice check and lease delete.Source: Coding guidelines
apps/api/src/modules/invoices/invoices.service.ts-236-257 (1)
236-257: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winKeep replacement mutations tenant-scoped.
The scoped lookup does not satisfy the tenant predicate requirement for the subsequent
deleteManyandupdate. IncludeorgIddirectly—or a relation filter for line items—in both transaction queries.Proposed scope changes
- await tx.invoiceLineItem.deleteMany({ where: { invoiceId } }); + await tx.invoiceLineItem.deleteMany({ + where: { invoiceId, invoice: { orgId } }, + }); return tx.invoice.update({ - where: { id: invoiceId }, + where: { id: invoiceId, orgId },As per coding guidelines, every Prisma query on a tenant-scoped model must include the tenant identifier.
🤖 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 `@apps/api/src/modules/invoices/invoices.service.ts` around lines 236 - 257, Update the transaction queries in the invoice update flow to include the current tenant’s orgId directly in both the invoiceLineItem.deleteMany and invoice.update predicates. Preserve the existing invoiceId filtering and replacement behavior, ensuring every mutation on the tenant-scoped models is tenant-scoped.Source: Coding guidelines
🟡 Minor comments (3)
apps/web/src/i18n/dictionaries/ar.json-21-21 (1)
21-21: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDifferentiate between "Billing" and "Invoices" in Arabic.
Currently, both
nav.billing(line 10) andnav.invoicesare translated as"الفواتير"(Invoices). Since both items appear in the navigation sidebar, having identical labels will confuse users.Consider renaming
nav.billingto"الفوترة"(Billing) or"الاشتراكات"(Subscriptions), depending on the platform's context, to distinguish it from"الفواتير"(Invoices).🤖 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 `@apps/web/src/i18n/dictionaries/ar.json` at line 21, Update the Arabic nav.billing translation to a distinct Billing or Subscriptions label, such as “الفوترة” or “الاشتراكات”, while preserving nav.invoices as “الفواتير”.apps/web/src/app/[lang]/dashboard/invoices/page.tsx-1-1 (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRun Prettier on the new invoice workflow files.
The formatting check currently fails for all three files.
apps/web/src/app/[lang]/dashboard/invoices/page.tsx#L1-L1: apply Prettier.apps/web/src/components/dashboard/invoice-detail-page.tsx#L1-L1: apply Prettier.apps/web/src/components/dashboard/invoices-page.tsx#L1-L1: apply Prettier.🤖 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 `@apps/web/src/app/`[lang]/dashboard/invoices/page.tsx at line 1, Run the repository’s Prettier formatter on apps/web/src/app/[lang]/dashboard/invoices/page.tsx (anchor), apps/web/src/components/dashboard/invoice-detail-page.tsx, and apps/web/src/components/dashboard/invoices-page.tsx, preserving their behavior while applying the formatter’s output.Source: Pipeline failures
apps/api/src/modules/invoices/invoices.service.spec.ts-1-1 (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRun Prettier on this test file.
The current
Prettier --checkCI step fails for this file.🤖 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 `@apps/api/src/modules/invoices/invoices.service.spec.ts` at line 1, Format the test file with Prettier so it passes the repository’s Prettier --check step, preserving the existing test behavior and content.Source: Pipeline failures
🧹 Nitpick comments (2)
apps/web/src/store/api/endpoints/invoice-payments.api.ts (1)
10-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract duplicated
unwraputility.This
unwrapfunction is duplicated ininvoices.api.tsand potentially other API endpoints. Consider extracting it to a shared utilities file (e.g.,src/store/api/utils.tsor exporting it frombase-api.ts) to adhere to the DRY principle.🤖 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 `@apps/web/src/store/api/endpoints/invoice-payments.api.ts` around lines 10 - 14, Extract the duplicated unwrap utility into a shared API utility module or export it from base-api.ts, then update invoices.api.ts and invoice-payments.api.ts to import and reuse that single implementation. Preserve the existing generic behavior and envelope detection logic.apps/api/src/modules/invoices/invoices.service.ts (1)
54-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the required NestJS
Logger.
InvoicesServicehas noLoggerinstance. Add and useprivate readonly logger = new Logger(InvoicesService.name)for unexpected operational failures.As per coding guidelines, “Every service in
apps/api/src/must use NestJS'sLoggerclass.”🤖 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 `@apps/api/src/modules/invoices/invoices.service.ts` around lines 54 - 60, Add NestJS Logger usage to InvoicesService by importing Logger and defining private readonly logger = new Logger(InvoicesService.name). Use this logger for unexpected operational failures within the service.Source: Coding guidelines
🤖 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 `@apps/api/src/modules/invoices/dto/create-invoice.dto.ts`:
- Around line 1-54: Replace the inline class-validator DTOs with Zod contracts
under packages/contracts/src/<resource>/: remove create-invoice.dto.ts and
update-invoice.dto.ts, define schemas with inferred request types, and update
invoices.controller.ts lines 47-50 and 57-61 to apply ZodValidationPipe in
`@Body`() and use those inferred types. Affected sites:
apps/api/src/modules/invoices/dto/create-invoice.dto.ts lines 1-54 and
update-invoice.dto.ts lines 1-31 require removal; both controller ranges require
the pipe and contract-derived payload types.
In `@apps/api/src/modules/invoices/dto/update-invoice.dto.ts`:
- Around line 13-31: Replace the inline class-validator definition in
UpdateInvoiceDto with the corresponding invoice update request Zod schema from
`@repo/contracts`. Define or extend the schema under the contracts invoice
resource for dueDate, nullable notes, and nested lineItems, then import and use
that shared schema in the API while preserving the existing validation and
optionality.
In `@apps/api/src/modules/invoices/invoices.controller.ts`:
- Around line 49-60: Update the invoice controller’s create and update body
parameters to use ZodValidationPipe with the corresponding invoiceCreateSchema
and invoiceUpdateSchema from `@repo/contracts`, replacing class-validator DTO
types with the matching request types. Import the pipe and schemas, while
preserving the existing service calls and authorization flow.
In `@packages/contracts/src/index.ts`:
- Around line 711-727: Define and export Zod schemas for CreateInvoiceBody and
PatchInvoiceBody in the contracts module, matching every field’s requiredness,
nullability, and types from the existing TypeScript definitions. Derive or align
the exported TypeScript types with these schemas where appropriate, and add
corresponding schemas for the other request/response wire shapes in this module
that currently have only type definitions.
---
Major comments:
In `@apps/api/src/common/invoice-summary/compute-invoice-summary.ts`:
- Around line 24-29: Update the amount calculation and comparison in the invoice
summary logic so totalAmount and paidAmount are normalized to integer cents (or
rounded to the nearest cent) before the paid status check. Ensure the existing
paid, partially_paid, and overdue status flow uses these normalized values and
correctly treats fully paid invoices as paid.
In `@apps/api/src/modules/invoice-payments/dto/create-invoice-payment.dto.ts`:
- Around line 1-36: Replace CreateInvoicePaymentDto’s class-validator decorators
with a shared Zod request schema defined and exported from packages/contracts,
including the inferred request type and an API-safe payment-method definition
rather than importing InvoicePaymentMethod from `@repo/db`. Update the
corresponding controller endpoint to accept this contract through
ZodValidationPipe and remove the inline DTO usage.
In `@apps/api/src/modules/invoice-payments/invoice-payments.controller.ts`:
- Around line 29-32: Update getInvoicePayments and the creation endpoint to
apply ZodValidationPipe to the invoiceId query parameter and request body, using
the corresponding schemas imported from `@repo/contracts`. Ensure both query
values and creation payloads are validated before reaching the service layer,
while preserving the existing controller behavior after validation.
In `@apps/api/src/modules/invoice-payments/invoice-payments.service.ts`:
- Around line 32-38: Update InvoicePaymentsService to inject DatabaseService in
place of PrismaService, and replace the corresponding field type and usage
references accordingly. Add a NestJS Logger field to the service, following the
existing service logging convention.
- Around line 165-185: The invoice payment create and delete flows can leave
database state and timeline events inconsistent. Update the relevant create flow
around prisma.invoicePayment.create and the delete flow at the specified sibling
site to use a shared Prisma transaction that atomically persists/deletes the
payment and records the corresponding timeline event; preserve the existing
event data and return behavior.
- Around line 63-73: Scope all invoice Prisma operations by organization ID:
update summarizeInvoice to accept orgId and include it with invoiceId in the
invoice lookup, update the deletion query to filter by both id and orgId, and
extend the corresponding service spec assertion to verify the scoped deletion
contract. Apply changes at
apps/api/src/modules/invoice-payments/invoice-payments.service.ts lines 63-73
and 203, and
apps/api/src/modules/invoice-payments/invoice-payments.service.spec.ts lines
250-252.
In `@apps/api/src/modules/invoices/invoices.service.ts`:
- Around line 285-294: Update invoice deletion in invoices.service.ts around the
payment check and prisma.invoice.delete to use an atomic, tenant-scoped deletion
guard including orgId; remove the race-prone pre-count or incorporate the
dependency condition into the delete transaction, and translate any resulting
foreign-key conflict into the existing ConflictException behavior. Apply the
same atomic invoice-reference guard, orgId scoping, and FK-conflict translation
to lease deletion in leases.service.ts around its dependent-invoice check and
lease delete.
- Around line 236-257: Update the transaction queries in the invoice update flow
to include the current tenant’s orgId directly in both the
invoiceLineItem.deleteMany and invoice.update predicates. Preserve the existing
invoiceId filtering and replacement behavior, ensuring every mutation on the
tenant-scoped models is tenant-scoped.
In `@apps/web/src/components/dashboard/invoice-detail-page.tsx`:
- Around line 154-155: Update the useListInvoicePaymentsQuery call to capture
its isError state, then adjust the payment history rendering to show an explicit
payment-loading error and retry action when the query fails instead of treating
undefined payments as an empty history. Preserve the existing empty-state
rendering for successful queries with no payments.
- Around line 70-75: Replace the raw Tailwind color classes in STATUS_STYLES
within apps/web/src/components/dashboard/invoice-detail-page.tsx at lines 70-75
with the semantic design tokens defined in globals.css. Apply the same
token-based status styling to the corresponding STATUS_STYLES mapping in
apps/web/src/components/dashboard/invoices-page.tsx at lines 84-89, keeping
status-specific variants consistent between both components.
- Around line 54-59: Replace the invoice and payment API type imports in
invoice-detail-page.tsx with their corresponding exports from `@repo/contracts`,
preserving the existing type usage. Apply the same migration in
invoices-page.tsx for its invoice response types and enums; both components must
consume shared wire-contract definitions directly rather than importing from
`@/types/api`.
- Around line 185-210: Update onRecordSubmit and handleDelete in
apps/web/src/components/dashboard/invoice-detail-page.tsx (lines 185-210) to
catch ApiError and display its message via toast, while retaining the generic
fallback for unknown errors. Apply the same ApiError-aware handling to the
create, edit, and delete mutation handlers in
apps/web/src/components/dashboard/invoices-page.tsx (lines 496-560), ensuring
actionable validation, authorization, and conflict messages are shown.
- Around line 63-110: Replace the hard-coded invoice labels and UI copy with
values from the active locale dictionary. In
apps/web/src/components/dashboard/invoice-detail-page.tsx#L63-L110, update
STATUS_LABELS, LINE_ITEM_CATEGORY_LABELS, PAYMENT_METHOD_LABELS, and related
dialog text to use the dictionary; in
apps/web/src/components/dashboard/invoices-page.tsx#L77-L117, localize its label
constants and remaining JSX literals through the same mechanism, preserving the
existing rendering behavior for both locales.
- Around line 265-269: Format date-only due dates as local calendar dates by
splitting the YYYY-MM-DD value into numeric parts and constructing a local Date
before calling toLocaleDateString. Apply this change at the due-date rendering
in apps/web/src/components/dashboard/invoice-detail-page.tsx (265-269) and
apps/web/src/components/dashboard/invoices-page.tsx (754-756), preserving the
existing display behavior aside from preventing UTC day shifts.
In `@apps/web/src/components/dashboard/invoices-page.tsx`:
- Around line 210-251: Add accessible labeling to the category Select,
description Input, and amount Input within the line-item rendering block. Use
unique per-row identifiers based on idPrefix and index, associate visible Label
elements through matching htmlFor/id attributes or provide descriptive
aria-labels, and preserve the existing form registration and validation
behavior.
- Around line 734-745: Prevent nested action controls from triggering navigation
in the TableRow keyboard handler: update the row’s onKeyDown around goToInvoice
so it only handles Enter or Space when e.target equals e.currentTarget,
preserving navigation for direct row activation while allowing the actions-menu
trigger to handle its own keyboard events.
In `@packages/contracts/src/index.ts`:
- Around line 646-779: Move the invoice and invoice-payment schema definitions
currently in packages/contracts/src/index.ts into separate resource/operation
files following the <resource>-<operation>.request.ts or .response.ts naming
convention. Update the folder index and main index exports so all existing
invoice symbols remain publicly available, including
invoiceLineItemCategorySchema, invoiceStatusSchema, invoicePaymentMethodSchema,
and the related request/response types.
In `@packages/database/prisma/schema.prisma`:
- Around line 344-360: Update the Invoice, InvoiceLineItem, and InvoicePayment
models in packages/database/prisma/schema.prisma (lines 344-360, 362-373, and
375-389) to use `@default`(uuid()) for their primary key IDs and append
@@schema("public") to each model.
---
Minor comments:
In `@apps/api/src/modules/invoices/invoices.service.spec.ts`:
- Line 1: Format the test file with Prettier so it passes the repository’s
Prettier --check step, preserving the existing test behavior and content.
In `@apps/web/src/app/`[lang]/dashboard/invoices/page.tsx:
- Line 1: Run the repository’s Prettier formatter on
apps/web/src/app/[lang]/dashboard/invoices/page.tsx (anchor),
apps/web/src/components/dashboard/invoice-detail-page.tsx, and
apps/web/src/components/dashboard/invoices-page.tsx, preserving their behavior
while applying the formatter’s output.
In `@apps/web/src/i18n/dictionaries/ar.json`:
- Line 21: Update the Arabic nav.billing translation to a distinct Billing or
Subscriptions label, such as “الفوترة” or “الاشتراكات”, while preserving
nav.invoices as “الفواتير”.
---
Nitpick comments:
In `@apps/api/src/modules/invoices/invoices.service.ts`:
- Around line 54-60: Add NestJS Logger usage to InvoicesService by importing
Logger and defining private readonly logger = new Logger(InvoicesService.name).
Use this logger for unexpected operational failures within the service.
In `@apps/web/src/store/api/endpoints/invoice-payments.api.ts`:
- Around line 10-14: Extract the duplicated unwrap utility into a shared API
utility module or export it from base-api.ts, then update invoices.api.ts and
invoice-payments.api.ts to import and reuse that single implementation. Preserve
the existing generic behavior and envelope detection logic.
🪄 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 Plus
Run ID: 90c47391-4671-437c-a8c3-2a75db79066d
📒 Files selected for processing (35)
apps/api/src/app.module.tsapps/api/src/common/invoice-summary/compute-invoice-summary.spec.tsapps/api/src/common/invoice-summary/compute-invoice-summary.tsapps/api/src/modules/invoice-payments/dto/create-invoice-payment.dto.tsapps/api/src/modules/invoice-payments/invoice-payments.controller.tsapps/api/src/modules/invoice-payments/invoice-payments.module.tsapps/api/src/modules/invoice-payments/invoice-payments.service.spec.tsapps/api/src/modules/invoice-payments/invoice-payments.service.tsapps/api/src/modules/invoices/dto/create-invoice.dto.tsapps/api/src/modules/invoices/dto/update-invoice.dto.tsapps/api/src/modules/invoices/invoices.controller.tsapps/api/src/modules/invoices/invoices.module.tsapps/api/src/modules/invoices/invoices.service.spec.tsapps/api/src/modules/invoices/invoices.service.tsapps/api/src/modules/leases/leases.service.spec.tsapps/api/src/modules/leases/leases.service.tsapps/web/src/app/[lang]/dashboard/invoices/[id]/page.tsxapps/web/src/app/[lang]/dashboard/invoices/page.tsxapps/web/src/app/api/invoice-payments/[id]/route.tsapps/web/src/app/api/invoice-payments/route.tsapps/web/src/app/api/invoices/[id]/route.tsapps/web/src/app/api/invoices/route.tsapps/web/src/auth/permissions.tsapps/web/src/components/dashboard/invoice-detail-page.tsxapps/web/src/components/dashboard/invoices-page.tsxapps/web/src/components/layout/dashboard-sidebar.tsxapps/web/src/i18n/dictionaries/ar.jsonapps/web/src/i18n/dictionaries/en.jsonapps/web/src/store/api/endpoints/invoice-payments.api.tsapps/web/src/store/api/endpoints/invoices.api.tsapps/web/src/store/api/tag-types.tspackages/contracts/src/index.tspackages/database/prisma/migrations/20260714175807_add_invoice_model/migration.sqlpackages/database/prisma/migrations/20260714201131_add_invoice_payment_model/migration.sqlpackages/database/prisma/schema.prisma
Comment on lines
+1
to
+54
| import { | ||
| ArrayMinSize, | ||
| IsArray, | ||
| IsDateString, | ||
| IsEnum, | ||
| IsNotEmpty, | ||
| IsNumber, | ||
| IsOptional, | ||
| IsString, | ||
| Min, | ||
| ValidateNested, | ||
| } from 'class-validator'; | ||
| import { Type } from 'class-transformer'; | ||
| import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; | ||
| import { InvoiceLineItemCategory } from '@repo/db'; | ||
|
|
||
| export class InvoiceLineItemInputDto { | ||
| @ApiProperty({ enum: InvoiceLineItemCategory }) | ||
| @IsEnum(InvoiceLineItemCategory) | ||
| category: InvoiceLineItemCategory; | ||
|
|
||
| @ApiPropertyOptional() | ||
| @IsOptional() | ||
| @IsString() | ||
| description?: string; | ||
|
|
||
| @ApiProperty() | ||
| @IsNumber({ maxDecimalPlaces: 2 }) | ||
| @Min(0) | ||
| amount: number; | ||
| } | ||
|
|
||
| export class CreateInvoiceDto { | ||
| @ApiProperty() | ||
| @IsString() | ||
| @IsNotEmpty() | ||
| leaseId: string; | ||
|
|
||
| @ApiProperty() | ||
| @IsDateString() | ||
| dueDate: string; | ||
|
|
||
| @ApiPropertyOptional() | ||
| @IsOptional() | ||
| @IsString() | ||
| notes?: string; | ||
|
|
||
| @ApiProperty({ type: [InvoiceLineItemInputDto] }) | ||
| @IsArray() | ||
| @ArrayMinSize(1) | ||
| @ValidateNested({ each: true }) | ||
| @Type(() => InvoiceLineItemInputDto) | ||
| lineItems: InvoiceLineItemInputDto[]; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔴 Critical | 🏗️ Heavy lift
Use Zod schemas from @repo/contracts instead of inline class-validator DTOs.
As per coding guidelines, you must define every request/response shape on the wire as a Zod schema in packages/contracts/src/<resource>/ and validate request bodies in the controller using ZodValidationPipe. Never define DTOs inline in apps/api.
apps/api/src/modules/invoices/dto/create-invoice.dto.ts#L1-L54: Remove this file and define the create-invoice schema inpackages/contracts.apps/api/src/modules/invoices/dto/update-invoice.dto.ts#L1-L31: Remove this file and define the update-invoice schema inpackages/contracts.apps/api/src/modules/invoices/invoices.controller.ts#L47-L50: InjectZodValidationPipein the@Body()decorator and type the payload using the inferred request type from your new contract.apps/api/src/modules/invoices/invoices.controller.ts#L57-L61: InjectZodValidationPipein the@Body()decorator and type the payload using the inferred request type from your new contract.
📍 Affects 3 files
apps/api/src/modules/invoices/dto/create-invoice.dto.ts#L1-L54(this comment)apps/api/src/modules/invoices/dto/update-invoice.dto.ts#L1-L31apps/api/src/modules/invoices/invoices.controller.ts#L47-L50apps/api/src/modules/invoices/invoices.controller.ts#L57-L61
🤖 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 `@apps/api/src/modules/invoices/dto/create-invoice.dto.ts` around lines 1 - 54,
Replace the inline class-validator DTOs with Zod contracts under
packages/contracts/src/<resource>/: remove create-invoice.dto.ts and
update-invoice.dto.ts, define schemas with inferred request types, and update
invoices.controller.ts lines 47-50 and 57-61 to apply ZodValidationPipe in
`@Body`() and use those inferred types. Affected sites:
apps/api/src/modules/invoices/dto/create-invoice.dto.ts lines 1-54 and
update-invoice.dto.ts lines 1-31 require removal; both controller ranges require
the pipe and contract-derived payload types.
Source: Coding guidelines
Comment on lines
+13
to
+31
| export class UpdateInvoiceDto { | ||
| @ApiPropertyOptional() | ||
| @IsOptional() | ||
| @IsDateString() | ||
| dueDate?: string; | ||
|
|
||
| @ApiPropertyOptional({ nullable: true }) | ||
| @IsOptional() | ||
| @IsString() | ||
| notes?: string | null; | ||
|
|
||
| @ApiPropertyOptional({ type: [InvoiceLineItemInputDto] }) | ||
| @IsOptional() | ||
| @IsArray() | ||
| @ArrayMinSize(1) | ||
| @ValidateNested({ each: true }) | ||
| @Type(() => InvoiceLineItemInputDto) | ||
| lineItems?: InvoiceLineItemInputDto[]; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔴 Critical | 🏗️ Heavy lift
Use Zod schemas from @repo/contracts instead of inline DTOs.
As per coding guidelines, DTOs should never be defined inline in apps/api using class-validator. Instead, define every request/response shape on the wire as a Zod schema in packages/contracts/src/<resource>/ and import it.
🤖 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 `@apps/api/src/modules/invoices/dto/update-invoice.dto.ts` around lines 13 -
31, Replace the inline class-validator definition in UpdateInvoiceDto with the
corresponding invoice update request Zod schema from `@repo/contracts`. Define or
extend the schema under the contracts invoice resource for dueDate, nullable
notes, and nested lineItems, then import and use that shared schema in the API
while preserving the existing validation and optionality.
Source: Coding guidelines
Comment on lines
+49
to
+60
| @Body() dto: CreateInvoiceDto, | ||
| ) { | ||
| const { orgId, role } = await this.orgScope.resolveForCaller(user); | ||
| return this.invoicesService.create(orgId, user.sub, role, dto); | ||
| } | ||
|
|
||
| @Roles(Role.ORG_ADMIN, Role.FINANCE) | ||
| @Patch(':id') | ||
| async updateInvoice( | ||
| @CurrentUser() user: AuthenticatedUser, | ||
| @Param('id') id: string, | ||
| @Body() dto: UpdateInvoiceDto, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔴 Critical | 🏗️ Heavy lift
Validate request bodies using ZodValidationPipe.
As per coding guidelines, every request body must be validated with ZodValidationPipe from src/common/pipes/ using Zod schemas from @repo/contracts, rather than using class-validator DTOs.
♻️ Proposed structure
`@Post`()
async createInvoice(
`@CurrentUser`() user: AuthenticatedUser,
`@Body`(new ZodValidationPipe(invoiceCreateSchema)) dto: InvoiceCreateRequest,
) {
// ...
}
`@Patch`(':id')
async updateInvoice(
`@CurrentUser`() user: AuthenticatedUser,
`@Param`('id') id: string,
`@Body`(new ZodValidationPipe(invoiceUpdateSchema)) dto: InvoiceUpdateRequest,
) {
// ...
}🤖 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 `@apps/api/src/modules/invoices/invoices.controller.ts` around lines 49 - 60,
Update the invoice controller’s create and update body parameters to use
ZodValidationPipe with the corresponding invoiceCreateSchema and
invoiceUpdateSchema from `@repo/contracts`, replacing class-validator DTO types
with the matching request types. Import the pipe and schemas, while preserving
the existing service calls and authorization flow.
Source: Coding guidelines
Comment on lines
+711
to
+727
| export type CreateInvoiceBody = { | ||
| leaseId: string; | ||
| dueDate: string; | ||
| notes?: string; | ||
| lineItems: InvoiceLineItemInput[]; | ||
| }; | ||
|
|
||
| /** | ||
| * When lineItems is provided, the full desired set is replaced wholesale | ||
| * (delete-then-recreate) — not diffed individually. Omit it to patch | ||
| * dueDate/notes only. | ||
| */ | ||
| export type PatchInvoiceBody = { | ||
| dueDate?: string; | ||
| notes?: string | null; | ||
| lineItems?: InvoiceLineItemInput[]; | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Define Zod schemas for request/response bodies.
You have only exported TypeScript types for the request bodies (CreateInvoiceBody, PatchInvoiceBody, etc.) without defining their corresponding Zod schemas. As per coding guidelines, every request/response shape on the wire must be defined as a Zod schema in @repo/contracts so the API can use them for validation.
🤖 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 `@packages/contracts/src/index.ts` around lines 711 - 727, Define and export
Zod schemas for CreateInvoiceBody and PatchInvoiceBody in the contracts module,
matching every field’s requiredness, nullability, and types from the existing
TypeScript definitions. Derive or align the exported TypeScript types with these
schemas where appropriate, and add corresponding schemas for the other
request/response wire shapes in this module that currently have only type
definitions.
Source: Coding guidelines
CI format:check failed on 7 files added by the Invoices/Payments PR (issues/012-014) that weren't run through prettier before commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixed the findings that were genuine bugs or matched real project convention (AGENTS.md, existing service patterns): - computeInvoiceSummary: compare paid/total in integer cents, not raw floats, so a fully-paid invoice can't get stuck partially_paid due to float rounding (e.g. 0.1 + 0.2). - InvoicePaymentsService.summarizeInvoice: scope the Invoice lookup by orgId (defense-in-depth; both call sites already passed a pre-validated invoiceId, but this method returns entity data, unlike the id-only reference-count guards elsewhere). - Invoice detail page: surface a payment-list fetch failure instead of silently rendering "No payments recorded yet." - Invoices page: add aria-labels to the line-item category/ description/amount controls; guard the row's onKeyDown so Enter/Space on the nested actions-menu trigger doesn't also navigate the row (keydown bubbles past the click stopPropagation guard). Skipped findings that contradict this repo's actual conventions (verified against ExpensesService/expenses-page.tsx/AGENTS.md): DatabaseService+Logger (no DatabaseService exists anywhere; every service uses PrismaService per AGENTS.md), UUID+@@Schema primary keys (all 17 existing models use cuid(), none declare @@Schema), per-file contracts (packages/contracts/src is a single index.ts), ZodValidationPipe on API controllers (backend uses class-validator DTOs; Zod is frontend-only per AGENTS.md), importing wire types from @repo/contracts in the browser (existing pages import from @/types/api), ApiError-specific toast messages and design-token status colors (existing pages use generic toasts and raw Tailwind colors identically), and wrapping writes+timeline emits in a transaction (no service in the codebase does this — verified in ExpensesService). Also verified the flagged delete-guard "atomicity" concerns in InvoicesService/LeasesService already match the identical id-only reference-count pattern used in VendorsService/WorkOrdersService. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
Invoice/InvoiceLineItemschema,computeInvoiceSummarydeep module (derives total/paid/status, never stored), newinvoicespermission area, filtered list page (issues/012)LeasesService.removeguarded against referenced invoices, New/Edit Invoice dialog (issues/013)InvoicePaymentschema + service (create/findAll/findOne/remove, no update), building-scoped via parent Invoice,InvoicesService.removeguarded against referenced payments, Invoice detail page with line items + payment history + Record Payment action (issues/014)Test plan
npm run testnpm run typecheck🤖 Generated with Claude Code
Summary by CodeRabbit