Skip to content

feat(billing): adds balance check job managing#2272

Merged
ygrishajev merged 3 commits intomainfrom
feature/wallet-setting
Nov 26, 2025
Merged

feat(billing): adds balance check job managing#2272
ygrishajev merged 3 commits intomainfrom
feature/wallet-setting

Conversation

@ygrishajev
Copy link
Contributor

@ygrishajev ygrishajev commented Nov 24, 2025

refs #1779

Summary by CodeRabbit

Release Notes

  • New Features

    • Added automatic wallet balance reload functionality, allowing users to configure automatic reloading when balance falls below a specified threshold.
  • Chores

    • Enhanced job scheduling infrastructure with improved job cancellation capabilities.
    • Updated database schema to support wallet auto-reload tracking.
    • Improved transaction handling for better data consistency.

✏️ Tip: You can customize this high-level summary in your review settings.

@ygrishajev ygrishajev requested a review from a team as a code owner November 24, 2025 12:57
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 24, 2025

Walkthrough

Wallet balance auto-reload functionality is added via a new WalletBalanceReloadCheck job type, handler, and database schema extension. The WalletSettingService integrates JobQueueService to schedule and cancel auto-reload jobs based on settings changes. Core services (JobQueueService, DomainEventsService, TxService) are enhanced with job cancellation, optional enqueue parameters, and generic transaction support.

Changes

Cohort / File(s) Change Summary
Database Schema & Migration
apps/api/drizzle/meta/_journal.json, apps/api/drizzle/0022_lazy_kabuki.sql, apps/api/drizzle/meta/0022_snapshot.json, apps/api/src/billing/model-schemas/wallet-setting/wallet-setting.schema.ts
Migration 0022 adds auto_reload_job_id column (UUID) to wallet_settings table; schema snapshot and journal updated.
Job Event & Handler
apps/api/src/billing/events/wallet-balance-reload-check.ts, apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.ts, apps/api/src/app/providers/jobs.provider.ts
New WalletBalanceReloadCheck job class and singleton handler registered; handler concurrency set to 10 with short policy.
Wallet Settings Service
apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts, apps/api/src/billing/services/wallet-settings/wallet-settings.service.spec.ts
Service refactored with JobQueueService dependency; upsertWalletSetting now transactional with job scheduling/cancellation logic; public methods omit autoReloadJobId from return types; tests verify job lifecycle.
Job Queue & Domain Events Infrastructure
apps/api/src/core/services/job-queue/job-queue.service.ts, apps/api/src/core/services/job-queue/job-queue.service.spec.ts, apps/api/src/core/services/domain-events/domain-events.service.ts, apps/api/src/core/services/tx/tx.service.ts
JobQueueService adds cancel() method and policy propagation per handler; DomainEventsService.publish() returns string | null instead of void and accepts optional EnqueueOptions; TxService.transaction made generic to support return values.
Test Updates
apps/api/test/seeders/wallet-setting.seeder.ts, apps/api/test/services/wallet-testing.service.ts, apps/api/test/functional/wallet-settings.spec.ts
Seeder adds autoReloadJobId field; WalletTestingService mock updated; functional test suite removed.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant WalletSettingsService
    participant TxService
    participant JobQueueService
    participant Repository
    participant DomainEventsService

    Client->>WalletSettingsService: upsertWalletSetting(userId, settings)
    activate WalletSettingsService
    
    WalletSettingsService->>TxService: transaction()
    activate TxService
    
    alt autoReloadEnabled changed to true
        WalletSettingsService->>Repository: get existing setting
        WalletSettingsService->>JobQueueService: enqueue(WalletBalanceReloadCheck)
        JobQueueService-->>WalletSettingsService: return jobId
        WalletSettingsService->>Repository: updateById(autoReloadJobId: jobId)
    else autoReloadEnabled changed to false
        WalletSettingsService->>JobQueueService: cancel(jobId)
        WalletSettingsService->>Repository: updateById(autoReloadJobId: null)
    end
    
    TxService-->>WalletSettingsService: return result
    deactivate TxService
    
    WalletSettingsService-->>Client: return publicSetting (without autoReloadJobId)
    deactivate WalletSettingsService
Loading
sequenceDiagram
    participant JobQueue as pg-boss
    participant JobQueueService
    participant Handler as WalletBalanceReloadCheckHandler
    participant Logger

    JobQueue->>JobQueueService: job ready (WalletBalanceReloadCheck)
    activate JobQueueService
    JobQueueService->>Handler: handle()
    activate Handler
    Note over Handler: (placeholder: no-op)
    Handler-->>JobQueueService: return
    deactivate Handler
    JobQueueService->>Logger: log JOB_COMPLETED
    deactivate JobQueueService
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • WalletSettingService refactoring: New transactional flow with job scheduling/cancellation orchestration and modified return types requires careful review of the state transitions and edge cases (creation, update, cancellation paths).
  • Return type changes: Public methods now return Omit<WalletSettingOutput, "autoReloadJobId">, impacting all callers; verify no sensitive data exposure and all call sites handle the new shape.
  • JobQueueService enhancements: New cancel() method and per-handler policy propagation need verification against pg-boss API and existing handler registrations.
  • DomainEventsService signature change: publish() now returns string | null instead of void; verify all call sites handle the new return value appropriately, especially error paths returning null.
  • TxService generics: Make sure generic type parameter <T> propagates correctly through existing callers and doesn't break void-returning callbacks.

Possibly related PRs

Suggested reviewers

  • baktun14
  • stalniy

Poem

🐰 The wallet now rebalances with grace,
Jobs queued and cancelled at perfect pace,
With transactions wrapped and handlers set,
Auto-reload's running—no regrets yet!
Schema grows, migrations done,
Balance checkups—one by one! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(billing): adds balance check job managing' directly describes the main changes: introducing a wallet balance reload check job handler and scheduling system for automatic wallet balance reloading based on settings.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/wallet-setting

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (4)
apps/api/drizzle/0021_strange_madripoor.sql (1)

1-1: Consider adding an index on job_id for job cancellation queries.

If the application will query wallet_settings by job_id when canceling jobs, an index would improve performance.

Apply this diff if job_id queries are expected:

 ALTER TABLE "wallet_settings" ADD COLUMN "job_id" uuid;
+CREATE INDEX "wallet_settings_job_id_idx" ON "wallet_settings"("job_id");
apps/api/src/core/services/job-queue/job-queue.service.ts (1)

39-45: Job queue policy propagation and cancel API look consistent

Passing handler.policy into createQueue and exposing JobQueueService.cancel(name, id) keeps queue configuration and lifecycle cleanly encapsulated, and the logging is in line with the rest of the service. If you want to be extra defensive, you could (optionally) omit the policy field when it’s undefined, or add error logging around pgBoss.cancel failures, but the current implementation is perfectly acceptable.

Also applies to: 95-102, 205-209

apps/api/src/billing/services/wallet-settings/wallet-settings.service.spec.ts (1)

2-19: Tests give good coverage of the new job lifecycle; a couple of minor polish opportunities

The spec nicely exercises:

  • Public surface behavior (getWalletSetting / upsertWalletSetting omitting autoReloadJobId).
  • Create vs update paths, including the duplicate-key retry case.
  • Scheduling on enable (enqueue + persisting autoReloadJobId) and cancellation on disable via jobQueueService.cancel.
  • The validation rules around autoReloadEnabled, threshold, and amount.

Two small nits you might consider (optional):

  • In a couple of tests (e.g., “updates existing wallet setting”), the randomly generated autoReloadEnabled / autoReloadJobId on the seeded setting don’t affect assertions, but can make the setup slightly harder to reason about. Explicitly fixing those flags in the overrides when they matter would make the intent clearer.
  • Expectations currently use WalletBalanceReloadCheck.name for the queue name; if you later centralize the job name (e.g., via the JOB_NAME symbol or the event instance’s name), these tests will need to be adjusted to avoid drifting from the implementation.

Overall, the test coverage around job enqueue/cancel behavior looks solid.

Also applies to: 23-69, 70-106, 172-203, 205-232, 276-308

apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts (1)

122-149: Auto‑reload scheduling/cancellation behavior is sound; consider tightening the transition and job naming

The scheduling helpers are nicely factored: arrangeSchedule centralizes the state transition logic, schedule ensures at most one job per setting by canceling any existing job and overwriting autoReloadJobId, and deleteWalletSetting takes care of canceling jobs when rows are removed.

A couple of small, future‑proofing tweaks you might consider:

  • In arrangeSchedule, you currently cancel whenever !next?.autoReloadEnabled && next?.autoReloadJobId. To make the intent clearer and more robust to future changes (e.g., if you ever start nulling autoReloadJobId on disable), you could express this explicitly as “enabled → disabled” and pick the job id from either side, e.g.:

    const jobIdToCancel = prev?.autoReloadJobId ?? next?.autoReloadJobId;
    if (prev?.autoReloadEnabled && !next?.autoReloadEnabled && jobIdToCancel) {
      await this.jobQueueService.cancel(WalletBalanceReloadCheck.name, jobIdToCancel);
    }
  • Queue naming currently relies on WalletBalanceReloadCheck.name in schedule and deleteWalletSetting. That matches WalletBalanceReloadCheck[JOB_NAME] today, but if the class name and job name ever diverge, cancellations and singleton keys could silently drift. Deriving the name from the event’s name property or from the JOB_NAME symbol (e.g., WalletBalanceReloadCheck[JOB_NAME] or a shared constant) would keep these call sites tied to the canonical job name.

Optionally, you could also assert that createdJobId === jobId in schedule for extra safety, though with options.id set that should already hold.

Also applies to: 150-160

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b426f67 and 1e7ea6a.

📒 Files selected for processing (17)
  • apps/api/drizzle/0021_strange_madripoor.sql (1 hunks)
  • apps/api/drizzle/meta/0021_snapshot.json (1 hunks)
  • apps/api/drizzle/meta/_journal.json (1 hunks)
  • apps/api/src/app/providers/jobs.provider.ts (2 hunks)
  • apps/api/src/billing/events/wallet-balance-reload-check.ts (1 hunks)
  • apps/api/src/billing/model-schemas/wallet-setting/wallet-setting.schema.ts (1 hunks)
  • apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.ts (1 hunks)
  • apps/api/src/billing/services/wallet-settings/wallet-settings.service.spec.ts (6 hunks)
  • apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts (3 hunks)
  • apps/api/src/core/services/domain-events/domain-events.service.ts (2 hunks)
  • apps/api/src/core/services/job-queue/job-queue.service.spec.ts (2 hunks)
  • apps/api/src/core/services/job-queue/job-queue.service.ts (3 hunks)
  • apps/api/src/core/services/tx/tx.service.ts (1 hunks)
  • apps/api/src/provider/services/provider/provider.service.spec.ts (3 hunks)
  • apps/api/test/functional/wallet-settings.spec.ts (0 hunks)
  • apps/api/test/seeders/wallet-setting.seeder.ts (1 hunks)
  • apps/api/test/services/wallet-testing.service.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • apps/api/test/functional/wallet-settings.spec.ts
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-10-15T16:39:55.348Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.

Applied to files:

  • apps/api/test/services/wallet-testing.service.ts
  • apps/api/src/provider/services/provider/provider.service.spec.ts
  • apps/api/src/billing/services/wallet-settings/wallet-settings.service.spec.ts
📚 Learning: 2025-07-30T10:18:24.469Z
Learnt from: baktun14
Repo: akash-network/console PR: 1750
File: apps/api/drizzle/meta/0017_snapshot.json:17-49
Timestamp: 2025-07-30T10:18:24.469Z
Learning: In the Akash Network console billing system, the `user_wallets.user_id` column can be NULL to support anonymous user wallets, allowing users to interact with the system before authentication. Multiple anonymous wallets with NULL user_id values are permitted by design.

Applied to files:

  • apps/api/drizzle/0021_strange_madripoor.sql
📚 Learning: 2025-06-08T03:07:13.871Z
Learnt from: stalniy
Repo: akash-network/console PR: 1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.

Applied to files:

  • apps/api/src/provider/services/provider/provider.service.spec.ts
📚 Learning: 2025-05-28T20:42:58.200Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.

Applied to files:

  • apps/api/src/provider/services/provider/provider.service.spec.ts
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.

Applied to files:

  • apps/api/src/provider/services/provider/provider.service.spec.ts
📚 Learning: 2025-09-04T04:21:26.067Z
Learnt from: stalniy
Repo: akash-network/console PR: 1868
File: apps/api/src/app/services/trial-deployment-lease-created/trial-deployment-lease-created.handler.ts:73-76
Timestamp: 2025-09-04T04:21:26.067Z
Learning: The apps/api codebase consistently uses .toISOString() for startAfter values when scheduling jobs with JobQueueService to ensure UTC timezone consistency. This pattern is used throughout trial-started.handler.ts, trial-deployment-lease-created.handler.ts and other job scheduling code. Do not suggest changing to Date objects as ISO strings provide explicit UTC representation for database storage.

Applied to files:

  • apps/api/src/app/providers/jobs.provider.ts
🧬 Code graph analysis (5)
apps/api/src/billing/events/wallet-balance-reload-check.ts (2)
apps/api/src/core/services/job-queue/job-queue.service.ts (2)
  • Job (186-194)
  • JOB_NAME (198-198)
apps/api/src/core/services/domain-events/domain-events.service.ts (1)
  • JOB_NAME (6-6)
apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.ts (2)
apps/api/src/core/services/job-queue/job-queue.service.ts (1)
  • JobHandler (205-210)
apps/api/src/billing/events/wallet-balance-reload-check.ts (1)
  • WalletBalanceReloadCheck (5-15)
apps/api/src/billing/services/wallet-settings/wallet-settings.service.spec.ts (1)
apps/api/src/billing/events/wallet-balance-reload-check.ts (1)
  • WalletBalanceReloadCheck (5-15)
apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts (4)
apps/api/src/billing/repositories/wallet-settings/wallet-settings.repository.ts (1)
  • WalletSettingOutput (19-22)
apps/api/src/core/services/tx/tx.service.ts (1)
  • WithTransaction (33-45)
apps/api/src/billing/model-schemas/wallet-setting/wallet-setting.schema.ts (1)
  • WalletSetting (7-37)
apps/api/src/billing/events/wallet-balance-reload-check.ts (1)
  • WalletBalanceReloadCheck (5-15)
apps/api/src/core/services/domain-events/domain-events.service.ts (2)
apps/api/src/core/services/job-queue/job-queue.service.ts (2)
  • Job (186-194)
  • EnqueueOptions (212-212)
packages/logging/src/services/logger/logger.service.ts (1)
  • error (141-143)
⏰ 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). (3)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (13)
apps/api/drizzle/meta/_journal.json (1)

152-158: LGTM!

The journal entry correctly documents the new migration with proper sequencing and metadata.

apps/api/test/services/wallet-testing.service.ts (1)

40-40: LGTM!

The mock correctly aligns with the updated DomainEventsService.publish signature that now returns string | null instead of void.

apps/api/src/app/providers/jobs.provider.ts (2)

3-3: LGTM!

The import follows the established pattern for job handlers.


21-22: LGTM!

The WalletBalanceReloadCheckHandler is correctly registered in the job queue service, following the existing handler registration pattern.

apps/api/src/billing/model-schemas/wallet-setting/wallet-setting.schema.ts (1)

29-29: LGTM!

The autoReloadJobId field correctly maps to the new job_id database column, with appropriate nullable semantics for tracking optional background jobs.

apps/api/src/core/services/job-queue/job-queue.service.spec.ts (2)

1-1: LGTM!

The faker import enables UUID generation for the new cancel tests.


108-123: LGTM!

The cancel test suite provides appropriate coverage by verifying both the pgBoss.cancel invocation and the JOB_CANCELLED logging event, following the established testing pattern.

apps/api/src/core/services/tx/tx.service.ts (1)

19-19: LGTM! Good enhancement.

Making the transaction method generic enables it to return values from the callback while remaining backward compatible with existing void-returning usage.

apps/api/test/seeders/wallet-setting.seeder.ts (1)

13-13: LGTM!

The autoReloadJobId field is correctly added to the test data generator, aligning with the schema changes and providing appropriate test coverage.

apps/api/drizzle/meta/0021_snapshot.json (1)

334-451: wallet_settings snapshot (including job_id) aligns with model/schema changes

The wallet_settings table definition, including the new nullable job_id UUID column and existing wallet_id uniqueness / user_id index, matches the TypeScript model schema and how the service uses autoReloadJobId. As this file is generated metadata, no manual adjustments are needed.

apps/api/src/billing/events/wallet-balance-reload-check.ts (1)

1-15: WalletBalanceReloadCheck event wiring looks correct

The event cleanly implements Job with a stable JOB_NAME-based name, a versioned payload, and a minimal data shape (userId) that matches how the scheduler uses it. No changes needed here.

apps/api/src/core/services/domain-events/domain-events.service.ts (1)

4-8: DomainEventsService publish API is consistent with the job queue changes

Aligning DomainEvent with Job and having publish return the underlying enqueue id (or null on failure) makes the domain‑events layer a thin, predictable wrapper over JobQueueService.enqueue, while still logging failures. Just ensure any callers that care about delivery treat a null return as a soft failure.

Also applies to: 18-29

apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts (1)

27-39: Upsert flow and validation for wallet settings look well-structured

The new getWalletSetting / upsertWalletSetting design is clean:

  • Public responses intentionally omit autoReloadJobId, keeping internal job bookkeeping out of the API.
  • upsertWalletSetting uses a clear update‑then‑create pattern, with a duplicate‑key retry path that falls back to update and asserts success.
  • validate correctly pulls threshold/amount from either the incoming settings or the existing record and enforces both when autoReloadEnabled is true, matching the tests.

This gives a solid foundation for managing auto‑reload configuration without leaking internal job details.

Also applies to: 41-75, 109-120

@codecov
Copy link

codecov bot commented Nov 24, 2025

Codecov Report

❌ Patch coverage is 87.50000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.28%. Comparing base (b819c85) to head (47200e1).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...eload-check/wallet-balance-reload-check.handler.ts 37.50% 5 Missing ⚠️
...re/services/domain-events/domain-events.service.ts 25.00% 3 Missing ⚠️
...ervices/wallet-settings/wallet-settings.service.ts 97.91% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2272      +/-   ##
==========================================
- Coverage   47.64%   47.28%   -0.37%     
==========================================
  Files        1034     1026       -8     
  Lines       29294    28994     -300     
  Branches     7624     7562      -62     
==========================================
- Hits        13957    13709     -248     
+ Misses      14949    14811     -138     
- Partials      388      474      +86     
Flag Coverage Δ *Carryforward flag
api 81.88% <87.50%> (-0.30%) ⬇️
deploy-web 26.39% <ø> (ø) Carriedforward from b819c85
log-collector ?
notifications 87.94% <ø> (ø) Carriedforward from b819c85
provider-console 81.48% <ø> (ø) Carriedforward from b819c85
provider-proxy 84.47% <ø> (ø) Carriedforward from b819c85

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
apps/api/src/app/providers/jobs.provider.ts 69.23% <100.00%> (+2.56%) ⬆️
.../src/billing/events/wallet-balance-reload-check.ts 100.00% <100.00%> (ø)
...el-schemas/wallet-setting/wallet-setting.schema.ts 75.00% <ø> (ø)
...i/src/core/services/job-queue/job-queue.service.ts 98.33% <100.00%> (+0.08%) ⬆️
apps/api/src/core/services/tx/tx.service.ts 100.00% <100.00%> (ø)
...ervices/wallet-settings/wallet-settings.service.ts 97.33% <97.91%> (-0.50%) ⬇️
...re/services/domain-events/domain-events.service.ts 58.33% <25.00%> (-5.31%) ⬇️
...eload-check/wallet-balance-reload-check.handler.ts 37.50% <37.50%> (ø)

... and 43 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (3)
apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts (2)

41-120: Upsert, duplicate‑handling, and validation logic are robust (optional minor refactor only)

The @WithTransaction upsert pattern (update first, then create, with a duplicate‑key retry that falls back to update) is race‑safe and easy to reason about. validate correctly enforces that both autoReloadThreshold and autoReloadAmount are set when enabling auto‑reload, either from the payload or the existing setting, and the PostgresError guard for 23505 looks appropriate.

If you later find the { prev?, next? } contract for update/create hard to follow, you could move to a small discriminated union (e.g. { kind: "updated"; prev; next } | { kind: "created"; next } | { kind: "none" }) to make the control flow around mutationResult more explicit, but that’s purely a readability tweak.


122-160: Scheduling/cancellation logic is sound; consider a small robustness tweak

arrangeSchedule and schedule correctly capture the state transitions:

  • Transition from disabled → enabled (!prev?.autoReloadEnabled && next?.autoReloadEnabled) schedules a WalletBalanceReloadCheck job and persists its id.
  • Disabled state with an existing job id (!next?.autoReloadEnabled && next?.autoReloadJobId) cancels the job.
  • deleteWalletSetting cancels any stored job id in parallel with deleting the row, which is a sensible best‑effort compromise across two systems.

schedule also asserts on the result of jobQueueService.enqueue, so if enqueue fails the surrounding transaction can roll back the DB update to autoReloadJobId.

Two optional tweaks you might consider:

  • To guard against future divergence between the class name and the queue name, use the same job name constant that JobQueueService relies on (e.g. WalletBalanceReloadCheck[JOB_NAME]) rather than WalletBalanceReloadCheck.name when calling cancel and constructing singletonKey.
  • In arrangeSchedule, cancelling based on prev.autoReloadJobId (the job that was active before this mutation) rather than next.autoReloadJobId would make the intent slightly clearer and more robust if a future change ever starts clearing autoReloadJobId as part of the update.

These are polish items; the current logic is functionally correct.

apps/api/src/billing/services/wallet-settings/wallet-settings.service.spec.ts (1)

44-105: Upsert and auto‑reload scheduling specs cover key paths; consider a delete‑path test

The updated upsertWalletSetting tests now exercise the important behaviors:

  • Updating an existing setting and asserting that the returned value matches the public shape (without autoReloadJobId).
  • Creating a new setting when none exists, ensuring a WalletBalanceReloadCheck job is enqueued with the expected singletonKey and id, and that autoReloadJobId is persisted via updateById.
  • Handling the duplicate‑key race by retrying through update.
  • Enabling auto‑reload using existing threshold/amount, and cancelling the job when auto‑reload is disabled, with correct WalletBalanceReloadCheck cancel arguments.

These all look consistent with the service implementation and the JobQueueService contract.

Given that deleteWalletSetting now also cancels any existing autoReloadJobId, you might want to add a small spec that seeds a setting with a non‑null autoReloadJobId and verifies jobQueueService.cancel is called on delete, to lock in that behavior.

Also applies to: 107-136, 172-203, 205-232, 265-274

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1e7ea6a and c1a1ccb.

⛔ Files ignored due to path filters (1)
  • packages/net/src/generated/netConfigData.ts is excluded by !**/generated/**
📒 Files selected for processing (16)
  • apps/api/drizzle/0022_lazy_kabuki.sql (1 hunks)
  • apps/api/drizzle/meta/0022_snapshot.json (1 hunks)
  • apps/api/drizzle/meta/_journal.json (1 hunks)
  • apps/api/src/app/providers/jobs.provider.ts (2 hunks)
  • apps/api/src/billing/events/wallet-balance-reload-check.ts (1 hunks)
  • apps/api/src/billing/model-schemas/wallet-setting/wallet-setting.schema.ts (1 hunks)
  • apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.ts (1 hunks)
  • apps/api/src/billing/services/wallet-settings/wallet-settings.service.spec.ts (6 hunks)
  • apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts (3 hunks)
  • apps/api/src/core/services/domain-events/domain-events.service.ts (2 hunks)
  • apps/api/src/core/services/job-queue/job-queue.service.spec.ts (2 hunks)
  • apps/api/src/core/services/job-queue/job-queue.service.ts (3 hunks)
  • apps/api/src/core/services/tx/tx.service.ts (1 hunks)
  • apps/api/test/functional/wallet-settings.spec.ts (0 hunks)
  • apps/api/test/seeders/wallet-setting.seeder.ts (1 hunks)
  • apps/api/test/services/wallet-testing.service.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • apps/api/test/functional/wallet-settings.spec.ts
✅ Files skipped from review due to trivial changes (1)
  • apps/api/drizzle/meta/0022_snapshot.json
🚧 Files skipped from review as they are similar to previous changes (7)
  • apps/api/test/services/wallet-testing.service.ts
  • apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.ts
  • apps/api/src/core/services/job-queue/job-queue.service.ts
  • apps/api/src/core/services/tx/tx.service.ts
  • apps/api/src/billing/model-schemas/wallet-setting/wallet-setting.schema.ts
  • apps/api/test/seeders/wallet-setting.seeder.ts
  • apps/api/src/billing/events/wallet-balance-reload-check.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-15T16:39:55.348Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.

Applied to files:

  • apps/api/src/billing/services/wallet-settings/wallet-settings.service.spec.ts
📚 Learning: 2025-09-04T04:21:26.067Z
Learnt from: stalniy
Repo: akash-network/console PR: 1868
File: apps/api/src/app/services/trial-deployment-lease-created/trial-deployment-lease-created.handler.ts:73-76
Timestamp: 2025-09-04T04:21:26.067Z
Learning: The apps/api codebase consistently uses .toISOString() for startAfter values when scheduling jobs with JobQueueService to ensure UTC timezone consistency. This pattern is used throughout trial-started.handler.ts, trial-deployment-lease-created.handler.ts and other job scheduling code. Do not suggest changing to Date objects as ISO strings provide explicit UTC representation for database storage.

Applied to files:

  • apps/api/src/app/providers/jobs.provider.ts
🧬 Code graph analysis (3)
apps/api/src/core/services/domain-events/domain-events.service.ts (2)
apps/api/src/core/services/job-queue/job-queue.service.ts (2)
  • Job (186-194)
  • EnqueueOptions (212-212)
packages/logging/src/services/logger/logger.service.ts (1)
  • error (141-143)
apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts (4)
apps/api/src/billing/repositories/wallet-settings/wallet-settings.repository.ts (1)
  • WalletSettingOutput (19-22)
apps/api/src/core/services/tx/tx.service.ts (1)
  • WithTransaction (33-45)
apps/api/src/billing/model-schemas/wallet-setting/wallet-setting.schema.ts (1)
  • WalletSetting (7-37)
apps/api/src/billing/events/wallet-balance-reload-check.ts (1)
  • WalletBalanceReloadCheck (5-15)
apps/api/src/core/services/job-queue/job-queue.service.spec.ts (1)
apps/api/src/core/services/job-queue/job-queue.service.ts (1)
  • setup (172-179)
⏰ 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). (10)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: test-build
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: test-build
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: Validate local packages
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
apps/api/src/core/services/job-queue/job-queue.service.spec.ts (1)

1-1: Cancel flow test and faker usage look correct

The new test cleanly exercises JobQueueService.cancel, asserting both the underlying pgBoss.cancel call and the JOB_CANCELLED log payload using a UUID from faker captured in a local variable, so there’s no flakiness. No changes needed.

Also applies to: 108-123

apps/api/src/app/providers/jobs.provider.ts (1)

3-3: WalletBalanceReloadCheckHandler registration is consistent

The new WalletBalanceReloadCheckHandler is correctly imported and registered with JobQueueService alongside existing handlers, preserving the existing ordering while wiring the new job type into the startup flow.

Also applies to: 21-23

apps/api/drizzle/0022_lazy_kabuki.sql (1)

1-1: Schema change for wallet_settings.auto_reload_job_id is aligned

Adding a nullable uuid auto_reload_job_id column matches the model/schema used to track scheduled auto‑reload jobs and won’t break existing rows. Looks good.

apps/api/drizzle/meta/_journal.json (1)

153-165: Journal entry for migration 0022_lazy_kabuki is consistent

The new entry at idx: 22 with tag 0022_lazy_kabuki and version "7" cleanly extends the journal after 0021_brave_warpath; metadata is coherent with the new SQL migration.

apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts (1)

4-11: Auth/job‑queue wiring and public wallet setting shape look correct

Injecting AuthService and JobQueueService into WalletSettingService and stripping autoReloadJobId from getWalletSetting’s return value keeps the public API free of job‑queue implementation details while still enforcing access control via accessibleBy. No functional issues here.

Also applies to: 20-25, 27-39

apps/api/src/billing/services/wallet-settings/wallet-settings.service.spec.ts (1)

2-6: UUID mocking and JobQueueService wiring in test setup look good

Mocking uuid.v4 to return a deterministic jobId and exposing both jobQueueService and jobId from setup lets the specs assert the exact enqueue options and persisted autoReloadJobId without randomness. Deriving publicSetting by omitting autoReloadJobId from the seeded walletSetting also keeps expectations in sync with the service’s new Omit<..., "autoReloadJobId"> return types.

Also applies to: 8-11, 17-19, 276-308

id: jobId
});

assert(createdJobId, 500, "Failed to schedule wallet balance reload check");
Copy link
Contributor

Choose a reason for hiding this comment

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

issue(blocking): you are throwing outside transaction, right? and this means that wallet settings will have a reference to a job which was not created

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is in tx. this is an internal method called by a method wrapped in a transaction

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments