feat: Implement consolidations for issues #411-414#563
Merged
OlufunbiIK merged 1 commit intoApr 28, 2026
Conversation
- OlufunbiIK#411 Fee Module Consolidation - Created unified fee entities (UnifiedPlatformFee, UnifiedFeeConfiguration) - Implemented UnifiedFeeCalculatorService with business logic - Created UnifiedFeesService consolidating fees/ and platinum-fee/ modules - Added comprehensive fee management endpoints - Created fee domain documentation - OlufunbiIK#412 Version Endpoint Consolidation - Created ApiVersionController for API version metadata - Moved API version endpoints to /api-version path - Clear separation from track versions in /tracks/:trackId/versions - OlufunbiIK#413 Redis Layer Consolidation - Created RedisPlatformModule unifying cache/, unify-redis/, and throttler - Implemented RedisPlatformService with comprehensive operations - Added health checks, metrics, and monitoring - Created RedisPlatformHealthController - OlufunbiIK#414 Async Tip Verification Pipeline - Created TipVerificationJob for async proce - OlufunbiIK#411 Fee Module Consolidation - Created unified fee entities (UnifiedPlatformFee, UnifiedFeeConfigural b - Created unified fee entitier - Implemented UnifiedFeeCalculatorService with business logic - Created Ud - Created UnifiedFeesService consolidating fees/ and platinuba - Added comprehensive fee management endpoints - Created fee domain docg - Created fee domain documenta
|
@Cybermaxi7 is attempting to deploy a commit to the olufunbiik's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR implements four major consolidated features: an API version endpoint returning metadata, a unified fee management system with configuration history and platform fee recording, a consolidated Redis platform layer with health checks and caching abstractions, and an async tip verification pipeline using BullMQ job processing with moderation gating and Stellar reconciliation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Tip Creator
participant Queue as Job Queue
participant Job as Verification Job
participant Moderation as Moderation Service
participant Stellar as Stellar Service
participant TipSvc as Tips Service
participant FeeSvc as Fees Service
participant Notify as Notifications Service
Client->>Queue: Enqueue tip verification
activate Queue
Queue->>Job: Process TipVerificationJob
deactivate Queue
activate Job
Job->>TipSvc: Load tip + sender/artist
Job->>Moderation: Check moderation gate
alt Moderation blocks
Moderation-->>Job: BLOCKED
Job->>TipSvc: Update status=BLOCKED, store reason
Job->>Notify: Send blockage notification
else Moderation passes
Moderation-->>Job: Approved
Job->>Stellar: Verify transaction by hash
alt Transaction valid
Stellar-->>Job: Success (validated amount/asset)
Job->>TipSvc: Update status=VERIFIED, store txn details
Job->>FeeSvc: Record platform fee for tip
Job->>Notify: Notify artist + sender
Job->>Job: Emit tip.verified event
else Transaction invalid
Stellar-->>Job: Invalid/not found
alt Retries remaining
Job->>Job: Retry with backoff
else Max retries exhausted
Job->>TipSvc: Update status=FAILED, store reason
Job->>Notify: Send failure notification
end
end
end
deactivate Job
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (13)
backend/src/redis-platform/redis-platform.health.controller.ts-10-15 (1)
10-15:⚠️ Potential issue | 🟠 MajorReturn a failing HTTP status when Redis is unhealthy.
healthCheck()encodes failure in the JSON body, but this endpoint still responds with 200. External probes and load balancers will treat Redis as healthy and keep routing traffic. Map the unhealthy case to a 503.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/redis-platform/redis-platform.health.controller.ts` around lines 10 - 15, The getHealth endpoint currently returns the healthCheck() payload with HTTP 200 even when Redis is unhealthy; change getHealth to await this.redisService.healthCheck(), inspect the returned object (e.g., check health.status !== 'up' or the failure flag in the payload) and when unhealthy throw a Nest HttpException with HttpStatus.SERVICE_UNAVAILABLE (503) carrying the health payload; return the health payload unchanged only when healthy. Use the getHealth method and redisService.healthCheck() identifiers to locate and modify the code.backend/src/redis-platform/redis-platform.service.ts-226-233 (1)
226-233:⚠️ Potential issue | 🟠 MajorAvoid
KEYSin the shared Redis layer.
keys(pattern)is O(N) and blocks Redis while scanning the full DB. In a canonical platform module this can become an outage multiplier under load. UseSCAN/scanStreamand delete in batches instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/redis-platform/redis-platform.service.ts` around lines 226 - 233, The use of redis.keys in clearPattern and getKeysByPattern must be replaced with a non-blocking SCAN approach: update getKeysByPattern (or remove it) to iterate with redis.scan/scanStream and collect matches incrementally, and change clearPattern to stream-scan the pattern and delete keys in bounded batches (e.g., accumulate ~100-1000 keys then call redis.del(...batch) or use a pipeline) until the scan is exhausted; ensure both functions avoid calling redis.keys and handle empty results and errors gracefully while keeping memory bounded.backend/src/redis-platform/redis-platform.service.ts-212-217 (1)
212-217:⚠️ Potential issue | 🟠 MajorSet the throttler TTL atomically with the increment.
INCRand the conditionalEXPIREare separate commands. If the process dies or the second call fails between them, the counter becomes permanent and that key can throttle users indefinitely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/redis-platform/redis-platform.service.ts` around lines 212 - 217, The current incrementThrottler uses two separate commands (this.redis.incr and this.redis.expire) which can leave a permanent key if the process dies between calls; replace that with an atomic operation (use a Redis EVAL Lua script or a MULTI/EXEC transaction) so the increment and TTL set happen together — e.g., implement incrementThrottler to call this.redis.eval with a small Lua script that runs redis.call('INCR', KEYS[1]) and if the result == 1 then redis.call('EXPIRE', KEYS[1], ARGV[1]) and returns the counter, or alternatively use this.redis.multi().incr(key).expire(key, ttl).exec() to ensure atomicity. Ensure you remove the separate this.redis.expire call and return the increment result as before.backend/src/redis-platform/redis-platform.module.ts-72-117 (1)
72-117:⚠️ Potential issue | 🟠 MajorClose the Redis client during application shutdown.
This factory opens a long-lived socket, but nothing in the module implements
OnModuleDestroy/OnApplicationShutdownto callquit()ordisconnect(). That leaks connections and can hang tests or graceful shutdowns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/redis-platform/redis-platform.module.ts` around lines 72 - 117, The Redis client created in RedisPlatformModule.createClient opens a long-lived connection but is never closed; implement a lifecycle hook (e.g., have RedisPlatformModule or the provider class implement OnModuleDestroy or OnApplicationShutdown) that retains a reference to the client created by createClient and calls client.quit() (or client.disconnect() if quit may hang) during shutdown; ensure the module/provider exposes the client instance (or stores it on the class) and invoke the graceful close inside the lifecycle method, handling errors and awaiting quit/disconnect before completing shutdown.backend/src/redis-platform/redis-platform.service.ts-137-148 (1)
137-148:⚠️ Potential issue | 🟠 MajorMake INFO parsing newline-agnostic.
This parser only handles CRLF. The test in
backend/src/redis-platform/redis-platform.service.spec.ts:207-217already uses LF-only output, so every section comes back empty and the metrics collapse to zeros.Suggested fix
- const lines = info.split('\r\n'); + const lines = info.split(/\r?\n/);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/redis-platform/redis-platform.service.ts` around lines 137 - 148, The INFO parser fails on LF-only Redis output because parseInfo currently splits info with '\r\n'; change it to normalize newlines (e.g. split on /\r?\n/ or replace /\r\n?/g with '\n') so parseInfo(info, section) correctly finds sectionStart and sectionEnd for both CRLF and LF inputs, and ensure you still guard against sectionStart === -1 before slicing to avoid returning empty sections.backend/src/redis-platform/redis-platform.service.spec.ts-170-201 (1)
170-201:⚠️ Potential issue | 🟠 MajorFix the
Date.now()mocking in the health-check tests.Both spies patch the same function, so the second
mockReturnValue(...)wins beforehealthCheck()executes. That makesresponseTimeevaluate to0, and these assertions fail for the wrong reason.Suggested fix
- const startSpy = jest.spyOn(Date, 'now').mockReturnValue(1000); - const endSpy = jest.spyOn(Date, 'now').mockReturnValue(1010); + const nowSpy = jest + .spyOn(Date, 'now') + .mockReturnValueOnce(1000) + .mockReturnValueOnce(1010); @@ - startSpy.mockRestore(); - endSpy.mockRestore(); + nowSpy.mockRestore();Apply the same pattern to the failure case with
1050as the second return value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/redis-platform/redis-platform.service.spec.ts` around lines 170 - 201, The tests for service.healthCheck incorrectly overwrite Date.now by calling mockReturnValue twice on the same spy; change each test to set Date.now to return the start value first and the end value second (e.g., use jest.spyOn(Date, 'now').mockReturnValueOnce(start).mockReturnValueOnce(end) or mockImplementationOnce for the two calls) so responseTime computes correctly in both the healthy and unhealthy cases (keep restoring the spy afterwards); update the two blocks that currently create startSpy/endSpy in the two it(...) specs to use the sequential mockReturnValueOnce/mockImplementationOnce pattern before calling service.healthCheck().backend/src/tips/tip-verification.job.ts-147-169 (1)
147-169:⚠️ Potential issue | 🟠 MajorValidate
assetIssuerfor non-XLM payments.Matching only
asset_codelets a payment from a different Stellar issuer pass verification for the same asset code. For issued assets, compare both code and issuer before marking the tip verified.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/tips/tip-verification.job.ts` around lines 147 - 169, The current verification only compares payment.asset_code to tip.assetCode, allowing different issuers with the same code to pass; update the verification in tip-verification.job.ts where payment is examined so that for non-native assets you compare both payment.asset_code === tip.assetCode and payment.asset_issuer === tip.assetIssuer (or, equivalently, require tip.assetIssuer to match payment.asset_issuer when tip.assetCode is present), and return false if either the code or issuer does not match (keeping native/XLM payments unaffected).backend/src/fees/entities/unified-platform-fee.entity.ts-19-21 (1)
19-21:⚠️ Potential issue | 🟠 MajorMake
tipIdunique to preserve replay idempotency.The async pipeline acceptance calls out idempotent replays, but this entity only adds a non-unique index on
tipId. A duplicated job or reconciliation replay can insert multiple fee rows for the same tip and double-count totals. This should be enforced at the database level.Suggested change
-@Index(['tipId']) +@Index(['tipId'], { unique: true })Also applies to: 27-32
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/fees/entities/unified-platform-fee.entity.ts` around lines 19 - 21, The platform_fees entity currently declares a non-unique index on tipId which allows duplicate fee rows and breaks idempotency; update the entity decorators to enforce a unique constraint on tipId (e.g., change the `@Index`(['tipId']) to a unique index or add unique: true on the tipId column) so the DB prevents duplicate inserts, and make the same change for the analogous index referenced in the later block (lines 27-32) to ensure idempotent replay at the database level.backend/src/fees/unified-fees.service.ts-175-186 (1)
175-186:⚠️ Potential issue | 🟠 MajorMake fee recording idempotent on
tipId.This method always inserts a new row. In the async verification pipeline, replay/retry of the same tip will either duplicate the ledger entry or fail on a unique key instead of returning the existing fee, which conflicts with the idempotency requirement in
#414.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/fees/unified-fees.service.ts` around lines 175 - 186, The platform fee creation is not idempotent for a given tipId; modify the logic in unified-fees.service.ts (the code using platformFeeRepo.create and platformFeeRepo.save) to first lookup an existing PlatformFee by tipId and return it if found, otherwise create and save a new record; if you need to allow updates on retries, perform an upsert/update-by-tipId (e.g., findOne({ tipId }) then assign result fields feePercentage/feeAmountXLM/feeAmountUSD/collectionStatus/stellarTxHash and save) so duplicate inserts are avoided and the existing fee row is returned on retries.backend/src/fees/unified-fees.service.ts-175-184 (1)
175-184:⚠️ Potential issue | 🟠 MajorDon't reuse
stellarTxHashfor two different transactions.Line 183 stores the tip-payment hash on the fee record, but Lines 199-200 overwrite the same field with the later fee-collection hash. That destroys the original chain reference and makes the field ambiguous. This needs separate fields, or
stellarTxHashshould only be populated during collection.Also applies to: 199-200
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/fees/unified-fees.service.ts` around lines 175 - 184, The code currently reuses the same stellarTxHash field for both the tip payment and the later fee-collection transaction; update the entity and calls so these are distinct: set a tipStellarTxHash (or tipPaymentStellarTxHash) when creating the platform fee in platformFeeRepo.create (where stellarTxHash is currently assigned) and introduce a separate feeCollectionStellarTxHash (or feeStellarTxHash) to be populated only during fee collection (replace the assignment currently happening on lines ~199-200). Ensure all references to stellarTxHash in the fee creation and collection flows (e.g., platformFeeRepo.create and the fee-collection update path) are updated to use the new dedicated fields and migrate any database schema/ORM mappings accordingly.backend/src/fees/unified-fees.service.ts-93-95 (1)
93-95:⚠️ Potential issue | 🟠 MajorValidate min/max with null checks instead of truthy checks.
Line 94 validates the min/max relationship using falsy checks, which allows numeric zero values to bypass validation. If
minimumFeeXLM = 1andmaximumFeeXLM = 0, the condition fails because0is falsy in JavaScript, skipping the entire validation block and allowing invalid configurations to be saved.Suggested fix
- if (dto.minimumFeeXLM && dto.maximumFeeXLM && dto.minimumFeeXLM > dto.maximumFeeXLM) { + if ( + dto.minimumFeeXLM != null && + dto.maximumFeeXLM != null && + dto.minimumFeeXLM > dto.maximumFeeXLM + ) { throw new BadRequestException('minimumFeeXLM cannot exceed maximumFeeXLM'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/fees/unified-fees.service.ts` around lines 93 - 95, The min/max validation uses truthy checks so zero values are skipped; update the condition in the validation block that checks dto.minimumFeeXLM and dto.maximumFeeXLM (the code around the throw new BadRequestException('minimumFeeXLM cannot exceed maximumFeeXLM')) to test for presence with explicit null/undefined checks (e.g., use dto.minimumFeeXLM != null and dto.maximumFeeXLM != null or !== undefined) before comparing values, then keep the existing throw when minimumFeeXLM > maximumFeeXLM.backend/src/fees/unified-fee-calculator.service.ts-60-68 (1)
60-68:⚠️ Potential issue | 🟠 MajorReject malformed periods with a 400, not a 500.
parseIntaccepts prefixes like30d(returns 30), and the plainErrorthrown here will bubble out ofbackend/src/fees/unified-fees.service.tslines 229 and 280 as an internal error. This helper should validate the entire string and throwBadRequestExceptioninstead.Suggested fix
-import { Injectable } from '@nestjs/common'; +import { BadRequestException, Injectable } from '@nestjs/common'; @@ parsePeriodToDate(period: string): Date { - const now = new Date(); - const value = parseInt(period, 10); - - if (isNaN(value) || value <= 0) { - throw new Error(`Invalid period: ${period}. Expected positive integer.`); + const now = new Date(); + const normalized = period.trim(); + + if (!/^[1-9]\d*$/.test(normalized)) { + throw new BadRequestException( + `Invalid period: ${period}. Expected positive integer.`, + ); } - return new Date(now.getTime() - value * 24 * 60 * 60 * 1000); + now.setDate(now.getDate() - Number(normalized)); + return now; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/fees/unified-fee-calculator.service.ts` around lines 60 - 68, The parsePeriodToDate helper currently uses parseInt which accepts strings like "30d" and throws a generic Error; update parsePeriodToDate to fully validate the input with a strict numeric check (e.g. /^\d+$/) before parsing, use Number or parseInt after validation, and replace the thrown Error with a BadRequestException containing a clear message (so callers in unified-fees.service.ts receive a 400). Keep the rest of the logic (calculating now - value * 24*60*60*1000) intact and reference the parsePeriodToDate function when making this change.backend/src/fees/unified-fee-calculator.service.ts-37-49 (1)
37-49:⚠️ Potential issue | 🟠 MajorUse null checks for numeric optionals instead of truthy checks.
Lines 37, 42, and 48 use truthy checks that silently skip valid
0values. A configuration likeminimumFeeXLM: 0will be ignored, andxlmToUsdRate: 0will never propagate. Use!= nullchecks for numeric optionals instead.Suggested fix
- if (config.minimumFeeXLM && feeAmountXLM < config.minimumFeeXLM) { + if (config.minimumFeeXLM != null && feeAmountXLM < config.minimumFeeXLM) { feeAmountXLM = config.minimumFeeXLM; } @@ - if (config.maximumFeeXLM && feeAmountXLM > config.maximumFeeXLM) { + if (config.maximumFeeXLM != null && feeAmountXLM > config.maximumFeeXLM) { feeAmountXLM = config.maximumFeeXLM; } @@ - if (xlmToUsdRate) { + if (xlmToUsdRate != null) { feeAmountUSD = feeAmountXLM * xlmToUsdRate; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/fees/unified-fee-calculator.service.ts` around lines 37 - 49, The truthy checks on numeric optionals cause valid zeros to be ignored; update the three conditions to use null/undefined-safe checks (e.g., != null) instead of truthy checks: replace checks on config.minimumFeeXLM, config.maximumFeeXLM, and xlmToUsdRate so they explicitly test for null/undefined before using them to adjust feeAmountXLM or compute feeAmountUSD (refer to variables/config fields feeAmountXLM, feeAmountUSD, config.minimumFeeXLM, config.maximumFeeXLM, and xlmToUsdRate).
🟡 Minor comments (1)
backend/src/fees/fee-domain.md-47-50 (1)
47-50:⚠️ Potential issue | 🟡 MinorUpdate the fee-recording integration point.
This says fees are recorded when tips are created, but the new flow records them in
backend/src/tips/tip-verification.job.tsafter verification. Leaving the old timing here will mislead anyone integrating with or debugging the async pipeline.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/fees/fee-domain.md` around lines 47 - 50, The documentation entry is outdated: fees are no longer recorded at tip creation but after verification; update the fee-recording integration point in this doc to state that fee records are created by the verification job (see tip-verification.job.ts) after a tip passes verification rather than by the Tips Service at create time, and remove or replace the bullet line that currently reads "Tips Service: Automatically records fees when tips are created" to reflect the new async flow involving tip-verification.job.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/api-version/api-version.module.ts`:
- Around line 4-7: App is not importing ApiVersionModule so ApiVersionController
never mounts; open the application root module (AppModule) and add
ApiVersionModule to the imports array so Nest can register the controller and
expose /api-version; reference the ApiVersionModule symbol (and
ApiVersionController for verification) when updating the imports list in the
AppModule.
In `@backend/src/fees/unified-fees.controller.ts`:
- Around line 44-56: The routes (e.g., the updateConfiguration method) are only
protected by JwtAuthGuard so any authenticated user can call admin-only
endpoints; add an authorization guard and role check to enforce admin
privileges. Update the controller methods that currently use JwtAuthGuard
(including updateConfiguration and the other fee endpoints) to use a role-based
guard (e.g., RolesGuard or a PoliciesGuard) and annotate them with an admin role
requirement (e.g., `@Roles`('admin') or equivalent) so the guard checks
CurrentUser role/permissions before calling feesService methods; ensure the
guard is registered via `@UseGuards` alongside JwtAuthGuard and that the role
decorator name matches your existing auth implementation (Roles, AdminOnly,
etc.).
- Around line 51-55: The controller passes the wrong property from the
CurrentUser object into feesService.updateConfiguration: CurrentUserData exposes
userId (not id), so updateConfiguration(`@Body`() updateConfigDto, `@CurrentUser`()
user: CurrentUserData) should pass user.userId (or change the decorator to
`@CurrentUser`('id') to extract the id directly). Update the call to
feesService.updateConfiguration to use the correct identifier (user.userId) or
adjust the CurrentUser decorator to return the id so the method receives the
actual current-user id.
In `@backend/src/fees/unified-fees.service.ts`:
- Around line 132-165: The code currently falls back to parseFloat(tip.amount)
for tipAmountXLM even when conversion from a non-XLM asset failed; update the
logic so that if stellarService.getConversionRate fails for a non-XLM tip (in
the block using convertedAmountXLM and xlmToUsdRate) you do NOT use tip.amount
as XLM — instead propagate the failure or mark the fee as uncalculable: set
convertedAmountXLM to null/undefined and ensure tipAmountXLM is derived only
from convertedAmountXLM (no fallback to tip.amount), or throw/return a clear
ConversionFailed/error from the function so upstream logic can handle an
uncalculable fee; touch the code around convertedAmountXLM, xlmToUsdRate,
tipAmountXLM and the stellarService.getConversionRate calls to implement this
behavior.
In `@backend/src/redis-platform/redis-platform.module.ts`:
- Around line 20-69: The module factory methods currently only register the raw
client token; update both RedisPlatformModule.forRoot and
RedisPlatformModule.forRootAsync to add RedisPlatformService to the providers
array (and to exports so it is injectable) and to add
RedisPlatformHealthController to the controllers array (so the HTTP endpoints
mount); also ensure forRootAsync registers REDIS_PLATFORM_OPTIONS if any other
code expects that token (add an optionsProvider similar to forRoot or provide a
noop/default provider) so the service can inject options alongside the
REDIS_PLATFORM_CLIENT.
In `@backend/src/tips/tip-verification-queue.module.ts`:
- Around line 1-3: Change the incorrect import from '@nestjs/bullmq' to import
BullModule (not BullMQModule) and register the queue via
BullModule.registerQueue(...) in the TipVerificationQueue module; additionally
ensure TipVerificationJob is implemented as a consumer class decorated with
`@Processor`(...) that extends WorkerHost and exposes an async process(job: Job)
method (not just provided as a provider) so the queue and processor are properly
wired (update module providers/exports to include the processor class and import
the BullModule registration for the queue name).
In `@backend/src/tips/tip-verification.job.ts`:
- Around line 53-57: The code is marking tips as TipStatus.VERIFIED before
performing durable side effects, which can cause missing fee records or
notifications if later steps fail; change the flow in the tip-verification job
so you first perform and persist the required side effects (call the fee
recording function—e.g., recordTipFee—and send notifications—e.g.,
sendTipNotifications) and only after those operations succeed mark the tip as
VERIFIED (via markTipVerified), or wrap them in a single DB transaction/atomic
operation so that markTipVerified is committed only when fee recording and
notification persistence succeed; update the code paths currently
short-circuited by the early return on TipStatus.VERIFIED (the checks and the
blocks around markTipVerified, recordTipFee, and sendTipNotifications)
accordingly.
- Line 4: The file imports and implements a non-existent JobHandler and calls
job.add(), so replace the wrong BullMQ/NestJS usage: remove the JobHandler
import and any implementation of it, annotate the class with `@Processor`(...) and
have it extend WorkerHost, implement an async process(job: Job) method (use the
existing handler logic inside process), inject the queue with `@InjectQueue`(...)
into the class constructor and replace any job.add(...) calls with
this.queue.add(...), and remove manual re-enqueueing logic so you rely on the
queue/module-level attempts/backoff instead of stacking retries.
- Around line 60-63: Replace non-existent method/property calls and bad partial
casts: call ModerationService.moderateTipMessage instead of checkTip, call
StellarService.getTransactionDetails instead of getTransaction, and use
NotificationsService.create (not createNotification). Replace
NotificationType.TIP_FAILED/TIP_BLOCKED/TIP_VERIFIED with existing enum values
(e.g. VERIFICATION_UPDATE or GENERAL) as appropriate, and map TipStatus.BLOCKED
to an existing status like FAILED or REVERSED per business rules. Use
Tip.fromUser instead of senderId, remove or stop referencing non-existent
Tip.verificationDetails, and persist moderation metadata using fields that exist
on Tip (or extend the entity if needed) rather than
Tip.moderationReason/Tip.moderatedAt. Finally, stop casting ({ id: tipId } as
Tip); load the full Tip entity (e.g., via repository findOne/getById) before
passing it to moderation and downstream logic to avoid missing properties.
---
Major comments:
In `@backend/src/fees/entities/unified-platform-fee.entity.ts`:
- Around line 19-21: The platform_fees entity currently declares a non-unique
index on tipId which allows duplicate fee rows and breaks idempotency; update
the entity decorators to enforce a unique constraint on tipId (e.g., change the
`@Index`(['tipId']) to a unique index or add unique: true on the tipId column) so
the DB prevents duplicate inserts, and make the same change for the analogous
index referenced in the later block (lines 27-32) to ensure idempotent replay at
the database level.
In `@backend/src/fees/unified-fee-calculator.service.ts`:
- Around line 60-68: The parsePeriodToDate helper currently uses parseInt which
accepts strings like "30d" and throws a generic Error; update parsePeriodToDate
to fully validate the input with a strict numeric check (e.g. /^\d+$/) before
parsing, use Number or parseInt after validation, and replace the thrown Error
with a BadRequestException containing a clear message (so callers in
unified-fees.service.ts receive a 400). Keep the rest of the logic (calculating
now - value * 24*60*60*1000) intact and reference the parsePeriodToDate function
when making this change.
- Around line 37-49: The truthy checks on numeric optionals cause valid zeros to
be ignored; update the three conditions to use null/undefined-safe checks (e.g.,
!= null) instead of truthy checks: replace checks on config.minimumFeeXLM,
config.maximumFeeXLM, and xlmToUsdRate so they explicitly test for
null/undefined before using them to adjust feeAmountXLM or compute feeAmountUSD
(refer to variables/config fields feeAmountXLM, feeAmountUSD,
config.minimumFeeXLM, config.maximumFeeXLM, and xlmToUsdRate).
In `@backend/src/fees/unified-fees.service.ts`:
- Around line 175-186: The platform fee creation is not idempotent for a given
tipId; modify the logic in unified-fees.service.ts (the code using
platformFeeRepo.create and platformFeeRepo.save) to first lookup an existing
PlatformFee by tipId and return it if found, otherwise create and save a new
record; if you need to allow updates on retries, perform an
upsert/update-by-tipId (e.g., findOne({ tipId }) then assign result fields
feePercentage/feeAmountXLM/feeAmountUSD/collectionStatus/stellarTxHash and save)
so duplicate inserts are avoided and the existing fee row is returned on
retries.
- Around line 175-184: The code currently reuses the same stellarTxHash field
for both the tip payment and the later fee-collection transaction; update the
entity and calls so these are distinct: set a tipStellarTxHash (or
tipPaymentStellarTxHash) when creating the platform fee in
platformFeeRepo.create (where stellarTxHash is currently assigned) and introduce
a separate feeCollectionStellarTxHash (or feeStellarTxHash) to be populated only
during fee collection (replace the assignment currently happening on lines
~199-200). Ensure all references to stellarTxHash in the fee creation and
collection flows (e.g., platformFeeRepo.create and the fee-collection update
path) are updated to use the new dedicated fields and migrate any database
schema/ORM mappings accordingly.
- Around line 93-95: The min/max validation uses truthy checks so zero values
are skipped; update the condition in the validation block that checks
dto.minimumFeeXLM and dto.maximumFeeXLM (the code around the throw new
BadRequestException('minimumFeeXLM cannot exceed maximumFeeXLM')) to test for
presence with explicit null/undefined checks (e.g., use dto.minimumFeeXLM !=
null and dto.maximumFeeXLM != null or !== undefined) before comparing values,
then keep the existing throw when minimumFeeXLM > maximumFeeXLM.
In `@backend/src/redis-platform/redis-platform.health.controller.ts`:
- Around line 10-15: The getHealth endpoint currently returns the healthCheck()
payload with HTTP 200 even when Redis is unhealthy; change getHealth to await
this.redisService.healthCheck(), inspect the returned object (e.g., check
health.status !== 'up' or the failure flag in the payload) and when unhealthy
throw a Nest HttpException with HttpStatus.SERVICE_UNAVAILABLE (503) carrying
the health payload; return the health payload unchanged only when healthy. Use
the getHealth method and redisService.healthCheck() identifiers to locate and
modify the code.
In `@backend/src/redis-platform/redis-platform.module.ts`:
- Around line 72-117: The Redis client created in
RedisPlatformModule.createClient opens a long-lived connection but is never
closed; implement a lifecycle hook (e.g., have RedisPlatformModule or the
provider class implement OnModuleDestroy or OnApplicationShutdown) that retains
a reference to the client created by createClient and calls client.quit() (or
client.disconnect() if quit may hang) during shutdown; ensure the
module/provider exposes the client instance (or stores it on the class) and
invoke the graceful close inside the lifecycle method, handling errors and
awaiting quit/disconnect before completing shutdown.
In `@backend/src/redis-platform/redis-platform.service.spec.ts`:
- Around line 170-201: The tests for service.healthCheck incorrectly overwrite
Date.now by calling mockReturnValue twice on the same spy; change each test to
set Date.now to return the start value first and the end value second (e.g., use
jest.spyOn(Date, 'now').mockReturnValueOnce(start).mockReturnValueOnce(end) or
mockImplementationOnce for the two calls) so responseTime computes correctly in
both the healthy and unhealthy cases (keep restoring the spy afterwards); update
the two blocks that currently create startSpy/endSpy in the two it(...) specs to
use the sequential mockReturnValueOnce/mockImplementationOnce pattern before
calling service.healthCheck().
In `@backend/src/redis-platform/redis-platform.service.ts`:
- Around line 226-233: The use of redis.keys in clearPattern and
getKeysByPattern must be replaced with a non-blocking SCAN approach: update
getKeysByPattern (or remove it) to iterate with redis.scan/scanStream and
collect matches incrementally, and change clearPattern to stream-scan the
pattern and delete keys in bounded batches (e.g., accumulate ~100-1000 keys then
call redis.del(...batch) or use a pipeline) until the scan is exhausted; ensure
both functions avoid calling redis.keys and handle empty results and errors
gracefully while keeping memory bounded.
- Around line 212-217: The current incrementThrottler uses two separate commands
(this.redis.incr and this.redis.expire) which can leave a permanent key if the
process dies between calls; replace that with an atomic operation (use a Redis
EVAL Lua script or a MULTI/EXEC transaction) so the increment and TTL set happen
together — e.g., implement incrementThrottler to call this.redis.eval with a
small Lua script that runs redis.call('INCR', KEYS[1]) and if the result == 1
then redis.call('EXPIRE', KEYS[1], ARGV[1]) and returns the counter, or
alternatively use this.redis.multi().incr(key).expire(key, ttl).exec() to ensure
atomicity. Ensure you remove the separate this.redis.expire call and return the
increment result as before.
- Around line 137-148: The INFO parser fails on LF-only Redis output because
parseInfo currently splits info with '\r\n'; change it to normalize newlines
(e.g. split on /\r?\n/ or replace /\r\n?/g with '\n') so parseInfo(info,
section) correctly finds sectionStart and sectionEnd for both CRLF and LF
inputs, and ensure you still guard against sectionStart === -1 before slicing to
avoid returning empty sections.
In `@backend/src/tips/tip-verification.job.ts`:
- Around line 147-169: The current verification only compares payment.asset_code
to tip.assetCode, allowing different issuers with the same code to pass; update
the verification in tip-verification.job.ts where payment is examined so that
for non-native assets you compare both payment.asset_code === tip.assetCode and
payment.asset_issuer === tip.assetIssuer (or, equivalently, require
tip.assetIssuer to match payment.asset_issuer when tip.assetCode is present),
and return false if either the code or issuer does not match (keeping native/XLM
payments unaffected).
---
Minor comments:
In `@backend/src/fees/fee-domain.md`:
- Around line 47-50: The documentation entry is outdated: fees are no longer
recorded at tip creation but after verification; update the fee-recording
integration point in this doc to state that fee records are created by the
verification job (see tip-verification.job.ts) after a tip passes verification
rather than by the Tips Service at create time, and remove or replace the bullet
line that currently reads "Tips Service: Automatically records fees when tips
are created" to reflect the new async flow involving tip-verification.job.ts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 21b329e3-0fc0-4e73-b1b0-68000552bed6
📒 Files selected for processing (16)
backend/src/api-version/api-version.controller.tsbackend/src/api-version/api-version.module.tsbackend/src/fees/entities/unified-fee-configuration.entity.tsbackend/src/fees/entities/unified-platform-fee.entity.tsbackend/src/fees/fee-domain.mdbackend/src/fees/unified-fee-calculator.service.tsbackend/src/fees/unified-fees.controller.tsbackend/src/fees/unified-fees.module.tsbackend/src/fees/unified-fees.service.spec.tsbackend/src/fees/unified-fees.service.tsbackend/src/redis-platform/redis-platform.health.controller.tsbackend/src/redis-platform/redis-platform.module.tsbackend/src/redis-platform/redis-platform.service.spec.tsbackend/src/redis-platform/redis-platform.service.tsbackend/src/tips/tip-verification-queue.module.tsbackend/src/tips/tip-verification.job.ts
Comment on lines
+4
to
+7
| @Module({ | ||
| controllers: [ApiVersionController], | ||
| }) | ||
| export class ApiVersionModule {} |
There was a problem hiding this comment.
Import ApiVersionModule into the application root.
backend/src/app.module.ts:86-140 still omits this module, so Nest never mounts ApiVersionController and /api-version stays unreachable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/api-version/api-version.module.ts` around lines 4 - 7, App is not
importing ApiVersionModule so ApiVersionController never mounts; open the
application root module (AppModule) and add ApiVersionModule to the imports
array so Nest can register the controller and expose /api-version; reference the
ApiVersionModule symbol (and ApiVersionController for verification) when
updating the imports list in the AppModule.
Comment on lines
+44
to
+56
| @Post('configuration') | ||
| @UseGuards(JwtAuthGuard) | ||
| @ApiBearerAuth() | ||
| @ApiOperation({ summary: 'Update fee configuration (admin only)' }) | ||
| @ApiResponse({ status: 201, description: 'Fee configuration created' }) | ||
| @ApiResponse({ status: 400, description: 'Invalid configuration' }) | ||
| @ApiResponse({ status: 401, description: 'Unauthorized' }) | ||
| async updateConfiguration( | ||
| @Body() updateConfigDto: UpdateFeeConfigDto, | ||
| @CurrentUser() user: CurrentUserData, | ||
| ) { | ||
| return this.feesService.updateConfiguration(updateConfigDto, user.id); | ||
| } |
There was a problem hiding this comment.
admin only routes are only authenticated, not authorized.
JwtAuthGuard only checks that a JWT is present; it does not enforce admin privileges. Right now any authenticated user can change the platform fee configuration or mark fees as collected.
Also applies to: 94-107
🧰 Tools
🪛 GitHub Actions: Backend CI
[error] 55-55: TS2339: Property 'id' does not exist on type 'CurrentUserData' (this.feesService.updateConfiguration(..., user.id)).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/fees/unified-fees.controller.ts` around lines 44 - 56, The routes
(e.g., the updateConfiguration method) are only protected by JwtAuthGuard so any
authenticated user can call admin-only endpoints; add an authorization guard and
role check to enforce admin privileges. Update the controller methods that
currently use JwtAuthGuard (including updateConfiguration and the other fee
endpoints) to use a role-based guard (e.g., RolesGuard or a PoliciesGuard) and
annotate them with an admin role requirement (e.g., `@Roles`('admin') or
equivalent) so the guard checks CurrentUser role/permissions before calling
feesService methods; ensure the guard is registered via `@UseGuards` alongside
JwtAuthGuard and that the role decorator name matches your existing auth
implementation (Roles, AdminOnly, etc.).
Comment on lines
+51
to
+55
| async updateConfiguration( | ||
| @Body() updateConfigDto: UpdateFeeConfigDto, | ||
| @CurrentUser() user: CurrentUserData, | ||
| ) { | ||
| return this.feesService.updateConfiguration(updateConfigDto, user.id); |
There was a problem hiding this comment.
Use the actual current-user field name here.
CurrentUserData exposes userId (or @CurrentUser('id')), not id, so this is a compile failure and currently passes undefined into updateConfiguration.
Suggested change
- async updateConfiguration(
- `@Body`() updateConfigDto: UpdateFeeConfigDto,
- `@CurrentUser`() user: CurrentUserData,
- ) {
- return this.feesService.updateConfiguration(updateConfigDto, user.id);
+ async updateConfiguration(
+ `@Body`() updateConfigDto: UpdateFeeConfigDto,
+ `@CurrentUser`('id') userId: string,
+ ) {
+ return this.feesService.updateConfiguration(updateConfigDto, userId);
}📝 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.
Suggested change
| async updateConfiguration( | |
| @Body() updateConfigDto: UpdateFeeConfigDto, | |
| @CurrentUser() user: CurrentUserData, | |
| ) { | |
| return this.feesService.updateConfiguration(updateConfigDto, user.id); | |
| async updateConfiguration( | |
| `@Body`() updateConfigDto: UpdateFeeConfigDto, | |
| `@CurrentUser`('id') userId: string, | |
| ) { | |
| return this.feesService.updateConfiguration(updateConfigDto, userId); | |
| } |
🧰 Tools
🪛 GitHub Actions: Backend CI
[error] 55-55: TS2339: Property 'id' does not exist on type 'CurrentUserData' (this.feesService.updateConfiguration(..., user.id)).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/fees/unified-fees.controller.ts` around lines 51 - 55, The
controller passes the wrong property from the CurrentUser object into
feesService.updateConfiguration: CurrentUserData exposes userId (not id), so
updateConfiguration(`@Body`() updateConfigDto, `@CurrentUser`() user:
CurrentUserData) should pass user.userId (or change the decorator to
`@CurrentUser`('id') to extract the id directly). Update the call to
feesService.updateConfiguration to use the correct identifier (user.userId) or
adjust the CurrentUser decorator to return the id so the method receives the
actual current-user id.
Comment on lines
+132
to
+165
| if (tip.assetCode !== 'XLM') { | ||
| try { | ||
| const conversion = await this.stellarService.getConversionRate( | ||
| tip.assetCode, | ||
| tip.assetIssuer || null, | ||
| 'XLM', | ||
| null, | ||
| tip.amount, | ||
| ); | ||
| const estimated = conversion.estimatedAmount; | ||
| const estimatedStr = | ||
| typeof estimated === 'string' ? estimated : estimated.toString(); | ||
| convertedAmountXLM = parseFloat(estimatedStr); | ||
|
|
||
| // Get USD rate for fee calculation | ||
| const usdConversion = await this.stellarService.getConversionRate( | ||
| 'XLM', | ||
| null, | ||
| 'USD', | ||
| null, | ||
| convertedAmountXLM, | ||
| ); | ||
| const usdEstimated = usdConversion.estimatedAmount; | ||
| const usdEstimatedStr = | ||
| typeof usdEstimated === 'string' ? usdEstimated : usdEstimated.toString(); | ||
| xlmToUsdRate = parseFloat(usdEstimatedStr) / convertedAmountXLM; | ||
| } catch { | ||
| convertedAmountXLM = null; | ||
| xlmToUsdRate = undefined; | ||
| } | ||
| } | ||
|
|
||
| const tipAmountXLM = convertedAmountXLM || parseFloat(tip.amount.toString()); | ||
|
|
There was a problem hiding this comment.
Don't price non-XLM tips with the raw source amount when conversion fails.
If the quote fails, Lines 158-165 fall back to tip.amount even though it is still denominated in the original asset. A failed 10 USDC -> XLM conversion will be charged as if it were 10 XLM, which records the wrong fee. Propagate the failure or mark the fee as uncalculable instead of guessing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/fees/unified-fees.service.ts` around lines 132 - 165, The code
currently falls back to parseFloat(tip.amount) for tipAmountXLM even when
conversion from a non-XLM asset failed; update the logic so that if
stellarService.getConversionRate fails for a non-XLM tip (in the block using
convertedAmountXLM and xlmToUsdRate) you do NOT use tip.amount as XLM — instead
propagate the failure or mark the fee as uncalculable: set convertedAmountXLM to
null/undefined and ensure tipAmountXLM is derived only from convertedAmountXLM
(no fallback to tip.amount), or throw/return a clear ConversionFailed/error from
the function so upstream logic can handle an uncalculable fee; touch the code
around convertedAmountXLM, xlmToUsdRate, tipAmountXLM and the
stellarService.getConversionRate calls to implement this behavior.
Comment on lines
+20
to
+69
| @Global() | ||
| @Module({}) | ||
| export class RedisPlatformModule { | ||
| private static readonly logger = new Logger(RedisPlatformModule.name); | ||
|
|
||
| static forRoot(options: RedisPlatformOptions): DynamicModule { | ||
| const optionsProvider: Provider = { | ||
| provide: REDIS_PLATFORM_OPTIONS, | ||
| useValue: options, | ||
| }; | ||
|
|
||
| const clientProvider: Provider = { | ||
| provide: REDIS_PLATFORM_CLIENT, | ||
| useFactory: () => RedisPlatformModule.createClient(options), | ||
| }; | ||
|
|
||
| return { | ||
| module: RedisPlatformModule, | ||
| imports: [], | ||
| providers: [optionsProvider, clientProvider], | ||
| exports: [REDIS_PLATFORM_CLIENT, REDIS_PLATFORM_OPTIONS], | ||
| }; | ||
| } | ||
|
|
||
| static forRootAsync(): DynamicModule { | ||
| const clientProvider: Provider = { | ||
| provide: REDIS_PLATFORM_CLIENT, | ||
| inject: [ConfigService], | ||
| useFactory: (config: ConfigService) => { | ||
| const options: RedisPlatformOptions = { | ||
| host: config.get<string>('REDIS_HOST', 'localhost'), | ||
| port: config.get<number>('REDIS_PORT', 6379), | ||
| password: config.get<string>('REDIS_PASSWORD'), | ||
| db: config.get<number>('REDIS_DB', 0), | ||
| keyPrefix: config.get<string>('REDIS_KEY_PREFIX', 'tip-tune:'), | ||
| connectTimeout: config.get<number>('REDIS_CONNECT_TIMEOUT', 5_000), | ||
| maxRetriesPerRequest: config.get<number>('REDIS_MAX_RETRIES', 3), | ||
| enableReadyCheck: true, | ||
| lazyConnect: false, | ||
| }; | ||
| return RedisPlatformModule.createClient(options); | ||
| }, | ||
| }; | ||
|
|
||
| return { | ||
| module: RedisPlatformModule, | ||
| imports: [ConfigModule], | ||
| providers: [clientProvider], | ||
| exports: [REDIS_PLATFORM_CLIENT], | ||
| }; |
There was a problem hiding this comment.
Wire the Redis service and controller into this module.
forRoot() / forRootAsync() only register the raw client token. RedisPlatformService and RedisPlatformHealthController are never declared here, so the wrapper service is not injectable from this module and the new HTTP endpoints are not mounted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/redis-platform/redis-platform.module.ts` around lines 20 - 69,
The module factory methods currently only register the raw client token; update
both RedisPlatformModule.forRoot and RedisPlatformModule.forRootAsync to add
RedisPlatformService to the providers array (and to exports so it is injectable)
and to add RedisPlatformHealthController to the controllers array (so the HTTP
endpoints mount); also ensure forRootAsync registers REDIS_PLATFORM_OPTIONS if
any other code expects that token (add an optionsProvider similar to forRoot or
provide a noop/default provider) so the service can inject options alongside the
REDIS_PLATFORM_CLIENT.
Comment on lines
+1
to
+3
| import { Module } from '@nestjs/common'; | ||
| import { BullMQModule } from '@nestjs/bullmq'; | ||
| import { TipVerificationJob } from './tip-verification.job'; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @nestjs/bullmq10.2.3, what APIs are used to register a queue and consume jobs? Does the package exportBullMQModule, or is it BullModulewith the Nest BullMQ processor API such as@processor(...)/WorkerHost?
💡 Result:
In @nestjs/bullmq version 10.2.3, queues are registered using BullModule.forRoot for global configuration and BullModule.registerQueue({ name: 'queueName' }) for specific queues. Jobs are consumed using the Nest BullMQ processor API: @Processor('queueName') decorator on a class extending WorkerHost, with an async process(job: Job) method. The package exports BullModule (not BullMQModule), along with Processor, WorkerHost, and OnWorkerEvent from '@nestjs/bullmq'.
Citations:
- 1: https://docs.bullmq.io/guide/nestjs
- 2: https://docs.nestjs.com/techniques/queues
- 3: https://github.com/nestjs/bull/releases
- 4: https://github.com/nestjs/bull/tree/master
- 5: https://github.com/nestjs/bull/blob/6f49817c/packages/bullmq/lib/index.ts
Fix the import and add proper queue/processor registration.
@nestjs/bullmq exports BullModule (not BullMQModule), causing a compile error. Additionally, registering the queue as a provider is insufficient—the job consumer must use the @Processor() decorator on a class extending WorkerHost with an async process(job: Job) method.
🧰 Tools
🪛 GitHub Actions: Backend CI
[error] 2-2: TS2724: '@nestjs/bullmq' has no exported member 'BullMQModule'. Did you mean 'BullModule'?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/tips/tip-verification-queue.module.ts` around lines 1 - 3, Change
the incorrect import from '@nestjs/bullmq' to import BullModule (not
BullMQModule) and register the queue via BullModule.registerQueue(...) in the
TipVerificationQueue module; additionally ensure TipVerificationJob is
implemented as a consumer class decorated with `@Processor`(...) that extends
WorkerHost and exposes an async process(job: Job) method (not just provided as a
provider) so the queue and processor are properly wired (update module
providers/exports to include the processor class and import the BullModule
registration for the queue name).
| import { Injectable, Logger } from '@nestjs/common'; | ||
| import { InjectRepository } from '@nestjs/typeorm'; | ||
| import { Repository } from 'typeorm'; | ||
| import { Job, JobHandler } from 'bullmq'; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In BullMQ and @nestjs/bullmqfor NestJS 10, what is the supported way to implement a worker and trigger retries from inside a handler? DoesbullmqexportJobHandler, and does Jobexpose anadd() method?
💡 Result:
In BullMQ with @nestjs/bullmq for NestJS 10, the supported way to implement a worker (processor) is to create a class decorated with @Processor('queueName') that extends WorkerHost and implements an async process(job: Job) method. To trigger retries from inside a handler: - For automatic retries on failure, configure attempts and backoff when adding the job via Queue.add (e.g., { attempts: 3, backoff: { type: 'exponential', delay: 1000 } }). If the process throws an Error, BullMQ automatically retries according to these options. - For manual retry of the current job (e.g., after completion or failure), call await job.retry('completed') or await job.retry('failed', { resetAttemptsMade: true }). This moves the job back to waiting. - To add a new job (for retry-like behavior), inject Queue via @InjectQueue('queueName') and call this.queue.add(name, data, opts). bullmq does not export a type named JobHandler; the processor is typed as (job: Job) => Promise, referred to as Processor type alias. Job does not expose an add method; static Job.create or Queue.add / addBulk are used to add jobs. Job has retry, updateProgress, log, etc.
Citations:
- 1: https://docs.bullmq.io/guide/nestjs
- 2: https://oneuptime.com/blog/post/2026-03-31-redis-nestjs-bullmq-jobs/view
- 3: https://docs.nestjs.com/techniques/queues
- 4: https://docs.bullmq.io/guide/workers
- 5: https://docs.bullmq.io/guide/jobs/retrying-job
- 6: https://docs.bullmq.io/guide/retrying-failing-jobs
- 7: https://api.docs.bullmq.io/classes/v3.Job.html
- 8: https://api.docs.bullmq.io/types/v2.Processor.html
- 9: https://api.docs.bullmq.io/classes/v5.Queue.html
🏁 Script executed:
cat -n backend/src/tips/tip-verification.job.ts | head -50Repository: OlufunbiIK/tip-tune
Length of output: 2476
🏁 Script executed:
cat -n backend/src/tips/tip-verification.job.ts | sed -n '250,275p'Repository: OlufunbiIK/tip-tune
Length of output: 818
Fix invalid BullMQ worker implementation.
The code imports non-existent JobHandler from bullmq (line 4) and attempts to implement it (line 22). Additionally, job.add() is called at line 261, but the Job class does not expose an add() method. This code will not compile.
The correct BullMQ/NestJS implementation uses @Processor() decorator with a class extending WorkerHost, and an async process(job: Job) method. To trigger retries, inject the queue via @InjectQueue() and call this.queue.add() rather than job.add().
Even after fixing the API usage, manually re-enqueueing jobs would stack on top of the queue-level attempts and backoff configured in the module, causing duplicate retry logic.
🧰 Tools
🪛 GitHub Actions: Backend CI
[error] 4-4: TS2305: Module 'bullmq' has no exported member 'JobHandler'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/tips/tip-verification.job.ts` at line 4, The file imports and
implements a non-existent JobHandler and calls job.add(), so replace the wrong
BullMQ/NestJS usage: remove the JobHandler import and any implementation of it,
annotate the class with `@Processor`(...) and have it extend WorkerHost, implement
an async process(job: Job) method (use the existing handler logic inside
process), inject the queue with `@InjectQueue`(...) into the class constructor and
replace any job.add(...) calls with this.queue.add(...), and remove manual
re-enqueueing logic so you rely on the queue/module-level attempts/backoff
instead of stacking retries.
Comment on lines
+53
to
+57
| // Skip if already verified | ||
| if (tip.status === TipStatus.VERIFIED) { | ||
| this.logger.log(`Tip ${tipId} already verified, skipping`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Don't set VERIFIED before the required side effects are durable.
Once markTipVerified runs, any later exception in fee recording or notifications schedules a retry, and the next run exits early on Line 54 because the tip is already VERIFIED. That can leave the system with a verified tip but no fee record and no notifications.
Also applies to: 82-92, 96-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/tips/tip-verification.job.ts` around lines 53 - 57, The code is
marking tips as TipStatus.VERIFIED before performing durable side effects, which
can cause missing fee records or notifications if later steps fail; change the
flow in the tip-verification job so you first perform and persist the required
side effects (call the fee recording function—e.g., recordTipFee—and send
notifications—e.g., sendTipNotifications) and only after those operations
succeed mark the tip as VERIFIED (via markTipVerified), or wrap them in a single
DB transaction/atomic operation so that markTipVerified is committed only when
fee recording and notification persistence succeed; update the code paths
currently short-circuited by the early return on TipStatus.VERIFIED (the checks
and the blocks around markTipVerified, recordTipFee, and sendTipNotifications)
accordingly.
Comment on lines
+60
to
+63
| const moderationResult = await this.moderationService.checkTip(tip); | ||
| if (!moderationResult.allowed) { | ||
| this.logger.warn(`Tip ${tipId} blocked by moderation: ${moderationResult.reason}`); | ||
| await this.handleModerationBlock(tip, moderationResult.reason); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 'class\s+ModerationService|checkTip\s*\(' backend/src/moderation backend/src/tips
rg -n -C2 'class\s+StellarService|getTransaction\s*\(' backend/src/stellar
rg -n -C2 'class\s+NotificationsService|createNotification\s*\(' backend/src/notifications
rg -n -C2 'enum\s+NotificationType|TIP_FAILED|TIP_BLOCKED|TIP_VERIFIED' backend/src/notifications
rg -n -C2 'enum\s+TipStatus|verificationDetails|moderationReason|moderatedAt|senderId' backend/src/tipsRepository: OlufunbiIK/tip-tune
Length of output: 5807
🏁 Script executed:
cat -n backend/src/moderation/moderation.service.tsRepository: OlufunbiIK/tip-tune
Length of output: 8781
🏁 Script executed:
cat -n backend/src/stellar/stellar.service.tsRepository: OlufunbiIK/tip-tune
Length of output: 7192
🏁 Script executed:
cat -n backend/src/notifications/notifications.service.tsRepository: OlufunbiIK/tip-tune
Length of output: 8855
🏁 Script executed:
cat -n backend/src/notifications/notification.entity.tsRepository: OlufunbiIK/tip-tune
Length of output: 1763
🏁 Script executed:
cat -n backend/src/tips/entities/tip.entity.tsRepository: OlufunbiIK/tip-tune
Length of output: 5028
🏁 Script executed:
cat -n backend/src/tips/tip-verification.job.ts | head -120Repository: OlufunbiIK/tip-tune
Length of output: 5328
🏁 Script executed:
cat -n backend/src/tips/tip-verification.job.ts | tail -150Repository: OlufunbiIK/tip-tune
Length of output: 5501
🏁 Script executed:
cat -n backend/src/artists/entities/artist.entity.ts | head -100Repository: OlufunbiIK/tip-tune
Length of output: 2900
Align service method calls and entity properties with actual codebase contracts.
This file calls methods and properties that don't exist in the current codebase:
- Line 60:
ModerationService.checkTip()does not exist (service hasmoderateTipMessage) - Line 117:
StellarService.getTransaction()does not exist (service hasgetTransactionDetails) - Lines 196, 216, 230, 244:
NotificationsService.createNotification()does not exist (service method iscreate) - Lines 198, 218, 246:
NotificationType.TIP_FAILED,TIP_BLOCKED,TIP_VERIFIEDdo not exist (enum only hasTIP_RECEIVED,BADGE_EARNED,SYSTEM,GENERAL,COLLABORATION_INVITE,COLLABORATION_RESPONSE,VERIFICATION_UPDATE,LICENSE_REQUEST,TRACK_RELEASED,LICENSE_RESPONSE,EVENT_LIVE,EVENT_REMINDER,EVENT_CANCELLED) - Line 209:
TipStatus.BLOCKEDdoes not exist (enum only hasPENDING,VERIFIED,FAILED,REVERSED) - Lines 197, 217, 239, 245:
Tip.senderIddoes not exist (entity usesfromUser) - Line 183:
Tip.verificationDetailsdoes not exist - Lines 210–211:
Tip.moderationReasonandTip.moderatedAtdo not exist - Line 102: Casting
{ id: tipId } as Tipcreates a partial object that causes failures downstream when properties likesenderIdare accessed
🧰 Tools
🪛 GitHub Actions: Backend CI
[error] 60-60: TS2339: Property 'checkTip' does not exist on type 'ModerationService'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/tips/tip-verification.job.ts` around lines 60 - 63, Replace
non-existent method/property calls and bad partial casts: call
ModerationService.moderateTipMessage instead of checkTip, call
StellarService.getTransactionDetails instead of getTransaction, and use
NotificationsService.create (not createNotification). Replace
NotificationType.TIP_FAILED/TIP_BLOCKED/TIP_VERIFIED with existing enum values
(e.g. VERIFICATION_UPDATE or GENERAL) as appropriate, and map TipStatus.BLOCKED
to an existing status like FAILED or REVERSED per business rules. Use
Tip.fromUser instead of senderId, remove or stop referencing non-existent
Tip.verificationDetails, and persist moderation metadata using fields that exist
on Tip (or extend the entity if needed) rather than
Tip.moderationReason/Tip.moderatedAt. Finally, stop casting ({ id: tipId } as
Tip); load the full Tip entity (e.g., via repository findOne/getById) before
passing it to moderation and downstream logic to avoid missing properties.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fee Module Consolidation #411 Fee Module Consolidation
Version Endpoint Consolidation #412 Version Endpoint Consolidation
Redis Layer Consolidation #413 Redis Layer Consolidation
Async Tip Verification and Reconciliation Pipeline #414 Async Tip Verification Pipeline
Fee Module Consolidation #411 Fee Module Consolidation
closes #411
closes #412
closes #413
closes #414
Summary by CodeRabbit
Release Notes
New Features
Tests