fix: return optional total balance in list apps response for subwallets#2057
fix: return optional total balance in list apps response for subwallets#2057im-adithya merged 5 commits intomasterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughComputes total subwallet balance and returns it in ListApps; refactors SubWallets filtering to tri-state (true/false/nil) and switches metadata key lookups to a new constant. Frontend requests subwallets and consumes totalCount and totalBalance. Changes
Sequence DiagramsequenceDiagram
participant FE as Frontend (SubwalletList)
participant API as API (ListApps)
participant DB as Database (Queries)
FE->>API: ListApps(subWallets: true)
API->>API: Apply SubWallets tri-state filtering (use METADATA_APPSTORE_APP_ID_KEY)
API->>DB: GetTotalSubwalletBalance(tx)
DB->>DB: Find app IDs where metadata[METADATA_APPSTORE_APP_ID_KEY] = SUBWALLET_APPSTORE_APP_ID
DB->>DB: SUM INCOMING amounts (state = SETTLED)
DB->>DB: SUM OUTGOING (amount+fee+fee_reserve) (state IN (SETTLED,PENDING))
DB-->>API: Return net balance (received - spent)
API-->>FE: ListAppsResponse { Apps, TotalCount, TotalBalance }
FE->>FE: Render list using totalCount and totalBalance
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@api/api.go`:
- Around line 530-535: The call to queries.GetTotalSubwalletBalance currently
assumes a single int64 return and masks DB errors by setting totalBalance to 0;
change the call-site in the block that checks filters.SubWallets to handle the
new signature (int64, error) from GetTotalSubwalletBalance: call
totalBalanceMsat, err := queries.GetTotalSubwalletBalance(api.db), on error log
the error via the existing logger and return the error (or otherwise degrade by
leaving totalBalance nil), and only set totalBalance = &totalBalanceMsat when
err == nil; update references to totalBalance accordingly to avoid exposing a
zero balance on DB failures.
In `@db/queries/get_total_subwallet_balance.go`:
- Around line 10-33: GetTotalSubwalletBalance currently ignores database errors
from the two Scan calls (Scan(&received) and Scan(&spent)), so change its
signature to return (int64, error), capture the returned *gorm.DB from each Scan
(e.g., res := tx.Table(...).Scan(&received)), check res.Error and return 0, err
on failure, and only compute and return received.Sum - spent.Sum with nil error
when both queries succeed; update the caller in api/api.go to handle the new
(int64, error) result accordingly.
🧹 Nitpick comments (1)
db/queries/get_total_subwallet_balance_test.go (1)
25-59: Consider checking errors fromsvc.DB.Save()calls.Multiple
svc.DB.Save(...)calls (Lines 25, 31, 39, 49, 59) don't assert on the returned error. A silent save failure would cause a misleading test result. Usingrequire.NoError(t, svc.DB.Save(...).Error)would make failures easier to diagnose.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/src/screens/subwallets/NewSubwallet.tsx (1)
50-53: UseappsData.totalCountfor quota checks instead of page length.Line 52 uses
subwalletApps?.length, which is page-bounded. UsingtotalCountkeeps this correct even if paging limits change.Proposed tweak
- if ( - !albyMe?.subscription.plan_code && - subwalletApps?.length >= MAX_FREE_SUBWALLETS - ) { + if ( + !albyMe?.subscription.plan_code && + (appsData.totalCount ?? 0) >= MAX_FREE_SUBWALLETS + ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/screens/subwallets/NewSubwallet.tsx` around lines 50 - 53, The quota check in NewSubwallet.tsx currently uses the paged array length (subwalletApps?.length) which can be incorrect; replace that check with the total count from the server (appsData?.totalCount) so the condition reads that the user has no plan_code and appsData.totalCount >= MAX_FREE_SUBWALLETS, keeping references to albyMe, appsData.totalCount, subwalletApps and MAX_FREE_SUBWALLETS so you only change the comparison source and leave the rest of the logic intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@db/queries/get_total_subwallet_balance.go`:
- Around line 20-22: The aggregate SELECT uses SUM(...) which returns NULL when
no rows match; update the Select call in get_total_subwallet_balance (the query
building that currently uses Select("SUM(amount_msat) as sum")) to use COALESCE
around the aggregate (e.g., Select("COALESCE(SUM(amount_msat), 0) AS sum")) so
GORM can scan a zero instead of NULL into the int64 variable; keep the WHERE and
Scan(&received) logic unchanged.
In `@nip47/controllers/get_info_controller.go`:
- Around line 94-96: The code in get_info_controller.go unsafely asserts
metadata["lud16"].(string) which can panic if lud16 exists but is not a string;
update the block that sets responsePayload.LightningAddress to perform a safe
type assertion using the comma-ok pattern (e.g., v, ok :=
metadata["lud16"].(string)) and only set LightningAddress when ok is true, and
mirror the same fix where the same pattern appears in api/api.go (look for uses
of metadata["lud16"] and responsePayload.LightningAddress).
---
Nitpick comments:
In `@frontend/src/screens/subwallets/NewSubwallet.tsx`:
- Around line 50-53: The quota check in NewSubwallet.tsx currently uses the
paged array length (subwalletApps?.length) which can be incorrect; replace that
check with the total count from the server (appsData?.totalCount) so the
condition reads that the user has no plan_code and appsData.totalCount >=
MAX_FREE_SUBWALLETS, keeping references to albyMe, appsData.totalCount,
subwalletApps and MAX_FREE_SUBWALLETS so you only change the comparison source
and leave the rest of the logic intact.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
api/api.goconstants/constants.godb/queries/get_total_subwallet_balance.godb/queries/get_total_subwallet_balance_test.gofrontend/src/constants.tsfrontend/src/screens/subwallets/NewSubwallet.tsxfrontend/src/screens/subwallets/SubwalletList.tsxnip47/controllers/get_info_controller.gonip47/controllers/get_info_controller_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- db/queries/get_total_subwallet_balance_test.go
|
Addressed all comments, merging! |
Fixes #2031
Summary by CodeRabbit
New Features
UI Changes
Bug Fixes / Accuracy
Tests