Skip to content

feat: implement Add Transaction functionality (balance, account deduc…#13

Merged
Caisere merged 3 commits intomainfrom
feat/transaction-implementation
Mar 19, 2026
Merged

feat: implement Add Transaction functionality (balance, account deduc…#13
Caisere merged 3 commits intomainfrom
feat/transaction-implementation

Conversation

@Caisere
Copy link
Copy Markdown
Owner

@Caisere Caisere commented Mar 19, 2026

…tion, proper success and error handling, validation and account ID handling for transactions)

  • Transaction Modal: Update form state to store accountId instead of account name

    • Modify AddTransactionModal to use accountId in form state and validation
    • Update form handler to lookup account ID from userAccounts array on selection
    • Ensure accountId is properly passed to addTransaction function
  • Transaction Processing: Implement complete balance checking and atomic updates

    • Add balance validation for expense transactions (prevent overdrafts)
    • Implement atomic database transactions for balance updates and transaction creation
    • Update account balances: deduct for expenses, add for income
    • Add proper error handling for insufficient funds and account not found
  • Type Safety: Update transaction schema and interfaces

    • Add accountId field to createTransactionSchema validation
    • Update Transaction interface to use accountId instead of account name
    • Ensure type safety across the transaction creation flow

Summary by CodeRabbit

  • New Features

    • Improved transaction modal: async submission, stronger validation, and clear success/error toasts
    • Server-side add transaction flow with atomic balance updates and explicit insufficient-balance errors
  • Bug Fixes

    • Account selector now uses account IDs and defaults to the first account for reliable selection
    • Transaction type labels standardized to INCOME / EXPENSE for clearer distinction

…tion, proper success and error handling, validation and account ID handling for transactions)

- Transaction Modal: Update form state to store accountId instead of account name
  - Modify AddTransactionModal to use accountId in form state and validation
  - Update form handler to lookup account ID from userAccounts array on selection
  - Ensure accountId is properly passed to addTransaction function

- Transaction Processing: Implement complete balance checking and atomic updates
  - Add balance validation for expense transactions (prevent overdrafts)
  - Implement atomic database transactions for balance updates and transaction creation
  - Update account balances: deduct for expenses, add for income
  - Add proper error handling for insufficient funds and account not found

- Type Safety: Update transaction schema and interfaces
  - Add accountId field to createTransactionSchema validation
  - Update Transaction interface to use accountId instead of account name
  - Ensure type safety across the transaction creation flow
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
wealthwise Ready Ready Preview, Comment Mar 19, 2026 9:17pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 19, 2026

📝 Walkthrough

Walkthrough

Introduces end-to-end transaction creation: UI form key renames and enums, client async submit with toasts, new server action validating and inserting transactions while updating account balance, updated types/seed data, and a Pool-based Neon DB initialization.

Changes

Cohort / File(s) Summary
Types & Seeds
app/types.ts, app/lib/data.ts
Changed TransactionType to `"INCOME"
Add Transaction UI
app/components/transactions/add-transaction-modal.tsx
Form state keys renamed (description, accountId), enum values switched to uppercase, account select uses raw account.id, submission made async with required-field validation, nanoid transactionId generation, amount coercion, try/catch around addTransaction, and toast notifications on success/failure.
Server Action
app/lib/actions.ts
Added exported addTransaction(data: Transaction) server action: session check, Zod validation, account ownership lookup inside DB transaction, balance sufficiency check for EXPENSE, atomic balance update, transaction insertion, and structured success/error responses with error logging.
Services
app/lib/services.ts
Extended UserAccountName to include id and updated getUserAccounts query to return account ids.
Database Init
db/index.ts
Switched Neon client initialization to a Pool-based setup and updated drizzle(pool, { schema }) wiring (imports/variable rename adjusted).

Sequence Diagram

sequenceDiagram
    participant User as User
    participant Modal as Add Transaction Modal
    participant Server as addTransaction (Server)
    participant Validation as createTransactionSchema
    participant DB as Database

    User->>Modal: Fill & submit form
    Modal->>Modal: generate transactionId, coerce amount, map accountId
    Modal->>Server: call addTransaction(payload)
    Server->>Validation: validate payload
    Validation-->>Server: success / error
    alt validated
      Server->>DB: fetch account by accountId & userId
      DB-->>Server: account record
      Server->>Server: check balance if EXPENSE
      Server->>DB: update account balance (atomic gte guard)
      DB-->>Server: balance updated
      Server->>DB: insert transaction row
      DB-->>Server: inserted
      Server-->>Modal: success response
      Modal->>User: show success toast, close modal
    else validation/error
      Server-->>Modal: error response
      Modal->>User: show error toast
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇 I hopped to stitch a ledger bright,

IDs crisp and balances right,
From modal nibble to DB burrow,
Toasts cheer loud — carrots for tomorrow. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: implement Add Transaction functionality' clearly summarizes the main change: implementing the Add Transaction feature with balance handling and account deduction. It is concise, directly related to the primary changeset objective, and meaningful.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/transaction-implementation
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown

@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: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/components/transactions/add-transaction-modal.tsx`:
- Around line 26-39: The form uses account IDs but the Select emits account
names and the setter writes the name back into form.accountId (and the initial
accountId unnecessarily lowercases the id); update the Select options to use
account.id as the option value (not account.name), remove the .toLowerCase() on
the initial accountId, and change the setter in set (and the similar handler
around the other occurrence) so when k === 'accountId' it writes
selectedAccount?.id (raw, not lowercased) into form.accountId and avoids then
overwriting it with the display value — keep the generic setForm((p) => ({ ...p,
[k]: value })) for other keys only.
- Around line 68-76: The UI currently only shows a success toast for
addTransaction and ignores business failures and exceptions; update the
add-transaction-modal.tsx flow around the addTransaction call (the result
variable and its handling) to surface failures: after awaiting
addTransaction(data) check if result.success is false and call
toast.error(result.message || "Failed to add transaction") and return/stop
further processing; in the catch block convert the caught error to a readable
message (e.g., error?.message || String(error)) and call toast.error(...)
instead of only console.log, ensuring both business-level failures (result) and
thrown exceptions produce user-facing error toasts.

In `@app/lib/actions.ts`:
- Around line 400-466: The transaction handler reads
parsedTransaction.data.category but never persists it, so the insert into
transactions (see tx.insert(transactions).values({...})) must include the
transaction's categoryId; extract the correct identifier (e.g., categoryId or
category.id) from parsedTransaction.data and add it to the insert payload as
categoryId, and optionally validate it against the categories table before
inserting (use the same tx to query categories) so transactions are saved with
the proper categoryId instead of being uncategorized.
- Around line 433-463: The current flow reads and mutates balance in JS causing
a race; instead perform the balance check and update atomically in the DB with a
single conditional UPDATE on userAccounts that uses SQL arithmetic to subtract
(for EXPENSE) or add (for INCOME) and includes a WHERE that ensures balance >=
amount for expenses, then check the number of affected rows to fail when no row
was updated; after a successful update insert the transaction row into
transactions including the passed-in category (persist as categoryId) so
category is not dropped. Use the existing tx, userAccounts, transactions,
accountId/userId/transactionId/amount/type symbols and replace the two-step
read/update with the conditional UPDATE + row count check, then include
category/categoryId in the tx.insert call.

In `@app/types.ts`:
- Around line 82-96: The exported Transaction[] fixture is out of sync with the
new Transaction type: replace each fixture object’s old keys (id, desc, cat,
account, icon) with the new keys (transactionId, description, category,
accountId) and ensure fields match the Transaction shape (amount:number,
date:string) and the type field uses the TransactionType union values ("INCOME"
or "EXPENSE" uppercase) instead of lowercase "income"/"expense"; remove any
unused fields like icon and keep the export as Transaction[] so the module's
fixture objects conform to the Transaction interface.
- Around line 57-59: The schema for the transaction "amount" currently allows
zero and negatives; update the Zod schema for the amount field (the `amount`
property in app/types.ts) to enforce strictly positive values (use Zod's
positive check or an equivalent `.refine`/`.min` > 0) so invalid amounts are
rejected at schema validation before `addTransaction` runs; keep or add a clear
validation message indicating "amount must be greater than 0" to make failures
actionable.

In `@db/index.ts`:
- Line 4: The code uses a non-null assertion on process.env.DATABASE_URL when
constructing pool (const pool = new Pool(...)) which delays a misconfiguration
failure; change this to validate the environment variable first and fail fast:
check process.env.DATABASE_URL exists, throw a clear Error (e.g., "DATABASE_URL
environment variable is required") if missing, then pass the validated string to
new Pool; remove the `!` non-null assertion so the Pool always receives a real
string.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 989cbaf8-1bd3-4fa2-a0f3-3882dbcd0c01

📥 Commits

Reviewing files that changed from the base of the PR and between 8b92b25 and 5019ea0.

📒 Files selected for processing (5)
  • app/components/transactions/add-transaction-modal.tsx
  • app/lib/actions.ts
  • app/lib/services.ts
  • app/types.ts
  • db/index.ts

Comment on lines +26 to +39
accountId: userAccounts?.[0]?.id?.toLowerCase() || "",
date: "",
});
const set =
(k: string) =>
(e: React.ChangeEvent<HTMLInputElement | HTMLSelectElement>) =>
setForm((p) => ({ ...p, [k]: e.target.value }));
(e: React.ChangeEvent<HTMLInputElement | HTMLSelectElement>) => {
const value = e.target.value

function handleAddTransaction(e: FormEvent) {
if(k === 'accountId') {
const selectedAccount = userAccounts?.find((acc) => acc.name.toLowerCase() === value)
setForm((prev) => ({...prev, accountId: selectedAccount?.id || ''}))
}
setForm((p) => ({ ...p, [k]: value }));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Use the account id as the <Select> value end-to-end.

The form state starts with an id, the options emit account names, and the setter ultimately writes the name back into form.accountId. addTransaction later queries userAccounts.id, so a changed selection will resolve to "Account not found". Line 26 also lowercases ids unnecessarily.

🔧 Suggested fix
@@
-    accountId: userAccounts?.[0]?.id?.toLowerCase() || "",
+    accountId: userAccounts?.[0]?.id || "",
@@
-      const value = e.target.value
-
-      if(k === 'accountId') {
-        const selectedAccount = userAccounts?.find((acc) => acc.name.toLowerCase() === value)
-        setForm((prev) => ({...prev, accountId: selectedAccount?.id || ''}))
-      }
-      setForm((p) => ({ ...p, [k]: value }));
-    }
+      const value = e.target.value;
+      setForm((p) => ({ ...p, [k]: value }));
+    };
@@
-            userAccounts?.map((account: { name: string, id:string }) => ({
-              // .toLowerCase().replace(/\s+/g, "-")
-              value: account.name,
+            userAccounts?.map((account) => ({
+              value: account.id,
               label: account.name,
             })) || []

Also applies to: 134-141

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/components/transactions/add-transaction-modal.tsx` around lines 26 - 39,
The form uses account IDs but the Select emits account names and the setter
writes the name back into form.accountId (and the initial accountId
unnecessarily lowercases the id); update the Select options to use account.id as
the option value (not account.name), remove the .toLowerCase() on the initial
accountId, and change the setter in set (and the similar handler around the
other occurrence) so when k === 'accountId' it writes selectedAccount?.id (raw,
not lowercased) into form.accountId and avoids then overwriting it with the
display value — keep the generic setForm((p) => ({ ...p, [k]: value })) for
other keys only.

Comment thread app/components/transactions/add-transaction-modal.tsx
Comment thread app/lib/actions.ts
Comment on lines +400 to +466
const {
type,
transactionId,
amount,
description,
date,
accountId,
category,
} = parsedTransaction.data;

const userId = session.id;

const queryCondition = [
eq(userAccounts.id, accountId),
eq(userAccounts.userId, userId),
];

return await db.transaction(async (tx) => {
// get user's current account balance
const [account] = await tx
.select()
.from(userAccounts)
.where(
and(...queryCondition),
);

if (!account) {
return {
success: false,
message: "Account not found",
};
}

// Convery balance tto Number
const userCurrentBalance = Number(account.balance);

// check for transaction type and and sufficient balance

if (type === "EXPENSE" && userCurrentBalance < amount) {
return {
success: false,
message: `Insufficient balance. Available: ₦${userCurrentBalance.toLocaleString()}`,
};
}

// calculate new balance
const newBalance =
type === "EXPENSE"
? userCurrentBalance - amount
: userCurrentBalance + amount;

await tx
.update(userAccounts)
.set({ balance: newBalance.toString() })
.where(
and(...queryCondition),
);

await tx.insert(transactions).values({
userId,
accountId,
transactionId,
type,
amount: amount.toString(),
description,
date: new Date(date),
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

The selected category is being discarded.

Line 407 extracts category, but Lines 458-466 never persist it, and db/schema.ts Lines 222-264 show that transactions only store categoryId. Every row created by this flow will end up uncategorized.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/actions.ts` around lines 400 - 466, The transaction handler reads
parsedTransaction.data.category but never persists it, so the insert into
transactions (see tx.insert(transactions).values({...})) must include the
transaction's categoryId; extract the correct identifier (e.g., categoryId or
category.id) from parsedTransaction.data and add it to the insert payload as
categoryId, and optionally validate it against the categories table before
inserting (use the same tx to query categories) so transactions are saved with
the proper categoryId instead of being uncategorized.

Comment thread app/lib/actions.ts
Comment on lines +433 to +463
// Convery balance tto Number
const userCurrentBalance = Number(account.balance);

// check for transaction type and and sufficient balance

if (type === "EXPENSE" && userCurrentBalance < amount) {
return {
success: false,
message: `Insufficient balance. Available: ₦${userCurrentBalance.toLocaleString()}`,
};
}

// calculate new balance
const newBalance =
type === "EXPENSE"
? userCurrentBalance - amount
: userCurrentBalance + amount;

await tx
.update(userAccounts)
.set({ balance: newBalance.toString() })
.where(
and(...queryCondition),
);

await tx.insert(transactions).values({
userId,
accountId,
transactionId,
type,
amount: amount.toString(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Expectation:
# - db/schema.ts documents money columns as numeric.
# - addTransaction currently converts them through Number()/toString().
sed -n '160,166p' db/schema.ts
rg -n -C2 'Number\(account\.balance\)|balance: newBalance\.toString\(\)|amount: amount\.toString\(\)' app/lib/actions.ts

Repository: Caisere/wealthwise

Length of output: 813


🏁 Script executed:

#!/bin/bash
# Check the complete addTransaction function to see:
# 1. The full transaction.insert values object (is category included?)
# 2. Any concurrency safety mechanisms (BEGIN, explicit locks, etc.)

# Get the function around lines 433-463 to see full context
sed -n '420,475p' app/lib/actions.ts

Repository: Caisere/wealthwise

Length of output: 1399


🏁 Script executed:

#!/bin/bash
# Check the addTransaction function signature to see if category is a parameter
# and look for the full transaction block context

sed -n '380,420p' app/lib/actions.ts

Repository: Caisere/wealthwise

Length of output: 925


🏁 Script executed:

#!/bin/bash
# Check the transactions schema to confirm category field exists
# and verify if there are any database constraints or triggers

sed -n '220,265p' db/schema.ts

Repository: Caisere/wealthwise

Length of output: 1486


🏁 Script executed:

#!/bin/bash
# Check if there's any isolation level set or explicit locking in the transaction
# Also verify if this is using Drizzle's db.transaction

rg -n -B5 -A5 'db\.transaction' app/lib/actions.ts | head -40

Repository: Caisere/wealthwise

Length of output: 406


🏁 Script executed:

#!/bin/bash
# Verify the full transaction.insert values call - is categoryId included?
sed -n '458,470p' app/lib/actions.ts

Repository: Caisere/wealthwise

Length of output: 364


Move the balance check and update into a single SQL statement, and persist the category field.

Lines 434 and 451–453 read the balance as a Number, perform arithmetic in JavaScript, and write back a stringified result. This creates a race condition: two concurrent transactions can read the same balance, each calculate independently, and overwrite each other's changes. Use a single UPDATE ... WHERE balance >= amount statement to atomically deduct the expense and prevent concurrent mutations from racing.

Additionally, the function receives category as a parameter but does not persist it. Line 458–470 inserts the transaction without including categoryId, so category information is silently dropped.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/actions.ts` around lines 433 - 463, The current flow reads and
mutates balance in JS causing a race; instead perform the balance check and
update atomically in the DB with a single conditional UPDATE on userAccounts
that uses SQL arithmetic to subtract (for EXPENSE) or add (for INCOME) and
includes a WHERE that ensures balance >= amount for expenses, then check the
number of affected rows to fail when no row was updated; after a successful
update insert the transaction row into transactions including the passed-in
category (persist as categoryId) so category is not dropped. Use the existing
tx, userAccounts, transactions, accountId/userId/transactionId/amount/type
symbols and replace the two-step read/update with the conditional UPDATE + row
count check, then include category/categoryId in the tx.insert call.

Comment thread app/types.ts Outdated
Comment thread app/types.ts
Comment thread db/index.ts Outdated
Copy link
Copy Markdown

@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.

♻️ Duplicate comments (1)
db/index.ts (1)

5-5: ⚠️ Potential issue | 🟠 Major

Fail fast when DATABASE_URL is missing.

Line 5 still relies on a non-null assertion, so misconfiguration fails later and less clearly. Validate once at startup and throw an explicit error.

Suggested fix
-const pool = new Pool({ connectionString: process.env.DATABASE_URL! });
-export const db = drizzle(pool, {schema});
+const connectionString = process.env.DATABASE_URL;
+
+if (!connectionString) {
+  throw new Error("DATABASE_URL environment variable is required");
+}
+
+const pool = new Pool({ connectionString });
+export const db = drizzle(pool, { schema });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@db/index.ts` at line 5, Replace the non-null assertion on
process.env.DATABASE_URL and fail fast: read the env var into a local (e.g.,
connectionString), validate it at module startup and throw a clear Error if
missing, then construct the Pool with new Pool({ connectionString }) instead of
using process.env.DATABASE_URL!; update the symbol creating the pool (const
pool) to use the validated connectionString so misconfiguration errors surface
immediately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@db/index.ts`:
- Line 5: Replace the non-null assertion on process.env.DATABASE_URL and fail
fast: read the env var into a local (e.g., connectionString), validate it at
module startup and throw a clear Error if missing, then construct the Pool with
new Pool({ connectionString }) instead of using process.env.DATABASE_URL!;
update the symbol creating the pool (const pool) to use the validated
connectionString so misconfiguration errors surface immediately.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 10680786-c367-4f79-984a-3398015d4085

📥 Commits

Reviewing files that changed from the base of the PR and between 5019ea0 and 56e1e63.

📒 Files selected for processing (1)
  • db/index.ts

…function, database_url check during connection
Copy link
Copy Markdown

@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: 3

♻️ Duplicate comments (2)
app/lib/actions.ts (2)

400-408: ⚠️ Potential issue | 🟠 Major

Persist the validated category when creating the row.

Line 407 extracts category, but Lines 469-477 never write it to the transaction insert. Every transaction created through this action loses its category, so category-based views will miss new data.

Also applies to: 469-477

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/actions.ts` around lines 400 - 408, The parsedTransaction.data
destructures category but the DB insert that creates the transaction row (in the
create transaction flow that uses parsedTransaction.data / variables like type,
transactionId, amount, description, date, accountId) never includes category;
update the insert logic to persist the validated category by adding the category
column to the INSERT statement (and to the values/object passed to whatever DB
helper or ORM is used) and ensure the inserted value uses the local category
variable and matches expected DB field name/type (e.g., include category in the
object passed to createTransaction/knex/pg insert).

458-466: ⚠️ Potential issue | 🔴 Critical

Gate the insert on a successful balance update, and credit income correctly.

Lines 458-466 always subtract amount and always apply the balance >= amount guard. That means "INCOME" transactions can never increase the balance, and a concurrent expense can make the guarded UPDATE affect 0 rows while Lines 469-482 still insert the transaction and return success. Make the arithmetic conditional on type and stop when no row was updated.

Suggested fix
-      await tx
+      const updatedAccount = await tx
         .update(userAccounts)
-        .set({ balance: sql`${userAccounts.balance} - ${amount}` })
-        .where(
-          and(
-            ...queryCondition,
-            gte(userAccounts.balance, String(amount)), // only deduct if enough funds
-          ),
-        );
+        .set({
+          balance:
+            type === "EXPENSE"
+              ? sql`${userAccounts.balance} - ${amount}`
+              : sql`${userAccounts.balance} + ${amount}`,
+        })
+        .where(
+          type === "EXPENSE"
+            ? and(...queryCondition, gte(userAccounts.balance, String(amount)))
+            : and(...queryCondition),
+        )
+        .returning({ id: userAccounts.id });
+
+      if (updatedAccount.length === 0) {
+        return {
+          success: false,
+          message: "Insufficient balance",
+        };
+      }
 
       await tx.insert(transactions).values({
         userId,

Also applies to: 469-482

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/actions.ts` around lines 458 - 466, The update currently always
subtracts amount and always uses gte(userAccounts.balance, String(amount)),
which prevents INCOME from increasing balance and can let inserts proceed when
the update affected 0 rows; change the update in the transaction to perform
conditional arithmetic based on the transaction type (add when type ===
"INCOME", subtract otherwise) and only apply the gte(balance, amount) guard for
non-INCOME cases, then check the result of tx.update(userAccounts) (rows
affected) and abort/return an error (do not proceed to tx.insert(transactions))
if no row was updated; reference userAccounts, tx.update, amount, type,
queryCondition and the later tx.insert(transactions) so the insert is gated on a
successful update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/components/transactions/add-transaction-modal.tsx`:
- Around line 41-45: The modal's client-side validation destructures amount,
description, category, accountId, date from form but fails to include
description in the required check, causing unnecessary server requests; update
the validation in add-transaction-modal.tsx to include description (e.g., check
!description or !description.trim()) alongside !accountId, !amount, !category,
and !date so empty/whitespace descriptions are rejected immediately and
toast.error("Invalid Inputs") is shown.

In `@app/lib/data.ts`:
- Around line 49-129: The seed data in the transactions array assigns the same
string "thhe" to every transactionId, breaking uniqueness; update the seed
objects in app/lib/data.ts (the transactions/seed array) to give each entry a
unique transactionId (use crypto.randomUUID(), nanoid(), or a deterministic
pattern like `txn-${Date.now()}-${index}`) so keyed rendering, lookups, and
updates work correctly; ensure each object’s transactionId field is replaced
with a unique value while preserving all other fields.
- Around line 41-132: The TRANSACTIONS mock uses human-readable account labels
("GTBank", "Opay", "Cash") as accountId values but production expects opaque
account ids; update each transaction object's accountId in the TRANSACTIONS
array to use the real opaque ids (or test-scope constants) that match your
userAccounts.id values (or import USER_ACCOUNTS and reference their .id) so
id-based lookup paths behave like production; edit the TRANSACTIONS constant
(and any Transaction type fixtures) to replace those display strings with the
corresponding opaque ids used elsewhere in the app.

---

Duplicate comments:
In `@app/lib/actions.ts`:
- Around line 400-408: The parsedTransaction.data destructures category but the
DB insert that creates the transaction row (in the create transaction flow that
uses parsedTransaction.data / variables like type, transactionId, amount,
description, date, accountId) never includes category; update the insert logic
to persist the validated category by adding the category column to the INSERT
statement (and to the values/object passed to whatever DB helper or ORM is used)
and ensure the inserted value uses the local category variable and matches
expected DB field name/type (e.g., include category in the object passed to
createTransaction/knex/pg insert).
- Around line 458-466: The update currently always subtracts amount and always
uses gte(userAccounts.balance, String(amount)), which prevents INCOME from
increasing balance and can let inserts proceed when the update affected 0 rows;
change the update in the transaction to perform conditional arithmetic based on
the transaction type (add when type === "INCOME", subtract otherwise) and only
apply the gte(balance, amount) guard for non-INCOME cases, then check the result
of tx.update(userAccounts) (rows affected) and abort/return an error (do not
proceed to tx.insert(transactions)) if no row was updated; reference
userAccounts, tx.update, amount, type, queryCondition and the later
tx.insert(transactions) so the insert is gated on a successful update.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 451ac800-e2be-4f37-bd5f-f3a41aa22426

📥 Commits

Reviewing files that changed from the base of the PR and between 56e1e63 and ab96fc3.

📒 Files selected for processing (5)
  • app/components/transactions/add-transaction-modal.tsx
  • app/lib/actions.ts
  • app/lib/data.ts
  • app/types.ts
  • db/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • db/index.ts
  • app/types.ts

Comment on lines +41 to +45
const { amount, description, category, accountId, date } = form;

if (!accountId || !amount || !category || !date) {
toast.error("Invalid Inputs");
return;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Validate description with the other required fields.

createTransactionSchema requires description, but Line 43 skips it. An empty description therefore does a needless server round-trip and falls back to the generic "Invalid inputs" path instead of failing immediately in the modal.

Suggested fix
-      if (!accountId || !amount || !category || !date) {
+      if (!accountId || !amount || !description.trim() || !category || !date) {
         toast.error("Invalid Inputs");
         return;
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { amount, description, category, accountId, date } = form;
if (!accountId || !amount || !category || !date) {
toast.error("Invalid Inputs");
return;
const { amount, description, category, accountId, date } = form;
if (!accountId || !amount || !description.trim() || !category || !date) {
toast.error("Invalid Inputs");
return;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/components/transactions/add-transaction-modal.tsx` around lines 41 - 45,
The modal's client-side validation destructures amount, description, category,
accountId, date from form but fails to include description in the required
check, causing unnecessary server requests; update the validation in
add-transaction-modal.tsx to include description (e.g., check !description or
!description.trim()) alongside !accountId, !amount, !category, and !date so
empty/whitespace descriptions are rejected immediately and toast.error("Invalid
Inputs") is shown.

Comment thread app/lib/data.ts
Comment on lines 41 to 132
export const TRANSACTIONS: Transaction[] = [
{
id: 1,
desc: "Grocery — Shoprite",
cat: "Food",
account: "GTBank",
description: "Grocery — Shoprite",
category: "Food",
accountId: "GTBank",
amount: -8500,
date: "Mar 8, 2026",
icon: "🛒",
type: "expense",
type: "EXPENSE",
transactionId: "thhe",
},
{
id: 2,
desc: "March Salary",
cat: "Income",
account: "GTBank",
description: "March Salary",
category: "Income",
accountId: "GTBank",
amount: 420000,
date: "Mar 8, 2026",
icon: "💰",
type: "income",
transactionId: "thhe",
type: "INCOME",
},
{
id: 3,
desc: "Uber to VI",
cat: "Transport",
account: "Opay",
description: "Uber to VI",
category: "Transport",
accountId: "Opay",
amount: -2300,
date: "Mar 7, 2026",
icon: "🚗",
type: "expense",
transactionId: "thhe",
type: "EXPENSE",
},
{
id: 4,
desc: "Netflix Subscription",
cat: "Entertainment",
account: "Opay",
description: "Netflix Subscription",
category: "Entertainment",
accountId: "Opay",
amount: -4600,
date: "Mar 5, 2026",
icon: "🎬",
type: "expense",
transactionId: "thhe",
type: "EXPENSE",
},
{
id: 5,
desc: "EKEDC Electricity",
cat: "Utilities",
account: "Cash",
description: "EKEDC Electricity",
category: "Utilities",
accountId: "Cash",
amount: -15000,
date: "Mar 4, 2026",
icon: "",
type: "expense",
transactionId: "thhe",
type: "EXPENSE",
},
{
id: 6,
desc: "Freelance Payment",
cat: "Income",
account: "GTBank",
description: "Freelance Payment",
category: "Income",
accountId: "GTBank",
amount: 75000,
date: "Mar 3, 2026",
icon: "💼",
type: "income",
transactionId: "thhe",
type: "INCOME",
},
{
id: 7,
desc: "Mr Biggs — Dinner",
cat: "Food",
account: "Opay",
description: "Mr Biggs — Dinner",
category: "Food",
accountId: "Opay",
amount: -3800,
date: "Mar 2, 2026",
icon: "🍔",
type: "expense",
transactionId: "thhe",
type: "EXPENSE",
},
{
id: 8,
desc: "Danfo — Ikeja",
cat: "Transport",
account: "Cash",
description: "Danfo — Ikeja",
category: "Transport",
accountId: "Cash",
amount: -200,
date: "Mar 1, 2026",
icon: "🚌",
type: "expense",
transactionId: "thhe",
type: "EXPENSE",
},
{
id: 9,
desc: "Rent — March",
cat: "Rent",
account: "GTBank",
description: "Rent — March",
category: "Rent",
accountId: "GTBank",
amount: -80000,
date: "Mar 1, 2026",
icon: "🏠",
type: "expense",
transactionId: "thhe",
type: "EXPENSE",
},
{
id: 10,
desc: "Spotify",
cat: "Entertainment",
account: "Opay",
description: "Spotify",
category: "Entertainment",
accountId: "Opay",
amount: -2900,
date: "Feb 28, 2026",
icon: "🎵",
type: "expense",
transactionId: "thhe",
type: "EXPENSE",
},
];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Don't use display labels as mock accountIds.

Lines 45-126 still store values like "GTBank" and "Opay", but the live transaction flow now treats accountId as the opaque id emitted by the account select and looked up against userAccounts.id. Keeping the seed data in a different namespace will make any id-based lookup path behave differently from production.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/data.ts` around lines 41 - 132, The TRANSACTIONS mock uses
human-readable account labels ("GTBank", "Opay", "Cash") as accountId values but
production expects opaque account ids; update each transaction object's
accountId in the TRANSACTIONS array to use the real opaque ids (or test-scope
constants) that match your userAccounts.id values (or import USER_ACCOUNTS and
reference their .id) so id-based lookup paths behave like production; edit the
TRANSACTIONS constant (and any Transaction type fixtures) to replace those
display strings with the corresponding opaque ids used elsewhere in the app.

Comment thread app/lib/data.ts
Comment on lines +49 to +129
transactionId: "thhe",
},
{
id: 2,
desc: "March Salary",
cat: "Income",
account: "GTBank",
description: "March Salary",
category: "Income",
accountId: "GTBank",
amount: 420000,
date: "Mar 8, 2026",
icon: "💰",
type: "income",
transactionId: "thhe",
type: "INCOME",
},
{
id: 3,
desc: "Uber to VI",
cat: "Transport",
account: "Opay",
description: "Uber to VI",
category: "Transport",
accountId: "Opay",
amount: -2300,
date: "Mar 7, 2026",
icon: "🚗",
type: "expense",
transactionId: "thhe",
type: "EXPENSE",
},
{
id: 4,
desc: "Netflix Subscription",
cat: "Entertainment",
account: "Opay",
description: "Netflix Subscription",
category: "Entertainment",
accountId: "Opay",
amount: -4600,
date: "Mar 5, 2026",
icon: "🎬",
type: "expense",
transactionId: "thhe",
type: "EXPENSE",
},
{
id: 5,
desc: "EKEDC Electricity",
cat: "Utilities",
account: "Cash",
description: "EKEDC Electricity",
category: "Utilities",
accountId: "Cash",
amount: -15000,
date: "Mar 4, 2026",
icon: "",
type: "expense",
transactionId: "thhe",
type: "EXPENSE",
},
{
id: 6,
desc: "Freelance Payment",
cat: "Income",
account: "GTBank",
description: "Freelance Payment",
category: "Income",
accountId: "GTBank",
amount: 75000,
date: "Mar 3, 2026",
icon: "💼",
type: "income",
transactionId: "thhe",
type: "INCOME",
},
{
id: 7,
desc: "Mr Biggs — Dinner",
cat: "Food",
account: "Opay",
description: "Mr Biggs — Dinner",
category: "Food",
accountId: "Opay",
amount: -3800,
date: "Mar 2, 2026",
icon: "🍔",
type: "expense",
transactionId: "thhe",
type: "EXPENSE",
},
{
id: 8,
desc: "Danfo — Ikeja",
cat: "Transport",
account: "Cash",
description: "Danfo — Ikeja",
category: "Transport",
accountId: "Cash",
amount: -200,
date: "Mar 1, 2026",
icon: "🚌",
type: "expense",
transactionId: "thhe",
type: "EXPENSE",
},
{
id: 9,
desc: "Rent — March",
cat: "Rent",
account: "GTBank",
description: "Rent — March",
category: "Rent",
accountId: "GTBank",
amount: -80000,
date: "Mar 1, 2026",
icon: "🏠",
type: "expense",
transactionId: "thhe",
type: "EXPENSE",
},
{
id: 10,
desc: "Spotify",
cat: "Entertainment",
account: "Opay",
description: "Spotify",
category: "Entertainment",
accountId: "Opay",
amount: -2900,
date: "Feb 28, 2026",
icon: "🎵",
type: "expense",
transactionId: "thhe",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Seed transactionIds must be unique.

Lines 49-129 assign "thhe" to every transaction. That collapses the mock set onto a single identity for keyed rendering, deduping, or any lookup/update by transactionId.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/data.ts` around lines 49 - 129, The seed data in the transactions
array assigns the same string "thhe" to every transactionId, breaking
uniqueness; update the seed objects in app/lib/data.ts (the transactions/seed
array) to give each entry a unique transactionId (use crypto.randomUUID(),
nanoid(), or a deterministic pattern like `txn-${Date.now()}-${index}`) so keyed
rendering, lookups, and updates work correctly; ensure each object’s
transactionId field is replaced with a unique value while preserving all other
fields.

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.

1 participant