-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Implement auto-incrementing invoice numbers #43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Implement auto-incrementing invoice numbers #43
Conversation
…creation and display
WalkthroughThis change introduces an auto-increment invoice number feature. A new asynchronous function ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant P as Invoice Page
participant IL as Invoice Library (getInvoiceCount)
participant IC as InvoiceCreator Component
participant CL as Client Library (generateInvoiceNumber)
U->>P: Initiate invoice creation
P->>IL: getInvoiceCount(user.id)
IL-->>P: Return invoiceCount
P->>IC: Pass invoiceCount prop
IC->>CL: generateInvoiceNumber(invoiceCount)
CL-->>IC: Return formatted invoiceNumber
Assessment against linked issues
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/lib/invoice/index.ts (2)
24-27: Consider variable invoice number length for future scalability.Currently, the invoice count is padded to exactly 4 digits, limiting the monthly invoices to 9999 per user. While this might be sufficient for now, consider making this configurable or adding logic to handle overflow cases for high-volume users.
5-31:❓ Verification inconclusive
Potential improvements to the invoice number generation function.
The function correctly implements the auto-incrementing invoice number requirement in the format YYYYMM-XXXX. However, there are a few opportunities for improvement:
- Multiple
new Date()instances are created when a single one could be reused- There's no error handling if the database query returns empty results
- Potential race condition if multiple invoices are created simultaneously
export const generateInvoiceNumber = async (userId: string) => { + // Use a single Date instance for all calculations + const now = new Date(); + const firstDayOfMonth = new Date(now.getFullYear(), now.getMonth(), 1); + const invoicesCountThisMonth = await db .select({ count: count(), }) .from(requestTable) .where( and( eq(requestTable.userId, userId), gte( requestTable.createdAt, - new Date(new Date().getFullYear(), new Date().getMonth(), 1), + firstDayOfMonth, ), ), ); - const now = new Date(); const year = now.getFullYear(); const month = String(now.getMonth() + 1).padStart(2, "0"); + // Add error handling for empty results + const count = invoicesCountThisMonth[0]?.count ?? 0; const invoiceCount = String(invoicesCountThisMonth[0].count + 1).padStart( 4, "0", ); // Pad with zeros to 4 digits const invoiceNumber = `${year}${month}-${invoiceCount}`; return invoiceNumber; };For addressing the race condition, consider adding a database transaction or implementing a more robust solution if uniqueness is critical.
🏁 Script executed:
#!/bin/bash # Check if the database schema has any uniqueness constraints on invoice numbers fd requestTable ./ --exec grep -A 20 "requestTable" {} \;Length of output: 60
Invoice Number Generation Function Enhancements
The function generally meets the requirement of returning a number in the YYYYMM-XXXX format but still has some actionable improvement points:
- Reuse Date Instance: Instead of calling
new Date()multiple times, store the current date and derive needed values (e.g., first day of the month) once.- Error Handling: Guard against cases where the database query might return an empty result by defaulting the count to zero.
- Race Condition Warning: As the auto-increment logic depends on the query result, there’s potential for race conditions if multiple invoices are generated concurrently. Consider using a database transaction or another concurrency control mechanism. Also, please verify manually whether a uniqueness constraint exists for invoice numbers in the database schema since the initial check produced no conclusive output.
export const generateInvoiceNumber = async (userId: string) => { + // Use a single Date instance for all calculations + const now = new Date(); + const firstDayOfMonth = new Date(now.getFullYear(), now.getMonth(), 1); + const invoicesCountThisMonth = await db .select({ count: count(), }) .from(requestTable) .where( and( eq(requestTable.userId, userId), gte( requestTable.createdAt, - new Date(new Date().getFullYear(), new Date().getMonth(), 1), + firstDayOfMonth, ), ), ); - const now = new Date(); const year = now.getFullYear(); const month = String(now.getMonth() + 1).padStart(2, "0"); + // Add error handling for empty results + const count = invoicesCountThisMonth[0]?.count ?? 0; const invoiceCount = String(invoicesCountThisMonth[0].count + 1).padStart( 4, "0", ); // Pad with zeros to 4 digits const invoiceNumber = `${year}${month}-${invoiceCount}`; return invoiceNumber; };Please verify that the database-level uniqueness (if required) is appropriately enforced since our initial check did not yield conclusive results.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/i/[id]/page.tsx(3 hunks)src/app/invoices/create/page.tsx(3 hunks)src/components/invoice-creator.tsx(2 hunks)src/components/invoice-form.tsx(1 hunks)src/lib/invoice/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
src/app/i/[id]/page.tsx (1)
src/lib/invoice/index.ts (1) (1)
generateInvoiceNumber(5-31)
src/app/invoices/create/page.tsx (1)
src/lib/invoice/index.ts (1) (1)
generateInvoiceNumber(5-31)
🔇 Additional comments (12)
src/components/invoice-form.tsx (1)
54-58: Successfully implemented read-only invoice number fieldThe input field for invoice number has been properly modified to include the
disabledattribute, which makes it read-only and prevents users from manually editing the auto-generated invoice number.src/app/i/[id]/page.tsx (3)
5-5: Properly imported the invoice number generator functionThe
generateInvoiceNumberfunction is correctly imported from the invoice library.
28-28: Successfully implemented invoice number generationThe code correctly generates a unique invoice number using the user ID from the invoice me link.
55-55: Correctly passing the generated invoice number to the componentThe invoice number is properly passed as a prop to the InvoiceCreator component.
src/app/invoices/create/page.tsx (3)
5-5: Properly imported the invoice number generator functionThe
generateInvoiceNumberfunction is correctly imported from the invoice library.
18-18: Successfully implemented invoice number generationThe code correctly generates a unique invoice number using the current user's ID.
44-44: Correctly passing the generated invoice number to the componentThe invoice number is properly passed as a prop to the InvoiceCreator component.
src/components/invoice-creator.tsx (3)
26-26: Properly extended interface with optional invoice number propertyThe
InvoiceCreatorPropsinterface has been correctly updated to include an optionalinvoiceNumberproperty.
32-32: Correctly destructured the invoice number propThe
invoiceNumberprop is properly destructured from the component parameters.
57-57: Successfully implemented invoice number in form default valuesThe form's default value for
invoiceNumberis now correctly set to use the providedinvoiceNumberprop.src/lib/invoice/index.ts (2)
1-4: LGTM! Appropriate imports for the task.All necessary dependencies are imported for database access and query operations.
29-31: Function works as expected for the given requirements.The invoice number format matches the specified requirement (YYYYMM-XXXX) and properly increments based on the user's invoice count for the current month.
MantisClone
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved 👍 Pending Comment Resolution 🚧
How do we handle timezones when determining the year and month used in the invoice number?
…val and update related components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/invoice/client.ts (1)
1-14: Add JSDoc comment and improve array destructuring logic.The function correctly generates invoice numbers in the YYYYMM-XXXX format as specified in the PR objectives. However, there are a few improvements that can be made:
- Add JSDoc comments to document the function's purpose, parameters, and return value
- The array destructuring to extract month and year assumes a specific order of parts, which could be fragile if locale settings change
+/** + * Generates an invoice number in the format YYYYMM-XXXX + * @param invoiceCount - The count of invoices for the current month (already zero-padded) + * @returns A formatted invoice number string + */ export const generateInvoiceNumber = (invoiceCount: string) => { const now = new Date(); const userTimeZone = Intl.DateTimeFormat().resolvedOptions().timeZone; const formatter = new Intl.DateTimeFormat(undefined, { year: "numeric", month: "2-digit", timeZone: userTimeZone, }); - const [{ value: month }, , { value: year }] = formatter.formatToParts(now); + // Find parts by type instead of relying on position + const parts = formatter.formatToParts(now); + const month = parts.find(part => part.type === 'month')?.value || ''; + const year = parts.find(part => part.type === 'year')?.value || ''; return `${year}${month}-${invoiceCount}`; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/i/[id]/page.tsx(3 hunks)src/app/invoices/create/page.tsx(3 hunks)src/components/invoice-creator.tsx(3 hunks)src/lib/invoice/client.ts(1 hunks)src/lib/invoice/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/app/invoices/create/page.tsx
- src/lib/invoice/index.ts
- src/components/invoice-creator.tsx
- src/app/i/[id]/page.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
Resolves #22
Automated Invoice Number Generation
Changes
YYYYMM-XXXXTechnical Details
generateInvoiceNumberthat:YYYYMM-XXXXwhere XXXX is zero-paddedScreenshots:
Summary by CodeRabbit