feat: TurboTenant deposits + rent roll import#106
Conversation
- POST /api/import/turbotenant-deposits — ingests payment history CSV with auto-resolution of property→tenant and bank→account mappings. Dedup via TurboTenant Payment ID. Classifies as rental income (4010) or fee income (4020) based on bank account pattern. - POST /api/import/turbotenant-rentroll — ingests rent roll snapshot CSV. Matches properties by address, returns structured rent roll with unpaid/past due amounts. Read-only (no transaction creation). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
chittyfinance | 68054a1 | Apr 24 2026, 12:10 PM |
|
@coderabbitai review Please evaluate:
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis pull request adds two new CSV import routes for TurboTenant data: a deposits importer that parses transaction records with tenant/account resolution and deduplication, and a rent roll importer that processes property occupancy snapshots with pattern-based tenant matching. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68054a189b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!tenantId) { | ||
| // Default to ARIBIA MGMT for unresolved | ||
| tenantId = slugToTenantId['aribia-mgmt'] || Object.values(slugToTenantId)[0]; |
There was a problem hiding this comment.
/api/import/turbotenant-deposits is mounted under the tenant-protected API, but this handler chooses tenantId from CSV-derived mappings (and defaults to aribia-mgmt) instead of the authenticated c.get('tenantId'). In the current app wiring, any user who can pass tenant middleware for one tenant can call this route and create transactions in other active tenants, which breaks tenant isolation and is a cross-tenant write vulnerability.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds TurboTenant-specific import endpoints under the existing /api/import route group to support ingesting deposit/payment history into transactions and parsing rent roll CSV snapshots for reporting.
Changes:
- Adds
POST /api/import/turbotenant-depositsto parse TurboTenant deposit CSVs, dedupe by TurboTenant Payment ID, and create income transactions with COA suggestions. - Adds
POST /api/import/turbotenant-rentrollto parse rent roll CSVs and return a structured snapshot with basic address-pattern matching. - Adds audit logging for both imports via
ledgerLog.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Default to ARIBIA MGMT for unresolved | ||
| tenantId = slugToTenantId['aribia-mgmt'] || Object.values(slugToTenantId)[0]; |
There was a problem hiding this comment.
The unresolved-tenant fallback Object.values(slugToTenantId)[0] can route imports into an arbitrary tenant (and can be undefined if no tenants exist). That’s risky for data integrity. Prefer returning a 400 with a clear error (or skipping the row with an error counter) when neither bank account nor address can resolve a tenant.
| // Default to ARIBIA MGMT for unresolved | |
| tenantId = slugToTenantId['aribia-mgmt'] || Object.values(slugToTenantId)[0]; | |
| skipped++; | |
| continue; |
| function resolveBankAccount(bankAcctStr: string): { tenantSlug: string; namePattern: string } | null { | ||
| const lower = bankAcctStr.toLowerCase(); | ||
| // Try most specific match first (longest key) | ||
| const sorted = Object.entries(BANK_ACCOUNT_MAP).sort((a, b) => b[0].length - a[0].length); | ||
| for (const [pattern, mapping] of sorted) { |
There was a problem hiding this comment.
resolveBankAccount sorts BANK_ACCOUNT_MAP entries on every call, which makes per-row processing O(n log n) unnecessarily. Precompute the sorted entries once (outside the function) and iterate that array in resolveBankAccount.
| function resolveBankAccount(bankAcctStr: string): { tenantSlug: string; namePattern: string } | null { | |
| const lower = bankAcctStr.toLowerCase(); | |
| // Try most specific match first (longest key) | |
| const sorted = Object.entries(BANK_ACCOUNT_MAP).sort((a, b) => b[0].length - a[0].length); | |
| for (const [pattern, mapping] of sorted) { | |
| // Try most specific match first (longest key) | |
| const sortedBankAccountEntries = Object.entries(BANK_ACCOUNT_MAP).sort( | |
| (a, b) => b[0].length - a[0].length, | |
| ); | |
| function resolveBankAccount(bankAcctStr: string): { tenantSlug: string; namePattern: string } | null { | |
| const lower = bankAcctStr.toLowerCase(); | |
| for (const [pattern, mapping] of sortedBankAccountEntries) { |
| // POST /api/import/turbotenant-rentroll — import TurboTenant rent roll snapshot | ||
| // Accepts raw CSV body with columns: Property, Unit, Tenants, Lease Start, | ||
| // Lease End, Security Deposit, Rent Amount, Total Unpaid, Total Past Due | ||
| // Upserts leases on matching properties. Does not create transactions. |
There was a problem hiding this comment.
The comment says this route “Upserts leases on matching properties”, but the implementation only parses CSV and returns a read-only snapshot (no storage writes). Update the comment to match current behavior (or implement the upsert if that’s intended).
| // Upserts leases on matching properties. Does not create transactions. | |
| // Parses the CSV and returns a read-only snapshot. Does not upsert leases or create transactions. |
| return c.json({ error: 'CSV must have header + data rows' }, 400); | ||
| } | ||
|
|
||
| const cols = parseCSVLine(lines[startLine]); |
There was a problem hiding this comment.
BOM handling here is incomplete: when the first line contains \uFEFF but is actually the header row (no “Pulled on…” preamble), cleaned is computed but never used, so parseCSVLine(lines[startLine]) may still see the BOM and colIdx(...) can fail. Strip a leading BOM from the actual header line unconditionally before parsing.
| const cols = parseCSVLine(lines[startLine]); | |
| const headerLine = (lines[startLine] ?? '').replace(/^\ufeff/, ''); | |
| const cols = parseCSVLine(headerLine); |
| const allTenants = await storage.getTenants(); | ||
| const slugToTenantId: Record<string, string> = {}; | ||
| for (const t of allTenants) slugToTenantId[t.slug] = t.id; | ||
|
|
There was a problem hiding this comment.
allTenants/slugToTenantId are computed but never used in this route, which adds unnecessary DB work and makes the code harder to follow. Either remove this block or use it to return richer match info (e.g., tenantId/propertyId) as part of the response.
| const allTenants = await storage.getTenants(); | |
| const slugToTenantId: Record<string, string> = {}; | |
| for (const t of allTenants) slugToTenantId[t.slug] = t.id; |
| // Load tenants and accounts | ||
| const allTenants = await storage.getTenants(); | ||
| const slugToTenantId: Record<string, string> = {}; | ||
| for (const t of allTenants) slugToTenantId[t.slug] = t.id; | ||
|
|
There was a problem hiding this comment.
This endpoint ignores the caller-scoped tenantId from tenantMiddleware and instead loads all active tenants (storage.getTenants()) and writes transactions into whatever tenantId is inferred from the CSV. That bypasses the membership check in tenantMiddleware and allows a caller with access to a single tenant to create transactions in other tenants. Restrict imports to c.get('tenantId') (and only resolve accounts/properties within that tenant), or explicitly verify the caller is a member of any resolved tenantId (e.g., via storage.getUserTenants(c.get('userId'))) before reading accounts/creating transactions.
| const lines = body.split('\n').filter((l) => l.trim()); | ||
| if (lines.length < 2) { | ||
| return c.json({ error: 'CSV must have header + data rows' }, 400); | ||
| } | ||
|
|
||
| const cols = parseCSVLine(lines[0]); |
There was a problem hiding this comment.
CSV header parsing doesn’t strip a UTF-8 BOM. If the export includes a BOM (common with Excel/Google Drive), colIdx('Payment ID') can fail and the route will incorrectly report missing required columns. Consider normalizing line endings and stripping a leading \uFEFF from the header line before calling parseCSVLine.
| const lines = body.split('\n').filter((l) => l.trim()); | |
| if (lines.length < 2) { | |
| return c.json({ error: 'CSV must have header + data rows' }, 400); | |
| } | |
| const cols = parseCSVLine(lines[0]); | |
| const normalizedBody = body.replace(/\r\n?/g, '\n'); | |
| const lines = normalizedBody.split('\n').filter((l) => l.trim()); | |
| if (lines.length < 2) { | |
| return c.json({ error: 'CSV must have header + data rows' }, 400); | |
| } | |
| const headerLine = lines[0].replace(/^\uFEFF/, ''); | |
| const cols = parseCSVLine(headerLine); |
Code Review -- PR 106: TurboTenant Deposits + Rent Roll ImportOverviewAdds two new import endpoints to
The overall structure is clean and consistent with the existing import handlers. A few issues need attention before merging. Issues to Fix1. Bypasses tenant middleware Every other import endpoint reads A caller authenticated as 2. Silent fallback to wrong tenant if aribia-mgmt slug not found If Issues Worth Fixing3. propertyId never set on created transactions Other import handlers resolve a 4. iSecDep column is parsed but never used
5. propertyName in ADDRESS_PROPERTY_MAP is dead code The rent roll handler declares 6. Metadata escape-fixup is fragile and inconsistent The deposits handler backslash-unescapes 7. classificationConfidence 0.900 is too high for bank-name heuristics The existing turbotenant importer uses 8. BOM handling in rent roll preamble detection is incomplete If Looks Good
Test Plan GapThe current test plan is a manual production smoke test (70 rows from Google Drive). For a route that writes financial transactions, consider a fixture CSV with assertions on import counts and the Summary: The multi-tenant boundary bypass (1) and silent misattribution fallback (2) should be resolved before merge. The missing |
Summary
POST /api/import/turbotenant-deposits— ingest TurboTenant deposit/payment historytt-deposit:<id>)POST /api/import/turbotenant-rentroll— ingest rent roll snapshotTest plan
npx tsc --noEmit)🤖 Generated with Claude Code
Summary by CodeRabbit