feat(invoice): add amendInvoice endpoint and related functionality - #36
Conversation
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesInvoice amendment flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Merchant
participant amendInvoiceController
participant amendInvoice
participant Prisma
Merchant->>amendInvoiceController: PATCH /:id/amend
amendInvoiceController->>amendInvoice: Pass merchant, invoice id, and body
amendInvoice->>Prisma: Find owned invoice
Prisma-->>amendInvoice: Return invoice
amendInvoice->>Prisma: Update validated fields
Prisma-->>amendInvoice: Return updated invoice
amendInvoice-->>amendInvoiceController: Return sanitized invoice
amendInvoiceController-->>Merchant: 200 JSON response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@src/services/invoice.services.ts`:
- Around line 189-190: Reformat the ternary assignment in the description
handling block so the `typeof data.description` condition and its branches are
split across lines according to Prettier, without changing the behavior of the
`description` value.
- Around line 177-179: Update the email handling in the invoice amendment flow
to validate that data.email is either null or a string before trimming it. For
any other runtime value, throw AppError with HTTP status 400; preserve the
existing null assignment and trimmed-string behavior.
- Around line 205-208: Update amendInvoice() so the Prisma invoice update
atomically requires the matching invoice id, merchantId, and status: PENDING in
its where clause. Handle the unmatched conditional-update result as the
appropriate failure path instead of treating the amendment as successful, while
preserving the existing update data.
In `@tests/unit/invoice.services.test.ts`:
- Around line 125-150: Add a focused partial-update test alongside the existing
PENDING invoice amendment test, calling amendInvoice with only one amendable
field and asserting prismaMock.invoice.update receives data containing only that
field. Keep the existing result assertions appropriate to the mocked response
and verify omitted fields are not included in update.data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 77b6c8d3-97a2-4ff5-b074-e7c1bff8ad2c
📒 Files selected for processing (5)
src/controllers/invoice.controllers.tssrc/routes/invoice.routes.tssrc/services/invoice.services.tstests/integration/invoice.routes.test.tstests/unit/invoice.services.test.ts
| if (data.email !== undefined) { | ||
| updateData.email = data.email === null ? null : data.email.trim(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate email before calling trim.
req.body bypasses AmendInvoiceInput at runtime; { "email": 1 } reaches data.email.trim() and becomes a 500. Reject non-string, non-null values with AppError(400).
Proposed fix
if (data.email !== undefined) {
+ if (data.email !== null && typeof data.email !== 'string') {
+ throw new AppError(400, 'email must be a string or null');
+ }
updateData.email = data.email === null ? null : data.email.trim();
}📝 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.
| if (data.email !== undefined) { | |
| updateData.email = data.email === null ? null : data.email.trim(); | |
| } | |
| if (data.email !== undefined) { | |
| if (data.email !== null && typeof data.email !== 'string') { | |
| throw new AppError(400, 'email must be a string or null'); | |
| } | |
| updateData.email = data.email === null ? null : data.email.trim(); | |
| } |
🤖 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 `@src/services/invoice.services.ts` around lines 177 - 179, Update the email
handling in the invoice amendment flow to validate that data.email is either
null or a string before trimming it. For any other runtime value, throw AppError
with HTTP status 400; preserve the existing null assignment and trimmed-string
behavior.
| const updated = await prisma.invoice.update({ | ||
| where: { id: invoice.id }, | ||
| data: updateData, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching invoice.services.ts:"
fd -a 'invoice.services.ts' . || true
file="$(fd 'invoice.services.ts' . | head -n 1 || true)"
if [ -z "${file:-}" ]; then
echo "No invoice.services.ts found"
exit 0
fi
echo
echo "File: $file"
wc -l "$file"
echo
echo "Outline:"
ast-grep outline "$file" --view compact || true
echo
echo "Relevant sections around lines 120-230:"
sed -n '120,230p' "$file" | nl -ba -v120
echo
echo "Price/Payment statuses and transaction-related references:"
rg -n "PENDING|VOID|PAID|void|payment|merchantId|invoice" "$file"Repository: ShadeProtocol/shade-backend
Length of output: 588
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="./src/services/invoice.services.ts"
echo "Outline:"
ast-grep outline "$file" --view expanded || true
echo
echo "Relevant section lines 1-235:"
awk 'NR>=1 && NR<=235 { printf "%6d %s\n", NR, $0 }' "$file"
echo
echo "Transaction/ref references:"
rg -n "transaction|wrap|prisma\\.$|createMany|createManyAndReturn|update\\(|updateMany\\(|delete\\(|deleteMany\\(|transaction" "$file"
echo
echo "Schema Invoice model if present:"
fd 'schema.prisma|*.prisma' . -x sh -c 'echo "--- $1"; sed -n "1,220p" "$1" | rg -n -C 4 "model Invoice|model Merchant|enum PaymentStatus|enum .*Status" || true' sh {}Repository: ShadeProtocol/shade-backend
Length of output: 10080
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Prisma files:"
fd -a '*.prisma' .
for f in $(fd '.*\\.prisma$' .); do
echo "--- $f"
rg -n -C 4 "model Invoice|model Merchant|PaymentStatus|InvoiceStatus|status|merchantId|paymentSlug" "$f" || true
done
echo
echo "Payment-related service files:"
rg -l "void|paid|status|InvoiceStatus|PENDING|CANCELLED|PAID" src || true
echo
echo "Potential transaction/lock usage:"
rg -n "promises|transaction|lock|isol|update\\(|updateMany|findUnique|findFirst|createMany|createManyAndReturn" src || trueRepository: ShadeProtocol/shade-backend
Length of output: 467
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Prisma files:"
fd . . -e prisma -t f
for f in $(fd . . -e prisma -t f); do
echo "--- $f"
rg -n -C 4 "model Invoice|model Merchant|PaymentStatus|InvoiceStatus|status|merchantId|paymentSlug" "$f" || true
done
echo
echo "Potential payment/void status writes:"
rg -n "voidInvoice|paid|status: InvoiceStatus|CANCELLED|PAID|update\\(|updateMany\\(|createMany|createManyAndReturn" src || true
echo
echo "Prisma client invocations:"
rg -n "prisma\\." src || trueRepository: ShadeProtocol/shade-backend
Length of output: 12314
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Pay service outline:"
ast-grep outline ./src/services/pay.services.ts --view expanded || true
echo
echo "Pay service lines 1-135:"
awk 'NR>=1 && NR<=135 { printf "%6d %s\n", NR, $0 }' ./src/services/pay.services.ts
echo
echo "Invoice controller lines 90-130:"
awk 'NR>=90 && NR<=130 { printf "%6d %s\n", NR, $0 }' ./src/controllers/invoice.controllers.ts
echo
echo "Prisma package version if present:"
if [ -f package.json ]; then
node -e "const p=require('./package.json'); console.log(JSON.stringify({dependencies:p.dependencies,devDependencies:p.devDependencies}, null, 2))" | rg -n '"`@prisma/client`"|"prisma"' || true
fiRepository: ShadeProtocol/shade-backend
Length of output: 6209
Make the PENDING precondition part of the write.
amendInvoice() checks invoice.status before prisma.invoice.update({ where: { id } }), so a concurrent void/payment can update a now-non-pending invoice. Use a conditional update/fault path that also filters by merchantId and status: PENDING, and handle the unmatched case.
🤖 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 `@src/services/invoice.services.ts` around lines 205 - 208, Update
amendInvoice() so the Prisma invoice update atomically requires the matching
invoice id, merchantId, and status: PENDING in its where clause. Handle the
unmatched conditional-update result as the appropriate failure path instead of
treating the amendment as successful, while preserving the existing update data.
| test('updates only the provided fields on a PENDING invoice', async () => { | ||
| prismaMock.invoice.findFirst.mockResolvedValue(baseInvoice as any); | ||
| prismaMock.invoice.update.mockResolvedValue({ | ||
| ...baseInvoice, | ||
| email: 'payer@example.com', | ||
| amount: 2000n, | ||
| description: 'Updated website design', | ||
| } as any); | ||
|
|
||
| const result = await amendInvoice(MERCHANT_ID, 'invoice-1', { | ||
| email: 'payer@example.com', | ||
| amount: '2000', | ||
| description: 'Updated website design', | ||
| }); | ||
|
|
||
| expect(result.email).toBe('payer@example.com'); | ||
| expect(result.amount).toBe('2000'); | ||
| expect(result.description).toBe('Updated website design'); | ||
| expect(prismaMock.invoice.update).toHaveBeenCalledWith({ | ||
| where: { id: 'invoice-1' }, | ||
| data: { | ||
| email: 'payer@example.com', | ||
| amount: 2000n, | ||
| description: 'Updated website design', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exercise an actual partial amendment.
This case supplies every amendable field, so it cannot catch an accidental overwrite of omitted fields. Add a case with only one field, and assert update.data contains only that field.
🤖 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 `@tests/unit/invoice.services.test.ts` around lines 125 - 150, Add a focused
partial-update test alongside the existing PENDING invoice amendment test,
calling amendInvoice with only one amendable field and asserting
prismaMock.invoice.update receives data containing only that field. Keep the
existing result assertions appropriate to the mocked response and verify omitted
fields are not included in update.data.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline tests/unit/invoice.services.test.ts --items all --type function --match amendInvoice
printf '\n--- current amendInvoice test suite ---\n'
sed -n '/describe('\''amendInvoice'\'',/,/^ describe('\''voidInvoice'\'')/p' tests/unit/invoice.services.test.ts
printf '\n--- uncommitted/current branch diff for test file ---\n'
git diff HEAD~1 -- tests/unit/invoice.services.test.ts || trueLength of output: 9815
@codebestia, the fix is not yet present. The current success case still submits all three amendable fields, so it does not verify that omitted fields are excluded from prismaMock.invoice.update(...).data.
Please add a focused case that amends just one field (for example, description) and asserts the update payload contains only that field.
🧬 Code Graph Analysis Results
Potentially relevant code snippets
src/services/invoice.services.ts
createInvoice (slug generation + amount validation) — lines 56-89
export const createInvoice = async (merchantId: string, data: CreateInvoiceInput) => {
const amount = parseAmount(data.amount);
if (amount === null) {
throw new AppError(400, 'amount must be a positive integer');
}
const status: PrismaInvoiceStatus = data.isDraft ? InvoiceStatus.DRAFT : InvoiceStatus.PENDING;
const expiresAt = data.expiresAt ? new Date(data.expiresAt) : null;
for (let attempt = 0; attempt < SLUG_MAX_RETRIES; attempt++) {
try {
const invoice = await prisma.invoice.create({
data: {
merchantId,
description: data.description.trim(),
amount,
token: data.token.trim(),
email: data.payerEmail?.trim() ?? null,
expiresAt,
status,
paymentSlug: generatePaymentSlug(),
},
});
return sanitizeInvoice(invoice);
} catch (error) {
if (isUniqueSlugError(error) && attempt < SLUG_MAX_RETRIES - 1) {
continue;
}
throw error;
}
}
throw new AppError(500, 'Failed to generate a unique payment slug');
};- Params:
(merchantId: string, data: CreateInvoiceInput) - Returns:
sanitizeInvoice(invoice)(created invoice) - Throws:
AppError(400)ifamountis not a positive integer; may throw from Prisma (including unique slug handling);AppError(500)if unique slug cannot be generated after retries.
listInvoices (merchant scoping + pagination metadata) — lines 91-130
export const listInvoices = async (
merchantId: string,
filters: InvoiceListFilters,
pagination: InvoicePagination,
) => {
const where: Prisma.InvoiceWhereInput = { merchantId };
if (filters.status) {
where.status = filters.status;
}
if (filters.token) {
where.token = filters.token;
}
if (filters.startDate || filters.endDate) {
where.createdAt = {};
if (filters.startDate) where.createdAt.gte = filters.startDate;
if (filters.endDate) where.createdAt.lte = filters.endDate;
}
const [invoices, total] = await Promise.all([
prisma.invoice.findMany({
where,
take: pagination.limit,
skip: pagination.offset,
orderBy: { createdAt: 'desc' },
}),
prisma.invoice.count({ where }),
]);
return {
data: invoices.map(sanitizeInvoice),
pagination: {
limit: pagination.limit,
offset: pagination.offset,
total,
},
};
};- Params:
(merchantId, filters, pagination) - Returns:
{ data: ..., pagination: { limit, offset, total } } - Error handling: none explicit in this function (errors propagate from Prisma).
getInvoice (404 when missing) — lines 132-142
export const getInvoice = async (merchantId: string, id: string) => {
const invoice = await prisma.invoice.findFirst({
where: { id, merchantId },
});
if (!invoice) {
throw new AppError(404, 'Invoice not found');
}
return sanitizeInvoice(invoice);
};- Params:
(merchantId: string, id: string) - Returns:
sanitizeInvoice(invoice) - Throws:
AppError(404)when invoice not found for that merchant.
amendInvoice (pending-only, validation, partial update) — lines 162-212
export const amendInvoice = async (merchantId: string, id: string, data: AmendInvoiceInput) => {
const invoice = await prisma.invoice.findFirst({
where: { id, merchantId },
});
if (!invoice) {
throw new AppError(404, 'Invoice not found');
}
if (invoice.status !== InvoiceStatus.PENDING) {
throw new AppError(400, 'Only pending invoices can be amended');
}
const updateData: Prisma.InvoiceUpdateInput = {};
if (data.email !== undefined) {
updateData.email = data.email === null ? null : data.email.trim();
}
if (data.amount !== undefined) {
const amount = parseAmount(data.amount);
if (amount === null) {
throw new AppError(400, 'amount must be a positive integer');
}
updateData.amount = amount;
}
if (data.description !== undefined) {
const description =
typeof data.description === 'string' ? data.description.trim() : data.description;
if (typeof description !== 'string') {
throw new AppError(400, 'description must be a string');
}
if (description.length > INVOICE_DESCRIPTION_MAX_LENGTH) {
throw new AppError(400, 'description exceeds the maximum length of 100 characters');
}
updateData.description = description;
}
if (Object.keys(updateData).length === 0) {
return sanitizeInvoice(invoice);
}
// This endpoint updates the DB record only. On-chain amend_invoice reconciliation is intentionally out of scope.
const updated = await prisma.invoice.update({
where: { id: invoice.id },
data: updateData,
});
return sanitizeInvoice(updated);
};- Params:
(merchantId: string, id: string, data: AmendInvoiceInput) - Returns: existing sanitized invoice if no updatable fields; otherwise sanitized updated invoice
- Throws:
AppError(404)if missing;AppError(400)if not pending, invalid/non-positive amount, or description too long (also type checks for description).
voidInvoice (pending-only, sets CANCELLED) — lines 214-233
export const voidInvoice = async (merchantId: string, id: string) => {
const invoice = await prisma.invoice.findFirst({
where: { id, merchantId },
});
if (!invoice) {
throw new AppError(404, 'Invoice not found');
}
if (invoice.status !== InvoiceStatus.PENDING) {
throw new AppError(400, 'Only pending invoices can be voided');
}
const updated = await prisma.invoice.update({
where: { id: invoice.id },
data: { status: InvoiceStatus.CANCELLED },
});
return sanitizeInvoice(updated);
};- Params:
(merchantId: string, id: string) - Returns: sanitized updated invoice
- Throws:
AppError(404)if missing;AppError(400)if status is not pending.
|
Hello @Amarjeet325 |
|
Hello @codebestia I have fix code |
Summary
Adds a new invoice amendment endpoint for merchants to update an invoice’s payer email, amount, and description while the invoice is still pending.
What changed
Notes
This change is intentionally scoped to off-chain DB updates only. On-chain reconciliation for amended invoices remains out of scope for this PR.
Testing
Summary by CodeRabbit
PATCH /invoices/:id/amend.PENDINGinvoices (email, amount, description) and receive the updated invoice.404; invalid input or non-PENDINGstatus returns400.