-
Notifications
You must be signed in to change notification settings - Fork 0
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: add invoice me link functionality #8
Conversation
WalkthroughThis pull request introduces multiple updates across the project. A new SQL table ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant P as Invoice Me Page
participant API as InvoiceMe Router (TRPC)
participant DB as Database
U->>P: Access Invoice Management
P->>API: Query getAll invoice links
API->>DB: Fetch invoice links
DB-->>API: Return list of links
API-->>P: Return invoice links
P->>U: Render InvoiceMeLinks component
U->>P: Click delete on an invoice link
P->>API: Call delete mutation with link ID
API->>DB: Delete invoice link
DB-->>API: Confirm deletion
API-->>P: Invalidate cache and update list
P->>U: Refresh InvoiceMeLinks list
sequenceDiagram
participant U as User
participant B as Browser
participant GA as Google Auth Server
participant S as Server (OAuth Callback)
participant DB as Database
U->>B: Click Google Login
B->>GA: Redirect to Google with scopes (incl. email)
GA-->>B: Redirect back with authorization code
B->>S: Send OAuth callback with code
S->>GA: Validate code and retrieve token & claims
GA-->>S: Return token and user claims (sub, name, email)
S->>DB: Check if user exists
alt User exists
DB-->>S: Return user info
else
S->>DB: Create new user with googleId, name, email
DB-->>S: Confirm creation
end
S->>S: Create session for user
S-->>B: Redirect user to /dashboard
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 9
🔭 Outside diff range comments (1)
src/components/invoice-creator.tsx (1)
70-81
: 🛠️ Refactor suggestionImprove error handling in onSubmit.
The error handling could be more specific and provide better feedback to users.
const onSubmit = async (data: InvoiceFormValues) => { try { await createInvoice(data); } catch (error) { + const errorMessage = error instanceof Error + ? error.message + : "An unexpected error occurred"; + + // Log error for debugging + console.error('Failed to create invoice:', error); + toast.error("Failed to create invoice", { - description: - error instanceof Error - ? error.message - : "An unexpected error occurred", + description: errorMessage, + duration: 5000, // Give users more time to read the error }); } };
🧹 Nitpick comments (28)
src/app/login/google/callback/route.ts (5)
42-48
: Defineemail_verified
usage policy
The newGoogleClaims
interface includesemail_verified
, but the code never checks or uses it. If you require verified emails, consider validatingclaims.email_verified
before creating or updating the user profile.
52-54
: Expand error message for missing claims
The 400 response could be more descriptive by indicating which claim(s) are missing (sub, name, or email). This helps troubleshoot any token-related issues.
63-76
: Clarify redirect path for existing users
Existing users are redirected to “/”. Consider standardizing the post-login destination (e.g., “/dashboard”) for both new and existing users, or explaining the difference in user flow.
96-96
: Optimize final redirect for new users
You have chosen “/dashboard” for newly created accounts. Ensure that this route gracefully handles first-time visits (e.g., no incomplete profile or setup steps needed).
99-103
: Improve error logging
Returning a 400 for all exceptions may obscure root causes. Logging the actual error details in the catch block (e.g.,console.error(error)
) would help in diagnosing unexpected Google OAuth failures.src/server/routers/invoice.ts (4)
10-57
: Centralize error handling increateInvoiceHelper
This helper method is well-structured but relies on the caller for error handling. Consider surrounding the external POST request with try/catch for more granular failure feedback (e.g., logging or partial rollbacks).try { const response = await apiClient.post("/v1/request", { amount: totalAmount.toString(), ... }); +} catch (error) { + // Optionally log or re-throw a more informative error + throw new TRPCError({ code: "BAD_REQUEST", message: "Failed to create external request." }); }
79-87
: Transactional approach
Enclosing all DB operations withindb.transaction
is a solid approach. Ensure that any external calls also handle rollback logic consistently if needed, especially if the external request fails after partially inserting data.
145-149
: Payable invoice query
Usingeq(requestTable.invoicedTo, user?.id as string)
is straightforward. If you ever add partial matches or multiple recipients, you might need a more complex condition.
152-161
: Large-sum overflow
Be aware thatNumber(inv.amount)
might cause issues with extremely large invoice sums beyond 2^53 in JavaScript. If that’s a possibility, consider using aBigInt
or a specialized library.Do you want me to open a new issue for handling large integer arithmetic safely?
src/components/invoice-table.tsx (3)
17-17
: Icon usage clarity
Importing multiple Lucide icons tidies up your UI. Keep an eye on bundle size if many icons accumulate.
72-98
:EmptyState
for each tab
Providing distinct messages for sent vs. received tabs avoids confusion and clarifies user actions.
121-214
: Tab-based table
This chunk effectively splits invoice views. If further customizations for “sent” vs. “received” appear, consider refactoring repeated logic (table structure) into a shared subcomponent.src/lib/schemas/invoice.ts (1)
8-11
: Consider reusing email validation logic.The email validation is duplicated for
clientEmail
andcreatorEmail
. Consider extracting the email validation into a reusable schema.+const emailSchema = z.string().email("Invalid email address"); + export const invoiceFormSchema = z.object({ // ... - clientEmail: z.string().email("Invalid email address"), + clientEmail: emailSchema, invoicedTo: z.string().optional(), creatorName: z.string().min(1, "Your name is required"), - creatorEmail: z.string().email("Invalid email address"), + creatorEmail: emailSchema, // ...src/server/routers/invoice-me.ts (1)
60-64
: Add return value for delete operation.The delete operation should return a success indicator or the deleted item.
- await db + const result = await db .delete(invoiceMeTable) .where( and(eq(invoiceMeTable.id, input), eq(invoiceMeTable.userId, user.id)), ); + return { success: true, id: input };src/app/invoice-me/page.tsx (1)
73-73
: Add loading state for InvoiceMeLinks.The data fetching could take time, but there's no loading state shown to users.
Consider adding a loading skeleton or spinner while the links are being fetched:
+ const [isLoading, setIsLoading] = useState(true); + useEffect(() => { + // Set loading to false after links are fetched + setIsLoading(false); + }, [links]); - <InvoiceMeLinks initialLinks={links} /> + {isLoading ? ( + <div>Loading invoice links...</div> + ) : ( + <InvoiceMeLinks initialLinks={links} /> + )}src/app/invoices/create/page.tsx (2)
16-16
: Remove console.log statement.Debug logging should not be present in production code.
- console.log("User : ", user);
71-74
: Use nullish coalescing for safer object destructuring.The current implementation could be made safer by using nullish coalescing operator for the entire user object.
- currentUser={{ - email: user.email ?? "", - name: user.name ?? "", - }} + currentUser={user ? { + email: user.email ?? "", + name: user.name ?? "", + } : undefined}src/components/invoice-creator.tsx (2)
16-26
: Enhance type safety with required fields.Consider making essential fields required in the interface.
interface InvoiceCreatorProps { recipientDetails?: { - clientName: string; - clientEmail: string; - userId: string; + clientName: string | null; + clientEmail: string | null; + userId: string; // Required as it's used for conditional logic }; currentUser?: { - name: string; - email: string; + name: string | null; + email: string | null; }; }
90-95
: Add loading state feedback in the UI.The loading state is passed to InvoiceForm but there's no visual feedback in the preview.
<InvoiceForm form={form} onSubmit={onSubmit} isLoading={isLoading} recipientDetails={recipientDetails} /> + {isLoading && ( + <div className="mt-4 text-sm text-gray-500"> + Creating invoice... + </div> + )}src/app/i/[id]/page.tsx (2)
73-77
: Improve type safety for user data.The current implementation assumes user data exists. Add proper type guards and fallbacks.
recipientDetails={{ - clientName: invoiceMeLink.user.name ?? "", - clientEmail: invoiceMeLink.user.email ?? "", - userId: invoiceMeLink.user.id, + clientName: invoiceMeLink.user?.name ?? "Unknown", + clientEmail: invoiceMeLink.user?.email ?? "", + userId: invoiceMeLink.user?.id ?? "", // Consider handling missing user ID }}
8-11
: Enhance SEO metadata.The current metadata could be more specific to improve SEO.
export const metadata: Metadata = { - title: "Invoice Me | EasyInvoice", - description: "Create an invoice for a service provider", + title: `Create Invoice for ${invoiceMeLink?.label ?? 'Service Provider'} | EasyInvoice`, + description: "Create and send a professional invoice quickly and securely", + openGraph: { + title: `Create Invoice for ${invoiceMeLink?.label ?? 'Service Provider'} | EasyInvoice`, + description: "Create and send a professional invoice quickly and securely", + }, };src/app/dashboard/page.tsx (1)
57-68
: Consider adding aria-current for active navigation.The navigation links are well-implemented, but adding
aria-current="page"
to the active link would improve accessibility.<Link href="/dashboard" className="text-zinc-900 hover:text-zinc-600 transition-colors" + aria-current="page" > Dashboard </Link>
src/components/invoice-me-links.tsx (2)
19-19
: Consider moving origin logic to a custom hook.Extract the origin determination logic into a reusable hook for better maintainability.
// useOrigin.ts export function useOrigin() { return typeof window !== "undefined" ? window.location.origin : ""; }
82-92
: Clear error state on input change.Consider clearing the error state when the user starts typing to provide better feedback.
<Input id="newLinkName" placeholder="e.g. Project X" value={newLinkName} - onChange={(e) => setNewLinkName(e.target.value)} + onChange={(e) => { + setNewLinkName(e.target.value); + setError(null); + }} onKeyDown={async (e) => { if (e.key === "Enter") { await handleCreateInvoiceMeLink(); } }} />src/components/invoice-form.tsx (1)
70-130
: Consider extracting form fields into reusable components.The conditional rendering creates duplicate form field structures. Consider extracting these into reusable components to improve maintainability and reduce code duplication.
Example refactor:
interface FormFieldProps { label: string; name: string; placeholder: string; type?: string; register: UseFormRegister<InvoiceFormValues>; error?: FieldError; } function FormField({ label, name, placeholder, type = "text", register, error }: FormFieldProps) { return ( <div className="space-y-2"> <Label htmlFor={name}>{label}</Label> <Input {...register(name)} type={type} placeholder={placeholder} /> {error && ( <p className="text-sm text-red-500"> {error.message} </p> )} </div> ); }Then use it like:
-<div className="space-y-2"> - <Label htmlFor="creatorName">Your Name</Label> - <Input - {...form.register("creatorName")} - placeholder="Enter your name" - /> - {form.formState.errors.creatorName && ( - <p className="text-sm text-red-500"> - {form.formState.errors.creatorName.message} - </p> - )} -</div> +<FormField + label="Your Name" + name="creatorName" + placeholder="Enter your name" + register={form.register} + error={form.formState.errors.creatorName} +/>drizzle/0001_brown_sebastian_shaw.sql (1)
8-10
: Consider making the email column NOT NULL.Since email is used for invoice notifications and is marked as unique, consider making it a required field.
-ALTER TABLE "easyinvoice_user" ADD COLUMN "email" text; +ALTER TABLE "easyinvoice_user" ADD COLUMN "email" text NOT NULL;drizzle/meta/0001_snapshot.json (2)
113-118
: Consider using a more specific JSON type for items.The
items
column uses a generic JSON type. Consider using JSONB for better query performance and indexing capabilities.- "type": "json", + "type": "jsonb",
71-82
: Consider using DATE type for date fields.The
dueDate
andissuedDate
columns are defined as text. Using DATE type would provide better data validation and query optimization.- "type": "text", + "type": "date",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (24)
drizzle/0001_brown_sebastian_shaw.sql
(1 hunks)drizzle/meta/0001_snapshot.json
(1 hunks)drizzle/meta/_journal.json
(1 hunks)package.json
(1 hunks)src/app/dashboard/page.tsx
(3 hunks)src/app/i/[id]/page.tsx
(1 hunks)src/app/invoice-me/page.tsx
(1 hunks)src/app/invoices/[ID]/page.tsx
(1 hunks)src/app/invoices/create/page.tsx
(2 hunks)src/app/login/google/callback/route.ts
(1 hunks)src/app/login/google/route.ts
(2 hunks)src/components/invoice-creator.tsx
(2 hunks)src/components/invoice-form.tsx
(2 hunks)src/components/invoice-me-link.tsx
(1 hunks)src/components/invoice-me-links.tsx
(1 hunks)src/components/invoice-preview.tsx
(2 hunks)src/components/invoice-table.tsx
(2 hunks)src/components/ui/alert-dialog.tsx
(1 hunks)src/lib/schemas/invoice.ts
(1 hunks)src/server/db/schema.ts
(5 hunks)src/server/index.ts
(1 hunks)src/server/routers/invoice-me.ts
(1 hunks)src/server/routers/invoice.ts
(1 hunks)src/server/trpc.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/server/trpc.ts
🔇 Additional comments (27)
src/app/login/google/callback/route.ts (1)
78-88
: Ensure unique email constraint alignment
When creating a new user, the code setsemail: emailFromClaims
. If theuserTable
enforces unique email constraints, confirm that multiple Google accounts cannot share the same email inadvertently.src/server/routers/invoice.ts (5)
3-5
: Imports look good
The added imports (userTable
,and
,desc
, etc.) neatly support the new logic introduced below.
64-70
: Validate user session security
Checkingif (!user)
is correct for gating invoice creation. If further role-based policies are introduced, expand or refine this check accordingly.
72-75
: Consider multi-account edge cases
Finding a single client user record by
95-127
:createFromInvoiceMe
logic
The new public endpoint properly verifies the recipient withinvoicedTo
. This is a good pattern for ensuring you only create an invoice if the user truly exists.
131-140
: Invoice separation logic
The conditionor(isNull(requestTable.invoicedTo), not(eq(requestTable.invoicedTo, user?.id)))
is a neat way to define “receivables.” Confirm it correctly excludes any “self-invoices” that might slip through.src/components/invoice-table.tsx (9)
12-12
: Tabs UI import
Bringing in the tabs component is a good call for segmented invoice views.
24-28
: Strongly typedInvoiceType
Defining an explicit type for invoices, total, and outstanding fosters clarity and maintainability.
37-41
:StatCard
component
Extracting a reusable stat card is a clean move. These small UI components help reduce duplication around invoice stats.Also applies to: 43-57
58-60
: Separate pagination states
Maintaining individual pages for receivable vs. payable invoices is a solid approach to avoid collisions in pagination logic.
69-70
: Single source of truth
activeData
elegantly switches betweeninvoices.issuedByMe
and.issuedToMe
, eliminating the need for repeated logic.
100-119
: Expandable metrics
The stat cards for total invoices, outstanding, and total amounts are easy to expand if you add new metrics (e.g., overdue count). Good forward-thinking design.
218-268
:InvoiceRow
time zone caution
isPast(dueDate)
works well for local time. If you need absolute UTC-based deadlines, ensure that the date-fns call aligns with your server’s time zone or that you store/parse dates consistently in UTC.
270-283
: Conditional headers
TableColumns
dynamically adjusts for “sent” vs. “received.” This helps keep user context clear in each tab.
285-324
: Pagination component
Providing a single UI for navigating pages is a neat approach. The minimal logic is easy to maintain and could be reused across your app if needed.src/server/index.ts (1)
3-3
: LGTM!The
invoiceMeRouter
is correctly imported and registered in the app router, following the established pattern.Also applies to: 9-9
src/app/login/google/route.ts (1)
12-13
: LGTM!The email scopes are correctly added to support the new email field in the user table.
src/app/dashboard/page.tsx (1)
23-23
: LGTM! Simplified data retrieval.The simplified invoice data retrieval improves code readability while maintaining functionality.
src/server/db/schema.ts (1)
88-97
: LGTM! Well-structured table definition.The
invoiceMeTable
is well-defined with appropriate constraints and relationships.src/components/ui/alert-dialog.tsx (1)
1-141
: LGTM! Well-structured Alert Dialog component.The implementation follows best practices:
- Uses Radix UI primitives for accessibility
- Proper TypeScript type safety
- Consistent styling with utility classes
- Good component composition and reusability
src/components/invoice-preview.tsx (1)
54-65
: LGTM! Well-structured creator information display.The implementation provides a clear visual hierarchy for creator details with consistent spacing and styling.
src/app/invoices/[ID]/page.tsx (1)
123-127
: LGTM! Consistent creator information display.The implementation maintains visual consistency with the invoice preview component and provides a clear hierarchy of information.
src/components/invoice-form.tsx (1)
25-33
: LGTM! Well-typed props interface.The interface clearly defines the optional recipientDetails prop with proper TypeScript types.
drizzle/meta/_journal.json (1)
12-18
: LGTM! Migration journal entry is properly structured.The new migration entry follows proper versioning practices with appropriate metadata.
drizzle/0001_brown_sebastian_shaw.sql (2)
1-6
: LGTM! Table creation follows best practices.The new
easyinvoice_invoice_me
table is well-structured with appropriate constraints and timestamps.
12-16
: LGTM! Proper error handling for constraint creation.The DO block with exception handling prevents errors from duplicate constraint creation.
package.json (1)
24-24
: LGTM! Dependencies align with new UI requirements.The addition of alert-dialog and updates to other Radix UI components support the new invoice link functionality.
Also applies to: 28-29
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 (3)
src/server/routers/invoice-me.ts (3)
11-13
: Enhance input validation for the label field.Consider adding constraints to the label field to prevent empty strings or excessive lengths.
- label: z.string(), + label: z.string().min(1).max(100),
25-30
: Consider returning the created invoice link.The mutation should return the created entity to allow immediate use by the client.
- await db.insert(invoiceMeTable).values({ + const [createdLink] = await db.insert(invoiceMeTable).values({ id: ulid(), label: input.label, userId: user.id, - }); + }).returning(); + return createdLink;
60-65
: Add deletion verification and return confirmation.Consider verifying the link exists and returning the deletion status.
- await db + const [deletedLink] = await db .delete(invoiceMeTable) .where( and(eq(invoiceMeTable.id, input), eq(invoiceMeTable.userId, user.id)), - ); + ) + .returning(); + + if (!deletedLink) { + throw new TRPCError({ + code: "NOT_FOUND", + message: "Invoice me link not found or you don't have permission to delete it", + }); + } + + return { success: true };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/invoice-me-link.tsx
(1 hunks)src/server/routers/invoice-me.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/invoice-me-link.tsx
🔇 Additional comments (3)
src/server/routers/invoice-me.ts (3)
1-8
: LGTM! Well-organized imports and router setup.The imports are well-structured and the router setup follows TRPC best practices.
31-47
: Consider pagination for getAll procedure.The
getAll
procedure fetches all invoice links without pagination, which could lead to performance issues as the number of links grows.
66-90
: LGTM! Well-implemented getById procedure.The procedure includes proper error handling and efficiently retrieves related user details.
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 (6)
src/components/version-badge.tsx (2)
3-3
: Consider using environment variables for version information.Direct import of
package.json
might cause issues in certain build configurations. Consider using environment variables or build-time constants for version information.-import packageInfo from "../../package.json"; +const VERSION = process.env.NEXT_PUBLIC_APP_VERSION || "0.0.0";
11-17
: Add aria-label for accessibility.The version badge link should have an aria-label for better accessibility.
- <Link target="_blank" href={githubRelease}> + <Link + target="_blank" + href={githubRelease} + aria-label={`Version ${packageInfo.version} - View release notes`} + >src/components/ui/badge.tsx (1)
30-34
: Add role attribute for accessibility.The badge div should have a role attribute for better accessibility.
- <div className={cn(badgeVariants({ variant }), className)} {...props} /> + <div role="status" className={cn(badgeVariants({ variant }), className)} {...props} />src/app/layout.tsx (1)
42-42
: Move GitHub release URL to environment variables.Hardcoded URLs should be moved to environment variables for better maintainability and configuration flexibility.
- <VersionDisplay githubRelease="https://github.com/RequestNetwork/easy-invoice/releases" /> + <VersionDisplay githubRelease={process.env.NEXT_PUBLIC_GITHUB_RELEASES_URL} />Add the following to your
.env
file:NEXT_PUBLIC_GITHUB_RELEASES_URL=https://github.com/RequestNetwork/easy-invoice/releases
src/components/invoice-form.tsx (2)
70-130
: Consider extracting form field component to reduce duplication.The error handling structure is repeated across multiple form fields. Consider creating a reusable form field component to reduce code duplication.
+// FormField.tsx +interface FormFieldProps { + label: string; + name: string; + type?: string; + placeholder: string; + register: any; + error?: { message?: string }; +} + +export function FormField({ + label, + name, + type = "text", + placeholder, + register, + error, +}: FormFieldProps) { + return ( + <div className="space-y-2"> + <Label htmlFor={name}>{label}</Label> + <Input + {...register} + type={type} + placeholder={placeholder} + /> + {error && ( + <p className="text-sm text-red-500"> + {error.message} + </p> + )} + </div> + ); +} // Usage in InvoiceForm -<div className="space-y-2"> - <Label htmlFor="creatorName">Your Name</Label> - <Input - {...form.register("creatorName")} - placeholder="Enter your name" - /> - {form.formState.errors.creatorName && ( - <p className="text-sm text-red-500"> - {form.formState.errors.creatorName.message} - </p> - )} -</div> +<FormField + label="Your Name" + name="creatorName" + placeholder="Enter your name" + register={form.register("creatorName")} + error={form.formState.errors.creatorName} +/>
134-139
: Consider using responsive grid column sizes.The current fixed pixel widths (
80px
,96px
,40px
) might not scale well on different screen sizes. Consider using relative units or CSS Grid'sminmax()
function for better responsiveness.-<div className="grid grid-cols-[1fr,80px,96px,40px] gap-4 mb-1"> +<div className="grid grid-cols-[minmax(200px,1fr),minmax(80px,auto),minmax(96px,auto),40px] gap-4 mb-1">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
package.json
(2 hunks)src/app/layout.tsx
(2 hunks)src/components/invoice-form.tsx
(2 hunks)src/components/ui/badge.tsx
(1 hunks)src/components/version-badge.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (3)
src/components/ui/badge.tsx (1)
6-24
: LGTM! Well-structured variant definitions.The badge variants are well-defined using class-variance-authority with consistent styling across different states.
src/components/invoice-form.tsx (2)
25-40
: LGTM! Well-structured interface changes.The addition of the optional
recipientDetails
prop and the updated component signature are well-typed and maintain backward compatibility.
70-130
: LGTM! Clear conditional rendering logic.The implementation cleanly separates the invoice-me flow from the normal invoice flow with clear comments and consistent error handling.
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 🚧
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 (4)
src/app/invoices/create/page.tsx (1)
68-73
: Consider validating email presence before invoice creation.While the code handles null values gracefully, creating an invoice without a valid email could lead to issues with the "Invoice Me" functionality. Consider adding a validation check or requiring email setup before allowing invoice creation.
- <InvoiceCreator - currentUser={{ - email: user.email ?? "", - name: user.name ?? "", - }} - /> + {!user.email ? ( + <div className="text-center"> + <p className="text-lg text-gray-600 mb-4"> + Please set up your email before creating invoices + </p> + <Link + href="/settings" + className="text-blue-600 hover:text-blue-800" + > + Go to Settings + </Link> + </div> + ) : ( + <InvoiceCreator + currentUser={{ + email: user.email, + name: user.name ?? "", + }} + /> + )}src/server/routers/invoice.ts (3)
61-92
: Improve error handling in the create procedure.While the authentication check and client email lookup are good additions, the error handling could be more specific:
Apply this diff to improve error handling:
create: protectedProcedure .input(invoiceFormSchema) .mutation(async ({ ctx, input }) => { const { db, user } = ctx; if (!user) { throw new TRPCError({ code: "UNAUTHORIZED", message: "You must be logged in to create an invoice", }); } // Check if client email exists as a user const clientUser = await db.query.userTable.findFirst({ where: eq(userTable.email, input.clientEmail), }); try { return await db.transaction(async (tx) => { return createInvoiceHelper( tx, { ...input, invoicedTo: clientUser?.id ?? undefined, }, user.id, ); }); } catch (error) { - console.error("Error: ", error); - return { success: false }; + if (error instanceof TRPCError) { + throw error; + } + console.error("Failed to create invoice:", error); + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Failed to create invoice", + cause: error, + }); } }),
94-128
: Improve error handling and clarify comments in the createFromInvoiceMe procedure.The procedure has good recipient validation but needs similar error handling improvements as the create procedure.
Apply this diff to improve error handling and clarify comments:
// Public route for invoice-me creation createFromInvoiceMe: publicProcedure .input(invoiceFormSchema) .mutation(async ({ ctx, input }) => { const { db } = ctx; if (!input.invoicedTo) { throw new TRPCError({ code: "BAD_REQUEST", message: "Invoice recipient ID is required", }); } // Verify the invoicedTo user exists const recipient = await db.query.userTable.findFirst({ where: eq(userTable.id, input.invoicedTo), }); if (!recipient) { throw new TRPCError({ code: "NOT_FOUND", message: "Invoice recipient not found", }); } try { return await db.transaction(async (tx) => { - // For invoice-me, the userId is the same as invoicedTo + // In invoice-me flow, the recipient becomes the invoice creator + // This ensures the invoice appears in their "issued by me" list return createInvoiceHelper(tx, input, input.invoicedTo as string); }); } catch (error) { - console.error("Error: ", error); - return { success: false }; + if (error instanceof TRPCError) { + throw error; + } + console.error("Failed to create invoice from invoice-me:", error); + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Failed to create invoice", + cause: error, + }); } }),
130-163
: Consider performance optimizations for the getAll procedure.The separation of invoices and the use of drizzle-orm operators is well implemented. However, consider these optimizations:
Apply this diff to improve type safety and performance:
- getAll: protectedProcedure.query(async ({ ctx }) => { + getAll: protectedProcedure.query(async ({ ctx }) => { const { db, user } = ctx; + + if (!user) { + throw new TRPCError({ + code: "UNAUTHORIZED", + message: "You must be logged in to view invoices", + }); + } // Get invoices to receive (issued by me) const receivables = await db.query.requestTable.findMany({ where: and( - eq(requestTable.userId, user?.id as string), + eq(requestTable.userId, user.id), or( isNull(requestTable.invoicedTo), - not(eq(requestTable.invoicedTo, user?.id as string)), + not(eq(requestTable.invoicedTo, user.id)), ), ), orderBy: desc(requestTable.createdAt), }); // Get invoices to pay (issued to me) const payables = await db.query.requestTable.findMany({ - where: eq(requestTable.invoicedTo, user?.id as string), + where: eq(requestTable.invoicedTo, user.id), orderBy: desc(requestTable.createdAt), }); + // Memoize calculated values + const receivablesTotal = receivables.reduce( + (acc, inv) => acc + Number(inv.amount), + 0, + ); + const receivablesOutstanding = receivables.filter( + (inv) => inv.status !== "paid", + ).length; + + const payablesTotal = payables.reduce( + (acc, inv) => acc + Number(inv.amount), + 0, + ); + const payablesOutstanding = payables.filter( + (inv) => inv.status !== "paid", + ).length; return { issuedByMe: { invoices: receivables, - total: receivables.reduce((acc, inv) => acc + Number(inv.amount), 0), - outstanding: receivables.filter((inv) => inv.status !== "paid").length, + total: receivablesTotal, + outstanding: receivablesOutstanding, }, issuedToMe: { invoices: payables, - total: payables.reduce((acc, inv) => acc + Number(inv.amount), 0), - outstanding: payables.filter((inv) => inv.status !== "paid").length, + total: payablesTotal, + outstanding: payablesOutstanding, }, }; }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
biome.json
(1 hunks)package.json
(2 hunks)src/app/invoices/create/page.tsx
(2 hunks)src/components/invoice-creator.tsx
(2 hunks)src/server/routers/invoice.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/components/invoice-creator.tsx
🔇 Additional comments (3)
biome.json (1)
25-31
: New Linter Rule for Console Usage AddedThe new
"noConsole"
rule is configured to trigger an error for any disallowed use ofconsole
methods while permittingerror
,warn
,info
, anddebug
. This change will enforce stricter logging practices and prevent accidental logging in production. Please ensure that all team members are aware of this update so that any console calls outside of the permitted set are refactored accordingly.src/app/invoices/create/page.tsx (1)
14-16
: LGTM! Clean session management implementation.The session management changes improve type safety while maintaining the authentication flow.
src/server/routers/invoice.ts (1)
3-3
: LGTM!The new imports for database tables and operators enhance the query capabilities and maintain proper database access.
Also applies to: 5-5
Resolves #6
Invoice Me Feature Implementation
Changes
Technical Updates
UI/UX Improvements
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements