FIN-25: Implement backend for existing screens #69
Conversation
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughComplete refactor of the database schema from a single "welcomes" table to a relational multi-table structure (accounts, categories, currencies, transactions, users), with corresponding updates to server routers, type definitions, authentication integration, UI components, and localization to support a more sophisticated financial application architecture. Changes
Sequence DiagramsequenceDiagram
participant User
participant App
participant Clerk
participant TRPC
participant DB
rect rgb(200, 220, 240)
Note over User,DB: Account Creation Flow
User->>App: Submit account form (bankName, currentBalance, reference, usage)
App->>TRPC: accounts.addAccount()
TRPC->>Clerk: Fetch current user context
Clerk-->>TRPC: Return ClerkUser
TRPC->>DB: Check if user exists in users table
alt User not in DB
TRPC->>DB: Insert new user row
end
TRPC->>DB: Resolve EUR currency ID
TRPC->>DB: INSERT new account (userId, bankName, currentBalance, reference, usage, currencyId)
DB-->>TRPC: Return inserted account + currency details
TRPC-->>App: AccountWithCurrency (account + currency data)
App->>User: Navigate to /(tabs)/banking
end
rect rgb(240, 200, 220)
Note over User,DB: Account Retrieval Flow
User->>App: Load banking dashboard
App->>TRPC: accounts.getAccounts.useQuery()
TRPC->>DB: SELECT accounts JOIN currencies WHERE userId = ?
DB-->>TRPC: Return AccountWithCurrency[]
TRPC-->>App: Filtered list of accounts
alt Accounts exist
App->>App: Pre-filter into reference buckets (Private, Business, Savings, Shared)
App->>User: Render AccountSections with staggered animations
else No accounts
App->>User: Redirect to /start onboarding
end
end
rect rgb(200, 240, 220)
Note over User,DB: Root Route Auth Redirect Logic
User->>App: Load / (root)
App->>TRPC: Check if signed in + fetch accounts
alt Signed in + accounts loaded + empty
App->>User: Redirect to /start
else Signed in + accounts exist
App->>User: Redirect to /(tabs)/banking
else Not signed in
App->>User: Render marketing/landing page
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
- Added account types to the database schema and created related migrations. - Introduced new API routes for managing accounts and retrieving user data. - Updated the frontend to handle account creation and retrieval with type support. - Added error handling for account-related operations. - Created a backend development guide for consistency and best practices.
- Enhanced the backend development documentation with additional guidelines and best practices. - Improved code formatting for better readability in various files, including schema and router definitions. - Added missing newlines in JSON files for consistency. - Updated mock data structure to align with new account properties.
…Group/finance-io into federicoleiva/fin-25-implement-backend-for-existing-screens
FedericoLeiva12
changed the title
- Changed the property name from 'bankName' to 'name' in the mock dashboard data to align with the updated data structure.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/data/mockData.ts (1)
35-47: Inconsistent property: mix ofbankNameandnamewill break consumers.Unify all account entries to use the same key. Since you introduced
bankName, align the rest.{ id: "2", - name: "DKB", + bankName: "DKB", amount: "232.93€", icon: React.createElement(DKBIcon), type: "dkb", }, ... { id: "3", - name: "Revolut", + bankName: "Revolut", amount: "12.232.93€", icon: React.createElement(RevolutIcon), type: "revolut", }, ... { id: "4", - name: "Depot", + bankName: "Depot", amount: "43.232.93€", icon: React.createElement(LandmarkIcon), type: "depot", },Optionally annotate
mockDashboardDatawith a type to catch these mismatches early (or remove the unusedAccountimport).Also applies to: 50-66
🧹 Nitpick comments (13)
src/db/schema.ts (1)
79-80: Consider adding updatedAt timestamps to track modifications.Add updatedAt to accounts and transactions for auditing and cache invalidation.
createdAt: timestamp({ withTimezone: true }).defaultNow().notNull(), + updatedAt: timestamp({ withTimezone: true }).defaultNow().notNull(),Note: Without DB triggers, update this field at write time in your services.
Also applies to: 103-104
src/app/index.tsx (1)
158-164: Add a basic loading fallback for UX.Show a spinner while accounts load.
- /// TODO: Add loading state - // if (isLoadingAccounts) { - // return <View className="flex-1 items-center justify-center"> - // <ActivityIndicator size="large" color={iconColor} /> - // </View>; - // } + if (isLoadingAccounts) { + return ( + <View className="flex-1 items-center justify-center"> + <ActivityIndicator size="large" color={iconColor} /> + </View> + ); + }Add import:
-import { - View, - ScrollView, - useColorScheme, - TouchableOpacity, - Modal, -} from "react-native"; +import { + View, + ScrollView, + useColorScheme, + TouchableOpacity, + Modal, + ActivityIndicator, +} from "react-native";src/scripts/seed.ts (3)
27-34: Make inserts idempotent and race‑safe withonConflictDoNothing().Avoid unique violations and extra reads by letting Postgres handle conflicts on unique keys.
[Based on learnings]
- if (toInsert.length > 0) { - await db.insert(currenciesTable).values(toInsert); + if (toInsert.length > 0) { + await db.insert(currenciesTable).values(toInsert).onConflictDoNothing(); console.log( `Inserted currencies: ${toInsert.map((c) => c.code).join(", ")}`, ); } else {
55-62: Apply the same idempotent pattern to categories.- if (toInsert.length > 0) { - await db.insert(categoriesTable).values(toInsert); + if (toInsert.length > 0) { + await db.insert(categoriesTable).values(toInsert).onConflictDoNothing(); console.log( `Inserted categories: ${toInsert.map((c) => c.slug).join(", ")}`, ); } else {
82-89: Apply the same idempotent pattern to account types.- if (toInsert.length > 0) { - await db.insert(accountTypesTable).values(toInsert); + if (toInsert.length > 0) { + await db.insert(accountTypesTable).values(toInsert).onConflictDoNothing(); console.log( `Inserted account types: ${toInsert.map((c) => c.slug).join(", ")}`, ); } else {src/errors/trpc.ts (1)
4-6: Make notAuthenticated accept an optional message (API consistency)Align with other factories to allow overrides.
- notAuthenticated: () => - new TRPCError({ code: "UNAUTHORIZED", message: "Not authenticated" }), + notAuthenticated: (message: string = "Not authenticated") => + new TRPCError({ code: "UNAUTHORIZED", message }),src/server/routers/transactions.ts (5)
13-21: Tighten input validation (uuid, decimal, coerce date, non-empty strings)Prevents bad data and client Date serialization issues.
- .input( - z.object({ - categoryId: z.string(), - company: z.string(), - amount: z.string(), - datetime: z.date(), - description: z.string().optional(), - accountId: z.string(), - }), - ) + .input( + z.object({ + categoryId: z.string().uuid(), + company: z.string().min(1), + amount: z.string().regex(/^\d+(\.\d{1,2})?$/, "Amount must be a decimal with up to 2 fraction digits"), + datetime: z.coerce.date(), + description: z.string().optional(), + accountId: z.string().uuid(), + }), + )
85-93: Tighten getTransactions input validation (uuid, coerce date)- .input( - z.object({ - accountId: z.string().optional(), - categoryId: z.string().optional(), - startDate: z.date().optional(), - endDate: z.date().optional(), - }), - ) + .input( + z.object({ + accountId: z.string().uuid().optional(), + categoryId: z.string().uuid().optional(), + startDate: z.coerce.date().optional(), + endDate: z.coerce.date().optional(), + }), + )
95-111: Validate date range before queryingReject inverted ranges early.
- .query( - async ({ ctx, input }): Promise<TransactionWithCategoryAndAccount[]> => { + .query( + async ({ ctx, input }): Promise<TransactionWithCategoryAndAccount[]> => { + if (input.startDate && input.endDate && input.startDate > input.endDate) { + throw errors.badRequest("Invalid date range: startDate must be <= endDate"); + } if (input.accountId) {
4-5: Return results in a deterministic order (e.g., newest first)Improves UX and test stability.
-import { eq, gte, lte, and } from "drizzle-orm"; +import { eq, gte, lte, and, desc } from "drizzle-orm"; @@ - const qb = db + const qb = db .select({ transactions: transactionsTable, categories: categoriesTable, accounts: accountsTable, }) .from(transactionsTable) .innerJoin( accountsTable, eq(transactionsTable.accountId, accountsTable.id), ) .innerJoin( categoriesTable, eq(transactionsTable.categoryId, categoriesTable.id), - ); + ) + .orderBy(desc(transactionsTable.datetime));Also applies to: 135-150
6-6: Use type-only imports forTransactionWithCategoryAndAccount-import { TransactionWithCategoryAndAccount } from "~/types"; +import type { TransactionWithCategoryAndAccount } from "~/types";Enable
"verbatimModuleSyntax": truein tsconfig.json to enforce stripping of type-only imports at compile time.src/server/routers/accounts.ts (2)
13-19: Strengthen input validation (uuid for typeId, decimal for currentBalance, non-empty strings)- z.object({ - bankName: z.string(), - currentBalance: z.string(), - reference: z.string(), - usage: z.string(), - typeId: z.string().optional(), - }), + z.object({ + bankName: z.string().min(1), + currentBalance: z + .string() + .regex(/^\d+(\.\d{1,2})?$/, "Balance must be a decimal with up to 2 fraction digits"), + reference: z.string().min(1), + usage: z.string().min(1), + typeId: z.string().uuid().optional(), + }),
23-31: Avoid hard-coding currency; allow configuration or inputConsider reading a default from config/env, or accept currencyCode in input.
- const currency = await db + const defaultCurrencyCode = process.env.DEFAULT_CURRENCY_CODE ?? "EUR"; + const currency = await db .select() .from(currenciesTable) - .where(eq(currenciesTable.code, "EUR")) + .where(eq(currenciesTable.code, defaultCurrencyCode)) .limit(1); // Hardcoded to EURIf you want to keep it hard-coded for now, add a TODO explaining when it will become configurable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
docs/backend-development.md(1 hunks)drizzle/0000_mute_thunderbolt_ross.sql(0 hunks)drizzle/0000_unique_mercury.sql(1 hunks)drizzle/0001_nostalgic_master_mold.sql(1 hunks)drizzle/0001_tense_satana.sql(0 hunks)drizzle/0002_icy_madrox.sql(0 hunks)drizzle/meta/0000_snapshot.json(1 hunks)drizzle/meta/0001_snapshot.json(2 hunks)drizzle/meta/0002_snapshot.json(0 hunks)drizzle/meta/_journal.json(1 hunks)package.json(1 hunks)src/app/(auth)/sign-in.tsx(1 hunks)src/app/(tabs)/banking/index.tsx(3 hunks)src/app/index.tsx(2 hunks)src/app/start/index.tsx(2 hunks)src/data/mockData.ts(2 hunks)src/db/schema.ts(1 hunks)src/errors/messages.ts(1 hunks)src/errors/trpc.ts(1 hunks)src/index.ts(1 hunks)src/scripts/seed.ts(1 hunks)src/server/api/root.ts(1 hunks)src/server/api/trpc.ts(2 hunks)src/server/index.ts(1 hunks)src/server/routers/accounts.ts(1 hunks)src/server/routers/categories.ts(1 hunks)src/server/routers/post.ts(0 hunks)src/server/routers/transactions.ts(1 hunks)src/server/routers/users.ts(1 hunks)src/server/routers/welcome.ts(0 hunks)src/types/index.ts(1 hunks)
💤 Files with no reviewable changes (6)
- drizzle/0002_icy_madrox.sql
- drizzle/0000_mute_thunderbolt_ross.sql
- src/server/routers/welcome.ts
- drizzle/meta/0002_snapshot.json
- src/server/routers/post.ts
- drizzle/0001_tense_satana.sql
🧰 Additional context used
📓 Path-based instructions (6)
src/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.tsx: Use instead of from react-native in UI components
All styling should use TailwindCSS classes via NativeWind (avoid inline StyleSheet definitions)
Files:
src/app/(auth)/sign-in.tsxsrc/app/(tabs)/banking/index.tsxsrc/app/index.tsxsrc/app/start/index.tsx
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow TypeScript strict mode requirements across the codebase
Files:
src/app/(auth)/sign-in.tsxsrc/server/routers/categories.tssrc/app/(tabs)/banking/index.tsxsrc/server/routers/accounts.tssrc/errors/trpc.tssrc/data/mockData.tssrc/server/routers/transactions.tssrc/index.tssrc/server/routers/users.tssrc/errors/messages.tssrc/app/index.tsxsrc/scripts/seed.tssrc/db/schema.tssrc/app/start/index.tsxsrc/server/api/root.tssrc/server/api/trpc.tssrc/types/index.tssrc/server/index.ts
src/app/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Expo Router file-based routing; screens and routes live under src/app/
Files:
src/app/(auth)/sign-in.tsxsrc/app/(tabs)/banking/index.tsxsrc/app/index.tsxsrc/app/start/index.tsx
src/server/routers/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Define tRPC procedures/routers under src/server/routers/
Files:
src/server/routers/categories.tssrc/server/routers/accounts.tssrc/server/routers/transactions.tssrc/server/routers/users.ts
src/{server,db}/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database operations must use Drizzle ORM queries (no raw SQL/ad-hoc clients)
Files:
src/server/routers/categories.tssrc/server/routers/accounts.tssrc/server/routers/transactions.tssrc/server/routers/users.tssrc/db/schema.tssrc/server/api/root.tssrc/server/api/trpc.tssrc/server/index.ts
src/db/schema.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain the database schema in src/db/schema.ts
Files:
src/db/schema.ts
🧠 Learnings (2)
📚 Learning: 2025-08-24T14:28:33.804Z
Learnt from: CR
PR: The-Creative-Programming-Group/finance-io#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-24T14:28:33.804Z
Learning: Applies to src/server/routers/**/*.ts : Define tRPC procedures/routers under src/server/routers/
Applied to files:
src/server/routers/categories.tssrc/server/routers/accounts.tssrc/server/routers/transactions.tsdocs/backend-development.mdsrc/server/routers/users.tssrc/server/api/root.tssrc/server/index.ts
📚 Learning: 2025-08-24T14:28:33.804Z
Learnt from: CR
PR: The-Creative-Programming-Group/finance-io#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-24T14:28:33.804Z
Learning: Applies to src/db/schema.ts : Maintain the database schema in src/db/schema.ts
Applied to files:
src/db/schema.ts
🧬 Code graph analysis (11)
src/app/(auth)/sign-in.tsx (1)
src/app/(auth)/sign-up.tsx (1)
router(293-293)
src/server/routers/categories.ts (4)
src/server/api/trpc.ts (2)
createTRPCRouter(114-114)protectedProcedure(131-148)src/types/index.ts (1)
Category(27-31)src/db/index.ts (1)
db(6-6)src/db/schema.ts (1)
categoriesTable(31-41)
src/server/routers/accounts.ts (6)
src/server/api/trpc.ts (2)
createTRPCRouter(114-114)protectedProcedure(131-148)src/types/index.ts (1)
AccountWithCurrencyAndType(38-41)src/db/index.ts (1)
db(6-6)src/db/schema.ts (3)
currenciesTable(18-28)accountTypesTable(44-54)accountsTable(57-86)src/errors/trpc.ts (1)
errors(3-18)src/errors/messages.ts (1)
error_messages(1-14)
src/server/routers/transactions.ts (6)
src/server/api/trpc.ts (2)
createTRPCRouter(114-114)protectedProcedure(131-148)src/types/index.ts (1)
TransactionWithCategoryAndAccount(53-56)src/db/index.ts (1)
db(6-6)src/db/schema.ts (3)
accountsTable(57-86)transactionsTable(89-110)categoriesTable(31-41)src/errors/trpc.ts (1)
errors(3-18)src/errors/messages.ts (1)
error_messages(1-14)
src/index.ts (2)
src/db/index.ts (1)
db(6-6)src/db/schema.ts (2)
currenciesTable(18-28)accountsTable(57-86)
src/server/routers/users.ts (4)
src/server/api/trpc.ts (1)
protectedProcedure(131-148)src/types/index.ts (1)
User(3-9)src/db/index.ts (1)
db(6-6)src/db/schema.ts (1)
usersTable(13-15)
src/app/index.tsx (1)
src/utils/trpc.ts (1)
trpc(4-4)
src/scripts/seed.ts (2)
src/db/index.ts (1)
db(6-6)src/db/schema.ts (3)
currenciesTable(18-28)categoriesTable(31-41)accountTypesTable(44-54)
src/app/start/index.tsx (1)
src/utils/trpc.ts (1)
trpc(4-4)
src/server/api/root.ts (3)
src/server/api/trpc.ts (1)
createTRPCRouter(114-114)src/server/routers/users.ts (1)
usersRouter(7-30)src/server/routers/accounts.ts (1)
accountsRouter(10-100)
src/server/api/trpc.ts (1)
src/errors/trpc.ts (1)
errors(3-18)
🔇 Additional comments (14)
src/app/(auth)/sign-in.tsx (1)
100-103: Redirect to root after sign‑in — LGTMCentralizing post‑login routing via the index screen matches the new accounts‑gating flow.
src/db/schema.ts (1)
12-110: New normalized schema with UUIDs, FKs, and indexes — solidGood use of decimal(mode:"string"), uniqueness, and relationship indexes. This aligns well with Drizzle’s best practices.
package.json (1)
10-13: Approve DB tooling scripts
drizzle.config.ts and src/scripts/seed.ts confirmed present and correct.drizzle/meta/_journal.json (1)
8-10: Journal tags match existing migration files. Both SQL files are present; no migration drift detected.src/data/mockData.ts (1)
11-12: Track/remove mock data soon.There’s a TODO to remove mocks. Ensure replacement is wired before merge to avoid stale UI.
src/server/index.ts (1)
1-2: LGTM: centralized re‑export.Keeps the entrypoint thin and aligned with router composition.
src/types/index.ts (2)
20-20: String type for monetary values is appropriate.Using
stringforcurrentBalance(andamounton line 47) is a good practice to avoid floating-point precision issues with monetary values. This aligns with the database'snumeric(18, 2)type, which will be serialized as a string by most PostgreSQL drivers.
38-56: Composite types align well with expected API responses.The
AccountWithCurrencyAndTypeandTransactionWithCategoryAndAccountinterfaces provide clean shapes for joined data from the database. This matches the relational model established in the migrations and will simplify data fetching in tRPC routers.drizzle/0000_unique_mercury.sql (3)
39-42: Foreign key cascades and restrictions are well-designed.The ON DELETE policies are appropriate:
cascadefor user-owned data (accounts, transactions) ensures cleanup when a user is deletedrestrictfor reference data (currencies, categories) prevents accidental deletion of entities still in useThis protects data integrity while allowing proper cleanup of user-specific resources.
43-49: Indexes support expected query patterns.The index strategy is comprehensive:
- User and foreign key indexes (accounts_user_idx, accounts_currency_idx, transactions_account_idx, transactions_category_idx) optimize joins and filtered queries
- Unique indexes on categories.slug and currencies.code prevent duplicates and enable efficient lookups
- transactions_datetime_idx supports time-range queries
This should provide good performance for typical banking app queries (user accounts, transaction history, category breakdowns).
1-10: Ensure migrations run before code referencing new column
Application code (e.g. src/index.ts, src/server/routers/accounts.ts, src/db/schema.ts) referencesaccounts.typeId, but this migration createsaccountswithout that column until 0001. Confirm migrations 0000 and 0001 run atomically before app startup to avoid missing-column errors.drizzle/meta/0000_snapshot.json (1)
1-375: Snapshot correctly reflects migration 0000.This auto-generated snapshot accurately documents the database state after applying migration 0000_unique_mercury.sql. The schema definition matches the SQL migration, including:
- accounts table without typeId (added in migration 0001)
- All foreign keys and indexes as defined
- Proper use of UUID and varchar types
No issues detected.
drizzle/meta/0001_snapshot.json (1)
1-453: Snapshot correctly reflects complete schema with account_types.This auto-generated snapshot accurately documents the database state after applying migration 0001_nostalgic_master_mold.sql. Key additions:
account_typestable with id, name, slug fields (lines 7-54)typeIdcolumn added to accounts (lines 96-101)accounts_type_idxindex for typeId lookups (lines 147-161)- Foreign key constraint linking accounts.typeId to account_types.id (lines 173-181)
The schema now fully aligns with the TypeScript type definitions in src/types/index.ts.
src/server/routers/accounts.ts (1)
5-5: Import types usingimport typeChange the import to a type-only import to ensure it’s erased at compile time and avoid runtime overhead:
-import { AccountWithCurrencyAndType } from "~/types"; +import type { AccountWithCurrencyAndType } from "~/types";
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
drizzle/meta/0001_snapshot.json (1)
13-114: Addpgcryptoextension before usinggen_random_uuid().All UUID defaults here rely on
gen_random_uuid(), but none of the migrations create thepgcryptoextension that provides that function. On a fresh Postgres instance these tables will fail to migrate becausegen_random_uuidis undefined. Please add a migration at the top of the stack withCREATE EXTENSION IF NOT EXISTS "pgcrypto";(or switch touuid_generate_v4()withuuid-ossp).Also applies to: 198-347
🧹 Nitpick comments (9)
package.json (1)
10-13: LGTM! Consider documenting the new scripts.The new database scripts follow standard Drizzle ORM conventions and enable essential workflows (seeding, migrations, schema generation, Studio UI). Ensure that
src/scripts/seed.tsexists and that the team is aware of when to run these scripts (e.g.,pnpm seedfor initial data,pnpm migrateafter schema changes).Consider adding a brief comment in the README or docs explaining:
- When to run
pnpm seed(initial setup, after migrations)- When to run
pnpm migrate(after runningpnpm generate)- What
pnpm studioprovides (Drizzle Studio UI for inspecting data)src/errors/messages.ts (1)
1-14: Consider whether error message values should differ from keys.The current structure has error message values that mirror their keys (e.g.,
forbidden: "forbidden"). This pattern works for internal error codes but might not be ideal if these messages are ever displayed to users or logged for debugging.If these are meant to be user-facing or logged for diagnostics, consider using descriptive messages:
export const error_messages = { // Generic - forbidden: "forbidden", - notFound: "notFound", - badRequest: "badRequest", + forbidden: "Access forbidden", + notFound: "Resource not found", + badRequest: "Invalid request", // Accounts - currencyNotFound: "currencyNotFound", - accountTypeNotFound: "accountTypeNotFound", - accountNotOwned: "accountNotOwned", + currencyNotFound: "Currency not found", + accountTypeNotFound: "Account type not found", + accountNotOwned: "Account does not belong to user", // Transactions - transactionNotFound: "transactionNotFound", + transactionNotFound: "Transaction not found", } as const;If the current pattern is intentional (e.g., for i18n keys), then this comment can be ignored.
drizzle/0000_unique_mercury.sql (1)
1-49: LGTM! Schema design is sound.The migration establishes a well-structured relational schema with appropriate:
- Primary keys (UUIDs with
gen_random_uuid())- Foreign key constraints (cascade for user/account deletion, restrict for reference data)
- Indexes on commonly queried fields (userId, currencyId, accountId, categoryId, datetime)
- Unique constraints on business keys (categories.slug, currencies.code)
Minor suggestions:
- The
accounts.usageandaccounts.referencefield names are somewhat generic. Consider renaming for clarity (e.g.,accountPurpose,referenceNumberorexternalReference).- Add a newline at the end of the file per SQL conventions.
These are low-priority refinements and can be addressed in a future migration if needed.
src/app/index.tsx (1)
144-155: Gate the accounts query on authentication
protectedProcedurerejects unauthenticated calls, but we still fire the query whileisSignedInis false, causing needless 401s/local errors. Guard the query withenabled: !!isSignedIn(and optionally reset on sign-out) so we only hit the endpoint after Clerk confirms the session.- const { data: accounts, isLoading: isLoadingAccounts } = - trpc.accounts.getAccounts.useQuery(); + const { + data: accounts, + isLoading: isLoadingAccounts, + } = trpc.accounts.getAccounts.useQuery(undefined, { + enabled: !!isSignedIn, + });docs/backend-development.md (1)
78-107: Fix example:and(...)doesn’t support.filter.
and()returns an SQL expression, so calling.filter(Boolean)on its result will throw. Build the array first, filter it, then spread intoand. Example fix:-const whereExpr = and( - eq(accountsTable.userId, ctx.userId!), - input.accountId - ? eq(transactionsTable.accountId, input.accountId) - : undefined, - input.categoryId - ? eq(transactionsTable.categoryId, input.categoryId) - : undefined, - input.startDate - ? gte(transactionsTable.datetime, input.startDate) - : undefined, - input.endDate ? lte(transactionsTable.datetime, input.endDate) : undefined, -).filter(Boolean) as any; // build conditionally, or build stepwise without arrays +const whereConditions = [ + eq(accountsTable.userId, ctx.userId!), + input.accountId + ? eq(transactionsTable.accountId, input.accountId) + : undefined, + input.categoryId + ? eq(transactionsTable.categoryId, input.categoryId) + : undefined, + input.startDate + ? gte(transactionsTable.datetime, input.startDate) + : undefined, + input.endDate ? lte(transactionsTable.datetime, input.endDate) : undefined, +].filter((expr): expr is NonNullable<typeof expr> => Boolean(expr)); + +const whereExpr = + whereConditions.length > 0 ? and(...whereConditions) : undefined;This keeps the snippet copy-pastable for readers.
src/server/routers/accounts.ts (2)
13-19: Tighten input validation (min lengths, decimal format, UUID)Current zod schema allows empty strings and any string for balances. Enforce minimal constraints and correct types.
- z.object({ - bankName: z.string(), - currentBalance: z.string(), - reference: z.string(), - usage: z.string(), - typeId: z.string().optional(), - }), + z.object({ + bankName: z.string().min(1), + // 18,2 decimal as string (optional leading minus) + currentBalance: z + .string() + .regex(/^-?\d{1,16}(\.\d{1,2})?$/, "Invalid decimal format"), + reference: z.string().min(1), + usage: z.string().min(1), + typeId: z.string().uuid().optional(), + }),
23-30: Parameterize hardcoded currency codeEUR is currently seeded but still hardcoded; accept a
currencyCode: string(orcurrencyId: uuid) in the input—defaulting to “EUR” when omitted—to avoid future 404s.src/server/routers/transactions.ts (2)
41-51: Pre-check category existence or map FK errors to TRPC errorsCurrently relies on FK to fail, bubbling a 500. Either pre-check category or catch DB error and respond with NOT_FOUND.
Example pre-check pattern:
const cat = await db .select({ id: categoriesTable.id }) .from(categoriesTable) .where(eq(categoriesTable.id, input.categoryId)) .limit(1); if (cat.length === 0) { throw errors.notFound(error_messages.notFound); }
15-21: Strengthen transaction router schemaReplace basic string/date validators with stricter checks:
--- src/server/routers/transactions.ts @@ -15,7 +15,13 @@ - categoryId: z.string(), - company: z.string(), - amount: z.string(), - datetime: z.date(), + categoryId: z.string().uuid(), + company: z.string().min(1), + amount: z + .string() + .regex(/^-?\d{1,16}(?:\.\d{1,2})?$/, "Invalid decimal format"), + datetime: z + .union([z.date(), z.string().datetime()]) + .transform(v => (v instanceof Date ? v : new Date(v))), description: z.string().optional(), - accountId: z.string(), + accountId: z.string().uuid(),Project already uses superjson for TRPC, so ISO date strings will be correctly handled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
docs/backend-development.md(1 hunks)drizzle/0000_mute_thunderbolt_ross.sql(0 hunks)drizzle/0000_unique_mercury.sql(1 hunks)drizzle/0001_nostalgic_master_mold.sql(1 hunks)drizzle/0001_tense_satana.sql(0 hunks)drizzle/0002_icy_madrox.sql(0 hunks)drizzle/meta/0000_snapshot.json(1 hunks)drizzle/meta/0001_snapshot.json(2 hunks)drizzle/meta/0002_snapshot.json(0 hunks)drizzle/meta/_journal.json(1 hunks)package.json(1 hunks)src/app/(auth)/sign-in.tsx(1 hunks)src/app/(tabs)/banking/index.tsx(3 hunks)src/app/index.tsx(2 hunks)src/app/start/index.tsx(2 hunks)src/data/mockData.ts(1 hunks)src/db/schema.ts(1 hunks)src/errors/messages.ts(1 hunks)src/errors/trpc.ts(1 hunks)src/index.ts(1 hunks)src/scripts/seed.ts(1 hunks)src/server/api/root.ts(1 hunks)src/server/api/trpc.ts(2 hunks)src/server/index.ts(1 hunks)src/server/routers/accounts.ts(1 hunks)src/server/routers/categories.ts(1 hunks)src/server/routers/post.ts(0 hunks)src/server/routers/transactions.ts(1 hunks)src/server/routers/users.ts(1 hunks)src/server/routers/welcome.ts(0 hunks)src/types/index.ts(1 hunks)
💤 Files with no reviewable changes (6)
- drizzle/0001_tense_satana.sql
- src/server/routers/welcome.ts
- drizzle/0002_icy_madrox.sql
- src/server/routers/post.ts
- drizzle/0000_mute_thunderbolt_ross.sql
- drizzle/meta/0002_snapshot.json
🧰 Additional context used
📓 Path-based instructions (6)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow TypeScript strict mode requirements across the codebase
Files:
src/errors/messages.tssrc/scripts/seed.tssrc/errors/trpc.tssrc/server/routers/accounts.tssrc/app/(auth)/sign-in.tsxsrc/index.tssrc/app/(tabs)/banking/index.tsxsrc/data/mockData.tssrc/server/routers/users.tssrc/server/index.tssrc/app/index.tsxsrc/server/api/root.tssrc/server/routers/categories.tssrc/server/api/trpc.tssrc/server/routers/transactions.tssrc/app/start/index.tsxsrc/types/index.tssrc/db/schema.ts
src/server/routers/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Define tRPC procedures/routers under src/server/routers/
Files:
src/server/routers/accounts.tssrc/server/routers/users.tssrc/server/routers/categories.tssrc/server/routers/transactions.ts
src/{server,db}/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database operations must use Drizzle ORM queries (no raw SQL/ad-hoc clients)
Files:
src/server/routers/accounts.tssrc/server/routers/users.tssrc/server/index.tssrc/server/api/root.tssrc/server/routers/categories.tssrc/server/api/trpc.tssrc/server/routers/transactions.tssrc/db/schema.ts
src/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.tsx: Use instead of from react-native in UI components
All styling should use TailwindCSS classes via NativeWind (avoid inline StyleSheet definitions)
Files:
src/app/(auth)/sign-in.tsxsrc/app/(tabs)/banking/index.tsxsrc/app/index.tsxsrc/app/start/index.tsx
src/app/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Expo Router file-based routing; screens and routes live under src/app/
Files:
src/app/(auth)/sign-in.tsxsrc/app/(tabs)/banking/index.tsxsrc/app/index.tsxsrc/app/start/index.tsx
src/db/schema.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain the database schema in src/db/schema.ts
Files:
src/db/schema.ts
🧠 Learnings (1)
📚 Learning: 2025-08-24T14:28:33.804Z
Learnt from: CR
PR: The-Creative-Programming-Group/finance-io#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-24T14:28:33.804Z
Learning: Applies to src/server/routers/**/*.ts : Define tRPC procedures/routers under src/server/routers/
Applied to files:
src/server/routers/accounts.tssrc/server/routers/users.tsdocs/backend-development.mdsrc/server/index.tssrc/server/api/root.tssrc/server/routers/categories.tssrc/server/routers/transactions.ts
🧬 Code graph analysis (10)
src/scripts/seed.ts (2)
src/db/index.ts (1)
db(6-6)src/db/schema.ts (3)
currenciesTable(18-28)categoriesTable(31-41)accountTypesTable(44-54)
src/server/routers/accounts.ts (6)
src/server/api/trpc.ts (2)
createTRPCRouter(114-114)protectedProcedure(131-148)src/types/index.ts (1)
AccountWithCurrencyAndType(38-41)src/db/index.ts (1)
db(6-6)src/db/schema.ts (3)
currenciesTable(18-28)accountTypesTable(44-54)accountsTable(57-86)src/errors/trpc.ts (1)
errors(3-18)src/errors/messages.ts (1)
error_messages(1-14)
src/index.ts (2)
src/db/index.ts (1)
db(6-6)src/db/schema.ts (2)
currenciesTable(18-28)accountsTable(57-86)
src/server/routers/users.ts (4)
src/server/api/trpc.ts (2)
createTRPCRouter(114-114)protectedProcedure(131-148)src/types/index.ts (1)
User(3-9)src/db/index.ts (1)
db(6-6)src/db/schema.ts (1)
usersTable(13-15)
src/app/index.tsx (1)
src/utils/trpc.ts (1)
trpc(4-4)
src/server/api/root.ts (3)
src/server/api/trpc.ts (1)
createTRPCRouter(114-114)src/server/routers/users.ts (1)
usersRouter(7-30)src/server/routers/accounts.ts (1)
accountsRouter(10-100)
src/server/routers/categories.ts (4)
src/server/api/trpc.ts (2)
createTRPCRouter(114-114)protectedProcedure(131-148)src/types/index.ts (1)
Category(27-31)src/db/index.ts (1)
db(6-6)src/db/schema.ts (1)
categoriesTable(31-41)
src/server/api/trpc.ts (1)
src/errors/trpc.ts (1)
errors(3-18)
src/server/routers/transactions.ts (6)
src/server/api/trpc.ts (2)
createTRPCRouter(114-114)protectedProcedure(131-148)src/types/index.ts (1)
TransactionWithCategoryAndAccount(53-56)src/db/index.ts (1)
db(6-6)src/db/schema.ts (3)
accountsTable(57-86)transactionsTable(89-110)categoriesTable(31-41)src/errors/trpc.ts (1)
errors(3-18)src/errors/messages.ts (1)
error_messages(1-14)
src/app/start/index.tsx (1)
src/utils/trpc.ts (1)
trpc(4-4)
🔇 Additional comments (9)
src/app/(auth)/sign-in.tsx (1)
102-102: LGTM! Redirect aligns with new account-state routing.The redirect to
"/"is appropriate given that the index route now handles conditional navigation based on account state. This change aligns with the broader first-login flow refactoring described in the PR objectives.src/app/(tabs)/banking/index.tsx (1)
64-92: LGTM! Defensive coding pattern is appropriate.The addition of optional chaining (
account?.name) and fallback empty strings prevents potential runtime errors if thenamefield is undefined or null. This pattern is applied consistently across private, business, and safe account sections.Note: The PR TODO mentions refactoring this screen to use real backend data instead of
mockDashboardData. Once that work is completed, ensure the backend API guarantees thatnameis always present, which would allow removal of the fallback logic.src/app/start/index.tsx (3)
47-47: Verify the unuseduserDataquery result.The
trpc.users.getUser.useQuery()is called, but the returneduserDatais not used elsewhere in this component. This might be:
- Intentionally prefetching user data for caching
- A leftover from refactoring that can be removed
- Intended for future use
If this query is not needed, consider removing it to avoid unnecessary API calls:
- const { data: userData } = trpc.users.getUser.useQuery(); -If it's intentionally prefetching data, consider adding a comment explaining the purpose.
49-59: LGTM! Backend integration updated correctly.The mutation change from the old API to
accounts.addAccountaligns with the new backend routers. The success callback appropriately resets the form, clears the amount display, and navigates to the banking tab.
79-82: LGTM! Field name updated to match backend schema.The change from
currentAmounttocurrentBalancealigns with the backend schema defined in the migration. The form still usescurrentAmountinternally (in the form state), which is fine—the transformation happens only at the mutation boundary.src/server/api/trpc.ts (2)
12-22: LGTM! Clerk client initialization is correct.The imports and initialization of
clerkClientfollow Clerk's recommended patterns. ThesecretKeyis appropriately sourced from the environment variable.
131-148: LGTM! Authentication middleware properly integrated.The
protectedProcedurecorrectly:
- Validates
ctx.userIdpresence- Fetches the full Clerk user object
- Throws
notAuthenticated()errors when auth fails- Augments the context with the verified
userobjectOne consideration: The
clerkClient.users.getUser()call adds latency to every protected procedure. If user data is frequently needed but doesn't change often, consider:
- Caching the user object in the request context or a short-lived cache
- Only fetching user details when required by specific procedures
However, this optimization is optional and can be deferred until performance becomes a concern. For now, the straightforward approach is appropriate.
src/db/schema.ts (2)
56-86: Schema structure looks solid (FKs, indexes, decimal-as-string)Good use of uuid PKs, decimal mode "string", and targeted indexes. Aligns with Drizzle best practices.
88-110: Nice transaction model and indexesAppropriate FKs and datetime index for range queries. No issues.
FleetAdmiralJakob
requested review from
FleetAdmiralJakob,
Gamius00 and
murtazanoori
- Refactored the query conditions in the transactions router for better readability and maintainability by using an array to build conditions. - Updated the user retrieval logic in the users router to use `onConflictDoNothing` for inserting users, ensuring no duplicates are created. - Enhanced the backend development documentation with clearer examples and improved code formatting.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/app/(tabs)/banking/index.tsx (1)
67-67: Critical: UI still readsnamebut backend providesbankName.The past review comment about this issue remains unresolved. The UI reads
account?.name, but the backend'sAccountWithCurrencyAndTypetype (seesrc/types/index.tsline 23) providesbankName. When you replacemockDashboardDatawith real backend data, all account names will render as empty strings.Update all three locations to use
bankName:- onPress={() => handleAccountPress(account?.name || "", account.id)} + onPress={() => handleAccountPress(account?.bankName || "", account.id)}Also update the rendered name prop in the
AccountItemcomponent at lines 64, 76, and 89:- name={account.name} + name={account.bankName}Also applies to: 79-79, 92-92
🧹 Nitpick comments (2)
src/server/routers/accounts.ts (2)
23-30: Hardcoded currency limits flexibility.The currency is hardcoded to
"EUR"at line 26, which prevents users from creating accounts in other currencies. This may be intentional for an MVP, but consider parameterizingcurrencyIdin the input schema if multi-currency support is planned.
61-71: Consider using spread syntax for cleaner response construction.The manual field-by-field construction of the
AccountWithCurrencyAndTyperesponse is verbose. Using spread syntax can improve readability and reduce duplication.- return { - id: account[0].id, - bankName: account[0].bankName, - currentBalance: account[0].currentBalance, - reference: account[0].reference, - usage: account[0].usage, - currencyId: account[0].currencyId, - currency: currency[0], - typeId: account[0].typeId, - type: accountType[0], - }; + return { + ...account[0], + currency: currency[0], + type: accountType[0], + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docs/backend-development.md(1 hunks)src/app/(tabs)/banking/index.tsx(3 hunks)src/app/index.tsx(2 hunks)src/index.ts(0 hunks)src/server/api/root.ts(1 hunks)src/server/routers/accounts.ts(1 hunks)src/server/routers/transactions.ts(1 hunks)src/server/routers/users.ts(1 hunks)src/types/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/backend-development.md
- src/server/api/root.ts
- src/server/routers/transactions.ts
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.tsx: Use instead of from react-native in UI components
All styling should use TailwindCSS classes via NativeWind (avoid inline StyleSheet definitions)
Files:
src/app/(tabs)/banking/index.tsxsrc/app/index.tsx
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow TypeScript strict mode requirements across the codebase
Files:
src/app/(tabs)/banking/index.tsxsrc/server/routers/accounts.tssrc/types/index.tssrc/server/routers/users.tssrc/app/index.tsx
src/app/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Expo Router file-based routing; screens and routes live under src/app/
Files:
src/app/(tabs)/banking/index.tsxsrc/app/index.tsx
src/server/routers/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Define tRPC procedures/routers under src/server/routers/
Files:
src/server/routers/accounts.tssrc/server/routers/users.ts
src/{server,db}/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database operations must use Drizzle ORM queries (no raw SQL/ad-hoc clients)
Files:
src/server/routers/accounts.tssrc/server/routers/users.ts
🧠 Learnings (1)
📚 Learning: 2025-08-24T14:28:33.804Z
Learnt from: CR
PR: The-Creative-Programming-Group/finance-io#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-24T14:28:33.804Z
Learning: Applies to src/server/routers/**/*.ts : Define tRPC procedures/routers under src/server/routers/
Applied to files:
src/server/routers/accounts.tssrc/server/routers/users.ts
🧬 Code graph analysis (4)
src/app/(tabs)/banking/index.tsx (1)
src/components/AccountItem.tsx (2)
AccountItemProps(7-15)useTheme(17-93)
src/server/routers/accounts.ts (6)
src/server/api/trpc.ts (1)
protectedProcedure(131-148)src/types/index.ts (1)
AccountWithCurrencyAndType(42-45)src/db/index.ts (1)
db(6-6)src/db/schema.ts (3)
currenciesTable(18-28)accountTypesTable(44-54)accountsTable(57-86)src/errors/trpc.ts (1)
errors(3-18)src/errors/messages.ts (1)
error_messages(1-14)
src/server/routers/users.ts (4)
src/server/api/trpc.ts (1)
protectedProcedure(131-148)src/types/index.ts (1)
ClerkUser(7-13)src/db/index.ts (1)
db(6-6)src/db/schema.ts (1)
usersTable(13-15)
src/app/index.tsx (1)
src/utils/trpc.ts (1)
trpc(4-4)
🔇 Additional comments (4)
src/types/index.ts (1)
3-13: LGTM! Past naming concern addressed.The rename from
UsertoClerkUserand the added comment clarify that this interface represents Clerk authentication data rather than the database schema. This resolves the previous review comment about potential confusion.src/server/routers/users.ts (1)
7-20: LGTM! Past runtime crash fixed.The previous critical issue where
existingUsers[0]was undefined after an insert has been resolved. The code now usesonConflictDoNothing()for an idempotent upsert and returns the knownuserIddirectly, preventing the runtime crash.src/server/routers/accounts.ts (1)
32-46: LGTM! Past UUID vs slug mismatch fixed.The previous critical bug where
typeId(a UUID) was compared againstaccountTypesTable.slughas been resolved. The code now correctly usesaccountTypesTable.idwhentypeIdis provided and falls back to slug"private"when absent.src/app/index.tsx (1)
144-157: LGTM! Past authentication and logging concerns resolved.The previous issues have been addressed:
- The accounts query is now properly guarded with
enabled: isSignedIn, preventing unauthorized calls.- PII logging has been removed.
- The conditional redirect logic correctly routes users based on their account status.
|
|
||
| const codes = currencies.map((c) => c.code); | ||
|
|
||
| const existing = await db |
There was a problem hiding this comment.
Please make more comments, for every function ... etc.
Gamius00
left a comment
Gamius00
left a comment
There was a problem hiding this comment.
Please comment:
- Functions
- Variables and States
But you dont have to comment your schema or something.
There was a problem hiding this comment.
Please remove this read me and add it to our wiki: https://github.com/[The-Creative-Programming-Group/finance-io](https://github.com/The-Creative-Programming-Group/finance-io/wiki)/wiki
There was a problem hiding this comment.
We want to have everything in one place, and we figured out that the wiki is the best for that.
There was a problem hiding this comment.
A problem we haven't thought about yet then is that if we put everything in the wiki AIs won't be able to access all of that information......... hmmmm......
There was a problem hiding this comment.
You mean its important for like GH Copilot if it creates a PR?
There was a problem hiding this comment.
Not only that but also if it works in your IDE. Then it has context and knows what to do.
There was a problem hiding this comment.
Pull Request Overview
This PR implements comprehensive backend infrastructure for the Finance-IO mobile app, replacing mock data with real database operations. The changes include new tRPC routers for users, accounts, transactions, and categories, a complete database schema redesign with proper relationships and indexes, and improved authentication flow with Clerk integration.
Key Changes:
- Replaced the temporary
welcomeRouterwith production-ready routers for users, accounts, transactions, and categories with proper authorization - Redesigned database schema from a simple
welcomestable to a full relational structure with users, accounts, account types, currencies, categories, and transactions - Updated sign-in flow to redirect to the root route, which now intelligently routes users to onboarding or banking based on account existence
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/index.ts | Added TypeScript interfaces for backend entities (ClerkUser, Account, Transaction, Category, Currency) with relationship types |
| src/server/routers/users.ts | New users router that fetches Clerk user details and creates user records |
| src/server/routers/transactions.ts | New transactions router with authorization checks for adding and querying transactions |
| src/server/routers/accounts.ts | New accounts router for creating and listing accounts with currency and type relationships |
| src/server/routers/categories.ts | New categories router for retrieving transaction categories |
| src/server/api/trpc.ts | Enhanced protectedProcedure to fetch and attach full Clerk user object to context |
| src/server/api/root.ts | Updated router registration to use new domain routers instead of deprecated ones |
| src/db/schema.ts | Complete schema redesign with proper tables for users, accounts, transactions, categories, currencies, and account types |
| src/scripts/seed.ts | New seed script to populate currencies, categories, and account types |
| src/app/index.tsx | Enhanced root route with conditional routing based on account existence |
| src/app/start/index.tsx | Updated to use new accounts router and getUser query |
| src/app/(auth)/sign-in.tsx | Changed post-sign-in redirect from /start to / for smart routing |
| src/errors/trpc.ts | New standardized error factory for consistent TRPC error handling |
| src/errors/messages.ts | New i18n-ready error message keys |
| docs/backend-development.md | Comprehensive backend development guide documenting patterns and conventions |
FleetAdmiralJakob
left a comment
FleetAdmiralJakob
left a comment
There was a problem hiding this comment.
Could you please create a video and upload it here showing how the app looks and works with these features added?
| { code: "USD", name: "US Dollar" }, | ||
| { code: "EUR", name: "Euro" }, | ||
| { code: "ARS", name: "Argentine Peso" }, |
There was a problem hiding this comment.
Maybe we create a type here with all the currencies we want to support and also use that in the schema. Unite everything in one file so it's easy to change and then reuse it everywhere
There was a problem hiding this comment.
Yes, let's do it with an enum
| export const currenciesTable = pgTable( | ||
| "currencies", | ||
| { | ||
| id: uuid().defaultRandom().primaryKey(), | ||
| name: varchar({ length: 100 }).notNull(), | ||
| code: varchar({ length: 10 }).notNull(), // e.g. USD, EUR, ARS | ||
| }, | ||
| (table) => ({ | ||
| codeUniqueIdx: uniqueIndex("currencies_code_unique_idx").on(table.code), | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Why do we need a table storing different currencies?
src/db/schema.ts
Outdated
| export const accountTypesTable = pgTable( | ||
| "account_types", | ||
| { | ||
| id: uuid().defaultRandom().primaryKey(), | ||
| name: varchar({ length: 100 }).notNull(), | ||
| slug: varchar({ length: 120 }).notNull(), | ||
| }, | ||
| (table) => ({ | ||
| slugUniqueIdx: uniqueIndex("account_types_slug_unique_idx").on(table.slug), | ||
| }), | ||
| ); | ||
|
|
There was a problem hiding this comment.
Can you name an example of an account type? Why can't this be an enum or something? Why should this be a table?
| // Categories | ||
| export const categoriesTable = pgTable( | ||
| "categories", | ||
| { | ||
| id: uuid().defaultRandom().primaryKey(), | ||
| name: varchar({ length: 100 }).notNull(), | ||
| slug: varchar({ length: 120 }).notNull(), | ||
| }, | ||
| (table) => ({ | ||
| slugUniqueIdx: uniqueIndex("categories_slug_unique_idx").on(table.slug), | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Same thing with currencies and account types: Does this need to be its own table?
| const categories = [ | ||
| { name: "Groceries", slug: "groceries" }, | ||
| { name: "Restaurants", slug: "restaurants" }, | ||
| { name: "Transport", slug: "transport" }, | ||
| { name: "Bills", slug: "bills" }, | ||
| { name: "Entertainment", slug: "entertainment" }, | ||
| ]; |
There was a problem hiding this comment.
Same question as with the currencies
There was a problem hiding this comment.
We need to make sure what happens if Clerk and our DB get out of sync.
Why did we choose not to use Clerk Webhooks to sync both?
https://clerk.com/docs/guides/development/webhooks/overview
| export interface ClerkUser { | ||
| id: string; | ||
| firstName: string; | ||
| lastName: string; | ||
| emails: string[]; | ||
| phoneNumbers: string[]; | ||
| } |
There was a problem hiding this comment.
This needs to be kept in sync if we change settings in the Clerk dashboard. Also, we don't have phone number support AFAIK.
src/types/index.ts
Outdated
| export interface AccountType { | ||
| id: string; | ||
| name: string; | ||
| slug: string; | ||
| } | ||
|
|
||
| export interface Account { | ||
| id: string; | ||
| bankName: string; | ||
| currentBalance: string; | ||
| reference: string; | ||
| usage: string; | ||
| currencyId: string; | ||
| typeId: string; | ||
| } | ||
|
|
||
| export interface Category { | ||
| id: string; | ||
| name: string; | ||
| slug: string; | ||
| } | ||
|
|
||
| export interface Currency { | ||
| id: string; | ||
| code: string; | ||
| } | ||
|
|
||
| export interface AccountWithCurrencyAndType extends Account { | ||
| currency: Currency; | ||
| type: AccountType; | ||
| } | ||
|
|
||
| export interface Transaction { | ||
| id: string; | ||
| categoryId: string; | ||
| company: string; | ||
| amount: string; | ||
| icon: React.ReactNode; | ||
| type: "paypal" | "dkb" | "revolut" | "depot"; | ||
| datetime: Date; | ||
| description: string; | ||
| accountId: string; | ||
| } | ||
|
|
||
| export interface TransactionWithCategoryAndAccount extends Transaction { | ||
| category: Category; | ||
| account: Account; | ||
| } |
There was a problem hiding this comment.
Can't these types be inferred or something from Drizzle e.g.
Like so we don't violate DRY.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/backend-development.md (1)
51-51: Minor: Hyphenate "floating-point" (compound adjective).Line 51 should read "floating-point" when modifying "issues."
Apply this diff:
-- For currency amounts, use Drizzle `decimal({ precision, scale, mode: "string" })` and handle values as strings at the API boundary to avoid floating point issues. +- For currency amounts, use Drizzle `decimal({ precision, scale, mode: "string" })` and handle values as strings at the API boundary to avoid floating-point issues.src/app/index.tsx (1)
161-162: Simplify redundant null coalescing and optional chaining.The expression
(accounts || [])?.length > 0is unnecessarily complex. Since(accounts || [])always returns an array, the optional chaining?.is redundant.Apply this diff to simplify:
- } else if (isSignedIn && !isLoadingAccounts && (accounts || [])?.length > 0) { + } else if (isSignedIn && !isLoadingAccounts && accounts && accounts.length > 0) { return <Redirect href={"./(tabs)/banking"} />;Alternatively:
- } else if (isSignedIn && !isLoadingAccounts && (accounts || [])?.length > 0) { + } else if (isSignedIn && !isLoadingAccounts && (accounts?.length ?? 0) > 0) { return <Redirect href={"./(tabs)/banking"} />;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/backend-development.md(1 hunks)src/app/index.tsx(2 hunks)src/server/routers/transactions.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/server/routers/transactions.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.tsx: Use instead of from react-native in all components
All styling should use TailwindCSS classes via NativeWind (avoid inline StyleSheet-based styles)
Files:
src/app/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow TypeScript strict mode requirements throughout the codebase
Files:
src/app/index.tsx
src/{app,components,services,utils,contexts}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Client-side API calls should use tRPC procedures (do not call REST endpoints directly)
Files:
src/app/index.tsx
src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place screens and routes under src/app/ to follow Expo Router file-based routing
Files:
src/app/index.tsx
🧠 Learnings (2)
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
PR: The-Creative-Programming-Group/finance-io#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/{server,services}/**/*.{ts,tsx} : Database operations must use Drizzle ORM queries (no raw SQL in app code)
Applied to files:
docs/backend-development.md
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
PR: The-Creative-Programming-Group/finance-io#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/server/routers/**/*.ts : Define tRPC procedures in src/server/routers/
Applied to files:
docs/backend-development.md
🧬 Code graph analysis (1)
src/app/index.tsx (1)
src/utils/trpc.ts (1)
trpc(4-4)
🪛 LanguageTool
docs/backend-development.md
[grammar] ~51-~51: Use a hyphen to join words.
Context: ...gs at the API boundary to avoid floating point issues. ### Zod Validation Patter...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (3)
docs/backend-development.md (2)
75-116: ✅ Where-clause example now correctly typed and compiles.The secure list query example has been corrected from the previous feedback: typed conditions array with type-guarded filter, no
anycasts, and proper SQL expression composition. This teaches the right pattern and aligns with the "no any" rule stated at line 139.
1-150: Comprehensive and practical backend development guide.This documentation successfully codifies the backend patterns, security practices (ownership checks, protectedProcedure), and type-safety principles across the new routers, schemas, and error handling. The concrete example (lines 78–116) demonstrates correct Drizzle composition and Zod validation, and the checklist (lines 143–150) gives developers clear entry points for adding features. Aligns well with the learnings on Drizzle ORM usage and tRPC router conventions.
src/app/index.tsx (1)
150-153: LGTM! Query is properly guarded.The accounts query is correctly guarded with
enabled: isSignedIn, which prevents unnecessary requests when the user is not authenticated.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
drizzle/0001_nostalgic_master_mold.sql(1 hunks)
🔇 Additional comments (1)
drizzle/0001_nostalgic_master_mold.sql (1)
1-41: Safe migration pattern now in place.The migration correctly applies the recommended safe pattern: add nullable column → seed default → backfill → enforce NOT NULL → add constraints. Once the column name typo above is fixed, this migration will safely handle both fresh databases and existing data.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/index.tsx (1)
161-161: Simplify the redundant condition.The expression
(accounts || [])?.length > 0is unnecessarily complex. Since the first condition already handles the case where accounts is falsy or empty, you can simplify this toaccounts.length > 0.- } else if (isSignedIn && !isLoadingAccounts && (accounts || [])?.length > 0) { + } else if (isSignedIn && !isLoadingAccounts && accounts.length > 0) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/index.tsx(2 hunks)src/server/routers/accounts.ts(1 hunks)src/types/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/server/routers/accounts.ts
- src/types/index.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.tsx: Use instead of from react-native in all components
All styling should use TailwindCSS classes via NativeWind (avoid inline StyleSheet-based styles)
Files:
src/app/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow TypeScript strict mode requirements throughout the codebase
Files:
src/app/index.tsx
src/{app,components,services,utils,contexts}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Client-side API calls should use tRPC procedures (do not call REST endpoints directly)
Files:
src/app/index.tsx
src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place screens and routes under src/app/ to follow Expo Router file-based routing
Files:
src/app/index.tsx
🧬 Code graph analysis (1)
src/app/index.tsx (1)
src/utils/trpc.ts (1)
trpc(4-4)
src/app/index.tsx
Outdated
| // TODO: Add loading state | ||
| // if (isLoadingAccounts) { | ||
| // return <View className="flex-1 items-center justify-center"> | ||
| // <ActivityIndicator size="large" color={iconColor} /> | ||
| // </View>; | ||
| // } |
There was a problem hiding this comment.
Implement the loading state to improve UX.
While accounts are loading, signed-in users see the full landing page content instead of a loading indicator. This creates a confusing experience where the page briefly flashes before redirecting.
Additionally, if this code were uncommented as-is, it would cause a reference error because iconColor is used on line 168 but not defined until line 172.
Based on past review comments, this TODO should be tracked as a ticket. However, the loading state should be implemented soon to avoid user confusion during the accounts fetch.
Here's a corrected implementation:
- // TODO: Add loading state
- // if (isLoadingAccounts) {
- // return <View className="flex-1 items-center justify-center">
- // <ActivityIndicator size="large" color={iconColor} />
- // </View>;
- // }
+ if (isSignedIn && isLoadingAccounts) {
+ const loadingIconColor = scheme === "dark" ? "#E0E0E0" : "#111827";
+ return (
+ <View className="flex-1 items-center justify-center">
+ <ActivityIndicator size="large" color={loadingIconColor} />
+ </View>
+ );
+ }You'll also need to import ActivityIndicator:
import {
View,
ScrollView,
useColorScheme,
TouchableOpacity,
Modal,
+ ActivityIndicator,
} from "react-native";📝 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.
| // TODO: Add loading state | |
| // if (isLoadingAccounts) { | |
| // return <View className="flex-1 items-center justify-center"> | |
| // <ActivityIndicator size="large" color={iconColor} /> | |
| // </View>; | |
| // } | |
| if (isSignedIn && isLoadingAccounts) { | |
| const loadingIconColor = scheme === "dark" ? "#E0E0E0" : "#111827"; | |
| return ( | |
| <View className="flex-1 items-center justify-center"> | |
| <ActivityIndicator size="large" color={loadingIconColor} /> | |
| </View> | |
| ); | |
| } |
| // TODO: Add loading state | |
| // if (isLoadingAccounts) { | |
| // return <View className="flex-1 items-center justify-center"> | |
| // <ActivityIndicator size="large" color={iconColor} /> | |
| // </View>; | |
| // } | |
| import { | |
| View, | |
| ScrollView, | |
| useColorScheme, | |
| TouchableOpacity, | |
| Modal, | |
| ActivityIndicator, | |
| } from "react-native"; |
🤖 Prompt for AI Agents
In src/app/index.tsx around lines 165 to 170, the TODO for the accounts loading
state must be implemented: when isLoadingAccounts is true, return a centered
View with an ActivityIndicator; ensure ActivityIndicator is imported from
react-native (or the UI lib used) and that iconColor is defined or calculated
before it's referenced (move iconColor above the loading check or replace with a
known color variable); remove the commented block and add a short ticket
reference comment if required.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 30 out of 31 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- .idea/sqldialects.xml: Language not supported
Comments suppressed due to low confidence (1)
src/app/start/index.tsx:47
- Unused variable userData.
const { data: userData } = trpc.users.getUser.useQuery();
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/app/index.tsx (3)
179-186: Simplify the accounts existence check.The condition
accounts && accounts.length > 0can be simplified using optional chaining.Apply this diff:
else if ( isSignedIn && !isLoadingAccounts && - accounts && - accounts.length > 0 + (accounts?.length ?? 0) > 0 ) { return <Redirect href={"./(tabs)/banking"} />; }
162-167: Add error handling for the accounts query.The query doesn't capture error states. If the fetch fails,
accountswill be undefined andisLoadingAccountswill be false, causing the user to be silently redirected to the start screen without any error feedback.Capture and handle the error state:
- const { data: accounts, isLoading: isLoadingAccounts } = - trpc.accounts.getAccounts.useQuery(undefined, { - enabled: isSignedIn, - }); + const { + data: accounts, + isLoading: isLoadingAccounts, + error: accountsError, + } = trpc.accounts.getAccounts.useQuery(undefined, { + enabled: isSignedIn, + });Then add error handling before the redirect logic:
if (accountsError && isSignedIn) { return ( <View className="flex-1 items-center justify-center"> <AppText className="text-danger dark:text-danger"> {t("errorLoadingAccounts")} </AppText> </View> ); }
189-194: Implement the loading state to improve UX.While accounts are loading, signed-in users see the full landing page before redirecting, causing a confusing flash. The commented code references
iconColorwhich is defined later (line 196), causing a scope error.Implement the loading state with a correctly scoped color variable:
- // TODO: Add loading state - // if (isLoadingAccounts) { - // return <View className="flex-1 items-center justify-center"> - // <ActivityIndicator size="large" color={iconColor} /> - // </View>; - // } + if (isSignedIn && isLoadingAccounts) { + const loadingIconColor = scheme === "dark" ? "#E0E0E0" : "#111827"; + return ( + <View className="flex-1 items-center justify-center"> + <ActivityIndicator size="large" color={loadingIconColor} /> + </View> + ); + }Import
ActivityIndicator:import { View, ScrollView, useColorScheme, TouchableOpacity, Modal, + ActivityIndicator, } from "react-native";
🧹 Nitpick comments (5)
src/scripts/seed.ts (3)
10-35: Add TSDoc comments and consider centralizing lookup data.Per previous review feedback, functions should include documentation. Additionally, the hardcoded currency list could be centralized as a typed constant or enum to enable reuse across the codebase (e.g., for validation or default values).
Example documentation:
+/** + * Seeds the currencies table with default currency entries (USD, EUR, ARS). + * Skips currencies that already exist based on their code. + */ async function seedCurrencies() {For centralizing lookup data, consider creating
src/constants/lookups.ts:export const SUPPORTED_CURRENCIES = [ { code: "USD", name: "US Dollar" }, { code: "EUR", name: "Euro" }, { code: "ARS", name: "Argentine Peso" }, ] as const; export type CurrencyCode = typeof SUPPORTED_CURRENCIES[number]["code"];This would allow the seed script and other parts of the codebase (validation, defaults) to reference the same source of truth.
37-63: Add TSDoc and consider refactoring to reduce duplication.This function follows the same pattern as
seedCurrencies. Adding TSDoc comments would improve maintainability, and the three seed functions could be refactored into a single generic helper to eliminate duplication.Example documentation:
+/** + * Seeds the categories table with default category entries. + * Skips categories that already exist based on their slug. + */ async function seedCategories() {Consider a generic helper like:
async function seedLookupTable<T extends { slug: string } | { code: string }>( table: PgTable, data: T[], uniqueField: keyof T, tableName: string ) { const keys = data.map((item) => item[uniqueField]); const existing = await db.select().from(table).where(inArray(table[uniqueField], keys)); const existingKeys = new Set(existing.map((e) => e[uniqueField])); const toInsert = data.filter((item) => !existingKeys.has(item[uniqueField])); if (toInsert.length > 0) { await db.insert(table).values(toInsert); console.log(`Inserted ${tableName}: ${toInsert.map(i => i[uniqueField]).join(", ")}`); } else { console.log(`${tableName} already seeded`); } }
65-90: Add TSDoc comment.+/** + * Seeds the account_types table with default account type entries. + * Skips account types that already exist based on their slug. + */ async function seedAccountTypes() {src/app/start/index.tsx (1)
76-81: Consider aligning form field names and types with the backend schema.The form collects
currentAmountas a number but the mutation expectscurrentBalanceas a string. While thetoString()conversion works, this mismatch creates maintenance overhead and potential bugs.Option 1: Rename the form field to match the backend:
defaultValues: { bankName: "", - currentAmount: 0, + currentBalance: "0.00", reference: "", usage: "", },Then update the field handling to work with strings directly, or keep the current UX and handle conversion at the form level.
Option 2: Update the backend schema to accept a number and handle conversion server-side (preferred if backend controls the precision).
src/app/index.tsx (1)
171-177: Simplify the null check with optional chaining.The condition
(!accounts || accounts?.length === 0)is redundant because optional chaining already handles null/undefined cases.Apply this diff:
if ( isSignedIn && !isLoadingAccounts && - (!accounts || accounts?.length === 0) + !accounts?.length ) { return <Redirect href={"./start"} />; }The
?.operator returnsundefinedwhenaccountsis null/undefined, making the explicit null check unnecessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/backend-development.md(1 hunks)src/app/index.tsx(2 hunks)src/app/start/index.tsx(2 hunks)src/scripts/seed.ts(1 hunks)src/server/api/trpc.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.tsx: Use instead of from react-native in all components
All styling should use TailwindCSS classes via NativeWind (avoid inline StyleSheet-based styles)
Files:
src/app/start/index.tsxsrc/app/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow TypeScript strict mode requirements throughout the codebase
Files:
src/app/start/index.tsxsrc/server/api/trpc.tssrc/scripts/seed.tssrc/app/index.tsx
src/{app,components,services,utils,contexts}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Client-side API calls should use tRPC procedures (do not call REST endpoints directly)
Files:
src/app/start/index.tsxsrc/app/index.tsx
src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place screens and routes under src/app/ to follow Expo Router file-based routing
Files:
src/app/start/index.tsxsrc/app/index.tsx
src/{server,services}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database operations must use Drizzle ORM queries (no raw SQL in app code)
Files:
src/server/api/trpc.ts
🧠 Learnings (2)
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
PR: The-Creative-Programming-Group/finance-io#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/server/routers/**/*.ts : Define tRPC procedures in src/server/routers/
Applied to files:
src/server/api/trpc.tsdocs/backend-development.md
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
PR: The-Creative-Programming-Group/finance-io#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/{server,services}/**/*.{ts,tsx} : Database operations must use Drizzle ORM queries (no raw SQL in app code)
Applied to files:
docs/backend-development.md
🧬 Code graph analysis (4)
src/app/start/index.tsx (1)
src/utils/trpc.ts (1)
trpc(4-4)
src/server/api/trpc.ts (1)
src/errors/trpc.ts (1)
errors(3-18)
src/scripts/seed.ts (2)
src/db/index.ts (1)
db(6-6)src/db/schema.ts (3)
currenciesTable(18-28)categoriesTable(31-41)accountTypesTable(44-54)
src/app/index.tsx (1)
src/utils/trpc.ts (1)
trpc(4-4)
🔇 Additional comments (4)
docs/backend-development.md (1)
1-150: Excellent backend development guidelines—comprehensive and well-structured.The documentation provides clear, actionable patterns for authentication, error handling, Drizzle ORM usage, and type safety. The code examples demonstrate proper TypeScript practices, and the guidelines align well with the project's strict typing requirements.
src/scripts/seed.ts (2)
1-8: LGTM—imports are clean and correct.The imports include only what's needed: environment config, database access, table schemas, and the
inArrayhelper for the read-before-write checks.
92-101: LGTM—clean orchestration and error handling.The sequential execution and top-level error handler with
process.exit(1)are appropriate for a seed script.src/app/start/index.tsx (1)
47-57: LGTM! Well-structured mutation with error handling.The mutation properly handles both success and error cases, resets the form state, and navigates the user appropriately. The error handler provides clear feedback to the user.
| let user; | ||
| try { | ||
| user = await clerkClient.users.getUser(ctx.userId); | ||
| } catch (err) { | ||
| console.error("Error fetching user from Clerk:", err); | ||
| throw errors.notAuthenticated(); | ||
| } |
There was a problem hiding this comment.
Consider caching or deferring the user fetch to avoid a round-trip to Clerk on every protected request.
Fetching the full user object from Clerk on every protected procedure call introduces latency and increases load on both Clerk's API and your server. In many procedures, only userId is required.
Recommended solutions:
-
Defer user fetch — Remove the fetch from the middleware and let individual procedures call
clerkClient.users.getUser(ctx.userId)only when they need the full user object. -
Add caching — If the user object is frequently needed, cache it with a short TTL:
import { LRUCache } from 'lru-cache';
const userCache = new LRUCache<string, User>({
max: 500,
ttl: 1000 * 60 * 5, // 5 minutes
});
export const protectedProcedure = t.procedure.use(async ({ ctx, next }) => {
if (!ctx.userId) {
throw errors.notAuthenticated();
}
let user = userCache.get(ctx.userId);
if (!user) {
try {
user = await clerkClient.users.getUser(ctx.userId);
userCache.set(ctx.userId, user);
} catch (err) {
console.error("Error fetching user from Clerk:", err);
throw errors.notAuthenticated();
}
}
return next({
ctx: {
...ctx,
userId: ctx.userId,
user,
},
});
});- Use a helper procedure — Create
protectedProcedureWithUserfor procedures that need the full user object, keepingprotectedProcedurelightweight with onlyuserId.
🤖 Prompt for AI Agents
In src/server/api/trpc.ts around lines 135-141, the middleware eagerly fetches
the full Clerk user for every protected request causing unnecessary latency and
load; either remove the fetch so middleware only validates ctx.userId and let
individual procedures fetch the full user when needed, or implement a short-TTL
LRU cache for user objects and read/write it in the middleware (or create a
separate protectedProcedureWithUser variant) so only procedures that require the
full user get it from Clerk or the cache; update the middleware to throw
errors.notAuthenticated() if no ctx.userId, and if you add caching ensure
thread-safe access, a reasonable max size/TTL, and proper error handling/logging
when Clerk fetches fail.
There was a problem hiding this comment.
@coderabbitai Does it make sense to use lru-cache in a serverless environment?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
https://github.com/coderabbitai We will use a serverless environment with a 99% probability. Please reevaluate your answer and tips. And update your review comment.
There was a problem hiding this comment.
@coderabbitai We will use a serverless environment with a 99% probability. Please reevaluate your answer and tips. And update your review comment.
There was a problem hiding this comment.
@FleetAdmiralJakob, you're absolutely right to question the LRU cache recommendation for a serverless environment. In-memory caching is ineffective in serverless since each function invocation is stateless and the cache would be cold on every (or most) requests.
Here's the revised recommendation for serverless:
Primary solution: Defer the user fetch
Remove the user fetch from the middleware entirely. Let individual procedures call clerkClient.users.getUser(ctx.userId) only when they actually need the full user object:
export const protectedProcedure = t.procedure.use(async ({ ctx, next }) => {
if (!ctx.userId) {
throw errors.notAuthenticated();
}
return next({
ctx: {
...ctx,
userId: ctx.userId,
},
});
});Then in procedures that need the full user:
myProcedure: protectedProcedure.query(async ({ ctx }) => {
const user = await clerkClient.users.getUser(ctx.userId);
// ... use user data
}),Alternative: Create a separate procedure variant
Keep the lightweight protectedProcedure and create protectedProcedureWithUser for procedures that need the full user object:
export const protectedProcedureWithUser = protectedProcedure.use(async ({ ctx, next }) => {
let user;
try {
user = await clerkClient.users.getUser(ctx.userId);
} catch (err) {
console.error("Error fetching user from Clerk:", err);
throw errors.notAuthenticated();
}
return next({
ctx: {
...ctx,
user,
},
});
});This keeps your serverless functions fast by avoiding unnecessary API calls to Clerk on every protected request.
🧠 Learnings used
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/server/routers/**/*.ts : Define tRPC procedures in src/server/routers/
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/{app,components,services,utils,contexts}/**/*.{ts,tsx} : Client-side API calls should use tRPC procedures (do not call REST endpoints directly)
Learnt from: FleetAdmiralJakob
Repo: The-Creative-Programming-Group/finance-io PR: 6
File: src/app/_layout.tsx:51-52
Timestamp: 2025-05-31T15:45:23.954Z
Learning: FleetAdmiralJakob prefers using .env.example files with sensible defaults (minus secrets) for environment variable management, allowing developers to copy and customize them for different deployment environments.
- improve types - removed "account types" table
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (16)
src/components/ui/select.tsx (2)
54-72: Consider using theme-aware color for ChevronDown icon.The hard-coded
#888color on line 71 may not adapt well to theme changes. Consider using a theme-aware color or extracting the color to match your design system.Example approach:
- <ChevronDown size={18} color="#888" /> + <ChevronDown size={18} className="text-text/50 dark:text-dark-text/50" />Note: Verify that
lucide-react-nativesupportsclassNameprop, otherwise you may need to use a theme hook or design token.
74-116: Consider theme-aware colors for drag handle and check icon.Two color-related refinements:
- Line 87: The drag handle uses
bg-gray-400which may need a dark mode variant for consistency.- Line 108: The Check icon uses hard-coded
#16a34a(green), which doesn't adapt to theme changes.Example approach:
- <View className="bg-gray-400 mb-3 h-1.5 w-12 self-center rounded-full" /> + <View className="mb-3 h-1.5 w-12 self-center rounded-full bg-gray-400 dark:bg-gray-600" />- {isActive ? <Check size={18} color="#16a34a" /> : null} + {isActive ? <Check size={18} className="text-green-600 dark:text-green-500" /> : null}Note: Verify that
lucide-react-nativesupportsclassNameprop for the Check icon, otherwise consider using a theme hook or design token.src/app/(auth)/sign-up.tsx (3)
35-36: Add error handling configuration to the mutation.The mutation lacks error handling configuration. If
createUser.mutateAsync()fails during sign-up, users won't see a helpful error message.Consider adding error callbacks:
- const createUser = trpc.users.createUser.useMutation(); + const createUser = trpc.users.createUser.useMutation({ + onError: (error) => { + console.error("Failed to create user:", error); + setError("Failed to create account. Please try again."); + }, + });
147-148: Add specific error handling for database user creation.If the database user creation fails, the generic error handler (line 153-180) may display a misleading "Verification failed" message to users when the actual issue is database connectivity.
Consider wrapping the mutation in a specific try-catch:
if (completeSignUp.status === "complete") { await setActive({ session: completeSignUp.createdSessionId }); - // Add the user in the database - await createUser.mutateAsync(); + // Add the user in the database + try { + await createUser.mutateAsync(); + } catch (dbError) { + console.error("Failed to create user in database:", dbError); + setError("Account created but failed to initialize profile. Please contact support."); + setIsSubmitting(false); + return; + } router.replace("/start");
164-165: Add specific error handling for database user creation in fallback path.Similar to the main verification path, if database user creation fails here, the catch block (line 170) will display "Email already verified. Please sign in." which is misleading when the actual issue is database connectivity.
Apply the same pattern as suggested for line 147-148:
if (signInAttempt?.status === "complete") { await setActive({ session: signInAttempt.createdSessionId }); - // Add the user in the database - await createUser.mutateAsync(); + // Add the user in the database + try { + await createUser.mutateAsync(); + } catch (dbError) { + console.error("Failed to create user in database:", dbError); + setError("Account exists but failed to initialize profile. Please contact support."); + setIsSubmitting(false); + return; + } router.replace("/start");src/scripts/seed.ts (3)
7-11: Consider centralizing the currencies list.The hardcoded currencies list should be defined in a shared location (e.g., in
src/db/schema.tsalongside the schema definition or in a constants file) and imported here. This ensures consistency across seeding, validation, and schema constraints.Based on past review feedback suggesting unifying currency definitions.
For example, in
src/db/schema.ts:export const SUPPORTED_CURRENCIES = [ { code: "USD", name: "US Dollar" }, { code: "EUR", name: "Euro" }, { code: "ARS", name: "Argentine Peso" }, ] as const;Then import and use in the seed script:
+import { SUPPORTED_CURRENCIES } from "~/db/schema"; + async function seedCurrencies() { - const currencies = [ - { code: "USD", name: "US Dollar" }, - { code: "EUR", name: "Euro" }, - { code: "ARS", name: "Argentine Peso" }, - ]; + const currencies = [...SUPPORTED_CURRENCIES];
34-40: Consider centralizing the categories list.Similar to currencies, the hardcoded categories list should be defined in a shared location to ensure consistency. This is especially important if categories are used for validation or displayed in the UI.
Based on past review feedback suggesting unifying category definitions.
61-64: Consider parallel execution for independent seeds.Since currencies and categories are independent, you could seed them in parallel for faster execution.
async function main() { - await seedCurrencies(); - await seedCategories(); + await Promise.all([ + seedCurrencies(), + seedCategories(), + ]); }src/types/index.ts (3)
19-26: Consider using Drizzle-inferred types to avoid duplication.As noted in past feedback, these manual type definitions duplicate the schema definitions and violate DRY. Drizzle ORM can infer types directly from schema definitions.
Based on past review feedback.
Example approach:
import { type InferSelectModel } from 'drizzle-orm'; import { accountsTable } from '~/db/schema'; export type Account = InferSelectModel<typeof accountsTable>;This ensures types stay in sync with schema changes automatically.
28-32: Apply Drizzle type inference here as well.The same recommendation to use Drizzle's
InferSelectModelapplies toCategoryto maintain consistency and avoid schema drift.export type Category = InferSelectModel<typeof categoriesTable>;
43-51: Apply Drizzle type inference to Transaction as well.Same recommendation to use
InferSelectModel<typeof transactionsTable>for consistency.src/server/routers/accounts.ts (3)
25-38: Consider moving user creation to sign-up flow.The user existence check and creation logic duplicates what should happen during user registration (handled by
createUsermutation in the users router). Creating users within the account creation flow can lead to:
- Race conditions if multiple accounts are created simultaneously
- Inconsistent user creation flow
- Logic duplication
Consider either:
- Ensuring
createUseris always called during sign-up (it already is per src/app/(auth)/sign-up.tsx)- Or adding a foreign key constraint check that will fail gracefully if user doesn't exist
If you choose to keep this logic, add a transaction wrapper:
await db.transaction(async (tx) => { const existingUser = await tx.select()... if (!existingUser.length) { await tx.insert(usersTable)... } // ... rest of account creation });
69-77: Simplify return object construction.The manual property mapping can be simplified using spread operators for cleaner, more maintainable code.
return { - id: account[0].id, - bankName: account[0].bankName, - currentBalance: account[0].currentBalance, - reference: account[0].reference, - usage: account[0].usage, - currencyId: account[0].currencyId, + ...account[0], currency: currency[0], };
95-103: Simplify result mapping.Similar to the addAccount return, the mapping can be simplified with spread operators.
return rows.map(({ accounts, currency }) => ({ - id: accounts.id, - bankName: accounts.bankName, - currentBalance: accounts.currentBalance, - reference: accounts.reference, - usage: accounts.usage, - currencyId: accounts.currencyId, + ...accounts, currency, }));.idea/sqldialects.xml (1)
1-8: Consider adding .idea/ directory to .gitignore.This IntelliJ IDEA configuration file is environment-specific. The
.idea/directory should typically be excluded from version control to avoid conflicts between team members' IDE settings.Add to
.gitignore:.idea/However, some teams choose to commit certain
.idea/files for consistency. Verify your team's convention.src/schemas/welcomeSchema.ts (1)
3-5: Consider renaming to avoid confusion with schema.ts export.The file creates a local
referencesEnum(Zod enum), butsrc/db/schema.tsalso exports areferencesEnum(Drizzle pgEnum). While they serve different purposes, the identical naming could cause confusion when importing.Consider one of these alternatives:
-export const referencesEnum = z.enum(references); +export const referenceValidation = z.enum(references);Or import and re-export from schema.ts if the Drizzle enum is compatible:
// If compatible with Zod's expected format import { referencesEnum as dbReferencesEnum } from "~/db/schema";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.idea/sqldialects.xml(1 hunks)drizzle/0002_careless_gladiator.sql(1 hunks)drizzle/meta/0002_snapshot.json(3 hunks)drizzle/meta/_journal.json(1 hunks)localization/it/common.json(1 hunks)src/app/(auth)/sign-up.tsx(4 hunks)src/app/start/index.tsx(4 hunks)src/components/ui/select.tsx(1 hunks)src/db/schema.ts(1 hunks)src/errors/messages.ts(1 hunks)src/schemas/welcomeSchema.ts(3 hunks)src/scripts/seed.ts(1 hunks)src/server/routers/accounts.ts(1 hunks)src/server/routers/users.ts(1 hunks)src/types/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- localization/it/common.json
🚧 Files skipped from review as they are similar to previous changes (4)
- src/app/start/index.tsx
- drizzle/meta/_journal.json
- src/server/routers/users.ts
- src/errors/messages.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.tsx: Use instead of from react-native in all components
All styling should use TailwindCSS classes via NativeWind (avoid inline StyleSheet-based styles)
Files:
src/app/(auth)/sign-up.tsxsrc/components/ui/select.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow TypeScript strict mode requirements throughout the codebase
Files:
src/app/(auth)/sign-up.tsxsrc/server/routers/accounts.tssrc/components/ui/select.tsxsrc/schemas/welcomeSchema.tssrc/scripts/seed.tssrc/db/schema.tssrc/types/index.ts
src/{app,components,services,utils,contexts}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Client-side API calls should use tRPC procedures (do not call REST endpoints directly)
Files:
src/app/(auth)/sign-up.tsxsrc/components/ui/select.tsx
src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place screens and routes under src/app/ to follow Expo Router file-based routing
Files:
src/app/(auth)/sign-up.tsx
src/server/routers/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Define tRPC procedures in src/server/routers/
Files:
src/server/routers/accounts.ts
src/{server,services}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database operations must use Drizzle ORM queries (no raw SQL in app code)
Files:
src/server/routers/accounts.ts
src/db/schema.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain the database schema in src/db/schema.ts
Files:
src/db/schema.ts
🧠 Learnings (7)
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/server/routers/**/*.ts : Define tRPC procedures in src/server/routers/
Applied to files:
src/server/routers/accounts.ts
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/{app,components,services,utils,contexts}/**/*.{ts,tsx} : Client-side API calls should use tRPC procedures (do not call REST endpoints directly)
Applied to files:
src/server/routers/accounts.ts
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to **/*.tsx : Use <AppText> instead of <Text> from react-native in all components
Applied to files:
src/components/ui/select.tsx
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/db/schema.ts : Maintain the database schema in src/db/schema.ts
Applied to files:
src/schemas/welcomeSchema.tssrc/scripts/seed.tssrc/db/schema.ts
📚 Learning: 2025-06-25T17:49:26.926Z
Learnt from: murtazanoori
Repo: The-Creative-Programming-Group/finance-io PR: 23
File: src/components/zodSchema.ts:4-8
Timestamp: 2025-06-25T17:49:26.926Z
Learning: In Zod schema validation for this project, the pattern of using chained min() validations with different messages is intentional - min(1) with "field is required" for empty inputs, followed by min(2) with "must be at least 2 characters" for inputs that are too short. This provides differentiated error messaging for better UX.
Applied to files:
src/schemas/welcomeSchema.ts
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/{server,services}/**/*.{ts,tsx} : Database operations must use Drizzle ORM queries (no raw SQL in app code)
Applied to files:
src/db/schema.tssrc/types/index.ts
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: If modifying authentication features, verify Clerk auth flow (sign-in/sign-up)
Applied to files:
src/types/index.ts
🧬 Code graph analysis (6)
src/app/(auth)/sign-up.tsx (1)
src/utils/trpc.ts (1)
trpc(4-4)
src/server/routers/accounts.ts (7)
src/server/api/trpc.ts (2)
createTRPCRouter(113-113)protectedProcedure(130-150)src/db/schema.ts (4)
referencesEnum(46-46)usersTable(14-16)currenciesTable(19-29)accountsTable(49-74)src/schemas/welcomeSchema.ts (1)
referencesEnum(5-5)src/types/index.ts (2)
AccountWithCurrency(39-41)Account(19-26)src/db/index.ts (1)
db(6-6)src/errors/trpc.ts (1)
errors(3-18)src/errors/messages.ts (1)
error_messages(1-13)
src/components/ui/select.tsx (1)
src/components/ui/AppText.tsx (1)
AppText(14-48)
src/schemas/welcomeSchema.ts (1)
src/db/schema.ts (2)
referencesEnum(46-46)references(44-44)
src/scripts/seed.ts (2)
src/db/index.ts (1)
db(6-6)src/db/schema.ts (2)
currenciesTable(19-29)categoriesTable(32-42)
src/db/schema.ts (1)
src/schemas/welcomeSchema.ts (1)
referencesEnum(5-5)
⏰ 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: Socket Security: Pull Request Alerts
🔇 Additional comments (21)
src/components/ui/select.tsx (3)
1-18: LGTM! Clean imports and well-defined types.The component correctly imports
AppTextinstead of React Native'sText, and the TypeScript types are explicit and comprehensive.
20-44: LGTM! Well-structured component logic.The component setup is clean with appropriate use of
useMemoto optimize the selected item lookup, and handlers are correctly implemented.
54-115: Excellent accessibility implementation!The component includes comprehensive accessibility props (
accessibilityRole,accessibilityState,accessibilityLabel,accessibilityHint) throughout, ensuring a good experience for users with assistive technologies.src/app/(auth)/sign-up.tsx (1)
20-20: LGTM!The TRPC client import is correctly structured and follows the expected pattern.
src/scripts/seed.ts (2)
1-5: LGTM!Imports are clean and all necessary for the seeding operations.
66-69: LGTM!Error handling follows the standard pattern for Node.js scripts with appropriate exit codes.
src/types/index.ts (3)
1-11: LGTM!The
ClerkUserinterface is well-documented and the naming clearly indicates it represents Clerk authentication data rather than database schema. The comment about phone numbers addresses past concerns.
39-41: LGTM!The composite type appropriately models the joined account-currency relationship returned by API queries.
53-56: LGTM!The composite type appropriately models the joined transaction data with category and account relationships.
drizzle/meta/0002_snapshot.json (1)
1-382: Auto-generated migration snapshot.This Drizzle Kit snapshot file is machine-generated and should not be manually edited. The schema definitions appear consistent with the related migration SQL files and schema.ts changes.
src/db/schema.ts (7)
1-11: LGTM!All imports are necessary and correctly sourced from Drizzle ORM.
13-16: LGTM!The minimal users table appropriately stores only the Clerk user ID as a foreign key anchor, with user details managed by Clerk.
18-29: LGTM!The currencies table enables multi-currency support with appropriate constraints. The unique index on
codeprevents duplicate currency codes. This design allows for extensibility if additional currencies need to be added via seeding rather than code changes.
31-42: LGTM!The categories table with unique slug index supports extensible transaction categorization. The slug field enables URL-friendly identifiers for potential filtering or routing.
44-46: LGTM!The references enum with a fixed set of values is an appropriate replacement for the account_types table, providing type safety without the overhead of a separate table.
48-74: LGTM!The accounts table is well-designed with:
- Appropriate foreign key cascade/restrict rules
- String mode for decimal fields to prevent JavaScript float precision issues
- Proper indexes on foreign keys for query optimization
- Timestamp tracking with defaultNow
76-98: LGTM!The transactions table follows consistent design patterns with appropriate:
- Cascade delete for accountId (transactions belong to accounts)
- Restrict delete for categoryId (preserve referential integrity)
- String mode for decimal amounts
- Indexes on foreign keys and datetime for efficient queries
src/server/routers/accounts.ts (2)
1-10: LGTM!All imports are necessary and correctly sourced. Note that
referencesEnumis imported fromwelcomeSchema.ts, which may be a legacy file name.
19-19: Remove unused typeId parameter.The
typeIdinput parameter is defined but never used in the procedure. This was likely left over from when account types were stored in a separate table (before migration 0002 removed theaccount_typestable).z.object({ bankName: z.string(), currentBalance: z.string(), reference: referencesEnum, usage: z.string(), - typeId: z.string().optional(), }),Likely an incorrect or invalid review comment.
drizzle/0002_careless_gladiator.sql (1)
1-7: Original review comment concerns are valid but require manual verification of actual data.The review comment correctly identifies the critical risk in this migration. Analysis of the full schema evolution shows:
Data loss is intentional: The migration consolidates from a two-column approach (typeId foreign key + reference varchar) to a single reference ENUM column. The account_types table drop is a deliberate schema redesign, not an oversight.
Statement ordering is redundant but safe: The
DROP INDEX "accounts_type_idx"on line 5 is redundant becauseDROP TABLE ... CASCADEon line 3 already removes dependent indexes. This won't cause failures, just unnecessary syntax.USING cast is the critical risk: Line 6 casts existing reference varchar values to the ENUM type. This will fail if ANY existing value in the reference column is outside the enum set ('private', 'business', 'savings', 'shared'). The seed.ts file contains no reference data population, so the actual runtime values are unknown.
Action required: Before deploying this migration to a database with existing data, verify that all values currently in
accounts.referenceare valid enum members. If the reference column contains values outside ('private', 'business', 'savings', 'shared'), the migration will fail with a constraint violation.src/schemas/welcomeSchema.ts (1)
15-15: LGTM!Using
referencesEnumdirectly in the base schema is appropriate for backend validation without internationalization.
| "Reference must be less than 50 characters", | ||
| ), | ||
| }), | ||
| reference: referencesEnum, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add internationalized error message for consistency.
The reference field lacks custom error messages, breaking the established pattern where all other fields (bankName, currentAmount, usage) provide detailed i18n messages. Default Zod enum errors won't be internationalized.
Based on learnings: The project intentionally uses differentiated error messaging for better UX.
Apply this diff to add i18n error handling:
- reference: referencesEnum,
+ reference: referencesEnum.error(() =>
+ t("referenceRequired", "Please select a valid reference type")
+ ),Or using Zod 4's unified error parameter for more specific messaging:
- reference: referencesEnum,
+ reference: z.enum(references, {
+ error: (issue) => {
+ if (issue.code === 'invalid_enum_value') {
+ return t("referenceInvalid", "Please select: Private, Business, Savings, or Shared");
+ }
+ }
+ }),Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/schemas/welcomeSchema.ts around line 52, the reference field currently
uses the raw enum (reference: referencesEnum,) and lacks an internationalized
custom error like the other fields; update this to provide the same i18n-aware
error handling by supplying a custom error message for invalid/required enum
values (either by using Zod 4's unified error param/errorMap or by passing a
message option to the enum) matching the project's i18n key pattern used for
bankName/currentAmount/usage so validation returns the localized error string.
- removed mockData
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
localization/ur/common.json (1)
1-1: Update frontend component references to use new dashboard localization keys.The UI components are not fully wired to the new keys. Specifically:
dashboardSharedis defined in all localization files but has zero references in frontend code (missing wiring)dashboardSafeAccounts(old key) is still used insrc/app/start/index.tsx:90and should be replaced withdashboardSavings- Consider also updating the "shared" reference in the same file from
sharedFundstodashboardSharedfor consistencyFix required:
src/app/start/index.tsx:90– Replacet("dashboardSafeAccounts", "Savings")witht("dashboardSavings")src/app/start/index.tsx:91– Replacet("sharedFunds", "Shared")witht("dashboardShared")if applicable
♻️ Duplicate comments (3)
src/types/index.ts (2)
15-19: Verify AccountType interface is still needed.A previous review flagged this interface as potentially unused dead code after the migration dropped the
account_typestable andaccounts.typeIdcolumn. Verify whether this interface is referenced anywhere in the codebase.Run this verification script:
#!/bin/bash # Description: Search for AccountType usage across the codebase # Search for AccountType references excluding this file rg -n '\bAccountType\b' --type ts --type tsx -g '!src/types/index.ts' -C 2If the script returns no results, remove this unused interface:
-export interface AccountType { - id: string; - name: string; - slug: string; -} -
36-39: Currency interface missing 'name' field from schema.The database schema defines three fields for currencies:
id,name, andcode. This interface is missing thenamefield, causing a mismatch between the TypeScript type and the database schema.Apply this diff to add the missing field:
export interface Currency { id: string; + name: string; code: string; }Alternatively, use Drizzle's type inference to automatically derive the type from the schema:
import type { InferSelectModel } from 'drizzle-orm'; import { currenciesTable } from '~/db/schema'; export type Currency = InferSelectModel<typeof currenciesTable>;Based on past review comments suggesting DRY principles for Drizzle types.
src/schemas/welcomeSchema.ts (1)
54-64: Restore localized validation message forreference.Switching to
referencesEnumwithout an error hook now surfaces Zod’s default English message for both empty and invalid selections, breaking the localized UX we established for other fields. Please wrap the enum with a localized error handler so end users still receive translated guidance.- reference: referencesEnum, + reference: referencesEnum.error((issue) => + issue.input === undefined || issue.input === "" + ? t("referenceRequired", "Reference is required") + : t("referenceRequired", "Please select a valid reference type"), + ),Feel free to introduce a dedicated
referenceInvalidkey if you want a distinct message for bad values.Based on learnings
🧹 Nitpick comments (3)
src/app/(tabs)/banking/index.tsx (3)
14-15: Use the loading state to improve UX.The
isLoadingAccountsvariable is defined but never used. Consider showing a loading indicator or skeleton UI while data is being fetched, and prevent rendering account sections until loading completes.Apply this diff to add loading state handling:
+ if (isLoadingAccounts) { + return ( + <SafeAreaView className="flex-1 bg-background dark:bg-dark-background"> + <StatusBar + barStyle={isDark ? "light-content" : "dark-content"} + backgroundColor={colors.background} + /> + <ScrollView className="flex-1 p-8"> + <AppText className="text-center" style={{ color: colors.text }}> + {t("loading")} + </AppText> + </ScrollView> + </SafeAreaView> + ); + } + const handleAccountPress = (accountName: string, accountId: string) => {
22-29: Add empty state handling for better UX.When
accountsis undefined or empty, the component renders a blank screen with no feedback. Add an empty state message or prompt to guide users when no accounts exist.Add this check after the loading state handling:
if (!accounts || accounts.length === 0) { return ( <SafeAreaView className="flex-1 bg-background dark:bg-dark-background"> <StatusBar barStyle={isDark ? "light-content" : "dark-content"} backgroundColor={colors.background} /> <ScrollView className="flex-1 p-8"> <AppText className="text-center" style={{ color: colors.text }}> {t("noAccountsFound")} </AppText> </ScrollView> </SafeAreaView> ); }
114-116: Use consistent conditional rendering pattern.This section uses the
&&operator for conditional rendering, while other sections (lines 59-61, 63-65, 80-82, 97-99) use the ternary operator withnull. Choose one pattern and apply it consistently throughout the component.Apply this diff to match the pattern used in other sections:
- {accounts?.find((account) => account.reference === "shared") && ( + {accounts?.find((account) => account.reference === "shared") ? ( <SectionHeader title={t("sharedFunds")} delay={1000} /> - )} + ) : null}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
localization/ar/common.json(1 hunks)localization/bn/common.json(1 hunks)localization/de/common.json(1 hunks)localization/en/common.json(1 hunks)localization/es/common.json(1 hunks)localization/fa/common.json(1 hunks)localization/fr/common.json(1 hunks)localization/ha/common.json(1 hunks)localization/hi/common.json(1 hunks)localization/id/common.json(1 hunks)localization/it/common.json(2 hunks)localization/ja/common.json(1 hunks)localization/ko/common.json(1 hunks)localization/ps/common.json(1 hunks)localization/pt/common.json(1 hunks)localization/ru/common.json(1 hunks)localization/sw/common.json(1 hunks)localization/th/common.json(1 hunks)localization/tr/common.json(1 hunks)localization/ur/common.json(1 hunks)localization/vi/common.json(1 hunks)localization/zh/common.json(1 hunks)src/app/(tabs)/banking/index.tsx(2 hunks)src/components/AccountItem.tsx(4 hunks)src/data/mockData.ts(0 hunks)src/schemas/welcomeSchema.ts(3 hunks)src/types/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/data/mockData.ts
🧰 Additional context used
📓 Path-based instructions (5)
localization/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store and edit language/i18n resources in the localization/ directory
Files:
localization/ur/common.jsonlocalization/ja/common.jsonlocalization/tr/common.jsonlocalization/vi/common.jsonlocalization/sw/common.jsonlocalization/es/common.jsonlocalization/fa/common.jsonlocalization/bn/common.jsonlocalization/hi/common.jsonlocalization/en/common.jsonlocalization/id/common.jsonlocalization/pt/common.jsonlocalization/zh/common.jsonlocalization/fr/common.jsonlocalization/ar/common.jsonlocalization/ps/common.jsonlocalization/ko/common.jsonlocalization/ha/common.jsonlocalization/th/common.jsonlocalization/ru/common.jsonlocalization/it/common.jsonlocalization/de/common.json
**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.tsx: Use instead of from react-native in all components
All styling should use TailwindCSS classes via NativeWind (avoid inline StyleSheet-based styles)
Files:
src/app/(tabs)/banking/index.tsxsrc/components/AccountItem.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow TypeScript strict mode requirements throughout the codebase
Files:
src/app/(tabs)/banking/index.tsxsrc/schemas/welcomeSchema.tssrc/types/index.tssrc/components/AccountItem.tsx
src/{app,components,services,utils,contexts}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Client-side API calls should use tRPC procedures (do not call REST endpoints directly)
Files:
src/app/(tabs)/banking/index.tsxsrc/components/AccountItem.tsx
src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place screens and routes under src/app/ to follow Expo Router file-based routing
Files:
src/app/(tabs)/banking/index.tsx
🧠 Learnings (7)
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/{app,components,services,utils,contexts}/**/*.{ts,tsx} : Client-side API calls should use tRPC procedures (do not call REST endpoints directly)
Applied to files:
src/app/(tabs)/banking/index.tsx
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/db/schema.ts : Maintain the database schema in src/db/schema.ts
Applied to files:
src/schemas/welcomeSchema.ts
📚 Learning: 2025-06-25T17:49:26.926Z
Learnt from: murtazanoori
Repo: The-Creative-Programming-Group/finance-io PR: 23
File: src/components/zodSchema.ts:4-8
Timestamp: 2025-06-25T17:49:26.926Z
Learning: In Zod schema validation for this project, the pattern of using chained min() validations with different messages is intentional - min(1) with "field is required" for empty inputs, followed by min(2) with "must be at least 2 characters" for inputs that are too short. This provides differentiated error messaging for better UX.
Applied to files:
src/schemas/welcomeSchema.ts
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: If modifying authentication features, verify Clerk auth flow (sign-in/sign-up)
Applied to files:
src/types/index.ts
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/{server,services}/**/*.{ts,tsx} : Database operations must use Drizzle ORM queries (no raw SQL in app code)
Applied to files:
src/types/index.ts
📚 Learning: 2025-10-03T19:02:03.226Z
Learnt from: FleetAdmiralJakob
Repo: The-Creative-Programming-Group/finance-io PR: 50
File: src/app/(tabs)/_layout.tsx:5-37
Timestamp: 2025-10-03T19:02:03.226Z
Learning: In lucide-react-native, icons can be imported with an "Icon" suffix (e.g., ChartColumnIcon, CreditCardIcon, SettingsIcon) in addition to the default plain names (ChartColumn, CreditCard, Settings). The suffixed names are available through alternate package exports.
Applied to files:
src/components/AccountItem.tsx
📚 Learning: 2025-07-01T08:06:09.826Z
Learnt from: murtazanoori
Repo: The-Creative-Programming-Group/finance-io PR: 29
File: src/components/Icons.tsx:58-68
Timestamp: 2025-07-01T08:06:09.826Z
Learning: In React Native projects using custom fonts, replace native Text components with a custom AppText component located in the components folder. AppText provides automatic font handling and consistent theming. Use the `bold`, `semibold`, or `medium` props instead of `fontWeight` in styles. This is enforced by ESLint rules (no-restricted-syntax).
Applied to files:
src/components/AccountItem.tsx
🧬 Code graph analysis (3)
src/app/(tabs)/banking/index.tsx (4)
src/contexts/ThemeContext.tsx (1)
useTheme(66-72)src/utils/trpc.ts (1)
trpc(4-4)src/components/SectionHeader.tsx (1)
SectionHeader(12-35)src/components/AccountItem.tsx (1)
AccountItem(17-96)
src/schemas/welcomeSchema.ts (1)
src/db/schema.ts (2)
referencesEnum(46-46)references(44-44)
src/types/index.ts (1)
src/schemas/welcomeSchema.ts (1)
References(7-7)
🔇 Additional comments (19)
localization/ha/common.json (1)
93-98: Frontend code not updated to use new localization keys—critical runtime failures will occur.The localization changes in
ha/common.jsonare incomplete. While the new keys are properly added to all 22 locales, the frontend code was not updated and contains three critical mismatches:
src/app/start/index.tsxline ~76: Usest("dashboardSafeAccounts")(removed from localization) instead oft("dashboardSavings")src/app/start/index.tsxline ~79: Usest("sharedFunds")instead oft("dashboardShared")src/app/(tabs)/banking/index.tsxline: Usest("sharedFunds")instead oft("dashboardShared")The "savings" and "shared" account types will fail to display translated labels at runtime. Update these files to reference the new localization keys.
⛔ Skipped due to learnings
Learnt from: murtazanoori Repo: The-Creative-Programming-Group/finance-io PR: 33 File: localization/en.json:1-73 Timestamp: 2025-08-04T14:05:24.979Z Learning: In the finance-io project, the contributor murtazanoori prefers flat translation structure over nested grouping due to implementation costs: bulk rename across components, coordination needed for all 16 language files, and lookup performance considerations. The flat structure with ~73-79 keys per language file is acceptable for their current scale.localization/it/common.json (1)
48-48: Localization updates align with new account reference schema.The changes consistently implement the transition from a single "safe accounts" label to distinct "savings" and "shared" categories across Italian localization. The
referencePlaceholderupdate also improves translation consistency by using "Aziendale" instead of mixed English/Italian.Also applies to: 103-104
localization/fa/common.json (1)
176-177: Persian dashboard labels correctly updated.The new dashboard category keys are properly translated and align with the consistent pattern across all locales.
localization/sw/common.json (1)
96-97: Swahili dashboard terminology updated consistently.The additions align with the multi-locale refactor replacing "safe accounts" with distinct savings/shared categories.
localization/ko/common.json (1)
104-105: Korean dashboard labels correctly translated.The new keys are properly localized and follow the established cross-locale pattern.
localization/tr/common.json (1)
103-104: Turkish dashboard terminology updated.The new keys are properly translated and consistent with the multi-locale refactor.
localization/vi/common.json (1)
96-97: Vietnamese dashboard labels properly translated.The additions follow the consistent pattern across all localized files.
localization/bn/common.json (1)
95-96: Bengali dashboard terminology correctly updated.The new keys are properly localized and consistent with the cross-locale refactor.
localization/ur/common.json (1)
104-105: Urdu dashboard labels correctly translated.The new keys are properly localized and complete the consistent pattern across all 8 localized files.
localization/hi/common.json (1)
104-105: LGTM! Localization keys updated consistently.The addition of
dashboardSavingsanddashboardSharedkeys aligns with the broader refactoring to split dashboard account categories across all locales.localization/fr/common.json (1)
96-97: LGTM! French translations added correctly.The translations for "Économies" (Savings) and "Partagé" (Shared) are appropriate and consistent with the refactoring pattern.
localization/de/common.json (2)
113-113: LGTM! Translation refinement applied.The update from "Tägliche Konten" to "Tageskonten" is a valid translation refinement.
116-117: LGTM! German translations for new dashboard keys."Ersparnisse" (Savings) and "Gemeinsam" (Shared) are appropriate translations.
localization/id/common.json (1)
96-97: LGTM! Indonesian translations added."Tabungan" (Savings) and "Bersama" (Shared) are appropriate Indonesian translations for the new dashboard keys.
localization/ru/common.json (1)
103-104: LGTM! Russian translations added correctly."Сбережения" (Savings) and "Совместный" (Shared) are appropriate Russian translations.
localization/ja/common.json (1)
104-105: LGTM! Japanese translations added."貯蓄" (Savings) and "共有" (Shared) are appropriate Japanese translations for the new dashboard keys.
localization/en/common.json (1)
96-97: LGTM! Base English translations for new dashboard keys.The addition of "Savings" and "Shared" keys properly establishes the base translations for all other locales in this refactoring.
src/types/index.ts (2)
3-13: Well-documented type with clear context.The comment clearly explains that
ClerkUserrepresents data from Clerk's authentication service rather than the database schema, preventing confusion. The note about phone numbers being a paid feature is also helpful.As noted in a past review comment, remember to keep this interface in sync with the Clerk dashboard configuration if you modify user fields there.
60-144: Excellent documentation and type safety for navigation.The extensive documentation for
NavigationItemsprovides clear context, usage examples, and maintenance guidelines. The enum ensures type-safe i18n keys for navigation labels, and the examples demonstrate integration with both Expo Router and react-i18next effectively.
20251031-2306-12.3153163.mp4 |
- cleaned up the types
The file is missing a trailing newline, which is required by POSIX standards and may cause issues with some tools.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/(tabs)/banking/index.tsx(2 hunks)src/types/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.tsx: Use instead of from react-native in all components
All styling should use TailwindCSS classes via NativeWind (avoid inline StyleSheet-based styles)
Files:
src/app/(tabs)/banking/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow TypeScript strict mode requirements throughout the codebase
Files:
src/app/(tabs)/banking/index.tsxsrc/types/index.ts
src/{app,components,services,utils,contexts}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Client-side API calls should use tRPC procedures (do not call REST endpoints directly)
Files:
src/app/(tabs)/banking/index.tsx
src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place screens and routes under src/app/ to follow Expo Router file-based routing
Files:
src/app/(tabs)/banking/index.tsx
🧠 Learnings (3)
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/db/schema.ts : Maintain the database schema in src/db/schema.ts
Applied to files:
src/types/index.ts
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: If modifying authentication features, verify Clerk auth flow (sign-in/sign-up)
Applied to files:
src/types/index.ts
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/{server,services}/**/*.{ts,tsx} : Database operations must use Drizzle ORM queries (no raw SQL in app code)
Applied to files:
src/types/index.ts
🧬 Code graph analysis (2)
src/app/(tabs)/banking/index.tsx (5)
src/utils/trpc.ts (1)
trpc(4-4)src/schemas/welcomeSchema.ts (1)
References(7-7)src/types/index.ts (1)
Account(17-24)src/components/SectionHeader.tsx (1)
SectionHeader(12-35)src/components/AccountItem.tsx (2)
AccountItem(17-98)AccountItemProps(7-15)
src/types/index.ts (2)
src/schemas/welcomeSchema.ts (1)
References(7-7)src/db/schema.ts (2)
categoriesTable(32-42)currenciesTable(19-29)
| import { References } from "~/schemas/welcomeSchema"; | ||
| import { categoriesTable, currenciesTable } from "~/db/schema"; | ||
| import { InferSelectModel } from "drizzle-orm"; |
There was a problem hiding this comment.
Prevent shipping server-only modules to the client
These value imports pull ~/db/schema and drizzle-orm into the React Native bundle, which will crash Metro because those modules depend on Node runtime APIs. Switch them to type-only imports so the compiler erases them at emit time.
-import { References } from "~/schemas/welcomeSchema";
-import { categoriesTable, currenciesTable } from "~/db/schema";
-import { InferSelectModel } from "drizzle-orm";
+import type { References } from "~/schemas/welcomeSchema";
+import type { categoriesTable, currenciesTable } from "~/db/schema";
+import type { InferSelectModel } from "drizzle-orm";📝 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.
| import { References } from "~/schemas/welcomeSchema"; | |
| import { categoriesTable, currenciesTable } from "~/db/schema"; | |
| import { InferSelectModel } from "drizzle-orm"; | |
| import type { References } from "~/schemas/welcomeSchema"; | |
| import type { categoriesTable, currenciesTable } from "~/db/schema"; | |
| import type { InferSelectModel } from "drizzle-orm"; |
🤖 Prompt for AI Agents
In src/types/index.ts lines 1-3, the imports from "~/db/schema" and
"drizzle-orm" are being pulled into the client bundle; change those to type-only
imports so the compiler erases them at emit time: use "import type" for
InferSelectModel and for categoriesTable and currenciesTable (and for References
if it's a type) so runtime modules that depend on Node APIs are not included in
the React Native bundle.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
drizzle/0002_careless_gladiator.sql (1)
1-7: Add trailing newline at end of file.The file is missing a trailing newline, which is required by POSIX standards and may cause issues with some tools and git workflows.
Apply this fix:
ALTER TABLE "accounts" ALTER COLUMN "reference" SET DATA TYPE "public"."references" USING "reference"::"public"."references";--> statement-breakpoint -ALTER TABLE "accounts" DROP COLUMN "typeId"; +ALTER TABLE "accounts" DROP COLUMN "typeId";
🧹 Nitpick comments (2)
src/app/(tabs)/banking/index.tsx (2)
25-35: Simplify static reference constants.The
REFERENCEobject is wrapped inuseMemowith an empty dependency array, but these are static values that never change. Move them outside the component for clarity and performance.Apply this refactor:
+// Type-safe reference constants (above component) +const REFERENCE = { + PRIVATE: "private" as References, + BUSINESS: "business" as References, + SAVINGS: "savings" as References, + SHARED: "shared" as References, +} as const; + const Dashboard = () => { const { colors, isDark } = useTheme(); const { t } = useTranslation(); const { data: accounts } = trpc.accounts.getAccounts.useQuery(); - // Type-safe reference constants - const REFERENCE = React.useMemo( - () => - ({ - PRIVATE: "private" as References, - BUSINESS: "business" as References, - SAVINGS: "savings" as References, - SHARED: "shared" as References, - }) as const, - [], - );Then remove
REFERENCEfrom all useMemo dependency arrays (lines 40, 44, 48, 52):const privateAccounts = React.useMemo( () => (accounts ?? []).filter((a) => a.reference === REFERENCE.PRIVATE), - [accounts, REFERENCE], + [accounts], );
117-119: Consider using && for conditional rendering.The ternary with
nullworks correctly but&&is more idiomatic for conditional rendering in React.- {hasDailyAccounts ? ( - <SectionHeader title={t("dashboardDailyAccounts")} delay={400} /> - ) : null} + {hasDailyAccounts && ( + <SectionHeader title={t("dashboardDailyAccounts")} delay={400} /> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
drizzle/0002_careless_gladiator.sql(1 hunks)src/app/(tabs)/banking/index.tsx(2 hunks)src/server/routers/accounts.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/server/routers/accounts.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.tsx: Use instead of from react-native in all components
All styling should use TailwindCSS classes via NativeWind (avoid inline StyleSheet-based styles)
Files:
src/app/(tabs)/banking/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow TypeScript strict mode requirements throughout the codebase
Files:
src/app/(tabs)/banking/index.tsx
src/{app,components,services,utils,contexts}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Client-side API calls should use tRPC procedures (do not call REST endpoints directly)
Files:
src/app/(tabs)/banking/index.tsx
src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place screens and routes under src/app/ to follow Expo Router file-based routing
Files:
src/app/(tabs)/banking/index.tsx
🧬 Code graph analysis (1)
src/app/(tabs)/banking/index.tsx (5)
src/utils/trpc.ts (1)
trpc(4-4)src/schemas/welcomeSchema.ts (1)
References(7-7)src/types/index.ts (1)
Account(17-24)src/components/SectionHeader.tsx (1)
SectionHeader(12-35)src/components/AccountItem.tsx (1)
AccountItem(17-98)
🔇 Additional comments (4)
drizzle/0002_careless_gladiator.sql (1)
6-6: Verifyaccounts.referencecolumn contains only valid enum values before applying migration.The CAST operation on line 6 will fail if the column contains NULL values or any data not matching the four enum variants. However, codebase analysis shows all application writes to the
referencecolumn are validated through the Zod schema (enforcing 'private', 'business', 'savings', 'shared' in lowercase), and no seed data or direct SQL bypasses were found. Run a pre-migration query to confirm the data state:SELECT DISTINCT "reference", COUNT(*) FROM "accounts" GROUP BY "reference"; SELECT COUNT(*) FROM "accounts" WHERE "reference" IS NULL;Proceed only if all existing values match the enum variants exactly.
src/app/(tabs)/banking/index.tsx (3)
8-10: LGTM!The new imports correctly support TRPC data fetching and type-safe account handling.
59-80: LGTM!The
AccountSectioncomponent is well-structured and reusable. It correctly usesaccount.bankName(addressing previous feedback), applies staggered animation delays, and handles empty states gracefully.
138-151: Verify identical animation delays for Savings and Shared sections.Both sections use
baseDelay={1000}, which will cause them to animate simultaneously. Confirm whether this is intentional or if Shared Funds should have a later delay (e.g., 1200) for sequential animation.
| const { data: accounts } = trpc.accounts.getAccounts.useQuery(); | ||
|
|
||
| // TODO: Implement loading and error states |
There was a problem hiding this comment.
Implement loading and error states for the accounts query.
The query runs without handling loading or error states. Users will see undefined behavior during loading and no feedback on failure.
Add state handling:
- const { data: accounts } = trpc.accounts.getAccounts.useQuery();
+ const { data: accounts, isLoading, isError, error } = trpc.accounts.getAccounts.useQuery();
- // TODO: Implement loading and error states
+ if (isLoading) {
+ return (
+ <SafeAreaView className="flex-1 bg-background dark:bg-dark-background">
+ <AppText className="text-center mt-8">{t("loading")}</AppText>
+ </SafeAreaView>
+ );
+ }
+
+ if (isError) {
+ return (
+ <SafeAreaView className="flex-1 bg-background dark:bg-dark-background">
+ <AppText className="text-center mt-8 text-red-500">
+ {t("errorLoadingAccounts")}: {error.message}
+ </AppText>
+ </SafeAreaView>
+ );
+ }🤖 Prompt for AI Agents
In src/app/(tabs)/banking/index.tsx around lines 16 to 18, the trpc query result
is used directly without handling loading or error states; update the component
to read isLoading, isError and error from trpc.accounts.getAccounts.useQuery(),
render a loading indicator (spinner/placeholder) while isLoading is true, render
a clear error message or retry UI when isError is true (use error.message), and
guard usages of accounts (e.g., render an empty state or return early) to avoid
undefined behavior; ensure the UI returns meaningful fallback content in both
loading and error cases before rendering the accounts list.
cleaned up the scripts structure
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
scripts/seed.ts (3)
6-31: Add function documentation and consider extracting currencies to a shared constant.Past reviewers requested more comments for functions, which hasn't been addressed yet. Additionally, FleetAdmiralJakob suggested creating a shared type or constant for currencies that can be reused in the schema to ensure consistency.
Add JSDoc documentation:
+/** + * Seeds the database with default currencies (USD, EUR, ARS). + * This function is idempotent and will only insert currencies that don't already exist. + */ async function seedCurrencies() {Consider extracting the currencies to a shared constant in
src/db/schema.tsor a new constants file:// In src/db/schema.ts or src/db/constants.ts export const SUPPORTED_CURRENCIES = [ { code: "USD", name: "US Dollar" }, { code: "EUR", name: "Euro" }, { code: "ARS", name: "Argentine Peso" }, ] as const;Then import and use it in the seed script:
+import { SUPPORTED_CURRENCIES } from "~/db/schema"; async function seedCurrencies() { - const currencies = [ - { code: "USD", name: "US Dollar" }, - { code: "EUR", name: "Euro" }, - { code: "ARS", name: "Argentine Peso" }, - ]; + const currencies = SUPPORTED_CURRENCIES;Based on past review comments.
33-59: Add function documentation and consider extracting categories to a shared constant.Similar to
seedCurrencies, this function needs JSDoc documentation as requested by past reviewers. FleetAdmiralJakob also suggested extracting categories to a shared constant that can be reused in the schema.Add JSDoc documentation:
+/** + * Seeds the database with default categories (Groceries, Restaurants, Transport, Bills, Entertainment). + * This function is idempotent and will only insert categories that don't already exist. + */ async function seedCategories() {Consider extracting the categories to a shared constant in
src/db/schema.tsor a new constants file:// In src/db/schema.ts or src/db/constants.ts export const DEFAULT_CATEGORIES = [ { name: "Groceries", slug: "groceries" }, { name: "Restaurants", slug: "restaurants" }, { name: "Transport", slug: "transport" }, { name: "Bills", slug: "bills" }, { name: "Entertainment", slug: "entertainment" }, ] as const;Then import and use it in the seed script:
+import { DEFAULT_CATEGORIES } from "~/db/schema"; async function seedCategories() { - const categories = [ - { name: "Groceries", slug: "groceries" }, - { name: "Restaurants", slug: "restaurants" }, - { name: "Transport", slug: "transport" }, - { name: "Bills", slug: "bills" }, - { name: "Entertainment", slug: "entertainment" }, - ]; + const categories = DEFAULT_CATEGORIES;Based on past review comments.
61-64: Add function documentation.The
mainfunction should have JSDoc documentation as requested by past reviewers.+/** + * Main entry point for the seed script. + * Seeds currencies and categories in sequence. + */ async function main() {Based on past review comments.
src/app/index.tsx (1)
169-186: Redirect logic is sound; consider simplifying the null check.The redirect flow correctly guards against premature redirects during loading. However, the condition at line 174 can be simplified.
Apply this diff to simplify the redundant check:
if ( isSignedIn && !isLoadingAccounts && - (!accounts || accounts?.length === 0) + !accounts?.length ) { return <Redirect href={"./start"} />;Note: The loading state UI (showing an ActivityIndicator while
isLoadingAccountsis true) is tracked in Linear issue FIN-31 and not yet implemented. While the redirect logic correctly waits for loading to complete, signed-in users will briefly see the landing page content before being redirected, which may cause a visual flash.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(1 hunks)scripts/seed.ts(1 hunks)src/app/index.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.tsx: Use instead of from react-native in all components
All styling should use TailwindCSS classes via NativeWind (avoid inline StyleSheet-based styles)
Files:
src/app/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow TypeScript strict mode requirements throughout the codebase
Files:
src/app/index.tsxscripts/seed.ts
src/{app,components,services,utils,contexts}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Client-side API calls should use tRPC procedures (do not call REST endpoints directly)
Files:
src/app/index.tsx
src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place screens and routes under src/app/ to follow Expo Router file-based routing
Files:
src/app/index.tsx
🧠 Learnings (3)
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/{app,components,services,utils,contexts}/**/*.{ts,tsx} : Client-side API calls should use tRPC procedures (do not call REST endpoints directly)
Applied to files:
src/app/index.tsx
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to **/*.{ts,tsx} : Follow TypeScript strict mode requirements throughout the codebase
Applied to files:
src/app/index.tsx
📚 Learning: 2025-10-19T13:03:20.337Z
Learnt from: CR
Repo: The-Creative-Programming-Group/finance-io PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-19T13:03:20.337Z
Learning: Applies to src/db/schema.ts : Maintain the database schema in src/db/schema.ts
Applied to files:
scripts/seed.ts
🧬 Code graph analysis (2)
src/app/index.tsx (1)
src/utils/trpc.ts (1)
trpc(4-4)
scripts/seed.ts (2)
src/db/index.ts (1)
db(6-6)src/db/schema.ts (2)
currenciesTable(19-29)categoriesTable(32-42)
🔇 Additional comments (1)
scripts/seed.ts (1)
66-69: LGTM!The error handling is appropriate for a seed script, properly logging errors and exiting with a non-zero code on failure.
| const { data: accounts, isLoading: isLoadingAccounts } = | ||
| trpc.accounts.getAccounts.useQuery(undefined, { | ||
| // Only fetch accounts when signed in to avoid unauthorized requests | ||
| enabled: isSignedIn, | ||
| // Note: You could tune caching behavior here (e.g., `staleTime`) if needed | ||
| }); |
There was a problem hiding this comment.
Add error handling for the accounts query.
A past review comment flagged that the query lacks error handling. If the fetch fails, accounts will be undefined and isLoadingAccounts will be false, causing the user to be silently redirected to the start screen without any error feedback.
Destructure the error and add error handling:
- const { data: accounts, isLoading: isLoadingAccounts } =
- trpc.accounts.getAccounts.useQuery(undefined, {
- // Only fetch accounts when signed in to avoid unauthorized requests
- enabled: isSignedIn,
- // Note: You could tune caching behavior here (e.g., `staleTime`) if needed
- });
+ const {
+ data: accounts,
+ isLoading: isLoadingAccounts,
+ error: accountsError,
+ } = trpc.accounts.getAccounts.useQuery(undefined, {
+ enabled: isSignedIn,
+ });Then add error handling before the redirect logic (after line 167):
if (accountsError && isSignedIn) {
return (
<View className="flex-1 items-center justify-center">
<AppText className="text-red-500 dark:text-red-400">
{t("errorLoadingAccounts")}
</AppText>
</View>
);
}
📝 Description
accounts,transactions,categoriesandusersroutes, schemas and services in backend.TODO (pending)
Refactor home screen to be able to manage new backend types and replace mock data with real backend data.
✅ Checklist
master/develop🔍 Screenshots / GIFs (optional)
🔗 Related Issues
Closes #65
Closes #68
Summary by CodeRabbit
New Features
Documentation
Improvements