Skip to content

Conversation

bassgeta
Copy link
Contributor

@bassgeta bassgeta commented Oct 1, 2025

Problem

We want our easy invoice users to create client IDs and test out our checkout flow

Solution

Implement a new Ecommerce page on easy invoice, where users can manage their client integrations.

Changes

First of all, I've opted for naming the client id integrations EcommerceClient so there is no confusing code bits like clientId.clientId.
Even though our API uses the term ClientId, let's imagine that builders are issuing Client IDs to their clients.

  • new ecommerceClient entity. NOTE: it has 2 external IDs, the externalId is the id field on our clientId API entity. The rnClientId is the API's clientId field, which is used for the checkout.
  • new EcommerceManage page with 2 tabs, the Sales one being stubbed for this PR
  • CreateDefaultEcommerceClient component which is displayed when there is no client for our checkout domain yet.
  • EcommerceClientForm which is used for creating and editing the client
  • EcommerceClientEdit and EcommerceClientCreate are dialogs implementing that form
  • EcommerceClientTable is a filterable table of all created ecommerce clients.
  • EcommerceClientDelete is a button that opens an alert dialog for deleting an ecommerce client.
  • added new clientPayment entity that stores all payments made with a client id
  • modified the webhook route to store a client payment instead of a general request if clientId is present on the webhook body
  • implemented an overview of all payments for the manage view
  • implemented an overview of all the user's "receipts" -> payments made via our API using clientIds

Testing

  1. Navigate to the new ecommerce page, verify that you have 2 tabs.
image 2. When you have no clients, verify that you can create a default one by clicking the button image 3. Verify that you can't delete or edit the default client. 4. Create a new client using the changed button image 5. Verify that the first two fields are required image 6. Verify that the domain needs to be a URL image 7. Verify that if you input only the fee address, the percentage is required. Also verify that it needs to be a valid address. image 8. Verify that if you input the percentage, but no address, the address should display an error image 9. Remove the fee fields and create the client image 10. Verify that you can edit it's domain, label and fee info image image 11. Verify that you can delete the client image image 12. Please put the form through its paces, check if you can unset the fee address and percentage, check if you can't create clients for the same domain, etc.

Testing the receipts and sales

It's going to be the easiest to test this alongside this PR running on localhost:3001 and having easy invoice on 3000.

  1. Create the default client ID and copy the value. Go to RN checkout and add something to your cart, get to the payment step.
  2. Enter your client ID here, go over the payment flow. NOTE: Use the same email for customer info as the email with which you're logged into Easy Invoice!!!
image image 3. Once a payment is done and you are on the success step, go over to easy invoice's sales table and refresh it. Verify that you get the sales in the table and you can filter (although we only have 1 merchant so far...) image 4. Also verify that you get this payment in your receipts tab image

Resolves #151 and resolves #152

Summary by CodeRabbit

  • New Features
    • Introduced an authenticated Ecommerce area with dedicated navigation.
    • Manage clients: list, filter, create, edit, and delete; quick-create a default client.
    • Sales: view client payments with filters, pagination, expandable customer details, and external request links.
    • Dashboard Receipts: new tab to view your receipts with merchant filter, pagination, and error/retry handling.
    • Header now includes a link to Ecommerce.
  • Chores
    • Added env example for the default ecommerce domain (NEXT_PUBLIC_DEFAULT_ECOMMERCE_DOMAIN).

Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

Walkthrough

Adds an e-commerce module with authenticated layout, navigation, client ID management (CRUD), sales and receipts views, webhook handling for client payments, TRPC router integration, DB schema/migrations for ecommerce clients and client payments, environment/config constants, and related UI components and pages.

Changes

Cohort / File(s) Summary
Env & Constants
./.env.example, src/lib/constants/ecommerce.ts
Introduces NEXT_PUBLIC_DEFAULT_ECOMMERCE_DOMAIN and DEFAULT_CLIENT_ID_DOMAIN constant with fallback to https://checkout.request.network.
App Shell & Pages
src/app/ecommerce/layout.tsx, src/app/ecommerce/page.tsx, src/app/ecommerce/manage/page.tsx, src/app/ecommerce/sales/page.tsx, src/app/dashboard/receipts/page.tsx, src/components/header.tsx, src/components/dashboard-navigation.tsx
Adds authenticated ecommerce layout; redirects /ecommerce → /ecommerce/manage; server pages for Manage, Sales, and Dashboard Receipts; header link to /ecommerce; dashboard tabs updated to include Receipts.
Ecommerce Navigation
src/components/ecommerce/ecommerce-navigation.tsx
New client-side tab navigation between Manage and Sales, syncing active tab with pathname.
Manage: Blocks & Index
src/components/ecommerce/manage/index.tsx, src/components/ecommerce/manage/blocks/types.ts, .../blocks/client-form.tsx, .../blocks/clients-table.tsx, .../blocks/create-client.tsx, .../blocks/create-default-client.tsx, .../blocks/edit-client.tsx, .../blocks/delete-client.tsx
Implements client ID management UI: form with zod validation, table with filters/actions, create default/client dialogs, edit and delete flows; exports types and schema re-exports.
Sales: Components
src/components/ecommerce/sales/index.tsx, .../sales/blocks/client-payments-table.tsx, src/components/dashboard/receipts.tsx
Adds Sales and Receipts UIs with TRPC-backed data, filtering, pagination, expandable details, and empty/error states.
Schemas & Types
src/lib/schemas/ecommerce.ts, src/lib/types/index.ts
Adds zod schemas for create/edit with cross-field fee validation; introduces ClientPaymentWithEcommerceClient derived type from TRPC router outputs.
Server Router Integration
src/server/index.ts, src/server/routers/ecommerce.ts
Adds ecommerceRouter with procedures: create, edit, getAll, delete, getAllClientPayments, getAllUserReceipts; registers router in appRouter.
DB Schema & Migrations
src/server/db/schema.ts, drizzle/0009_slippery_penance.sql, drizzle/0010_peaceful_lionheart.sql, drizzle/meta/0009_snapshot.json, drizzle/meta/0010_snapshot.json, drizzle/meta/_journal.json
Defines ecommerceClient and clientPayment tables, relations, indexes; adds SQL migrations and snapshots for new tables and constraints.
Webhook Handling
src/app/api/webhook/route.ts
Extends POST handler to record client payments via addClientPayment(), validating ecommerce client, deduplicating by (requestId, txHash), and logging duplicates.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Browser
  participant NextApp as Next.js App
  participant TRPC as tRPC Router (ecommerce)
  participant DB as Database
  participant ExtAPI as External API (Client IDs)

  rect rgba(200,230,255,0.2)
  note over User,Browser: Manage Client IDs (/ecommerce/manage)
  User->>Browser: Navigate
  Browser->>NextApp: GET /ecommerce/manage
  NextApp->>NextApp: getCurrentSession
  alt No user
    NextApp-->>Browser: redirect /
  else Authenticated
    NextApp->>TRPC: ecommerce.getAll (SSR)
    TRPC->>DB: SELECT ecommerce clients by user
    DB-->>TRPC: rows
    TRPC-->>NextApp: clients
    NextApp-->>Browser: Render page (table + actions)
  end
  end

  rect rgba(200,255,200,0.2)
  note over Browser,TRPC: Create/Edit/Delete Client ID
  Browser->>TRPC: create/edit/delete via mutations
  opt create/edit propagates to external
    TRPC->>ExtAPI: POST/PUT v2/client-ids
    ExtAPI-->>TRPC: clientId/externalId
  end
  TRPC->>DB: INSERT/UPDATE/DELETE
  DB-->>TRPC: OK
  TRPC-->>Browser: success/error
  end
Loading
sequenceDiagram
  autonumber
  participant Checkout as Checkout Webhook Sender
  participant API as Next.js API /api/webhook
  participant DB as Database
  participant TRPC as tRPC (queries)
  participant UI as Sales/Receipts UI

  rect rgba(255,240,200,0.3)
  note over Checkout,API: Webhook: Payment event
  Checkout->>API: POST {event: "payment", clientId, requestId, txHash, ...}
  API->>DB: Validate clientId exists (ecommerce_client)
  alt Valid client
    API->>DB: INSERT client_payment (on conflict do nothing)
    DB-->>API: OK or duplicate
    API-->>Checkout: 200
  else Invalid client
    API-->>Checkout: 400/ignored
  end
  end

  rect rgba(235,235,255,0.3)
  note over UI,TRPC: Read models
  UI->>TRPC: getAllClientPayments / getAllUserReceipts
  TRPC->>DB: SELECT with joins
  DB-->>TRPC: rows
  TRPC-->>UI: data
  UI-->>UI: Filter/Paginate/Render
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested reviewers

  • rodrigopavezi
  • MantisClone
  • aimensahnoun

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "feat: Ecommerce client management" accurately summarizes the primary new feature added by this changeset, focusing on client management within the ecommerce domain without extraneous detail.
Linked Issues Check ✅ Passed The pull request fully implements the objectives from issues #151 and #152 by adding the ecommerce management and sales pages with tab navigation, filterable client and payment tables, default client creation, CRUD operations, webhook integration for payments, and a receipts dashboard, matching all specified requirements.
Out of Scope Changes Check ✅ Passed All modifications in the pull request directly support the linked issues around ecommerce client management, sales tracking, and receipt reporting, and no unrelated or extraneous functionality has been introduced.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/151-client-id-management

Comment @coderabbitai help to get the list of available commands and usage tips.

@bassgeta
Copy link
Contributor Author

bassgeta commented Oct 1, 2025

@coderabbitai review

Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Nitpick comments (11)
src/components/ecommerce/manage/blocks/clients-table.tsx (1)

78-99: Consider empty state for actions cell.

When canShowActions is false (default domain clients), the actions cell renders an empty div. This creates visual inconsistency with an empty column. Consider either:

  • Adding a lock icon or "Protected" badge for default clients
  • Hiding the Actions column entirely when all visible rows are default clients
src/components/ecommerce/manage/blocks/types.ts (1)

4-4: Consider whether the schema alias adds value.

The ecommerceClientFormSchema is a direct alias of ecommerceClientApiSchema with no transformation. If the form validation will always match the API schema, you could import the API schema directly in consuming components. However, if you anticipate future divergence or prefer semantic separation, the alias is acceptable.

src/components/ecommerce/manage/blocks/client-form.tsx (2)

119-124: Number coercion may produce unexpected values.

The onChange handler converts empty strings to undefined, but the zod schema uses z.coerce.number() which may accept other non-numeric strings. Consider validating or sanitizing the input before coercion.

The current implementation is functional, but you could make it more robust:

                 <Input
                   type="number"
                   placeholder="5"
                   min="0"
                   max="100"
                   step="0.1"
                   disabled={isLoading}
                   value={field.value || ""}
                   onChange={(e) =>
-                    field.onChange(
-                      e.target.value ? Number(e.target.value) : undefined,
-                    )
+                    field.onChange(
+                      e.target.value === "" ? undefined : Number(e.target.value)
+                    )
                   }
                 />

This makes the empty-string check more explicit. The HTML5 type="number" input already constrains the input, but defensive validation is good practice.


147-149: Loading text logic is fragile.

The loading text selection relies on string matching submitButtonText.includes("Create"), which is brittle if button text changes.

Consider adding an explicit prop or deriving the mode from the presence of defaultValues:

 interface EcommerceClientFormProps {
   onSubmit: (data: EcommerceClientFormValues) => void;
   isLoading?: boolean;
   defaultValues?: Partial<EcommerceClientFormValues>;
   submitButtonText?: string;
+  mode?: "create" | "edit";
   onCancel?: () => void;
 }

 export function EcommerceClientForm({
   onSubmit,
   isLoading = false,
   defaultValues,
   submitButtonText = "Create Client ID",
+  mode = "create",
   onCancel,
 }: EcommerceClientFormProps) {
   // ... form setup ...

   return (
     // ... form fields ...
           <Button type="submit" disabled={isLoading}>
             {isLoading ? (
               <>
                 <Loader2 className="mr-2 h-4 w-4 animate-spin" />
-                {submitButtonText.includes("Create")
-                  ? "Creating..."
-                  : "Updating..."}
+                {mode === "create" ? "Creating..." : "Updating..."}
               </>
             ) : (
               submitButtonText
             )}
           </Button>
src/components/ecommerce/manage/blocks/create-default-client.tsx (1)

24-28: Hardcoded label may not suit all use cases.

The label "Default Ecommerce" is hardcoded, which may not be descriptive for users who have multiple environments or want to customize their default client name.

Consider allowing customization or using a more descriptive label:

 const handleCreateDefault = () => {
   createEcommerceClientMutation({
-    label: "Default Ecommerce",
+    label: `Default Client (${new URL(DEFAULT_CLIENT_ID_DOMAIN).hostname})`,
     domain: DEFAULT_CLIENT_ID_DOMAIN,
   });
 };

This would create labels like "Default Client (checkout.example.com)" which is more informative.

src/components/ecommerce/manage/index.tsx (3)

24-26: Business logic for default client detection may be incomplete.

The condition checks if there's no client with DEFAULT_CLIENT_ID_DOMAIN, but it doesn't verify if the existing default client is valid or active. If a default client is soft-deleted or becomes invalid, this logic won't detect it.

Consider adding an explicit isDefault or isActive field to the ecommerce client schema, or verify that the domain-based check is sufficient for your use case:

#!/bin/bash
# Check if there's a soft-delete or active status field in the schema
ast-grep --pattern 'export const ecommerceClients = $$$({
  $$$
})'

If soft-deletes or status tracking is needed, update the schema and this check accordingly.


40-44: Conditional rendering may confuse users.

Switching between CreateDefaultEcommerceClient and CreateEcommerceClient based on data could be confusing. After creating a default client, the button changes, which might not be immediately clear to users.

Consider showing both buttons with different visual hierarchy, or adding explanatory text:

 return (
   <div className="flex flex-col items-start gap-3">
+    {shouldCreateDefault && (
+      <p className="text-sm text-muted-foreground">
+        Create a default client to get started with the checkout domain.
+      </p>
+    )}
     {shouldCreateDefault ? (
       <CreateDefaultEcommerceClient />
     ) : (
       <CreateEcommerceClient />
     )}
     <EcommerceClientsTable ecommerceClients={data} />
   </div>
 );

17-23: Adjust refetchOnMount when using initialData. This is the only instance of initialData paired with refetchOnMount in the codebase—remove refetchOnMount: true to avoid an unnecessary immediate refetch when SSR data is fresh, or swap it for refetchOnWindowFocus to keep data updated in the background.

src/components/ecommerce/manage/blocks/create-client.tsx (1)

50-55: Form might retain stale data between dialog opens.

When the dialog is closed and reopened, the form retains previous values because EcommerceClientForm doesn't reset on unmount. This could confuse users who expect a clean form.

Add a key prop to force form remount when dialog opens:

       <DialogContent>
         <DialogHeader>
           <DialogTitle>Create New Ecommerce Client</DialogTitle>
         </DialogHeader>
         <EcommerceClientForm
+          key={isOpen ? "open" : "closed"}
           onSubmit={handleSubmit}
           isLoading={isLoading}
           submitButtonText="Create Client"
           onCancel={() => setIsOpen(false)}
         />
       </DialogContent>

Alternatively, call form.reset() in the dialog's onOpenChange handler when opening.

src/lib/schemas/ecommerce.ts (2)

39-43: Consider allowing decimal precision control for fee percentage.

The current schema allows any decimal precision for fee percentages (e.g., 5.123456789), which may not align with your business requirements or backend precision limits.

Add precision control if needed:

   feePercentage: z.coerce
     .number()
     .min(0, "Fee percentage must be at least 0")
     .max(100, "Fee percentage cannot exceed 100")
+    .transform((val) => val !== undefined ? Math.round(val * 100) / 100 : undefined)
     .optional(),

This would round to 2 decimal places (e.g., 5.126 → 5.13). Adjust the multiplier for different precision requirements.


31-38: Use checksum-aware validator for Ethereum addresses
validator.js’s isEthereumAddress only checks basic format/regex and does not enforce the EIP-55 mixed-case checksum. If strict checksum validation is required, replace it with a checksum-aware utility such as isAddress from viem or ethers.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2b31c13 and fb78fc8.

📒 Files selected for processing (19)
  • .env.example (1 hunks)
  • src/app/ecommerce/layout.tsx (1 hunks)
  • src/app/ecommerce/manage/page.tsx (1 hunks)
  • src/app/ecommerce/page.tsx (1 hunks)
  • src/app/ecommerce/sales/page.tsx (1 hunks)
  • src/components/ecommerce/ecommerce-navigation.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/client-form.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/clients-table.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/create-client.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/create-default-client.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/delete-client.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/edit-client.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/types.ts (1 hunks)
  • src/components/ecommerce/manage/index.tsx (1 hunks)
  • src/lib/constants/ecommerce.ts (1 hunks)
  • src/lib/schemas/ecommerce.ts (1 hunks)
  • src/server/db/schema.ts (3 hunks)
  • src/server/index.ts (2 hunks)
  • src/server/routers/ecommerce.ts (1 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.example

[warning] 21-21: [EndingBlankLine] No blank line at the end of the file

(EndingBlankLine)

🔇 Additional comments (20)
src/app/ecommerce/page.tsx (1)

3-5: LGTM! Clean redirect pattern.

The immediate redirect to /ecommerce/manage is appropriate for an index route. The authentication check is correctly handled by the layout (in src/app/ecommerce/layout.tsx as noted in the AI summary), so no duplicate check is needed here.

src/app/ecommerce/sales/page.tsx (1)

5-15: Placeholder acknowledged.

The authentication pattern is consistent with the broader ecommerce area. The TODO comment and placeholder render clearly indicate this is a stub for future implementation.

src/components/ecommerce/ecommerce-navigation.tsx (1)

8-18: LGTM! Navigation state management is correct.

The pathname-based tab synchronization using useEffect properly handles the active state for the Manage and Sales tabs.

src/components/ecommerce/manage/blocks/delete-client.tsx (1)

31-47: LGTM! Proper mutation and error handling.

The delete mutation correctly invalidates the cache, provides user feedback via toasts, and manages dialog state. The disabled states during deletion prevent duplicate submissions.

src/components/ecommerce/manage/blocks/clients-table.tsx (3)

104-140: LGTM! Clean filter implementation.

The filter logic correctly handles "All Clients" via null state and filters by client ID. The Select component properly manages the active filter state.


35-45: LGTM! Clear table structure.

The column headers appropriately label all displayed fields including the conditional Actions column.


47-102: Use timezone‐aware timestamps for createdAt

In src/server/db/schema.ts line 315, createdAt: timestamp("created_at").defaultNow() defines a timestamp without time zone, which discards offset information. Switch to timestamp({ withTimezone: true }).defaultNow() (i.e. timestamptz) or lock your Postgres timezone to UTC to ensure dates render correctly for users in all time zones. (orm.drizzle.team)

⛔ Skipped due to learnings
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#91
File: drizzle/0007_awesome_gertrude_yorkes.sql:1-12
Timestamp: 2025-07-14T14:27:58.530Z
Learning: In the RequestNetwork/easy-invoice codebase, the user bassgeta prefers to maintain consistent timestamp formats across all database tables. They use `timestamp DEFAULT now()` rather than `timestamptz NOT NULL DEFAULT now()` and prefer not to change this in isolated places unless it's done consistently throughout the entire codebase.
src/server/index.ts (1)

4-4: LGTM!

The ecommerceRouter import and registration follow the established pattern for other routers in the application.

Also applies to: 21-21

src/app/ecommerce/layout.tsx (2)

13-14: LGTM! Auth guard is correctly implemented.

The authentication check and redirect pattern is appropriate for a server component layout.


16-32: LGTM! Layout structure is well-organized.

The component properly composes the UI sections with semantic HTML, responsive design classes, and clear hierarchy.

src/components/ecommerce/manage/blocks/edit-client.tsx (3)

26-39: LGTM! Mutation setup follows best practices.

The mutation correctly handles success with cache invalidation, toast notification, and dialog closure. Error handling includes a fallback message.


41-46: LGTM! Submit handler correctly augments form data.

The handler properly spreads form values and adds the client ID before calling the mutation.


48-71: LGTM! Dialog structure is well-organized.

The component properly wires the dialog state, form props, and handlers. The render-prop pattern for the trigger is correctly implemented.

src/components/ecommerce/manage/blocks/create-default-client.tsx (1)

12-21: LGTM! Mutation and cache invalidation handled correctly.

The mutation correctly invalidates the cache on success and provides user feedback via toasts. Error handling includes fallback messaging.

src/components/ecommerce/manage/index.tsx (1)

28-36: LGTM! Error state handled properly.

The error handling provides a clear retry mechanism with loading state feedback.

src/components/ecommerce/manage/blocks/create-client.tsx (1)

22-32: LGTM! Mutation handling is correct.

Cache invalidation and dialog state management are properly handled. Error messages provide good user feedback.

src/lib/schemas/ecommerce.ts (1)

4-26: LGTM! Cross-field validation logic is correct.

The fee validation properly enforces that both feeAddress and feePercentage must be provided together or not at all. Error paths are correctly assigned to the respective fields.

src/server/routers/ecommerce.ts (3)

1-13: LGTM!

Imports are appropriate and well-organized for the router's functionality.


106-116: LGTM!

The query correctly filters by user ID and properly handles errors.


125-125: LGTM: Default domain protection.

Correctly prevents deletion of the default client by filtering it out in the query condition.

@bassgeta
Copy link
Contributor Author

bassgeta commented Oct 1, 2025

@CodeRabbit review

Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (1)
src/components/ecommerce/manage/blocks/delete-client.tsx (1)

60-63: Fix grammatical error in warning message.

Line 62 contains "it's ID" which should be "its ID" (possessive, not contraction).

Apply this diff:

-              removed and any integrations using it's ID will stop working.
+              removed and any integrations using its ID will stop working.
🧹 Nitpick comments (4)
drizzle/0009_slippery_penance.sql (1)

8-9: Consider numeric type for feePercentage.

Storing feePercentage as text requires string-to-number conversion in application code. Using a numeric type (e.g., numeric(5,2)) would provide stronger type safety and simpler queries.

src/lib/schemas/ecommerce.ts (1)

39-48: Inconsistent approach: manual parsing vs zod coercion.

The schema uses manual Number(value) parsing instead of zod's .coerce.number(). This is inconsistent and error-prone.

Replace manual parsing with zod coercion for consistency and better error messages:

   feePercentage: z
     .string()
     .transform((val) => (val === "" ? undefined : val))
-    .refine((value) => {
-      if (value === undefined) return true;
-      const num = Number(value);
-
-      return !Number.isNaN(num) && num >= 0 && num <= 100;
-    }, "Fee percentage must be a number between 0 and 100")
-    .optional(),
+    .pipe(
+      z.coerce.number()
+        .min(0, "Fee percentage must be at least 0")
+        .max(100, "Fee percentage cannot exceed 100")
+    )
+    .optional()
+    .or(z.literal(undefined)),
src/server/routers/ecommerce.ts (1)

145-147: Consider logging external API delete failures.

If the external API delete fails after the DB record is removed, the orphaned external client won't be visible for cleanup. Consider logging the failure for manual reconciliation.

         await db
           .delete(ecommerceClientTable)
           .where(eq(ecommerceClientTable.id, existingEcommerceClient.id));

-        await apiClient.delete(
-          `v2/client-ids/${existingEcommerceClient.externalId}`,
-        );
+        try {
+          await apiClient.delete(
+            `v2/client-ids/${existingEcommerceClient.externalId}`,
+          );
+        } catch (apiError) {
+          console.error(
+            `Failed to delete external client ${existingEcommerceClient.externalId}:`,
+            apiError
+          );
+          // Local record already deleted - log for manual cleanup
+        }
       } catch (error) {
         throw toTRPCError(error);
       }
src/components/ecommerce/manage/blocks/delete-client.tsx (1)

45-47: Remove unnecessary async keyword.

The handler doesn't await anything, and the mutation call is synchronous. The async keyword can be removed.

Apply this diff:

-  const handleDeleteEcommerceClient = async () => {
+  const handleDeleteEcommerceClient = () => {
     deleteEcommerceClientMutation(ecommerceClient.id);
   };
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb78fc8 and f1a0c93.

📒 Files selected for processing (12)
  • drizzle/0009_slippery_penance.sql (1 hunks)
  • drizzle/meta/0009_snapshot.json (1 hunks)
  • drizzle/meta/_journal.json (1 hunks)
  • src/components/ecommerce/ecommerce-navigation.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/client-form.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/create-client.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/delete-client.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/edit-client.tsx (1 hunks)
  • src/components/ecommerce/manage/index.tsx (1 hunks)
  • src/lib/schemas/ecommerce.ts (1 hunks)
  • src/server/db/schema.ts (4 hunks)
  • src/server/routers/ecommerce.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/server/db/schema.ts
  • src/components/ecommerce/ecommerce-navigation.tsx
  • src/components/ecommerce/manage/blocks/edit-client.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-01T11:59:14.685Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#154
File: src/server/routers/ecommerce.ts:79-101
Timestamp: 2025-10-01T11:59:14.685Z
Learning: In the RequestNetwork/easy-invoice repository, transaction safety between external API calls and local database updates is intentionally not implemented as it's a demo application. The team prefers to keep patterns consistent across all endpoints rather than implementing it piecemeal. If transaction safety is needed, it should be addressed globally across all applicable endpoints.

Applied to files:

  • src/server/routers/ecommerce.ts
📚 Learning: 2025-06-23T09:14:42.979Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#83
File: src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx:127-138
Timestamp: 2025-06-23T09:14:42.979Z
Learning: In the RequestNetwork/easy-invoice codebase, when suggesting improvements like error handling for transaction approvals, the user bassgeta prefers consistency over isolated improvements. They prefer not to implement error handling in one place unless it's done consistently across all similar transaction flows in the codebase.

Applied to files:

  • src/server/routers/ecommerce.ts
📚 Learning: 2025-02-13T21:59:01.427Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#8
File: drizzle/meta/0001_snapshot.json:125-130
Timestamp: 2025-02-13T21:59:01.427Z
Learning: Drizzle migration files (drizzle/meta/*.json) are auto-generated and should not be modified directly. Schema changes should be made in the source files.

Applied to files:

  • drizzle/meta/0009_snapshot.json
🔇 Additional comments (22)
drizzle/0009_slippery_penance.sql (1)

19-19: LGTM: Unique constraint prevents race conditions.

The unique index on (userId, domain) enforces one-client-per-domain at the database level, preventing race conditions in concurrent creates/updates.

src/lib/schemas/ecommerce.ts (2)

4-26: LGTM: Cross-field validation correctly enforced.

The validation ensures feeAddress and feePercentage are provided together, with clear error messages on the appropriate field paths.


51-58: LGTM: Schema composition is clean.

The base schema is appropriately extended for create and edit operations, with consistent validation applied via superRefine.

drizzle/meta/0009_snapshot.json (1)

1-1068: Auto-generated snapshot - no review required.

This file is generated by drizzle-kit and should not be manually modified. Schema changes should be made in source files.

Based on learnings.

src/server/routers/ecommerce.ts (5)

20-30: Domain uniqueness is enforced by DB constraint.

The application-level check (lines 20-26) provides a user-friendly error message, and the unique index on (userId, domain) in the database prevents race conditions at the data layer.


75-88: LGTM: Domain conflict check prevents duplicate domains.

The validation correctly checks for domain conflicts when updating, excluding the current client from the check using not(eq(...)).


113-123: LGTM: Simple query with proper user scoping.

The getAll procedure correctly filters clients by userId with appropriate error handling.


141-147: Delete ordering improved: DB first, then external API.

The delete now removes the local record before calling the external API, which is safer. If the external delete fails, the local state is already clean.


132-132: LGTM: Default client protection.

The delete procedure correctly prevents deletion of the default client using not(eq(ecommerceClientTable.domain, DEFAULT_CLIENT_ID_DOMAIN)).

src/components/ecommerce/manage/blocks/delete-client.tsx (3)

1-27: LGTM!

Imports and interface definition are clean and well-structured.


28-43: LGTM!

State management and mutation setup follow best practices with proper success/error handling and cache invalidation.


49-80: LGTM!

Dialog structure and user feedback (loading states, disabled buttons, warning message) are well-implemented. The red styling for the destructive action is appropriate.

src/components/ecommerce/manage/index.tsx (3)

1-23: LGTM!

Query setup with initialData and refetchOnMount is a best practice for SSR hydration and ensuring fresh data on mount.


24-36: LGTM!

The default client logic correctly ensures a client with DEFAULT_CLIENT_ID_DOMAIN exists, and error handling with retry is well-implemented.


38-53: LGTM!

Conditional rendering logic is clear and aligns with the PR objective to provide a default client setup flow before allowing general client creation.

src/components/ecommerce/manage/blocks/create-client.tsx (3)

1-32: LGTM!

Mutation setup and state management follow the established patterns with proper cache invalidation and user feedback.


34-36: LGTM!

Submit handler is straightforward and correct.


38-60: LGTM!

Dialog structure is correct. The conditional rendering of the form (line 50) ensures the form remounts on each open, providing fresh state—a solid pattern for this use case.

src/components/ecommerce/manage/blocks/client-form.tsx (4)

1-27: LGTM!

Imports and interface definition are clean and well-typed.


29-45: LGTM!

Form initialization with merged default values is correct. The parent components remount this form on each dialog open (via conditional rendering), ensuring fresh state without needing a reset effect here.


47-125: LGTM!

Form fields are well-structured with appropriate input types, placeholders, constraints, and validation messaging. The font-mono for the fee address field is a nice touch for readability.


127-151: LGTM!

Button layout and loading states are well-implemented with clear user feedback. The type="button" on Cancel prevents accidental form submission.

@bassgeta bassgeta changed the base branch from feat/merchant-page to main October 3, 2025 11:47
@bassgeta bassgeta force-pushed the feat/151-client-id-management branch from e6d0631 to ff3c2df Compare October 3, 2025 11:52
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
src/app/ecommerce/page.tsx (1)

3-5: LGTM! Optional: Consider using Next.js redirect configuration.

The redirect pattern is valid for a Server Component in Next.js 15.x. As an optional improvement, you could configure this redirect in next.config.js to avoid the extra page file:

// In next.config.js
module.exports = {
  async redirects() {
    return [
      {
        source: '/ecommerce',
        destination: '/ecommerce/manage',
        permanent: true, // or false for temporary redirect
      },
    ]
  },
}

This reduces overhead by handling the redirect at the routing level rather than rendering a component.

src/app/ecommerce/sales/page.tsx (1)

12-14: TODO: Implement sales data fetching.

The sales page is currently a placeholder. When you're ready to implement the data fetching and UI, I can help generate the TRPC query integration and component structure similar to the manage page.

Do you want me to generate a proposal for the sales page implementation or open a tracking issue?

src/components/ecommerce/ecommerce-navigation.tsx (1)

8-32: Optional: Derive activeTab from pathname instead of storing state.

The current implementation uses useState and useEffect to sync activeTab with the pathname. As an optional simplification, you can derive the active tab directly from the pathname without maintaining separate state:

 export function EcommerceNavigation() {
   const pathname = usePathname();
-  const [activeTab, setActiveTab] = useState("manage");
-
-  useEffect(() => {
-    if (pathname.includes("/sales")) {
-      setActiveTab("sales");
-    } else {
-      setActiveTab("manage");
-    }
-  }, [pathname]);
+  const activeTab = pathname.includes("/sales") ? "sales" : "manage";

   return (
     <Tabs value={activeTab} className="w-full mb-8">

This eliminates the useEffect and reduces complexity while maintaining the same behavior.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1a0c93 and ff3c2df.

📒 Files selected for processing (23)
  • .env.example (1 hunks)
  • drizzle/0009_slippery_penance.sql (1 hunks)
  • drizzle/meta/0009_snapshot.json (1 hunks)
  • drizzle/meta/_journal.json (1 hunks)
  • src/app/ecommerce/layout.tsx (1 hunks)
  • src/app/ecommerce/manage/page.tsx (1 hunks)
  • src/app/ecommerce/page.tsx (1 hunks)
  • src/app/ecommerce/sales/page.tsx (1 hunks)
  • src/components/ecommerce/ecommerce-navigation.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/client-form.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/clients-table.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/create-client.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/create-default-client.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/delete-client.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/edit-client.tsx (1 hunks)
  • src/components/ecommerce/manage/blocks/types.ts (1 hunks)
  • src/components/ecommerce/manage/index.tsx (1 hunks)
  • src/components/header.tsx (1 hunks)
  • src/lib/constants/ecommerce.ts (1 hunks)
  • src/lib/schemas/ecommerce.ts (1 hunks)
  • src/server/db/schema.ts (4 hunks)
  • src/server/index.ts (2 hunks)
  • src/server/routers/ecommerce.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
  • drizzle/meta/_journal.json
  • src/app/ecommerce/layout.tsx
  • src/components/ecommerce/manage/index.tsx
  • src/lib/constants/ecommerce.ts
  • src/components/ecommerce/manage/blocks/create-client.tsx
  • src/components/ecommerce/manage/blocks/client-form.tsx
  • .env.example
  • src/components/ecommerce/manage/blocks/create-default-client.tsx
  • src/components/ecommerce/manage/blocks/delete-client.tsx
  • src/components/ecommerce/manage/blocks/clients-table.tsx
  • drizzle/0009_slippery_penance.sql
  • src/lib/schemas/ecommerce.ts
  • src/server/index.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-02-13T21:59:01.427Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#8
File: drizzle/meta/0001_snapshot.json:125-130
Timestamp: 2025-02-13T21:59:01.427Z
Learning: Drizzle migration files (drizzle/meta/*.json) are auto-generated and should not be modified directly. Schema changes should be made in the source files.

Applied to files:

  • drizzle/meta/0009_snapshot.json
📚 Learning: 2025-10-01T11:59:14.710Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#154
File: src/server/routers/ecommerce.ts:79-101
Timestamp: 2025-10-01T11:59:14.710Z
Learning: In the RequestNetwork/easy-invoice repository, transaction safety between external API calls and local database updates is intentionally not implemented as it's a demo application. The team prefers to keep patterns consistent across all endpoints rather than implementing it piecemeal. If transaction safety is needed, it should be addressed globally across all applicable endpoints.

Applied to files:

  • src/server/routers/ecommerce.ts
📚 Learning: 2025-06-23T09:14:42.979Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#83
File: src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx:127-138
Timestamp: 2025-06-23T09:14:42.979Z
Learning: In the RequestNetwork/easy-invoice codebase, when suggesting improvements like error handling for transaction approvals, the user bassgeta prefers consistency over isolated improvements. They prefer not to implement error handling in one place unless it's done consistently across all similar transaction flows in the codebase.

Applied to files:

  • src/server/routers/ecommerce.ts
📚 Learning: 2025-10-01T11:39:24.398Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#154
File: src/app/ecommerce/manage/page.tsx:7-15
Timestamp: 2025-10-01T11:39:24.398Z
Learning: In the Easy Invoice codebase, `getCurrentSession()` from `server/auth/index.ts` returns a discriminated union type `SessionValidationResult` that always includes both `session` and `user` properties. When unauthenticated, it returns `{ session: null, user: null }`. Destructuring `const { user } = await getCurrentSession()` is safe and is the standard pattern used across multiple pages. Do not flag this pattern as unsafe.

Applied to files:

  • src/app/ecommerce/sales/page.tsx
  • src/app/ecommerce/manage/page.tsx
🧬 Code graph analysis (5)
src/components/ecommerce/manage/blocks/edit-client.tsx (3)
src/server/db/schema.ts (1)
  • EcommerceClient (432-432)
src/components/ecommerce/manage/blocks/types.ts (1)
  • EcommerceClientFormValues (6-8)
src/components/ecommerce/manage/blocks/client-form.tsx (1)
  • EcommerceClientForm (29-152)
src/components/ecommerce/manage/blocks/types.ts (1)
src/lib/schemas/ecommerce.ts (1)
  • ecommerceClientApiSchema (51-52)
src/server/routers/ecommerce.ts (7)
src/server/trpc.ts (2)
  • router (11-11)
  • protectedProcedure (39-39)
src/lib/schemas/ecommerce.ts (2)
  • ecommerceClientApiSchema (51-52)
  • editecommerceClientApiSchema (54-58)
src/server/db/index.ts (1)
  • db (10-12)
src/server/db/schema.ts (1)
  • ecommerceClientTable (303-326)
src/lib/axios.ts (1)
  • apiClient (3-8)
src/lib/errors.ts (1)
  • toTRPCError (37-78)
src/lib/constants/ecommerce.ts (1)
  • DEFAULT_CLIENT_ID_DOMAIN (1-3)
src/app/ecommerce/sales/page.tsx (1)
src/server/auth/index.ts (1)
  • getCurrentSession (96-103)
src/app/ecommerce/manage/page.tsx (2)
src/server/auth/index.ts (1)
  • getCurrentSession (96-103)
src/components/ecommerce/manage/index.tsx (1)
  • EcommerceManage (14-54)
⏰ 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 (14)
src/server/db/schema.ts (4)

13-13: LGTM!

The uniqueIndex import is correctly added to support the unique constraint on the ecommerce_client table.


387-395: LGTM!

The relation correctly maps ecommerceClientTable to userTable via the userId foreign key.


432-432: LGTM!

The EcommerceClient type export correctly infers the select model from the table definition and follows the established pattern.


303-326: Enforce numeric type for feePercentage and verify need for indexes on externalId/rnClientId

  • Replace
    feePercentage: text(),
    with
    feePercentage: numeric("fee_percentage", { precision: 5, scale: 2 }),
    to enforce type safety at the DB level.
  • Only add indexes on externalId and rnClientId if you actually filter by those columns in queries—no such usage found in the current codebase; please confirm before indexing.
src/components/ecommerce/manage/blocks/types.ts (1)

1-8: LGTM!

The form schema alias and type inference follow standard Zod patterns and provide proper type safety for the ecommerce client forms.

src/components/ecommerce/manage/blocks/edit-client.tsx (2)

29-46: LGTM!

The mutation setup correctly handles success and error cases, invalidates the query cache to refresh the UI, and properly augments the form data with the client ID.


48-72: LGTM!

The dialog structure is well-implemented. The conditional rendering ({isOpen && ...}) ensures the form has fresh state when opened, and the key prop correctly forces a remount when editing different clients.

drizzle/meta/0009_snapshot.json (1)

1-1068: Auto-generated migration snapshot—no manual changes needed.

This file is generated by Drizzle and should not be edited directly. Schema changes should be made in source files and regenerated via Drizzle Kit. Based on learnings.

src/server/routers/ecommerce.ts (6)

20-30: Race condition mitigated by DB-level unique constraint.

The existence check is not atomic, but the unique index on (userId, domain) in the ecommerceClientTable schema will prevent duplicate inserts at the database level. Concurrent requests will fail with a constraint violation error, which is then wrapped by toTRPCError.


32-52: External API and DB insert lack transaction safety (by design).

If the external API call succeeds but the DB insert fails, an orphaned client will exist in the external system. This is a known limitation acknowledged by the team. Based on learnings, transaction safety is intentionally not implemented as this is a demo application, and the team prefers consistency across all endpoints rather than piecemeal improvements.


75-88: Domain conflict check added (addresses previous review).

The code now validates that the new domain is not already in use by another client for the same user before updating. This addresses the concern raised in previous reviews about missing domain uniqueness validation during edits.


90-108: External API and DB update lack transaction safety (by design).

If the external API update succeeds but the DB update fails, the external state and local state will diverge. This is a known limitation. Based on learnings, transaction safety is intentionally not implemented as this is a demo application, and the team prefers to address such patterns globally rather than in isolated endpoints.


113-123: LGTM!

The getAll procedure correctly queries all ecommerce clients for the authenticated user with appropriate error handling.


141-147: Delete order minimizes stale local records (transaction safety by design).

The code deletes from the local DB first, then from the external API. If the API delete fails, the local record is already removed, which is safer than leaving a stale local record. If the external client needs cleanup, it can be handled manually or via a background job. Based on learnings, full transaction safety is intentionally not implemented as this is a demo application.

@bassgeta
Copy link
Contributor Author

bassgeta commented Oct 3, 2025

@CodeRabbit review

Copy link
Contributor

coderabbitai bot commented Oct 3, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (6)
src/components/ecommerce/sales/index.tsx (1)

13-18: Consider increasing the refetch interval.

A 10-second polling interval may generate unnecessary server load for payment data that typically doesn't change frequently. Consider:

  • Increasing to 30-60 seconds
  • Using on-demand refresh via user action
  • Implementing WebSocket updates for real-time changes
     api.ecommerce.getAllClientPayments.useQuery(undefined, {
       initialData: initialClientPayments,
       refetchOnMount: true,
-      refetchInterval: 10000,
+      refetchInterval: 30000,
     });
src/server/db/schema.ts (1)

331-364: Consider adding validation for customerInfo JSON structure.

The customerInfo field uses a typed JSON structure but lacks runtime validation. Consider adding a Zod schema or similar validation to ensure data integrity, especially since this data comes from external sources (webhooks).

Example validation pattern:

import { z } from 'zod';

export const customerInfoSchema = z.object({
  firstName: z.string().optional(),
  lastName: z.string().optional(),
  email: z.string().email().optional(),
  address: z.object({
    street: z.string().optional(),
    city: z.string().optional(),
    state: z.string().optional(),
    postalCode: z.string().optional(),
    country: z.string().optional(),
  }).optional(),
});

Then use this schema when inserting/updating clientPaymentTable data.

src/components/ecommerce/sales/blocks/client-payments-table.tsx (1)

185-192: Consider memoizing the ecommerceClients derivation.

The reduce operation runs on every render. For large payment lists, consider using useMemo to cache the result.

+  const ecommerceClients = useMemo(
+    () =>
-  const ecommerceClients = clientPayments.reduce(
-    (acc, payment) => {
-      if (acc[payment.ecommerceClient.id]) return acc;
-      acc[payment.ecommerceClient.id] = payment.ecommerceClient;
-      return acc;
-    },
-    {} as Record<string, ClientPaymentWithEcommerceClient["ecommerceClient"]>,
-  );
+      clientPayments.reduce(
+        (acc, payment) => {
+          if (acc[payment.ecommerceClient.id]) return acc;
+          acc[payment.ecommerceClient.id] = payment.ecommerceClient;
+          return acc;
+        },
+        {} as Record<string, ClientPaymentWithEcommerceClient["ecommerceClient"]>,
+      ),
+    [clientPayments],
+  );
drizzle/0010_special_warbird.sql (1)

29-29: Index name could be clearer.

The unique index correctly enforces uniqueness on rnClientId, but the index name ecommerce_client_user_id_client_id_unique mentions user_id even though only rnClientId is indexed. Consider renaming to something like ecommerce_client_rn_client_id_unique for clarity.

-CREATE UNIQUE INDEX IF NOT EXISTS "ecommerce_client_user_id_client_id_unique" ON "easyinvoice_ecommerce_client" USING btree ("rnClientId");
+CREATE UNIQUE INDEX IF NOT EXISTS "ecommerce_client_rn_client_id_unique" ON "easyinvoice_ecommerce_client" USING btree ("rnClientId");
src/components/dashboard/receipts.tsx (2)

43-62: Date parsing could fail on invalid timestamps.

Line 50 uses new Date(receipt.createdAt) which will produce an invalid date if createdAt is malformed. While the fallback "N/A" handles null/undefined, it won't catch invalid date strings that produce Invalid Date.

Consider adding validation before parsing:

       <TableCell>
-        {receipt.createdAt
-          ? format(new Date(receipt.createdAt), "do MMM yyyy")
-          : "N/A"}
+        {receipt.createdAt &&
+        !isNaN(new Date(receipt.createdAt).getTime())
+          ? format(new Date(receipt.createdAt), "do MMM yyyy")
+          : "N/A"}
       </TableCell>

Alternatively, wrap the format call in a try-catch:

       <TableCell>
-        {receipt.createdAt
-          ? format(new Date(receipt.createdAt), "do MMM yyyy")
-          : "N/A"}
+        {receipt.createdAt ? (() => {
+          try {
+            return format(new Date(receipt.createdAt), "do MMM yyyy");
+          } catch {
+            return "N/A";
+          }
+        })() : "N/A"}
       </TableCell>

66-104: Consider making refetch interval configurable.

The 10-second refetch interval (line 76) keeps data fresh but may increase server load if many users have this page open. Consider:

  1. Making it configurable based on user preference
  2. Increasing the interval to 30s or 60s for a demo app
  3. Using refetchOnWindowFocus instead of automatic polling

The current implementation with filtering and pagination logic is correct and handles edge cases appropriately.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff3c2df and 2b60636.

📒 Files selected for processing (14)
  • drizzle/0010_special_warbird.sql (1 hunks)
  • drizzle/meta/0010_snapshot.json (1 hunks)
  • drizzle/meta/_journal.json (1 hunks)
  • src/app/api/webhook/route.ts (3 hunks)
  • src/app/dashboard/receipts/page.tsx (1 hunks)
  • src/app/ecommerce/sales/page.tsx (1 hunks)
  • src/components/dashboard-navigation.tsx (2 hunks)
  • src/components/dashboard/receipts.tsx (1 hunks)
  • src/components/ecommerce/sales/blocks/client-payments-table.tsx (1 hunks)
  • src/components/ecommerce/sales/index.tsx (1 hunks)
  • src/components/header.tsx (1 hunks)
  • src/lib/types/index.ts (2 hunks)
  • src/server/db/schema.ts (4 hunks)
  • src/server/routers/ecommerce.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • drizzle/meta/_journal.json
  • src/components/header.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-01T11:59:14.710Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#154
File: src/server/routers/ecommerce.ts:79-101
Timestamp: 2025-10-01T11:59:14.710Z
Learning: In the RequestNetwork/easy-invoice repository, transaction safety between external API calls and local database updates is intentionally not implemented as it's a demo application. The team prefers to keep patterns consistent across all endpoints rather than implementing it piecemeal. If transaction safety is needed, it should be addressed globally across all applicable endpoints.

Applied to files:

  • src/server/routers/ecommerce.ts
📚 Learning: 2025-06-23T09:14:42.979Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#83
File: src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx:127-138
Timestamp: 2025-06-23T09:14:42.979Z
Learning: In the RequestNetwork/easy-invoice codebase, when suggesting improvements like error handling for transaction approvals, the user bassgeta prefers consistency over isolated improvements. They prefer not to implement error handling in one place unless it's done consistently across all similar transaction flows in the codebase.

Applied to files:

  • src/server/routers/ecommerce.ts
🧬 Code graph analysis (8)
src/components/ecommerce/sales/index.tsx (3)
src/lib/types/index.ts (1)
  • ClientPaymentWithEcommerceClient (39-41)
src/components/ui/table/error-state.tsx (1)
  • ErrorState (12-44)
src/components/ecommerce/sales/blocks/client-payments-table.tsx (1)
  • ClientPaymentsTable (162-267)
src/app/dashboard/receipts/page.tsx (2)
src/server/auth/index.ts (1)
  • getCurrentSession (96-103)
src/components/dashboard/receipts.tsx (1)
  • DashboardReceipts (66-187)
src/components/ecommerce/sales/blocks/client-payments-table.tsx (5)
src/lib/types/index.ts (1)
  • ClientPaymentWithEcommerceClient (39-41)
src/components/ui/table/table-head-cell.tsx (1)
  • TableHeadCell (9-13)
src/components/short-address.tsx (1)
  • ShortAddress (12-37)
src/components/ui/table/empty-state.tsx (1)
  • EmptyState (10-24)
src/components/ui/table/pagination.tsx (1)
  • Pagination (10-45)
src/app/api/webhook/route.ts (3)
src/server/db/index.ts (1)
  • db (10-12)
src/server/db/schema.ts (2)
  • ecommerceClientTable (303-329)
  • clientPaymentTable (331-364)
src/lib/errors.ts (1)
  • ResourceNotFoundError (5-10)
src/lib/types/index.ts (1)
src/server/routers/ecommerce.ts (1)
  • ecommerceRouter (14-182)
src/components/dashboard/receipts.tsx (5)
src/lib/types/index.ts (1)
  • ClientPaymentWithEcommerceClient (39-41)
src/components/ui/table/table-head-cell.tsx (1)
  • TableHeadCell (9-13)
src/components/ui/table/error-state.tsx (1)
  • ErrorState (12-44)
src/components/ui/table/empty-state.tsx (1)
  • EmptyState (10-24)
src/components/ui/table/pagination.tsx (1)
  • Pagination (10-45)
src/app/ecommerce/sales/page.tsx (2)
src/server/auth/index.ts (1)
  • getCurrentSession (96-103)
src/components/ecommerce/sales/index.tsx (1)
  • EcommerceSales (12-41)
src/server/routers/ecommerce.ts (6)
src/server/trpc.ts (2)
  • router (11-11)
  • protectedProcedure (39-39)
src/lib/schemas/ecommerce.ts (2)
  • ecommerceClientApiSchema (51-52)
  • editecommerceClientApiSchema (54-58)
src/server/db/schema.ts (2)
  • ecommerceClientTable (303-329)
  • clientPaymentTable (331-364)
src/lib/axios.ts (1)
  • apiClient (3-8)
src/lib/errors.ts (1)
  • toTRPCError (37-78)
src/lib/constants/ecommerce.ts (1)
  • DEFAULT_CLIENT_ID_DOMAIN (1-3)
⏰ 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 (26)
src/app/api/webhook/route.ts (3)

19-63: LGTM! Transaction and error handling are well structured.

The function correctly uses a database transaction to ensure atomicity, properly handles the missing client case with ResourceNotFoundError, and implements idempotent duplicate detection with onConflictDoNothing. The early return for duplicates (with a warning log) is appropriate for webhook reliability.


183-190: Verify mutual exclusivity of payment types
Could not find evidence in the codebase that clientId and recurringPayment.id cannot both appear in the webhook payload. Please confirm in the webhook specification and add validation to enforce a single payment type (or handle cases where both are present).


51-53: Ensure a unique composite constraint on (requestId, txHash) exists in your database schema
onConflictDoNothing relies on a DB‐level unique index/constraint; add one in your migration if it’s not already defined.

src/components/dashboard-navigation.tsx (1)

17-18: LGTM! Receipts tab implementation is consistent.

The receipts tab follows the established pattern for other tabs (Pay, Subscriptions), maintaining consistency in pathname detection and navigation structure.

Also applies to: 26-26, 36-38

src/app/ecommerce/sales/page.tsx (1)

6-16: LGTM! Server-side data fetching follows established patterns.

The authentication guard and TRPC data fetching align with the existing codebase patterns seen in similar server components.

src/components/ecommerce/sales/index.tsx (1)

20-28: LGTM! Error handling with retry is well-implemented.

The error state provides clear feedback and a retry mechanism, improving user experience.

src/app/dashboard/receipts/page.tsx (1)

6-16: LGTM! Consistent implementation with sales page.

The receipts page follows the same server-side patterns as the sales page, maintaining consistency across the ecommerce features.

src/lib/types/index.ts (1)

2-3: LGTM! Type extraction follows TRPC best practices.

Using inferRouterOutputs ensures type safety and keeps the type definition in sync with the router implementation.

Also applies to: 39-41

drizzle/meta/0010_snapshot.json (1)

1-1194: Migration snapshot appears consistent with schema changes.

This is an auto-generated Drizzle migration snapshot. The new tables (easyinvoice_client_payment, easyinvoice_ecommerce_client) include appropriate foreign keys, unique constraints, and cascade behaviors.

src/server/db/schema.ts (2)

303-329: LGTM! ecommerceClientTable structure is well-designed.

The table includes:

  • Proper separation of external API ID and Request Network client ID
  • User-level unique constraint on domain (prevents duplicate domains per user)
  • Global unique constraint on rnClientId (ensures client IDs are unique)
  • Cascade delete on user removal (appropriate for user-owned data)

373-373: LGTM! Relations and type exports are properly configured.

The bidirectional relations between users, ecommerce clients, and client payments are correctly set up with appropriate cascade behaviors.

Also applies to: 426-448, 485-486

src/components/ecommerce/sales/blocks/client-payments-table.tsx (4)

42-102: LGTM! CustomerInfoDisplay handles edge cases well.

The component properly:

  • Handles missing customerInfo
  • Only shows expand button when there's expandable content
  • Uses controlled state for expand/collapse

124-128: Verify date handling for invalid dates.

The date formatting assumes clientPayment.createdAt is a valid date. While there's a fallback to "N/A", consider adding error handling for invalid date values that might cause format() to throw.

// Consider more defensive date handling
{clientPayment.createdAt
  ? (() => {
      try {
        return format(new Date(clientPayment.createdAt), "do MMM yyyy HH:mm");
      } catch {
        return "Invalid date";
      }
    })()
  : "N/A"}

147-156: Verify requestId is always present for external links.

The external link construction assumes clientPayment.requestId exists. If it can be null/undefined, the link would be malformed. Consider adding a check or using optional chaining.

-      <a
-        href={`https://scan.request.network/request/${clientPayment.requestId}`}
-        target="_blank"
-        rel="noopener noreferrer"
-        className="inline-flex items-center gap-1 text-blue-600 hover:text-blue-800 transition-colors"
-      >
-        <span className="text-sm">View Request</span>
-        <ExternalLink className="h-3 w-3" />
-      </a>
+      {clientPayment.requestId ? (
+        <a
+          href={`https://scan.request.network/request/${clientPayment.requestId}`}
+          target="_blank"
+          rel="noopener noreferrer"
+          className="inline-flex items-center gap-1 text-blue-600 hover:text-blue-800 transition-colors"
+        >
+          <span className="text-sm">View Request</span>
+          <ExternalLink className="h-3 w-3" />
+        </a>
+      ) : (
+        <span className="text-zinc-500">-</span>
+      )}

161-267: LGTM! Overall component structure is well-organized.

The component provides:

  • Clear filtering UI
  • Responsive pagination
  • Comprehensive payment details
  • Good empty states

The implementation follows React best practices and integrates well with the existing UI components.

drizzle/0010_special_warbird.sql (2)

1-15: LGTM!

The table structure is well-defined with appropriate types. Storing amount as text is correct for handling arbitrary-precision currency values in crypto transactions. The nullable customerInfo JSON field provides flexibility for customer data storage.


17-27: LGTM!

The foreign key constraints are correctly configured with CASCADE delete behavior, ensuring referential integrity. The DO blocks with duplicate_object exception handling make the migration idempotent and safe to re-run.

src/components/dashboard/receipts.tsx (3)

1-30: LGTM!

Imports are well-organized and types are clearly defined. The component interface appropriately accepts initial data for SSR/hydration support.


101-113: LGTM!

The filter handler correctly resets pagination when the filter changes. The merchant list derivation efficiently deduplicates clients using a reduce pattern.


115-186: LGTM!

The rendering logic is well-structured with appropriate conditional rendering for empty states and pagination. The filter UI provides good user experience with clear labeling and accessible components.

src/server/routers/ecommerce.ts (6)

1-14: LGTM!

Imports are well-organized and the router setup follows TRPC best practices.


15-56: Domain uniqueness enforced by DB constraint.

The check-then-insert pattern (lines 20-30) has a theoretical race window, but the database schema includes a unique index ecommerce_client_user_id_domain_unique on (userId, domain) (see src/server/db/schema.ts line 319-322), which will cause the insert to fail with a constraint violation if a concurrent request sneaks through. The current error handling via toTRPCError will map this to an appropriate TRPC error.


57-112: Domain conflict validation implemented correctly.

The edit procedure now includes proper domain uniqueness validation (lines 75-88) when the domain changes, addressing the previous review concern. The logic correctly excludes the current client from the conflict check using not(eq(ecommerceClientTable.id, input.id)).


113-123: LGTM!

The query correctly fetches all ecommerce clients for the authenticated user with appropriate error handling.


124-151: Delete order improved.

The procedure now deletes from the local database first (line 141) before calling the external API (line 145), which is safer than the previous order. If the external delete fails, the local record is already removed, avoiding inconsistent state where the local record exists but the external client is gone.

Based on learnings


152-181: LGTM!

Both payment query procedures are well-implemented:

  • getAllClientPayments correctly fetches payments for the authenticated user with related client data.
  • getAllUserReceipts properly uses PostgreSQL's JSON operator (->>) to filter by email from the customerInfo JSON field. The SQL template tag from drizzle-orm safely parameterizes the user email, preventing SQL injection.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f3b82c and 258e817.

📒 Files selected for processing (1)
  • src/app/api/webhook/route.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/app/api/webhook/route.ts (3)
src/server/db/index.ts (1)
  • db (10-12)
src/server/db/schema.ts (2)
  • ecommerceClientTable (303-329)
  • clientPaymentTable (331-364)
src/lib/errors.ts (1)
  • ResourceNotFoundError (5-10)
⏰ 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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 258e817 and bef6a6f.

📒 Files selected for processing (4)
  • drizzle/0010_peaceful_lionheart.sql (1 hunks)
  • drizzle/meta/0010_snapshot.json (1 hunks)
  • drizzle/meta/_journal.json (1 hunks)
  • src/server/db/schema.ts (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/server/db/schema.ts
  • drizzle/meta/_journal.json

@rodrigopavezi
Copy link
Member

Pull Request Review: feat/fees-breakdown

📋 Summary

This PR introduces a comprehensive ecommerce client management system with fee configuration, payment tracking, and webhook integration. The implementation follows existing patterns and maintains high code quality standards.

✅ What's Working Well

Database & Schema

  • Well-structured new tables (ecommerce_client, client_payment) with proper relationships
  • Unique constraints prevent duplicate clients and payments
  • Proper foreign key cascading and indexes

Security

  • All API endpoints properly authenticated with protectedProcedure
  • Users can only access their own data (proper authorization)
  • Webhook HMAC signature verification implemented
  • Comprehensive input validation with Zod schemas

Type Safety

  • Strong TypeScript usage throughout
  • Custom validation for Ethereum addresses and fee percentages
  • Proper type exports and inference

UI/UX

  • Consistent with existing design system
  • Proper loading states and error handling
  • User-friendly confirmation dialogs for destructive actions
  • Responsive tables with pagination and filtering

⚠️ Minor Improvements Suggested

  1. Rate Limiting: Consider adding rate limiting to the webhook endpoint
  2. Logging: Implement structured logging instead of console.error
  3. Environment Variables: Add runtime validation for required env vars
  4. API Response Validation: Validate external API responses

🎯 Assessment

Overall Score: 8.5/10

This is a well-implemented feature that:

  • Follows security best practices
  • Maintains code consistency
  • Provides excellent user experience
  • Handles errors gracefully

🚀 Recommendation

✅ APPROVED - Ready to merge

The core functionality is solid and secure. The suggested improvements are minor and can be addressed in follow-up PRs if desired.


Review completed with comprehensive analysis of database schema, API security, type safety, error handling, and UI components.

@bassgeta bassgeta merged commit c015b84 into main Oct 7, 2025
5 checks passed
@bassgeta bassgeta deleted the feat/151-client-id-management branch October 7, 2025 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EasyInvoice - Develop Sales and receipts for users EasyInvoice - Add Client ID Managment Tab
2 participants