Conversation
implement adding income transactions
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRefactors dashboard metrics to fetch concurrent income/expense totals, adds month-over-month helpers and percentage change logic, tightens transaction expense/budget atomic updates, adjusts StatCard props/formatting, and cleans up budgets and login UI text. Changes
Sequence DiagramsequenceDiagram
participant Stats as Stats Component
participant Services as Services Layer
participant DB as Database
participant StatCard as StatCard Component
Stats->>Services: getUserAccountData()
Stats->>Services: getTotalIncAndExp()
par Parallel aggregates
Services->>DB: Sum current-month incomes
Services->>DB: Sum current-month expenses
Services->>DB: Sum last-month incomes
Services->>DB: Sum last-month expenses
end
DB-->>Services: Aggregate results
Services->>Services: Convert & compute percentage changes
Services-->>Stats: Return totals and percentages
Stats->>StatCard: Render numeric values + optional lastMonth/change/up
StatCard-->>StatCard: Format via fmt() and display comparison
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (6)
app/components/dashboard/user-greeting.tsx (2)
9-11: Consider memoizing these values to avoid unnecessary recalculations.The
fullDateandperiodvalues are recalculated on every render. Since these are derived from date/time utilities, wrapping them inuseMemowould prevent unnecessary recalculations and improve performance.⚡ Proposed optimization using useMemo
+'use client' + +import { getCompleteDate, getPeriod } from "@/app/lib/nameAbbr"; +import { T } from "@/app/lib/theme"; +import { ReactNode, useMemo } from "react"; + export function UserGreeting ({children}:{children: ReactNode}) { - const fullDate = getCompleteDate() - const period = getPeriod(); + const fullDate = useMemo(() => getCompleteDate(), []); + const period = useMemo(() => getPeriod(), []);Note: If you want the greeting to update in real-time (e.g., when the period changes from "Morning" to "Afternoon"), you would need to add state and an interval to trigger re-renders, or use a dependency that changes over time.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/dashboard/user-greeting.tsx` around lines 9 - 11, Wrap the computed values fullDate and period in React's useMemo to avoid recalculating them on every render: replace the direct assignments const fullDate = getCompleteDate() and const period = getPeriod() with useMemo calls that call getCompleteDate and getPeriod respectively (e.g., useMemo(() => getCompleteDate(), [])) and include appropriate dependency arrays; ensure you import useMemo from React and, if you need the greeting to update over time, note that you must add a time-based dependency or interval to trigger updates rather than using an empty deps array.
30-30: Hardcoded month should be dynamic.The text references "March" explicitly, but given that this PR adds real-time data fetching to dashboard statistics, this month should probably be dynamic and reflect the current or selected month.
📅 Proposed fix to make the month dynamic
</h1> <p style={{ fontSize: 14, color: T.mu }}> - Here's your financial overview for March. + Here's your financial overview for {new Date().toLocaleDateString('en-US', { month: 'long' })}. </p> </div>Or if you have a date formatting utility in
@/app/lib/nameAbbr, consider using that for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/dashboard/user-greeting.tsx` at line 30, The greeting text in the UserGreeting component hardcodes "March"; update the component (user-greeting.tsx / UserGreeting) to compute the displayed month dynamically—either derive it from the current date (new Date()) or from the component's selectedMonth/props/state used by the dashboard statistics—and format it using your existing date utility (e.g., the nameAbbr formatter in `@/app/lib/nameAbbr`) so the string "Here's your financial overview for March." becomes "Here's your financial overview for {formattedMonth}."app/lib/actions.ts (1)
469-488: Remove commented-out dead code.This block of commented-out code adds noise and should be removed. If it's needed for reference, consider documenting the logic in a comment or keeping it in version control history.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/actions.ts` around lines 469 - 488, Remove the commented-out dead code that calculates newBalance and the duplicated tx.update(...) blocks (the lines referencing newBalance, type === "EXPENSE", userCurrentBalance, amount, and the two tx.update(userAccounts).set({ balance: newBalance.toString() }).where(and(...queryCondition)) calls); either delete these commented lines entirely or replace them with a single concise explanatory comment noting that balance calculation is handled elsewhere or preserved in VCS for history, ensuring no commented duplicate database updates remain.app/components/dashboard/start-card.tsx (1)
66-79: String-based label comparison is fragile.Using
label !== "Net Worth"for conditional rendering will break silently if the label text changes. Consider using a dedicated prop (e.g.,showChange?: boolean) or a semantic card type prop for more robust control.♻️ Example: dedicated prop
+ showChange?: boolean; }) { // ... - {label !== "Net Worth" && ( + {showChange && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/dashboard/start-card.tsx` around lines 66 - 79, The current conditional uses a fragile string comparison (label !== "Net Worth") to hide the change badge; modify the component in start-card.tsx to accept an explicit prop (e.g., showChange?: boolean or cardType: "netWorth" | "default") and replace the condition with that prop (e.g., showChange) in the render where label is checked; update all call sites of the component to pass showChange={false} for the Net Worth card (or the appropriate cardType) so behavior is explicit and stable, and ensure the new prop has a sensible default (true) in the component props/typing.app/components/dashboard/stats.tsx (2)
31-31: Hardcoded month labels reduce reusability.The labels "Income (Mar)", "Expenses (Mar)", and "Savings (Mar)" are hardcoded. Consider deriving the month dynamically for accuracy across different periods.
♻️ Example: dynamic month label
const currentMonth = new Date().toLocaleString('default', { month: 'short' }); // Then use: label={`Income (${currentMonth})`}Also applies to: 38-39, 46-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/dashboard/stats.tsx` at line 31, The hardcoded month strings in the label props (e.g., "Income (Mar)", "Expenses (Mar)", "Savings (Mar)") reduce reusability; add a single variable (e.g., const currentMonth = new Date().toLocaleString('default', { month: 'short' })) at the top of the component in app/components/dashboard/stats.tsx and replace those literal labels with template strings using currentMonth (e.g., label={`Income (${currentMonth})`}) for each occurrence (the three label props shown in the diff).
4-15: Consider wrapping with Suspense for better loading UX.The
Statscomponent is now async and performs database queries viaPromise.all. Based on the dashboard page implementation, this component is rendered without a Suspense boundary, which means the entire page will block until both queries complete. Wrapping<Stats />in<Suspense fallback={...}>in the parent page would enable streaming and show a loading state.In
app/(main)/dashboard/page.tsx, wrap the Stats component:import { Suspense } from "react"; // ... <Suspense fallback={<StatsLoadingSkeleton />}> <Stats /> </Suspense>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/dashboard/stats.tsx` around lines 4 - 15, The Stats component is async and awaits getUserAccountData and getTotalIncAndExp which blocks rendering; wrap the <Stats /> usage in its parent with a React Suspense boundary and a sensible fallback (e.g. <StatsLoadingSkeleton />), and add the import { Suspense } from "react" in that parent; this will enable streaming/loading UX while Stats resolves.
🤖 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/`(main)/budgets/page.tsx:
- Around line 125-127: The percentage is rendered as "%{l.rate}" which produces
"%50"; change the JSX to put the percent sign after the numeric value (render
l.rate followed by "%"—e.g., use {l.rate}% or an equivalent template string) in
the same conditional block that contains the <p> element so the output shows
"50%"; locate the occurrence referencing l.rate inside the JSX fragment that
renders the <p style={{ fontSize: 12, color: T.mu }}> element and swap the order
accordingly.
In `@app/components/dashboard/start-card.tsx`:
- Around line 82-86: The span uses an incomplete Tailwind class ("text-") and
the render guard {lastMonth && ...} hides valid zero values; update the class to
a concrete Tailwind color (e.g., "text-gray-500") and change the condition to
explicitly check for non-null/undefined (e.g., lastMonth !== undefined &&
lastMonth !== null) so zero is rendered; adjust the JSX around the lastMonth and
fmt usages in the StartCard component accordingly.
- Line 63: The rendered value uses fmt(Number(value)) but Number(value) can be
NaN for non-numeric strings, producing "₦NaN"; update the component to validate
and sanitize the input before calling fmt (or add defensive logic inside fmt).
Specifically, where the JSX renders {fmt(Number(value))} ensure you check
Number.isFinite(Number(value)) or !Number.isNaN(Number(value)) and provide a
safe fallback (e.g., 0 or a placeholder like "-" / "N/A") when the value is not
numeric; alternatively, update fmt to detect NaN and return a safe formatted
string instead—adjust the rendering of value in start-card.tsx (the prop named
value and the fmt call) to use the validated/fallback value.
In `@app/components/dashboard/stats.tsx`:
- Line 34: Replace the non-null assertions on percentage values with nullish
coalescing: instead of using percentageIncome! and percentageExpense! in the up
and change props, use (percentageIncome ?? 0) and (percentageExpense ?? 0) so
comparisons like (percentageIncome ?? 0) > 0 and displayed change values use a
safe default; update both the up prop (previously up={percentageIncome! > 0})
and the corresponding change usages to use the null-coalesced expressions to
avoid runtime errors when the service returns null/undefined.
- Around line 17-18: The percentage calculation for percentageSavings divides by
totalExpenses and can produce Infinity/NaN when totalExpenses is 0; update the
logic around the savings/percentageSavings computation to guard against
division-by-zero by checking totalExpenses (and optionally both totalIncomes and
totalExpenses) before dividing: compute savings as currently done with savings =
totalIncomes - totalExpenses and set percentageSavings to a safe fallback (e.g.,
0 or a defined sentinel) when totalExpenses === 0, otherwise compute
Number((((totalIncomes - totalExpenses) / totalExpenses) * 100).toFixed(2));
ensure you update the code paths that reference percentageSavings to expect the
fallback value.
In `@app/components/transactions/add-transaction-modal.tsx`:
- Around line 157-158: The Cancel Button components in add-transaction-modal.tsx
are currently using type="submit" which triggers form submission; update both
Cancel buttons (the one calling onClose on click and the second occurrence
referenced in the review) to use type="button" instead so clicking Cancel runs
onClose without submitting the form—look for the Button elements that have
onClick={onClose} and replace their type value from "submit" to "button".
- Around line 110-220: The form JSX is duplicated for both EXPENSE and INCOME
branches; remove the conditional and render a single form that uses existing
state/handlers (type, form, set, handleAddTransaction) and the same inputs
(Amount, Description, Category, Account, Date) with options from categories and
userAccounts, keeping the buttons wired to onClose and isLoading; ensure
handleAddTransaction continues to include the current type value from state in
the submission payload so behavior remains unchanged.
In `@app/lib/actions.ts`:
- Around line 500-513: The budget update is executed unconditionally inside the
EXPENSE branch even though categoryId is optional; guard the update so it only
runs when categoryId is present (e.g., if (categoryId) { ... }). Specifically,
wrap the tx.update(budgets).set({ spent: sql`${budgets.spent} + ${amount}`
}).where(and(eq(budgets.userId, userId), eq(budgets.categoryId, categoryId),
gte(budgets.monthlyLimit, budgets.spent))) call with a check for categoryId to
avoid passing undefined into eq(budgets.categoryId, categoryId).
- Around line 500-513: The update to budgets is missing a month filter so it can
modify the wrong month's row; compute the transaction month string (e.g.,
txMonth = new Date(date).toISOString().slice(0,7) + "-01") from the transaction
date and add eq(budgets.month, txMonth) to the where clause of the
tx.update(...) call (alongside the existing eq(budgets.userId, userId) and
eq(budgets.categoryId, categoryId) conditions) so only the budget row for that
specific month is incremented.
In `@app/lib/services.ts`:
- Around line 284-287: The catch block in getTotalIncAndExp logs the wrong
message ("getCategories failed"); update the console.error call inside the
getTotalIncAndExp catch to use a correct, descriptive message like
"getTotalIncAndExp failed" (preserving the error object payload), so replace the
incorrect string in the existing console.error({ error }) call in the
getTotalIncAndExp function.
- Around line 261-264: The expressions that convert query results to numbers
(totalIncomes, totalExpenses, lastMonthTotalIncomes, lastMonthTotalExpenses)
assume currentMonthTotalInc?.[0].total etc. always exist; when the query returns
an empty array those index accesses will be undefined and throw. Update each
conversion to safely handle missing rows by using optional chaining and a
default (e.g., treat missing .total as 0) so you call something like
Number(currentMonthTotalInc?.[0]?.total ?? 0) for totalIncomes and similarly for
totalExpenses, lastMonthTotalIncomes, and lastMonthTotalExpenses.
- Around line 209-219: The monthly totals queries in currentMonthQueryArr and
lastMonthQueryArr are filtering on transactions.createdAt (insertion time) but
should use the user-entered transaction date; update both arrays to replace
gte(transactions.createdAt, ...) and lte(transactions.createdAt, ...) with
gte(transactions.date, ...) and lte(transactions.date, ...). Locate the two
occurrences in app/lib/services.ts within the currentMonthQueryArr and
lastMonthQueryArr definitions and change them to use transactions.date so
backdated transactions fall into the correct month totals.
In `@package.json`:
- Around line 34-35: Remove the incompatible dependency "tailwindcss-animate"
from package.json (it conflicts with Tailwind v4) and keep "tw-animate-css";
also search for and remove any references to the plugin (e.g.,
require('tailwindcss-animate') or "tailwindcss-animate" in tailwind.config.js or
postcss config). After removal, run your package manager (npm/yarn/pnpm install)
to update lockfiles and rebuild to ensure no leftover references remain.
---
Nitpick comments:
In `@app/components/dashboard/start-card.tsx`:
- Around line 66-79: The current conditional uses a fragile string comparison
(label !== "Net Worth") to hide the change badge; modify the component in
start-card.tsx to accept an explicit prop (e.g., showChange?: boolean or
cardType: "netWorth" | "default") and replace the condition with that prop
(e.g., showChange) in the render where label is checked; update all call sites
of the component to pass showChange={false} for the Net Worth card (or the
appropriate cardType) so behavior is explicit and stable, and ensure the new
prop has a sensible default (true) in the component props/typing.
In `@app/components/dashboard/stats.tsx`:
- Line 31: The hardcoded month strings in the label props (e.g., "Income (Mar)",
"Expenses (Mar)", "Savings (Mar)") reduce reusability; add a single variable
(e.g., const currentMonth = new Date().toLocaleString('default', { month:
'short' })) at the top of the component in app/components/dashboard/stats.tsx
and replace those literal labels with template strings using currentMonth (e.g.,
label={`Income (${currentMonth})`}) for each occurrence (the three label props
shown in the diff).
- Around line 4-15: The Stats component is async and awaits getUserAccountData
and getTotalIncAndExp which blocks rendering; wrap the <Stats /> usage in its
parent with a React Suspense boundary and a sensible fallback (e.g.
<StatsLoadingSkeleton />), and add the import { Suspense } from "react" in that
parent; this will enable streaming/loading UX while Stats resolves.
In `@app/components/dashboard/user-greeting.tsx`:
- Around line 9-11: Wrap the computed values fullDate and period in React's
useMemo to avoid recalculating them on every render: replace the direct
assignments const fullDate = getCompleteDate() and const period = getPeriod()
with useMemo calls that call getCompleteDate and getPeriod respectively (e.g.,
useMemo(() => getCompleteDate(), [])) and include appropriate dependency arrays;
ensure you import useMemo from React and, if you need the greeting to update
over time, note that you must add a time-based dependency or interval to trigger
updates rather than using an empty deps array.
- Line 30: The greeting text in the UserGreeting component hardcodes "March";
update the component (user-greeting.tsx / UserGreeting) to compute the displayed
month dynamically—either derive it from the current date (new Date()) or from
the component's selectedMonth/props/state used by the dashboard statistics—and
format it using your existing date utility (e.g., the nameAbbr formatter in
`@/app/lib/nameAbbr`) so the string "Here's your financial overview for March."
becomes "Here's your financial overview for {formattedMonth}."
In `@app/lib/actions.ts`:
- Around line 469-488: Remove the commented-out dead code that calculates
newBalance and the duplicated tx.update(...) blocks (the lines referencing
newBalance, type === "EXPENSE", userCurrentBalance, amount, and the two
tx.update(userAccounts).set({ balance: newBalance.toString()
}).where(and(...queryCondition)) calls); either delete these commented lines
entirely or replace them with a single concise explanatory comment noting that
balance calculation is handled elsewhere or preserved in VCS for history,
ensuring no commented duplicate database updates remain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cbf411d7-447d-4756-ac72-5d5155347e6e
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
app/(main)/budgets/page.tsxapp/(main)/dashboard/page.tsxapp/components/dashboard/start-card.tsxapp/components/dashboard/stats.tsxapp/components/dashboard/user-greeting.tsxapp/components/transactions/add-transaction-modal.tsxapp/components/transactions/transactions.tsxapp/lib/actions.tsapp/lib/helper.tsapp/lib/services.tsapp/lib/theme.tspackage.json
💤 Files with no reviewable changes (1)
- app/(main)/dashboard/page.tsx
| {l.rate !== null && ( | ||
| <p style={{ fontSize: 12, color: T.mu }}>%{l.rate}</p> | ||
| )} |
There was a problem hiding this comment.
Minor: Percentage display has incorrect format.
The display shows %{l.rate} which renders as %50 instead of the conventional 50%. Consider swapping the order:
💅 Proposed fix
- <p style={{ fontSize: 12, color: T.mu }}>%{l.rate}</p>
+ <p style={{ fontSize: 12, color: T.mu }}>{l.rate}%</p>📝 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.
| {l.rate !== null && ( | |
| <p style={{ fontSize: 12, color: T.mu }}>%{l.rate}</p> | |
| )} | |
| {l.rate !== null && ( | |
| <p style={{ fontSize: 12, color: T.mu }}>{l.rate}%</p> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`(main)/budgets/page.tsx around lines 125 - 127, The percentage is
rendered as "%{l.rate}" which produces "%50"; change the JSX to put the percent
sign after the numeric value (render l.rate followed by "%"—e.g., use {l.rate}%
or an equivalent template string) in the same conditional block that contains
the <p> element so the output shows "50%"; locate the occurrence referencing
l.rate inside the JSX fragment that renders the <p style={{ fontSize: 12, color:
T.mu }}> element and swap the order accordingly.
| await tx | ||
| .update(budgets) | ||
| .set({ | ||
| spent: sql`${budgets.spent} + ${amount}`, | ||
| }) | ||
| .where( | ||
| and( | ||
| eq(budgets.userId, userId), | ||
| eq(budgets.categoryId, categoryId), | ||
| gte(budgets.monthlyLimit, budgets.spent), | ||
| // const txMonth = new Date(date).toISOString().slice(0, 7) + "-01"; | ||
| // eq(budgets.month, txMonth), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Budget update executes even when categoryId is not provided.
The budget update block runs unconditionally within the EXPENSE branch, but categoryId is optional. If a user submits an expense without a category, eq(budgets.categoryId, categoryId) with undefined may produce unexpected query behavior.
Wrap the budget update in a conditional:
🛡️ Proposed fix
+ if (categoryId) {
await tx
.update(budgets)
.set({
spent: sql`${budgets.spent} + ${amount}`,
})
.where(
and(
eq(budgets.userId, userId),
eq(budgets.categoryId, categoryId),
gte(budgets.monthlyLimit, budgets.spent),
),
);
+ }📝 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.
| await tx | |
| .update(budgets) | |
| .set({ | |
| spent: sql`${budgets.spent} + ${amount}`, | |
| }) | |
| .where( | |
| and( | |
| eq(budgets.userId, userId), | |
| eq(budgets.categoryId, categoryId), | |
| gte(budgets.monthlyLimit, budgets.spent), | |
| // const txMonth = new Date(date).toISOString().slice(0, 7) + "-01"; | |
| // eq(budgets.month, txMonth), | |
| ), | |
| ); | |
| if (categoryId) { | |
| await tx | |
| .update(budgets) | |
| .set({ | |
| spent: sql`${budgets.spent} + ${amount}`, | |
| }) | |
| .where( | |
| and( | |
| eq(budgets.userId, userId), | |
| eq(budgets.categoryId, categoryId), | |
| gte(budgets.monthlyLimit, budgets.spent), | |
| // const txMonth = new Date(date).toISOString().slice(0, 7) + "-01"; | |
| // eq(budgets.month, txMonth), | |
| ), | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/lib/actions.ts` around lines 500 - 513, The budget update is executed
unconditionally inside the EXPENSE branch even though categoryId is optional;
guard the update so it only runs when categoryId is present (e.g., if
(categoryId) { ... }). Specifically, wrap the tx.update(budgets).set({ spent:
sql`${budgets.spent} + ${amount}` }).where(and(eq(budgets.userId, userId),
eq(budgets.categoryId, categoryId), gte(budgets.monthlyLimit, budgets.spent)))
call with a check for categoryId to avoid passing undefined into
eq(budgets.categoryId, categoryId).
Budget update lacks month filtering — may update wrong month's budget.
The budget spent update uses only userId and categoryId without filtering by the transaction's month. This means an expense dated in March could increment a February budget's spent value if that's the only matching record.
The commented-out month filter (lines 510-511) should be uncommented and implemented:
🐛 Proposed fix to add month filtering
await tx
.update(budgets)
.set({
spent: sql`${budgets.spent} + ${amount}`,
})
.where(
and(
eq(budgets.userId, userId),
eq(budgets.categoryId, categoryId),
gte(budgets.monthlyLimit, budgets.spent),
- // const txMonth = new Date(date).toISOString().slice(0, 7) + "-01";
- // eq(budgets.month, txMonth),
+ eq(budgets.month, new Date(date).toISOString().slice(0, 7) + "-01"),
),
);📝 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.
| await tx | |
| .update(budgets) | |
| .set({ | |
| spent: sql`${budgets.spent} + ${amount}`, | |
| }) | |
| .where( | |
| and( | |
| eq(budgets.userId, userId), | |
| eq(budgets.categoryId, categoryId), | |
| gte(budgets.monthlyLimit, budgets.spent), | |
| // const txMonth = new Date(date).toISOString().slice(0, 7) + "-01"; | |
| // eq(budgets.month, txMonth), | |
| ), | |
| ); | |
| await tx | |
| .update(budgets) | |
| .set({ | |
| spent: sql`${budgets.spent} + ${amount}`, | |
| }) | |
| .where( | |
| and( | |
| eq(budgets.userId, userId), | |
| eq(budgets.categoryId, categoryId), | |
| gte(budgets.monthlyLimit, budgets.spent), | |
| eq(budgets.month, new Date(date).toISOString().slice(0, 7) + "-01"), | |
| ), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/lib/actions.ts` around lines 500 - 513, The update to budgets is missing
a month filter so it can modify the wrong month's row; compute the transaction
month string (e.g., txMonth = new Date(date).toISOString().slice(0,7) + "-01")
from the transaction date and add eq(budgets.month, txMonth) to the where clause
of the tx.update(...) call (alongside the existing eq(budgets.userId, userId)
and eq(budgets.categoryId, categoryId) conditions) so only the budget row for
that specific month is incremented.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
app/lib/services.ts (1)
261-264:⚠️ Potential issue | 🟡 MinorHandle potential
undefinedwhen accessing.totalon empty query results.The optional chaining on
?.[0]doesn't protect the subsequent.totalaccess. If the query returns an empty array,[0]isundefined, and accessing.totalonundefinedthrows aTypeError.🛡️ Proposed fix
- const totalIncomes = Number(currentMonthTotalInc?.[0].total ?? 0); - const totalExpenses = Number(currentMonthTotalExp?.[0].total ?? 0); - const lastMonthTotalIncomes = Number(lastMonthTotalInc?.[0].total ?? 0); - const lastMonthTotalExpenses = Number(lastMonthTotalExp?.[0].total ?? 0); + const totalIncomes = Number(currentMonthTotalInc?.[0]?.total ?? 0); + const totalExpenses = Number(currentMonthTotalExp?.[0]?.total ?? 0); + const lastMonthTotalIncomes = Number(lastMonthTotalInc?.[0]?.total ?? 0); + const lastMonthTotalExpenses = Number(lastMonthTotalExp?.[0]?.total ?? 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/services.ts` around lines 261 - 264, The four variables totalIncomes, totalExpenses, lastMonthTotalIncomes, and lastMonthTotalExpenses access .total on the first element of query results which may be undefined; change the expressions to safely access the nested property (e.g., guard the array access with an additional optional chain or nullish check such as using currentMonthTotalInc?.[0]?.total ?? 0) so Number(...) never receives undefined, and apply the same safe access pattern for currentMonthTotalExp, lastMonthTotalInc, and lastMonthTotalExp.
🧹 Nitpick comments (1)
app/lib/actions.ts (1)
470-489: Remove commented-out code.These commented blocks are dead code that reduces readability. If needed for reference, version control history preserves them.
♻️ Proposed cleanup
} else { - // calculate new balance - // const newBalance = - // type === "EXPENSE" - // ? userCurrentBalance - amount - // : userCurrentBalance + amount; - - // await tx - // .update(userAccounts) - // .set({ balance: newBalance.toString() }) - // .where( - // and(...queryCondition), - // ); - - // await tx - // .update(userAccounts) - // .set({ balance: newBalance.toString() }) - // .where( - // and(...queryCondition), - // ); - await tx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/actions.ts` around lines 470 - 489, Remove the dead commented-out calculation and duplicate update blocks around the user account balance logic: delete the commented definitions and uses of newBalance, the conditional `type === "EXPENSE"` expression, and the two commented `await tx.update(userAccounts).set({ balance: newBalance.toString() }).where(and(...queryCondition))` blocks so the file no longer contains those stale fragments; keep any real balance-update logic intact and rely on VCS for history if you need the removed lines.
🤖 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/globals.css`:
- Line 2: The dialog animations referenced in components/ui/dialog.tsx (classes
animate-in, animate-out, fade-in-0, fade-out-0, zoom-in-95, zoom-out-95) are
missing because the tw-animate-css import was commented out in globals.css;
either restore the import line in globals.css so those utilities are available,
or add equivalent CSS keyframes and utility classes to globals.css that define
animate-in/animate-out and the fade-/zoom- variants used by the Dialog component
to preserve the expected transitions.
In `@app/lib/actions.ts`:
- Around line 464-498: The transaction insertion can proceed even if the balance
UPDATE did nothing due to a race; after calling
tx.update(userAccounts).set(...).where(and(..., gte(userAccounts.balance,
String(amount)))), check the update result (rows affected) and if zero, abort
and return the insufficient-balance response instead of continuing to insert the
transaction. Update the logic around the balance check/tx.update (referencing
userAccounts, tx.update, gte, and queryCondition) to either remove the pre-check
and rely solely on the DB guard or keep the pre-check but still verify the
update affected rows before calling the transaction insert logic.
In `@app/lib/services.ts`:
- Around line 252-258: The inline comment above the query incorrectly says
"current month total Incomes" while the code is selecting total for EXPENSE;
update the comment to accurately reflect what the query does (e.g., "current
month total Expenses") next to the db.select block that uses transactions,
sum(transactions.amount), and currentMonthQueryArr so the comment matches the
filter eq(transactions.type, "EXPENSE").
---
Duplicate comments:
In `@app/lib/services.ts`:
- Around line 261-264: The four variables totalIncomes, totalExpenses,
lastMonthTotalIncomes, and lastMonthTotalExpenses access .total on the first
element of query results which may be undefined; change the expressions to
safely access the nested property (e.g., guard the array access with an
additional optional chain or nullish check such as using
currentMonthTotalInc?.[0]?.total ?? 0) so Number(...) never receives undefined,
and apply the same safe access pattern for currentMonthTotalExp,
lastMonthTotalInc, and lastMonthTotalExp.
---
Nitpick comments:
In `@app/lib/actions.ts`:
- Around line 470-489: Remove the dead commented-out calculation and duplicate
update blocks around the user account balance logic: delete the commented
definitions and uses of newBalance, the conditional `type === "EXPENSE"`
expression, and the two commented `await tx.update(userAccounts).set({ balance:
newBalance.toString() }).where(and(...queryCondition))` blocks so the file no
longer contains those stale fragments; keep any real balance-update logic intact
and rely on VCS for history if you need the removed lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 75e0f259-6e03-4c2b-a647-f356078fe122
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
app/(auth)/login/page.tsxapp/components/dashboard/start-card.tsxapp/components/dashboard/stats.tsxapp/globals.cssapp/lib/actions.tsapp/lib/services.ts
✅ Files skipped from review due to trivial changes (1)
- app/(auth)/login/page.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- app/components/dashboard/stats.tsx
- app/components/dashboard/start-card.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/lib/actions.ts (1)
463-468:⚠️ Potential issue | 🟠 MajorDrop the stale balance pre-check.
This read can still reject a valid expense if another transaction credits the account after the select but before the update. The guarded balance update below is the authoritative concurrency-safe check, so failure handling should hang off
updateResultinstead of this early return.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/actions.ts` around lines 463 - 468, Remove the stale optimistic pre-check that returns when type === "EXPENSE" and userCurrentBalance < amount; instead delete that early if/return block and let the concurrency-safe guarded update handle insufficient funds by inspecting updateResult (the DB update that decrements balance). Ensure any user-facing "Insufficient balance" response is produced based on the updateResult outcome (failed/no rows affected) rather than userCurrentBalance, keeping variables referenced like type, userCurrentBalance, amount and updateResult to locate the code to change.
♻️ Duplicate comments (1)
app/lib/actions.ts (1)
517-521:⚠️ Potential issue | 🟠 MajorStill scope the budget update to the transaction month.
This same file stores budgets by category and month, but this
UPDATEonly filters byuserIdandcategoryId. A single expense can increment the wrong month's budget row unless you addeq(budgets.month, txMonth)derived from the transaction date.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/actions.ts` around lines 517 - 521, The UPDATE currently filters only by userId and categoryId so it can bump the wrong month's row; compute txMonth from the transaction date (use new Date(date).toISOString().slice(0,7) + "-01") and add eq(budgets.month, txMonth) to the predicates alongside eq(budgets.userId, userId) and eq(budgets.categoryId, categoryId) so the update on budgets.monthlyLimit / budgets.spent targets the correct month row.
🧹 Nitpick comments (1)
app/lib/services.ts (1)
221-259: Collapse these aggregates into one snapshot.Running four separate
SUMqueries here means the dashboard can compute percentages from mixed committed states under concurrent writes, and it scans the same user/month slice four times per render. A single grouped aggregate, or at least a read transaction, would keep the numbers consistent and cheaper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/services.ts` around lines 221 - 259, The four separate SUM queries (using db.select(...).from(transactions).where(...)) must be collapsed into a single consistent read—either wrap them in a database read transaction or, better, replace them with one grouped/conditional aggregate that computes all four totals in one roundtrip (e.g., a single select with conditional SUMs or GROUP BY on transaction type + month using lastMonthQueryArr and currentMonthQueryArr as filters). Update the code that assigns lastMonthTotalInc, currentMonthTotalInc, lastMonthTotalExp, currentMonthTotalExp to extract values from the single query result (using transactions, sum, and the same query arrays) so the dashboard uses consistent aggregates and avoids scanning the same slice four times.
🤖 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/lib/actions.ts`:
- Line 519: The predicate gte(budgets.monthlyLimit, budgets.spent) uses the
pre-update spent value and prevents further expense accumulation after a budget
first goes over limit; instead compute the prospective/post-update spent value
and compare that to the limit. Replace the direct check with a comparison
against budgets.spent plus the incoming delta (e.g., compute newSpent =
budgets.spent + amount/spentDelta from the update or join the pending
transaction amount) and use gte(budgets.monthlyLimit, newSpent) (or remove the
predicate and enforce limits via the service using budgets.spent) so that the
check reflects the post-update total rather than the old value.
---
Outside diff comments:
In `@app/lib/actions.ts`:
- Around line 463-468: Remove the stale optimistic pre-check that returns when
type === "EXPENSE" and userCurrentBalance < amount; instead delete that early
if/return block and let the concurrency-safe guarded update handle insufficient
funds by inspecting updateResult (the DB update that decrements balance). Ensure
any user-facing "Insufficient balance" response is produced based on the
updateResult outcome (failed/no rows affected) rather than userCurrentBalance,
keeping variables referenced like type, userCurrentBalance, amount and
updateResult to locate the code to change.
---
Duplicate comments:
In `@app/lib/actions.ts`:
- Around line 517-521: The UPDATE currently filters only by userId and
categoryId so it can bump the wrong month's row; compute txMonth from the
transaction date (use new Date(date).toISOString().slice(0,7) + "-01") and add
eq(budgets.month, txMonth) to the predicates alongside eq(budgets.userId,
userId) and eq(budgets.categoryId, categoryId) so the update on
budgets.monthlyLimit / budgets.spent targets the correct month row.
---
Nitpick comments:
In `@app/lib/services.ts`:
- Around line 221-259: The four separate SUM queries (using
db.select(...).from(transactions).where(...)) must be collapsed into a single
consistent read—either wrap them in a database read transaction or, better,
replace them with one grouped/conditional aggregate that computes all four
totals in one roundtrip (e.g., a single select with conditional SUMs or GROUP BY
on transaction type + month using lastMonthQueryArr and currentMonthQueryArr as
filters). Update the code that assigns lastMonthTotalInc, currentMonthTotalInc,
lastMonthTotalExp, currentMonthTotalExp to extract values from the single query
result (using transactions, sum, and the same query arrays) so the dashboard
uses consistent aggregates and avoids scanning the same slice four times.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bdc9be4d-b45f-4cba-b3e3-e174f7ce0356
📒 Files selected for processing (2)
app/lib/actions.tsapp/lib/services.ts
added real-time data fetching to dashboard statistics
Summary by CodeRabbit