Conversation
WalkthroughAdds a full invoice feature: UI components and pages, create/update forms and modals, server API endpoints, DB schema and repository for invoices, validation schemas, balance recount service, and i18n strings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Partner Invoice Page / InvoiceCard
participant ModalCreate as ModalCreateInvoice
participant FormCreate as CreateInvoice Form
participant API as POST /api/partner/id/{partnerId}/invoice
participant DB as Invoice Repository
participant Service as recountPartnerBalance
participant Store as Partner Store
User->>UI: Click "Create"
UI->>ModalCreate: open({ partnerId })
ModalCreate->>FormCreate: render
User->>FormCreate: submit form data
FormCreate->>API: POST validated body
API->>DB: Invoice.create(data)
DB-->>API: created invoice
API->>Service: recountPartnerBalance(partnerId)
Service->>DB: Invoice.listForPartner(partnerId)
Service->>DB: Partner.update(balance)
API->>Store: trigger refresh
API-->>FormCreate: { ok: true }
FormCreate->>ModalCreate: emit success
ModalCreate->>UI: close overlay
sequenceDiagram
participant User
participant UI as InvoiceCard
participant ModalUpdate as ModalUpdateInvoice
participant FormUpdate as UpdateInvoice Form
participant API as PATCH /api/partner/invoice/id/{invoiceId}
participant DB as Invoice Repository
participant Service as recountPartnerBalance
participant Store as Partner Store
User->>UI: Click Edit
UI->>ModalUpdate: open({ invoiceId })
ModalUpdate->>FormUpdate: render with pre-filled data
User->>FormUpdate: submit changes
FormUpdate->>API: PATCH validated body
API->>DB: Invoice.update(invoiceId, data)
DB-->>API: updated invoice
API->>Service: recountPartnerBalance(partnerId)
Service->>DB: Invoice.listForPartner(partnerId)
Service->>DB: Partner.update(balance)
API->>Store: trigger refresh
API-->>FormUpdate: { ok: true }
FormUpdate->>ModalUpdate: emit success
ModalUpdate->>UI: close overlay
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 12
🧹 Nitpick comments (15)
apps/web-app/i18n/locales/ru-RU.json (1)
147-147: Verify completeness of i18n coverage for invoice feature.While menu and toast notifications are translated, ensure that all invoice-related UI strings (form labels, field placeholders, validation messages, error messages, and descriptions) have corresponding translations in this file and other supported locales. Additionally, confirm whether the "activity-schedules" addition at line 147 is part of this PR or should be in a separate commit.
Also applies to: 437-437
apps/web-app/app/pages/partner/[id]/invoice.vue (1)
32-32: Naming consistency: consider renaming toinvoices.Similar to the parent route file,
activeInvoicesdoesn't filter by status—it returns all invoices. For consistency and clarity, consider renaming toinvoices.Apply this diff:
-const activeInvoices = computed(() => partner.value?.invoices) +const invoices = computed(() => partner.value?.invoices)And update line 7:
<InvoiceCard - v-for="invoice in activeInvoices" + v-for="invoice in invoices" :key="invoice.id" :invoice="invoice" />apps/web-app/app/components/PartnerBalanceCard.vue (1)
8-14: Consider internationalizing the text and specifying a locale for currency formatting.The balance heading and description contain hardcoded Russian text, while other parts of the application use i18n (e.g.,
$t('common.title')). Additionally, the currency formatter on line 9 doesn't specify a locale, which may lead to inconsistent formatting.Consider applying this pattern:
<h3 class="text-xl md:text-xl/6 font-semibold"> - Баланс {{ new Intl.NumberFormat().format(balance) }} ₽ + {{ $t('partner.balance') }} {{ new Intl.NumberFormat('ru-RU').format(balance) }} ₽ </h3> <p class="text-base/5"> - Может уходить в минус, если партнер не будет оплачивать выставленные ему счета. + {{ $t('partner.balance-description') }} </p>Then add the corresponding keys to your i18n translation files.
apps/web-app/app/components/form/CreateInvoice.vue (2)
31-43: Consider internationalizing the hardcoded label.The label "Тип" is hardcoded in Russian, while other labels use i18n (e.g.,
$t('common.title')). For consistency, consider using$t('invoice.type')or a similar key.- <UFormField label="Тип" name="type"> + <UFormField :label="$t('invoice.type')" name="type">
45-57: Consider internationalizing the hardcoded label and currency symbol.The label "Сумма, руб" is hardcoded. Consider using i18n for consistency with other form fields.
<UFormField - label="Сумма, руб" + :label="$t('invoice.total-rub')" name="total" required >apps/web-app/server/api/partner/id/[partnerId]/invoice.post.ts (1)
32-33: Unsafe type casting of invoice type and status.The schema validation on line 17 accepts
typeandstatusas generic strings, but lines 32-33 cast them toInvoice['type']andInvoice['status']without runtime validation. If invalid values are provided, this could cause data integrity issues.The validation schema should enforce valid enum values. See the related comment on
apps/web-app/shared/services/partner.tsfor strengthening the schema definitions.apps/web-app/server/api/partner/invoice/id/[invoiceId]/index.patch.ts (1)
30-34: Unsafe type casting of invoice type and status.Similar to the POST endpoint, the schema validation accepts
typeandstatusas generic strings, but lines 32-33 cast them toInvoice['type']andInvoice['status']without runtime validation.Strengthen the validation schema to enforce valid enum values. See the comment on
apps/web-app/shared/services/partner.ts.apps/web-app/app/components/form/UpdateInvoice.vue (1)
9-9: Consider internationalizing hardcoded labels.The labels "Статус", "Тип", and "Сумма, руб" are hardcoded in Russian. For consistency with other fields that use i18n (e.g.,
$t('common.title')), consider using translation keys.Also applies to: 48-48, 62-62
apps/web-app/shared/services/partner.ts (1)
56-57: Consider adding length constraints to title and description fields.The
titleanddescriptionfields currently have no length constraints. Other schemas in this file use patterns like'8 <= string <= 150'(line 13) to enforce reasonable limits. Consider adding similar constraints to prevent unreasonably long values.export const createPartnerInvoiceSchema = type({ - title: type('string').describe('error.length.invalid'), - description: type('string | undefined').describe('error.length.invalid').optional(), + title: type('1 <= string <= 200').describe('error.length.invalid'), + description: type('0 <= string <= 500 | undefined').describe('error.length.invalid').optional(), total: type('number').describe('error.length.invalid'), type: invoiceType.describe('error.invalid-invoice-type'), status: invoiceStatus.describe('error.invalid-invoice-status'), }) export const updatePartnerInvoiceSchema = type({ - title: type('string | undefined').describe('error.length.invalid').optional(), - description: type('string | undefined').describe('error.length.invalid').optional(), + title: type('1 <= string <= 200 | undefined').describe('error.length.invalid').optional(), + description: type('0 <= string <= 500 | undefined').describe('error.length.invalid').optional(), total: type('number | undefined').describe('error.length.invalid').optional(), type: invoiceType.describe('error.invalid-invoice-type').optional(), status: invoiceStatus.describe('error.invalid-invoice-status').optional(), })Also applies to: 65-66
apps/web-app/app/components/InvoiceCard.vue (2)
18-18: Hardcoded currency symbol lacks i18n support.The currency symbol
₽is hardcoded, which prevents internationalization. If this application might support multiple currencies or locales in the future, consider usingIntl.NumberFormatwith currency options or extracting the symbol to a configuration/i18n resource.Example refactor using
Intl.NumberFormatwith currency:- {{ new Intl.NumberFormat().format(invoice.total) }} ₽ + {{ new Intl.NumberFormat('ru-RU', { style: 'currency', currency: 'RUB' }).format(invoice.total) }}
76-77: Remove unused variable.The
overlayvariable is created but never used directly. ThemodalUpdateInvoiceis sufficient.Apply this diff:
-const overlay = useOverlay() -const modalUpdateInvoice = overlay.create(ModalUpdateInvoice) +const modalUpdateInvoice = useOverlay().create(ModalUpdateInvoice)packages/database/src/repository/invoice.ts (4)
7-11: Consider adding error handling and return type clarity.The method returns
undefinedwhen an invoice is not found. While this may be intentional, consider whether throwing a specific error or returningnullwould make the API contract clearer for consumers.
19-22: Add error handling for insert failures.The
createmethod assumes the insert will succeed. Database constraints (e.g., foreign key violations ifpartnerIdis invalid) could cause unhandled errors. Consider wrapping in try-catch or allowing the error to propagate with clear documentation.
36-38: Consider checking existence before deletion.The
deletemethod doesn't verify whether the invoice exists before attempting deletion. This could silently succeed even when deleting a non-existent record. Consider returning a boolean or throwing an error if the invoice doesn't exist, depending on your API contract.Example with existence check:
static async delete(id: string) { const result = await useDatabase() .delete(invoices) .where(eq(invoices.id, id)) .returning() if (result.length === 0) { throw new Error(`Invoice with id ${id} not found`) } return result[0] }
24-34: Validate update data to prevent overwriting protected fields.The method accepts
Partial<InvoiceDraft>which allows updating any field, including potentially protected fields likeid,createdAt, orpartnerId. Consider creating a dedicated update type that excludes these fields.Example type definition (add to types file):
export type InvoiceUpdate = Omit<Partial<InvoiceDraft>, 'id' | 'createdAt' | 'partnerId'>Then update the method signature:
-static async update(id: string, data: Partial<InvoiceDraft>) { +static async update(id: string, data: InvoiceUpdate) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/web-app/app/components/InvoiceCard.vue(1 hunks)apps/web-app/app/components/PartnerAgreementCard.vue(1 hunks)apps/web-app/app/components/PartnerBalanceCard.vue(1 hunks)apps/web-app/app/components/PartnerCard.vue(1 hunks)apps/web-app/app/components/form/CreateInvoice.vue(1 hunks)apps/web-app/app/components/form/UpdateInvoice.vue(1 hunks)apps/web-app/app/components/modal/CreateInvoice.vue(1 hunks)apps/web-app/app/components/modal/UpdateInvoice.vue(1 hunks)apps/web-app/app/pages/partner/[id].vue(1 hunks)apps/web-app/app/pages/partner/[id]/index.vue(1 hunks)apps/web-app/app/pages/partner/[id]/invoice.vue(1 hunks)apps/web-app/i18n/locales/ru-RU.json(2 hunks)apps/web-app/server/api/partner/id/[partnerId]/invoice.post.ts(1 hunks)apps/web-app/server/api/partner/invoice/id/[invoiceId]/index.patch.ts(1 hunks)apps/web-app/shared/services/partner.ts(1 hunks)packages/database/src/repository/index.ts(2 hunks)packages/database/src/repository/invoice.ts(1 hunks)packages/database/src/tables.ts(1 hunks)packages/database/src/types/entities.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
packages/database/src/repository/index.ts (1)
packages/database/src/repository/invoice.ts (1)
Invoice(6-39)
packages/database/src/tables.ts (1)
packages/database/src/types/entities.ts (1)
InvoiceType(100-100)
packages/database/src/repository/invoice.ts (2)
packages/database/src/tables.ts (1)
invoices(786-797)packages/database/src/types/tables.ts (1)
InvoiceDraft(176-176)
apps/web-app/server/api/partner/invoice/id/[invoiceId]/index.patch.ts (3)
apps/web-app/shared/services/partner.ts (1)
updatePartnerInvoiceSchema(64-70)apps/web-app/server/services/db.ts (1)
db(164-164)packages/database/src/repository/invoice.ts (1)
Invoice(6-39)
apps/web-app/server/api/partner/id/[partnerId]/invoice.post.ts (3)
apps/web-app/shared/services/partner.ts (1)
createPartnerInvoiceSchema(55-61)apps/web-app/server/services/db.ts (1)
db(164-164)packages/database/src/repository/invoice.ts (1)
Invoice(6-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (11)
apps/web-app/i18n/locales/ru-RU.json (2)
148-149: Invoice menu items appropriately added with consistent naming.The new "invoices" (plural) and "invoice" (singular) keys follow the established menu item pattern in the file (similar to "chat"/"chats", "ticket"/"tickets").
437-440: Invoice toast notifications use correct Russian past tense and follow established pattern.The four new toast keys ("invoice-created", "invoice-updated", "invoice-deleted") align with the existing pattern and use grammatically correct Russian for past-tense notifications ("создан", "обновлен", "удален").
apps/web-app/app/components/PartnerAgreementCard.vue (1)
80-80: LGTM!The line height adjustment for the city text is a minor cosmetic improvement.
apps/web-app/app/components/PartnerCard.vue (1)
69-69: LGTM!Currency symbol standardization improves consistency with the new PartnerBalanceCard component.
packages/database/src/types/entities.ts (1)
100-100: LGTM!The InvoiceType definition is clear and follows the established pattern for entity types in this file.
apps/web-app/app/pages/partner/[id]/index.vue (1)
6-7: LGTM!Proper use of nullish coalescing provides a safe fallback for the balance prop.
packages/database/src/repository/index.ts (1)
12-12: LGTM!The Invoice repository is properly wired and maintains alphabetical ordering.
Also applies to: 41-41
apps/web-app/app/pages/partner/[id].vue (1)
39-39: Route change verified and correct.The route has been properly updated to the singular form
/kitchen. The route file exists atapps/web-app/app/pages/partner/[id]/kitchen.vue, line 39 correctly references/partner/${partner.value?.id}/kitchen, and no orphaned references to the old plural/kitchenspath remain. The "kitchens" references found in the codebase are legitimate data properties, type definitions, and UI labels—not route paths.apps/web-app/app/components/modal/UpdateInvoice.vue (1)
1-19: LGTM!The modal wrapper correctly passes the invoiceId prop to the form component and handles the overlay lifecycle on submission and success events.
apps/web-app/app/components/form/CreateInvoice.vue (1)
84-90: Verify the default value of 0 for invoice total.The form initializes
totalto 0, which may not be appropriate for invoice creation. Users might accidentally submit an invoice with zero value. Consider usingundefinedto force explicit input or a more appropriate default value.const state = ref<Partial<CreatePartnerInvoice>>({ title: undefined, description: undefined, - total: 0, + total: undefined, type: 'royalties', status: 'unpaid', })apps/web-app/server/api/partner/invoice/id/[invoiceId]/index.patch.ts (1)
55-75: Verify the balance calculation logic for unpaid invoices.The balance calculation logic (lines 60-69) only adds replenishment invoices when they are
paid, but always subtractsroyaltiesandotherinvoice types regardless of payment status. This means unpaid royalties immediately reduce the partner's balance.Please confirm whether this is the intended business logic:
- Should unpaid royalties/other invoices affect the balance, or only paid ones?
- Is the balance meant to represent "amount owed to/by the partner" or "current settled balance"?
If only paid invoices should affect the balance, consider:
if (invoice.type === 'royalties') { - balance -= invoice.total + if (invoice.status === 'paid') { + balance -= invoice.total + } } if (invoice.type === 'other') { - balance -= invoice.total + if (invoice.status === 'paid') { + balance -= invoice.total + } }
| <UFormField label="Тип" name="type"> | ||
| <USelect | ||
| v-model="state.type" | ||
| :items="[ | ||
| { label: 'Оплата роялти', value: 'royalties' }, | ||
| { label: 'Пополнение', value: 'replenishment' }, | ||
| ]" | ||
| :placeholder="$t('common.select')" | ||
| size="xl" | ||
| class="w-full" | ||
| /> | ||
| </UFormField> |
There was a problem hiding this comment.
Missing 'other' invoice type option.
The UpdateInvoice form only includes 'royalties' and 'replenishment' type options, but the CreateInvoice form (lines 34-38 in apps/web-app/app/components/form/CreateInvoice.vue) includes a third option: 'other'. This discrepancy prevents users from updating invoices that were created with type: 'other'.
Add the missing option:
<USelect
v-model="state.type"
:items="[
{ label: 'Оплата роялти', value: 'royalties' },
{ label: 'Пополнение', value: 'replenishment' },
+ { label: 'Другое', value: 'other' },
]"
:placeholder="$t('common.select')"
size="xl"
class="w-full"
/>📝 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.
| <UFormField label="Тип" name="type"> | |
| <USelect | |
| v-model="state.type" | |
| :items="[ | |
| { label: 'Оплата роялти', value: 'royalties' }, | |
| { label: 'Пополнение', value: 'replenishment' }, | |
| ]" | |
| :placeholder="$t('common.select')" | |
| size="xl" | |
| class="w-full" | |
| /> | |
| </UFormField> | |
| <UFormField label="Тип" name="type"> | |
| <USelect | |
| v-model="state.type" | |
| :items="[ | |
| { label: 'Оплата роялти', value: 'royalties' }, | |
| { label: 'Пополнение', value: 'replenishment' }, | |
| { label: 'Другое', value: 'other' }, | |
| ]" | |
| :placeholder="$t('common.select')" | |
| size="xl" | |
| class="w-full" | |
| /> | |
| </UFormField> |
🤖 Prompt for AI Agents
In apps/web-app/app/components/form/UpdateInvoice.vue around lines 48 to 59, the
USelect items only include 'royalties' and 'replenishment', causing invoices
with type 'other' to be unselectable; add the missing option { label: 'Другое'
(or the same localized label used in CreateInvoice.vue), value: 'other' } to the
items array so the UpdateInvoice form matches CreateInvoice and can update
invoices created with type 'other'.
| function getInfoByType(type: Invoice['type']) { | ||
| switch (type) { | ||
| case 'replenishment': | ||
| return 'Пополнение' | ||
| case 'royalties': | ||
| return 'Роялти' | ||
| case 'other': | ||
| return 'Другое' | ||
| } | ||
| } |
There was a problem hiding this comment.
Add default case to prevent undefined returns.
The switch statement lacks a default case. If an unexpected type value is passed, the function returns undefined, which could cause rendering issues.
Apply this diff:
function getInfoByType(type: Invoice['type']) {
switch (type) {
case 'replenishment':
return 'Пополнение'
case 'royalties':
return 'Роялти'
case 'other':
return 'Другое'
+ default:
+ return 'Неизвестно'
}
}📝 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.
| function getInfoByType(type: Invoice['type']) { | |
| switch (type) { | |
| case 'replenishment': | |
| return 'Пополнение' | |
| case 'royalties': | |
| return 'Роялти' | |
| case 'other': | |
| return 'Другое' | |
| } | |
| } | |
| function getInfoByType(type: Invoice['type']) { | |
| switch (type) { | |
| case 'replenishment': | |
| return 'Пополнение' | |
| case 'royalties': | |
| return 'Роялти' | |
| case 'other': | |
| return 'Другое' | |
| default: | |
| return 'Неизвестно' | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web-app/app/components/InvoiceCard.vue around lines 56 to 65, the
getInfoByType switch lacks a default branch so it can return undefined for
unknown types; add a default case that returns a safe fallback string (e.g.,
'Неизвестно' or an empty string) so the function always returns a string and
adjust typing if needed to reflect a guaranteed string return.
| function getInfoByStatus(status: Invoice['status']) { | ||
| switch (status) { | ||
| case 'unpaid': | ||
| return 'Не оплачен' | ||
| case 'paid': | ||
| return 'Оплачен' | ||
| } | ||
| } |
There was a problem hiding this comment.
Add default case to prevent undefined returns.
The switch statement lacks a default case. If an unexpected status value is passed, the function returns undefined, which could cause rendering issues.
Apply this diff:
function getInfoByStatus(status: Invoice['status']) {
switch (status) {
case 'unpaid':
return 'Не оплачен'
case 'paid':
return 'Оплачен'
+ default:
+ return 'Неизвестен'
}
}📝 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.
| function getInfoByStatus(status: Invoice['status']) { | |
| switch (status) { | |
| case 'unpaid': | |
| return 'Не оплачен' | |
| case 'paid': | |
| return 'Оплачен' | |
| } | |
| } | |
| function getInfoByStatus(status: Invoice['status']) { | |
| switch (status) { | |
| case 'unpaid': | |
| return 'Не оплачен' | |
| case 'paid': | |
| return 'Оплачен' | |
| default: | |
| return 'Неизвестен' | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web-app/app/components/InvoiceCard.vue around lines 67 to 74, the
getInfoByStatus switch lacks a default branch so it can return undefined for
unexpected statuses; add a default case that returns a sensible fallback string
(e.g., an empty string or a localized "Unknown" like 'Неизвестно') so the
function always returns a string and prevents rendering issues.
| <template> | ||
| <UModal title="Создание счета"> | ||
| <template #body> | ||
| <FormCreateInvoice | ||
| :partner-id="partnerId ?? ''" | ||
| @submitted="overlay.closeAll" | ||
| @success="overlay.closeAll" | ||
| /> | ||
| </template> | ||
| </UModal> | ||
| </template> | ||
|
|
||
| <script setup lang="ts"> | ||
| defineProps<{ | ||
| partnerId?: string | ||
| }>() | ||
|
|
||
| const overlay = useOverlay() | ||
| </script> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the empty string fallback for partnerId.
Line 5 uses partnerId ?? '' to provide an empty string when partnerId is undefined. Ensure that the FormCreateInvoice component and the downstream API endpoint at /api/partner/id/${partnerId}/invoice correctly handle empty strings, or consider passing undefined instead.
🏁 Script executed:
#!/bin/bash
# Check how FormCreateInvoice and the API handle empty partnerId
# Check FormCreateInvoice component for partnerId handling
ast-grep --pattern $'const { partnerId } = defineProps<{ partnerId$$$}>()'
# Check API route parameter usage
rg -n "partnerId" apps/web-app/server/api/partner/id/Length of output: 857
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full API route to see what happens when partnerId is empty
cat apps/web-app/server/api/partner/id/\[partnerId\]/invoice.post.ts
echo "---"
# Find FormCreateInvoice component and check how it uses partnerId
find apps/web-app -name "*FormCreateInvoice*" -o -name "*form-create-invoice*" | head -20
Length of output: 1036
---
🏁 Script executed:
```shell
#!/bin/bash
# Search more broadly for FormCreateInvoice
rg -n "FormCreateInvoice" apps/web-app --type vue --type ts -l
echo "---"
# Try to find it by searching for component definition
rg -n "defineProps.*partnerId" apps/web-app --type vue
Length of output: 59
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for FormCreateInvoice without type filtering
rg -n "FormCreateInvoice" apps/web-app -l
echo "---"
# Find .vue files with FormCreate in name
fd "FormCreateInvoice" apps/web-app
echo "---"
# Search for any $fetch or API calls involving invoice endpoint
rg -n "invoice" apps/web-app/app/components --context 3 -i
Length of output: 13333
Pass undefined instead of empty string, or add validation guards.
The API endpoint explicitly rejects empty strings: if (!partnerId) returns a 400 error. The current code passes partnerId ?? '' to FormCreateInvoice, which constructs the URL /api/partner/id/${partnerId}/invoice. When partnerId is empty, this fails validation.
Recommended fix: Remove the ?? '' fallback to pass undefined directly. Then, either (1) add a guard in FormCreateInvoice to prevent submission if partnerId is missing, or (2) conditionally show the modal only when partnerId is defined.
🤖 Prompt for AI Agents
In apps/web-app/app/components/modal/CreateInvoice.vue lines 1-19, the template
is passing partnerId ?? '' which sends an empty string to FormCreateInvoice and
causes the API to reject it; remove the `?? ''` fallback so the prop is left
undefined when absent (i.e., pass `:partner-id="partnerId"`), and then either
add a validation guard inside FormCreateInvoice to prevent submission/URL
construction when partnerId is missing, or only render/show this UModal/Form
when partnerId is defined (e.g., conditional rendering) to ensure no request is
made with an empty partnerId.
| const activeInvoices = computed(() => partner.value?.invoices) | ||
|
|
There was a problem hiding this comment.
Naming inconsistency: activeInvoices returns all invoices.
The computed property is named activeInvoices but doesn't filter by status—it returns all invoices from partner.value?.invoices. Consider either:
- Renaming to
invoicesfor clarity, or - Adding a filter to match the name:
.filter(invoice => invoice.status === 'unpaid')or similar logic.
Apply this diff if you want to rename for clarity:
-const activeInvoices = computed(() => partner.value?.invoices)
+const invoices = computed(() => partner.value?.invoices)And update references on line 35:
- badge: activeInvoices.value?.length,
+ badge: invoices.value?.length,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web-app/app/pages/partner/[id].vue around lines 22–23, the computed
property named activeInvoices actually returns all invoices; rename it to
invoices for clarity and update any usages (notably the reference on line 35) to
use invoices instead of activeInvoices so the name matches the returned data.
Ensure imports/refs and template bindings reflect the new name and run a quick
grep to catch any other occurrences to update.
| // Recount partner balance | ||
| const partner = await db.partner.find(invoice.partnerId ?? '') | ||
| if (!partner) { | ||
| throw createError({ | ||
| statusCode: 404, | ||
| message: 'Partner not found', | ||
| }) | ||
| } |
There was a problem hiding this comment.
Unsafe null handling for partnerId.
Line 37 uses invoice.partnerId ?? '' which could pass an empty string to db.partner.find(). If partnerId is null/undefined, the lookup will fail silently or return incorrect results. Consider returning an error if partnerId is missing from the invoice.
// Recount partner balance
- const partner = await db.partner.find(invoice.partnerId ?? '')
+ if (!invoice.partnerId) {
+ throw createError({
+ statusCode: 500,
+ message: 'Invoice has no associated partner',
+ })
+ }
+ const partner = await db.partner.find(invoice.partnerId)
if (!partner) {
throw createError({
statusCode: 404,
message: 'Partner not found',
})
}📝 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.
| // Recount partner balance | |
| const partner = await db.partner.find(invoice.partnerId ?? '') | |
| if (!partner) { | |
| throw createError({ | |
| statusCode: 404, | |
| message: 'Partner not found', | |
| }) | |
| } | |
| // Recount partner balance | |
| if (!invoice.partnerId) { | |
| throw createError({ | |
| statusCode: 500, | |
| message: 'Invoice has no associated partner', | |
| }) | |
| } | |
| const partner = await db.partner.find(invoice.partnerId) | |
| if (!partner) { | |
| throw createError({ | |
| statusCode: 404, | |
| message: 'Partner not found', | |
| }) | |
| } |
🤖 Prompt for AI Agents
In apps/web-app/server/api/partner/invoice/id/[invoiceId]/index.patch.ts around
lines 36 to 43, the code uses invoice.partnerId ?? '' which can pass an empty
string to db.partner.find; instead validate that invoice.partnerId is present
and non-empty, and if it's missing throw a createError (e.g., 400 Bad Request
with a clear message like "Missing partnerId on invoice"); only call
db.partner.find with the validated partnerId (do not fall back to an empty
string).
| export const createPartnerInvoiceSchema = type({ | ||
| title: type('string').describe('error.length.invalid'), | ||
| description: type('string | undefined').describe('error.length.invalid').optional(), | ||
| total: type('number').describe('error.length.invalid'), | ||
| type: type('string').describe('error.length.invalid'), | ||
| status: type('string').describe('error.length.invalid'), | ||
| }) | ||
| export type CreatePartnerInvoice = typeof createPartnerInvoiceSchema.infer | ||
|
|
||
| export const updatePartnerInvoiceSchema = type({ | ||
| title: type('string | undefined').describe('error.length.invalid').optional(), | ||
| description: type('string | undefined').describe('error.length.invalid').optional(), | ||
| total: type('number | undefined').describe('error.length.invalid').optional(), | ||
| type: type('string | undefined').describe('error.length.invalid').optional(), | ||
| status: type('string | undefined').describe('error.length.invalid').optional(), | ||
| }) | ||
| export type UpdatePartnerInvoice = typeof updatePartnerInvoiceSchema.infer |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Strengthen validation for invoice type and status fields.
The type and status fields are validated as generic 'string', which doesn't enforce the allowed enum values. This can lead to invalid data being accepted and causes unsafe type casting in the API endpoints (see comments on apps/web-app/server/api/partner/id/[partnerId]/invoice.post.ts and index.patch.ts).
Following the pattern used for patentStatus on line 38, consider defining explicit union types:
+const invoiceType = type('"royalties" | "replenishment" | "other"')
+const invoiceStatus = type('"unpaid" | "paid"')
+
export const createPartnerInvoiceSchema = type({
title: type('string').describe('error.length.invalid'),
description: type('string | undefined').describe('error.length.invalid').optional(),
total: type('number').describe('error.length.invalid'),
- type: type('string').describe('error.length.invalid'),
- status: type('string').describe('error.length.invalid'),
+ type: invoiceType.describe('error.invalid-invoice-type'),
+ status: invoiceStatus.describe('error.invalid-invoice-status'),
})
export type CreatePartnerInvoice = typeof createPartnerInvoiceSchema.infer
export const updatePartnerInvoiceSchema = type({
title: type('string | undefined').describe('error.length.invalid').optional(),
description: type('string | undefined').describe('error.length.invalid').optional(),
total: type('number | undefined').describe('error.length.invalid').optional(),
- type: type('string | undefined').describe('error.length.invalid').optional(),
- status: type('string | undefined').describe('error.length.invalid').optional(),
+ type: invoiceType.describe('error.invalid-invoice-type').optional(),
+ status: invoiceStatus.describe('error.invalid-invoice-status').optional(),
})
export type UpdatePartnerInvoice = typeof updatePartnerInvoiceSchema.inferThis will eliminate the need for unsafe type casting in the server endpoints and provide better error messages to API consumers.
🤖 Prompt for AI Agents
In apps/web-app/shared/services/partner.ts around lines 55 to 71, the invoice
schema currently allows any string for the `type` and `status` fields; replace
those generic string validators with explicit union validators that mirror the
pattern used for `patentStatus` (i.e., a type union of the allowed literal
values), and update the corresponding optional fields in
`updatePartnerInvoiceSchema` to use the same union-with-undefined and
.optional() pattern; ensure the exported CreatePartnerInvoice and
UpdatePartnerInvoice types continue to be inferred from the schemas so server
endpoints no longer need unsafe casts and consumers get proper validation
errors.
| static async update(id: string, data: Partial<InvoiceDraft>) { | ||
| const [invoice] = await useDatabase() | ||
| .update(invoices) | ||
| .set({ | ||
| ...data, | ||
| updatedAt: sql`now()`, | ||
| }) | ||
| .where(eq(invoices.id, id)) | ||
| .returning() | ||
| return invoice | ||
| } |
There was a problem hiding this comment.
Critical: Update method will crash if invoice doesn't exist.
Line 25 attempts to destructure the first element from the returning array. If no invoice matches the given id, the update returns an empty array, and destructuring undefined will throw a runtime error.
Apply this diff to handle the case safely:
static async update(id: string, data: Partial<InvoiceDraft>) {
- const [invoice] = await useDatabase()
+ const result = await useDatabase()
.update(invoices)
.set({
...data,
updatedAt: sql`now()`,
})
.where(eq(invoices.id, id))
.returning()
- return invoice
+
+ if (result.length === 0) {
+ throw new Error(`Invoice with id ${id} not found`)
+ }
+
+ return result[0]
}🤖 Prompt for AI Agents
In packages/database/src/repository/invoice.ts around lines 24 to 34, the update
method destructures the first element from the returning array which will be
undefined if no invoice matches the id; change the implementation to capture the
returning array into a variable, check whether the array is empty, and then
return null (or undefined per project convention) when no rows were updated
instead of destructuring unconditionally; preserve the existing update logic
(including setting updatedAt to now()) and ensure the method's return type
reflects the possibility of a null/undefined result.
| description: varchar('description'), | ||
| total: numeric('total', { mode: 'number' }).notNull().default(0), | ||
| paid: numeric('paid', { mode: 'number' }).notNull().default(0), | ||
| type: varchar('type').notNull().$type<entities.InvoiceType>(), |
There was a problem hiding this comment.
🧩 Analysis chain
Verify migration handles the notNull constraint.
The type column is marked as notNull() but has no default value. If the invoices table already contains records, adding this column will require a migration strategy to populate existing rows.
Run the following script to check if there are existing invoice records and verify the migration approach:
🏁 Script executed:
#!/bin/bash
# Description: Check for existing migrations related to invoices table and verify the migration strategy
# Search for invoice-related migrations
fd -e sql -e ts migration | xargs rg -l "invoices"
# Search for any default value handling or backfill logic for the type column
rg -A5 -B5 "type.*varchar.*notNull" --type=tsLength of output: 9250
I need to search more comprehensively for migration files and understand the invoices table context better.
Let me search for the actual migration files and determine if the invoices table is new or already exists:
Now let me search the codebase for the drizzle config and migration files:
🏁 Script executed:
#!/bin/bash
# Find drizzle config and migration files
echo "=== Looking for drizzle.config.ts ==="
find . -name "drizzle.config.ts" -o -name "drizzle.config.js" 2>/dev/null | head -20
echo ""
echo "=== Looking for migrations directory ==="
find . -type d -name "migrations" -o -type d -name "drizzle" 2>/dev/null | head -20
echo ""
echo "=== Looking for .sql migration files related to invoices ==="
find . -name "*.sql" -type f 2>/dev/null | xargs rg -l "invoices" 2>/dev/null | head -20Length of output: 590
Let me check the drizzle config and look for when the invoices table was first introduced:
🏁 Script executed:
#!/bin/bash
# Check drizzle config
echo "=== drizzle.config.ts content ==="
cat packages/database/drizzle.config.ts
echo ""
echo "=== Check if migrations are in .gitignore ==="
grep -r "migrations\|drizzle" .gitignore 2>/dev/null || echo "No gitignore patterns found for migrations"
echo ""
echo "=== Check if migrations are in git ==="
git ls-files | grep -E "(migrations|\.sql)" | head -20 || echo "No migrations tracked in git"
echo ""
echo "=== Search for invoices table definition context ==="
rg -B10 "export const invoices" packages/database/src/tables.ts | head -30Length of output: 782
Let me check git history and .gitignore to understand if invoices is a new table:
🏁 Script executed:
#!/bin/bash
# Check .gitignore for migrations
echo "=== Full .gitignore content (if exists) ==="
cat .gitignore 2>/dev/null | grep -E "migrations|^\*\*|\*\.sql|drizzle" || echo "No matching patterns"
echo ""
echo "=== Git history of invoices table ==="
git log --oneline -p packages/database/src/tables.ts 2>/dev/null | grep -A15 -B5 "export const invoices" | head -40
echo ""
echo "=== Check if invoices is in current HEAD ==="
git show HEAD:packages/database/src/tables.ts 2>/dev/null | grep -c "export const invoices" || echo "Not in HEAD"
echo ""
echo "=== Check if invoices table exists before current HEAD ==="
git show HEAD~1:packages/database/src/tables.ts 2>/dev/null | grep -c "export const invoices" || echo "Not in HEAD~1 (or HEAD~1 doesn't exist)"Length of output: 2234
Add a default value or nullable option to the type column in the invoices table.
This is an ALTER TABLE ADD COLUMN operation being applied to an existing table with existing records. The type column is being added with .notNull() but no .default() value, which will cause the migration to fail when applied to a database containing existing invoice records.
The git history confirms the invoices table was created in commit b1a6d47 and already exists in the codebase. The current PR adds the type column, but the schema definition doesn't provide a migration strategy for existing rows.
Fix options:
- Add
.default('...')with a reasonable default invoice type - Remove
.notNull()to make the column nullable initially, then migrate existing data and add the constraint in a follow-up migration - Create a multi-step migration: add nullable column, populate existing rows, then alter to add constraint
🤖 Prompt for AI Agents
In packages/database/src/tables.ts around line 794, the new invoices.type column
is defined as .notNull() without a default which will break ALTER TABLE on
existing data; either (A) add a sensible default via
.default('<DEFAULT_INVOICE_TYPE>') to keep the NOT NULL constraint, (B) remove
.notNull() to make the column nullable now and add a follow-up migration to
backfill and tighten the constraint, or (C) implement a multi-step migration:
add the column nullable, update existing rows to a chosen default, then alter
the column to NOT NULL; pick one approach and update the schema/migration
accordingly so existing records are handled.
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/web-app/server/api/partner/invoice/id/[invoiceId]/index.patch.ts (1)
38-38: Unsafe null handling for partnerId (addressed by reordering).This line uses
invoice.partnerId ?? ''which could pass an empty string todb.partner.find(). However, this issue is now addressed by the earlier suggestion to reorder partner validation before the invoice update, which includes proper null checking.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web-app/server/api/partner/id/[partnerId]/invoice.post.ts(1 hunks)apps/web-app/server/api/partner/invoice/id/[invoiceId]/index.patch.ts(1 hunks)apps/web-app/server/services/invoice.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web-app/server/api/partner/id/[partnerId]/invoice.post.ts
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web-app/server/services/invoice.ts (1)
apps/web-app/server/services/db.ts (1)
db(164-164)
apps/web-app/server/api/partner/invoice/id/[invoiceId]/index.patch.ts (3)
apps/web-app/shared/services/partner.ts (1)
updatePartnerInvoiceSchema(64-70)packages/database/src/repository/invoice.ts (1)
Invoice(6-39)apps/web-app/server/services/invoice.ts (1)
recountPartnerBalance(3-23)
| await db.invoice.update(invoiceId, { | ||
| ...data, | ||
| type: data.type as Invoice['type'], | ||
| status: data.status as Invoice['status'], | ||
| }) | ||
|
|
||
| // Recount partner balance | ||
| const partner = await db.partner.find(invoice.partnerId ?? '') | ||
| if (!partner) { | ||
| throw createError({ | ||
| statusCode: 404, | ||
| message: 'Partner not found', | ||
| }) | ||
| } | ||
|
|
||
| await recountPartnerBalance(partner.id) |
There was a problem hiding this comment.
Partner validation occurs after invoice update.
The invoice is updated (lines 31-35) before checking whether the associated partner exists (lines 38-44). If the partner lookup fails, the invoice remains in its updated state while an error is thrown, creating an inconsistent system state.
Apply this diff to validate the partner before updating the invoice:
const invoice = await db.invoice.find(invoiceId)
if (!invoice) {
throw createError({
statusCode: 404,
message: 'Invoice not found',
})
}
+ // Validate partner exists before updating invoice
+ if (!invoice.partnerId) {
+ throw createError({
+ statusCode: 500,
+ message: 'Invoice has no associated partner',
+ })
+ }
+ const partner = await db.partner.find(invoice.partnerId)
+ if (!partner) {
+ throw createError({
+ statusCode: 404,
+ message: 'Partner not found',
+ })
+ }
+
await db.invoice.update(invoiceId, {
...data,
type: data.type as Invoice['type'],
status: data.status as Invoice['status'],
})
- // Recount partner balance
- const partner = await db.partner.find(invoice.partnerId ?? '')
- if (!partner) {
- throw createError({
- statusCode: 404,
- message: 'Partner not found',
- })
- }
-
await recountPartnerBalance(partner.id)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web-app/server/api/partner/invoice/id/[invoiceId]/index.patch.ts around
lines 31 to 46, validate the partner before performing the invoice update: move
the partner lookup (using invoice.partnerId ?? '') to before db.invoice.update,
throw the 404 via createError if partner is not found, then proceed to call
db.invoice.update with the same payload (preserving the type/status casts) and
finally call recountPartnerBalance(partner.id); this ensures the partner exists
before mutating the invoice and avoids leaving the system in an inconsistent
state.
| await db.invoice.update(invoiceId, { | ||
| ...data, | ||
| type: data.type as Invoice['type'], | ||
| status: data.status as Invoice['status'], | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
Unsafe type assertions without validation.
Lines 33-34 cast data.type and data.status to Invoice['type'] and Invoice['status'] without validating that the string values match the expected enum values. The schema (from apps/web-app/shared/services/partner.ts) only validates these as string | undefined, allowing any string to pass through. This could result in invalid enum values being stored in the database.
Consider enhancing the validation schema to constrain allowed values:
export const updatePartnerInvoiceSchema = type({
title: type('string | undefined').optional(),
description: type('string | undefined').optional(),
total: type('number | undefined').optional(),
type: type('"replenishment" | "royalties" | "other" | undefined').optional(),
status: type('"paid" | "pending" | "cancelled" | undefined').optional(),
})Run this script to verify the allowed enum values for Invoice type and status:
🏁 Script executed:
#!/bin/bash
# Find Invoice type and status enum definitions
ast-grep --pattern 'type Invoice$$$= {
$$$
}'
# Also check for InvoiceType and InvoiceStatus type definitions
rg -nP --type=ts 'type Invoice(Type|Status)\s*='Length of output: 199
I need to verify the actual Invoice type definition and the current validation schema to confirm the correct enum values.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Find Invoice type definition in database types
rg -nP --type=ts 'type Invoice\s*=' -A 10
# Also check for interface Invoice
rg -nP --type=ts 'interface Invoice\s*{' -A 10Length of output: 214
I need to check the database schema definition and the validation schema in partner.ts to understand the current constraints.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Find invoices table definition (likely in drizzle schema)
fd -e ts -e js -path "*/drizzle/*" -o -path "*/schema*" | head -20
# Search for invoices table definition
rg -nP --type=ts 'invoices\s*=.*table\(' -A 15
# Also check for the partner validation schema
rg -nP --type=ts 'updatePartnerInvoiceSchema' -A 10Length of output: 3340
Let me find the invoices table schema to confirm the exact status field definition.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Search for invoices table definition in drizzle/database schema
rg -nP 'export const invoices' -A 30
# Also search for status field in invoice-related files
rg -nP 'status.*\(.*\)' packages/database/src -A 3 | grep -A 3 invoiceLength of output: 2476
Unsafe type assertions without validation—fix suggested enums.
Lines 33-34 cast data.type and data.status to Invoice['type'] and Invoice['status'] without validating that the string values match the expected enum values. The validation schema in apps/web-app/shared/services/partner.ts (lines 68-69) only validates these as string | undefined, allowing any string to pass through. This could result in invalid enum values being stored in the database.
The database defines these enums as:
InvoiceType: 'replenishment' | 'royalties' | 'other'InvoiceStatus: 'paid' | 'unpaid'
Update the validation schema to constrain to allowed values:
export const updatePartnerInvoiceSchema = type({
title: type('string | undefined').describe('error.length.invalid').optional(),
description: type('string | undefined').describe('error.length.invalid').optional(),
total: type('number | undefined').describe('error.length.invalid').optional(),
type: type('"replenishment" | "royalties" | "other" | undefined').optional(),
status: type('"paid" | "unpaid" | undefined').optional(),
})| for (const invoice of partnerInvoices) { | ||
| if (invoice.type === 'replenishment' && invoice.status === 'paid') { | ||
| balance += invoice.total | ||
| } | ||
|
|
||
| if (invoice.type === 'royalties') { | ||
| balance -= invoice.total | ||
| } | ||
| if (invoice.type === 'other') { | ||
| balance -= invoice.total | ||
| } | ||
| } |
There was a problem hiding this comment.
Inconsistent status handling across invoice types.
The balance calculation only checks status === 'paid' for replenishment invoices (line 8), but applies deductions for royalties and other invoice types regardless of their payment status (lines 12-17). This means unpaid or pending royalties and other invoices would reduce the partner's balance, which is likely incorrect.
Apply this diff to ensure consistent status checking:
let balance = 0
for (const invoice of partnerInvoices) {
- if (invoice.type === 'replenishment' && invoice.status === 'paid') {
+ if (invoice.type === 'replenishment' && invoice.status === 'paid') {
balance += invoice.total
- }
-
- if (invoice.type === 'royalties') {
+ } else if (invoice.type === 'royalties' && invoice.status === 'paid') {
balance -= invoice.total
- }
- if (invoice.type === 'other') {
+ } else if (invoice.type === 'other' && invoice.status === 'paid') {
balance -= invoice.total
}
}📝 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.
| for (const invoice of partnerInvoices) { | |
| if (invoice.type === 'replenishment' && invoice.status === 'paid') { | |
| balance += invoice.total | |
| } | |
| if (invoice.type === 'royalties') { | |
| balance -= invoice.total | |
| } | |
| if (invoice.type === 'other') { | |
| balance -= invoice.total | |
| } | |
| } | |
| for (const invoice of partnerInvoices) { | |
| if (invoice.type === 'replenishment' && invoice.status === 'paid') { | |
| balance += invoice.total | |
| } else if (invoice.type === 'royalties' && invoice.status === 'paid') { | |
| balance -= invoice.total | |
| } else if (invoice.type === 'other' && invoice.status === 'paid') { | |
| balance -= invoice.total | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web-app/server/services/invoice.ts around lines 7 to 18, the balance
calculation applies additions only when replenishment invoices are 'paid' but
subtracts royalties and other invoices regardless of status; update the logic so
royalties and other invoice types also modify the balance only when
invoice.status === 'paid' (either by adding the status check to those ifs or
refactoring the loop into a single conditional/switch that checks both type and
status), ensuring only paid invoices affect the partner balance.
1 participant
Summary by CodeRabbit
New Features
Style