TASK #00000 : Transaction Manual override API#666
TASK #00000 : Transaction Manual override API#666vijaykhollam merged 2 commits intotekdi:aspire-leadersfrom
Conversation
WalkthroughAdds a manual payment status override feature: new DTO and API IDs, controller endpoints, entity field, transaction lookup, and service workflows to update intents and related transactions, record an override reason, and trigger PAID-related side effects. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Controller as PaymentsController
participant Service as PaymentService
participant TransactionService as PaymentTransactionService
participant DB as Database
Client->>Controller: PATCH /payments/.../status/override (OverridePaymentStatusDto)
Controller->>Controller: validate DTO
Controller->>Service: overridePaymentStatus(paymentIntentId|transactionId, status, reason)
alt called by transactionId
Service->>TransactionService: findById(transactionId)
TransactionService->>DB: fetch transaction -> returns paymentIntentId
end
Service->>DB: fetch PaymentIntent by id
Service->>Service: mapIntentStatusToTransactionStatus(status)
Service->>DB: update PaymentIntent (status, statusReason)
Service->>DB: fetch related transactions
Service->>DB: update transactions with mapped status (and failureReason if applicable)
alt status == PAID
Service->>DB: unlock related targets
Service->>Service: trigger certificate generation / coupon redemption
end
DB-->>Service: updated intent + transactions
Service-->>Controller: PaymentStatusResponseDto (includes statusReason)
Controller-->>Client: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/payments/payments.controller.ts (1)
65-127:⚠️ Potential issue | 🔴 CriticalAdd authentication/authorization guards to override endpoints.
The
overridePaymentStatusByTransactionIdandoverridePaymentStatusendpoints lack authentication and authorization guards (@UseGuards). These are sensitive admin/support operations that require explicit protection. No global guards (APP_GUARD), module-level guards, or@Rolesdecorators are configured for the PaymentsModule or its endpoints. Add@UseGuards(JwtAuthGuard, RbacAuthGuard)and@Roles('admin')decorators to both override endpoints to enforce authorization checks. The existing RbacAuthGuard will allow requests when no permissions are required, so explicit role restrictions must be added at the controller method level.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/payments/payments.controller.ts` around lines 65 - 127, The two admin endpoints overridePaymentStatusByTransactionId and overridePaymentStatus are missing authentication/authorization; add method-level guards and role restriction by annotating both methods with `@UseGuards`(JwtAuthGuard, RbacAuthGuard) and `@Roles`('admin'), and ensure the controller file imports UseGuards, Roles, JwtAuthGuard and RbacAuthGuard so the decorators resolve; keep the existing ValidationPipe/filters as-is and apply the decorators directly above each method signature for both overridePaymentStatusByTransactionId and overridePaymentStatus.
🧹 Nitpick comments (2)
src/payments/services/payment.service.ts (1)
694-700: Inefficient N+1 update pattern for transactions.Each transaction is updated individually within the loop, resulting in N database calls. Consider using a bulk update operation for better performance, especially when there are multiple transactions.
Proposed fix using bulk update
- for (const tx of intentInTx.transactions) { - tx.status = transactionStatus; - if (status === PaymentIntentStatus.FAILED && !tx.failureReason) { - tx.failureReason = 'Status manually overridden'; - } - await manager.save(PaymentTransaction, tx); - } + // Bulk update all transactions + await manager.update( + PaymentTransaction, + { paymentIntentId: intentInTx.id }, + { + status: transactionStatus, + ...(status === PaymentIntentStatus.FAILED && { + failureReason: manager + .createQueryBuilder() + .update(PaymentTransaction) + .set({ + failureReason: () => + "COALESCE(failure_reason, 'Status manually overridden')", + }) + .getQuery() + ? undefined + : 'Status manually overridden', + }), + }, + );Alternatively, a simpler approach that preserves the conditional logic:
- for (const tx of intentInTx.transactions) { - tx.status = transactionStatus; - if (status === PaymentIntentStatus.FAILED && !tx.failureReason) { - tx.failureReason = 'Status manually overridden'; - } - await manager.save(PaymentTransaction, tx); - } + if (intentInTx.transactions.length > 0) { + const transactionIds = intentInTx.transactions.map((tx) => tx.id); + await manager.update( + PaymentTransaction, + { id: In(transactionIds) }, + { status: transactionStatus }, + ); + // Set failureReason only for FAILED status where not already set + if (status === PaymentIntentStatus.FAILED) { + await manager + .createQueryBuilder() + .update(PaymentTransaction) + .set({ failureReason: 'Status manually overridden' }) + .where('id IN (:...ids)', { ids: transactionIds }) + .andWhere('failure_reason IS NULL') + .execute(); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/payments/services/payment.service.ts` around lines 694 - 700, The loop over intentInTx.transactions causes N individual saves; instead prepare all transaction entities first (iterate intentInTx.transactions, set tx.status = transactionStatus and if (status === PaymentIntentStatus.FAILED && !tx.failureReason) set tx.failureReason = 'Status manually overridden'), collect them into an array, then perform a single bulk save via manager.save(PaymentTransaction, transactionsArray) or use a single update query (e.g., queryBuilder update by ids) to persist changes; update references: intentInTx.transactions, tx.status, status, PaymentIntentStatus, tx.failureReason, PaymentTransaction, and manager.save.src/payments/dtos/override-payment-status.dto.ts (1)
1-21: Well-structured DTO with proper validation.The DTO correctly validates the
statusas an enum and ensuresreasonis a non-empty string for audit purposes.Consider adding a maximum length constraint on
reasonto prevent excessively long audit entries and potential database issues.Optional: Add MaxLength validation
import { ApiProperty } from '@nestjs/swagger'; -import { IsEnum, IsNotEmpty, IsString } from 'class-validator'; +import { IsEnum, IsNotEmpty, IsString, MaxLength } from 'class-validator'; import { PaymentIntentStatus } from '../enums/payment.enums'; export class OverridePaymentStatusDto { `@ApiProperty`({ enum: PaymentIntentStatus, description: 'New payment status to set (CREATED, PAID, FAILED, REFUNDED). When set to PAID, targets are unlocked and certificate generation is triggered if applicable.', }) `@IsEnum`(PaymentIntentStatus) status: PaymentIntentStatus; `@ApiProperty`({ description: 'Reason for the manual status override (required for audit).', example: 'Customer confirmed bank transfer received', }) `@IsNotEmpty`({ message: 'reason is required for manual status override' }) `@IsString`() + `@MaxLength`(1000, { message: 'reason must not exceed 1000 characters' }) reason: string; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/payments/dtos/override-payment-status.dto.ts` around lines 1 - 21, Add a maximum length constraint to the reason field to prevent excessively long audit entries: update OverridePaymentStatusDto to import and use MaxLength (from class-validator) on the reason property (e.g., `@MaxLength`(500)) and add a matching maxLength in the ApiProperty metadata so both validation and OpenAPI docs reflect the limit; ensure the import for MaxLength is added alongside IsNotEmpty and IsString.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/payments/entities/payment-intent.entity.ts`:
- Around line 56-57: The PaymentIntent entity's statusReason property is
declared as non-nullable string but the column is nullable; update the
TypeScript type for statusReason to reflect the DB (e.g., change to string |
null) in the PaymentIntent entity so consumers handle null values correctly;
also scan other nullable `@Column` fields in the same entity (and their property
types) to ensure they match nullable: true.
In `@src/payments/services/payment.service.ts`:
- Around line 720-750: The code checks intent.targets but uses the outer-scope
intent (fetched via findById) which may not have relations loaded; update the
flow to use the transaction-loaded intent (intentInTx) or otherwise re-fetch
intent with relations before running side-effects: ensure the targets array
comes from intentInTx.targets (or call findById with relations ['targets'] into
a variable you then use) and then call processPaymentTargets with that data
(referencing paymentIntentId, intentInTx or the newly fetched intent,
processPaymentTargets, and intent.metadata) so you never assume intent.targets
is populated when running the post-payment logic.
- Around line 665-768: The controller endpoints overridePaymentStatus and
overridePaymentStatusByTransactionId are missing auth/role guards allowing any
user to call admin/support actions; add NestJS guards to both controller methods
by decorating them with `@UseGuards`(AuthGuard('jwt'), RolesGuard) (or your
project's equivalent auth guard) and `@Roles`('admin','support') (or the
appropriate role strings), and import the Roles decorator and
RolesGuard/AuthGuard symbols used by your app; ensure the decorators are applied
to the method definitions in PaymentsController so only authorized admin/support
users can invoke overridePaymentStatus and overridePaymentStatusByTransactionId.
---
Outside diff comments:
In `@src/payments/payments.controller.ts`:
- Around line 65-127: The two admin endpoints
overridePaymentStatusByTransactionId and overridePaymentStatus are missing
authentication/authorization; add method-level guards and role restriction by
annotating both methods with `@UseGuards`(JwtAuthGuard, RbacAuthGuard) and
`@Roles`('admin'), and ensure the controller file imports UseGuards, Roles,
JwtAuthGuard and RbacAuthGuard so the decorators resolve; keep the existing
ValidationPipe/filters as-is and apply the decorators directly above each method
signature for both overridePaymentStatusByTransactionId and
overridePaymentStatus.
---
Nitpick comments:
In `@src/payments/dtos/override-payment-status.dto.ts`:
- Around line 1-21: Add a maximum length constraint to the reason field to
prevent excessively long audit entries: update OverridePaymentStatusDto to
import and use MaxLength (from class-validator) on the reason property (e.g.,
`@MaxLength`(500)) and add a matching maxLength in the ApiProperty metadata so
both validation and OpenAPI docs reflect the limit; ensure the import for
MaxLength is added alongside IsNotEmpty and IsString.
In `@src/payments/services/payment.service.ts`:
- Around line 694-700: The loop over intentInTx.transactions causes N individual
saves; instead prepare all transaction entities first (iterate
intentInTx.transactions, set tx.status = transactionStatus and if (status ===
PaymentIntentStatus.FAILED && !tx.failureReason) set tx.failureReason = 'Status
manually overridden'), collect them into an array, then perform a single bulk
save via manager.save(PaymentTransaction, transactionsArray) or use a single
update query (e.g., queryBuilder update by ids) to persist changes; update
references: intentInTx.transactions, tx.status, status, PaymentIntentStatus,
tx.failureReason, PaymentTransaction, and manager.save.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (7)
src/common/utils/api-id.config.tssrc/payments/dtos/override-payment-status.dto.tssrc/payments/dtos/payment-status.dto.tssrc/payments/entities/payment-intent.entity.tssrc/payments/payments.controller.tssrc/payments/services/payment-transaction.service.tssrc/payments/services/payment.service.ts
| @Column({ type: 'text', nullable: true, name: 'status_reason' }) | ||
| statusReason: string; |
There was a problem hiding this comment.
Type should reflect nullable column definition.
The column is defined as nullable: true, but the TypeScript type is string instead of string | null (or string | undefined). This can lead to type safety issues when the value is actually null from the database.
Proposed fix
`@Column`({ type: 'text', nullable: true, name: 'status_reason' })
- statusReason: string;
+ statusReason: string | null;📝 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.
| @Column({ type: 'text', nullable: true, name: 'status_reason' }) | |
| statusReason: string; | |
| `@Column`({ type: 'text', nullable: true, name: 'status_reason' }) | |
| statusReason: string | null; |
🧰 Tools
🪛 ESLint
[error] 56-56: Replace 'text',·nullable:·true,·name:·'status_reason' with "text",·nullable:·true,·name:·"status_reason"
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/payments/entities/payment-intent.entity.ts` around lines 56 - 57, The
PaymentIntent entity's statusReason property is declared as non-nullable string
but the column is nullable; update the TypeScript type for statusReason to
reflect the DB (e.g., change to string | null) in the PaymentIntent entity so
consumers handle null values correctly; also scan other nullable `@Column` fields
in the same entity (and their property types) to ensure they match nullable:
true.
| /** | ||
| * Manually override payment status (admin/support use). | ||
| * Updates payment intent status and status_reason, all related transactions, and optionally unlocks targets when set to PAID. | ||
| */ | ||
| async overridePaymentStatus( | ||
| paymentIntentId: string, | ||
| status: PaymentIntentStatus, | ||
| reason: string, | ||
| ) { | ||
| const intent = await this.paymentIntentService.findById(paymentIntentId); | ||
| const transactionStatus = | ||
| this.mapIntentStatusToTransactionStatus(status); | ||
|
|
||
| await this.dataSource.transaction(async (manager: EntityManager) => { | ||
| const intentInTx = await manager.findOne(PaymentIntent, { | ||
| where: { id: intent.id }, | ||
| relations: ['transactions', 'targets'], | ||
| }); | ||
|
|
||
| if (!intentInTx) { | ||
| throw new BadRequestException( | ||
| `Payment intent ${paymentIntentId} not found`, | ||
| ); | ||
| } | ||
|
|
||
| intentInTx.status = status; | ||
| intentInTx.statusReason = reason; | ||
| await manager.save(PaymentIntent, intentInTx); | ||
|
|
||
| for (const tx of intentInTx.transactions) { | ||
| tx.status = transactionStatus; | ||
| if (status === PaymentIntentStatus.FAILED && !tx.failureReason) { | ||
| tx.failureReason = 'Status manually overridden'; | ||
| } | ||
| await manager.save(PaymentTransaction, tx); | ||
| } | ||
|
|
||
| if (status === PaymentIntentStatus.PAID) { | ||
| await manager.update( | ||
| PaymentTarget, | ||
| { | ||
| paymentIntentId: intentInTx.id, | ||
| unlockStatus: PaymentTargetUnlockStatus.LOCKED, | ||
| }, | ||
| { | ||
| unlockStatus: PaymentTargetUnlockStatus.UNLOCKED, | ||
| unlockedAt: new Date(), | ||
| }, | ||
| ); | ||
| this.logger.log( | ||
| `Manual override: unlocked targets for payment intent ${intentInTx.id}`, | ||
| ); | ||
| } | ||
| }); | ||
|
|
||
| if (status === PaymentIntentStatus.PAID && intent.userId) { | ||
| const purpose = intent.purpose; | ||
| // Run after-payment success steps: certificate generation per target (same as webhook flow) | ||
| if (purpose === PaymentPurpose.CERTIFICATE_BUNDLE && intent.targets?.length) { | ||
| for (const target of intent.targets) { | ||
| this.processPaymentTargets( | ||
| paymentIntentId, | ||
| intent.userId, | ||
| target.contextId, | ||
| purpose, | ||
| ).catch((error) => { | ||
| this.logger.error( | ||
| `Failed to process payment target (certificate) after manual override for ${paymentIntentId}, contextId ${target.contextId}: ${error.message}`, | ||
| error.stack, | ||
| ); | ||
| }); | ||
| } | ||
| } else if (purpose && intent.metadata?.contextId) { | ||
| // Fallback: single context from metadata (e.g. when targets not loaded or legacy) | ||
| this.processPaymentTargets( | ||
| paymentIntentId, | ||
| intent.userId, | ||
| intent.metadata.contextId, | ||
| purpose, | ||
| ).catch((error) => { | ||
| this.logger.error( | ||
| `Failed to process payment targets after manual override for ${paymentIntentId}: ${error.message}`, | ||
| error.stack, | ||
| ); | ||
| }); | ||
| } | ||
| if (intent.metadata?.couponId) { | ||
| this.couponService | ||
| .recordRedemption( | ||
| intent.metadata.couponId, | ||
| intent.userId, | ||
| paymentIntentId, | ||
| ) | ||
| .catch((error) => { | ||
| this.logger.error( | ||
| `Failed to record coupon redemption after manual override for ${paymentIntentId}: ${error.message}`, | ||
| error.stack, | ||
| ); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| return this.getPaymentStatus(paymentIntentId); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's check if payments.controller.ts exists
find . -name "*.controller.ts" -path "*/payments/*"Repository: tekdi/user-microservice
Length of output: 205
🏁 Script executed:
#!/bin/bash
# Search for overridePaymentStatus usage in the codebase
rg -n 'overridePaymentStatus' --type tsRepository: tekdi/user-microservice
Length of output: 843
🏁 Script executed:
#!/bin/bash
# Check for auth guards and decorators in the payments module
rg -n '@UseGuards|@Roles|AuthGuard|@Auth' src/payments/ --type tsRepository: tekdi/user-microservice
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Read the payments controller to see the endpoint definitions
sed -n '70,130p' src/payments/payments.controller.tsRepository: tekdi/user-microservice
Length of output: 2564
🏁 Script executed:
#!/bin/bash
# Check the entire payments controller for any decorators
head -100 src/payments/payments.controller.tsRepository: tekdi/user-microservice
Length of output: 3542
Add authorization guards to payment status override endpoints.
The controller endpoints for overridePaymentStatus and overridePaymentStatusByTransactionId (lines 118 and 80 in src/payments/payments.controller.ts) lack authorization guards. These methods are documented as "admin/support use" but have no @UseGuards() or @Roles() decorators, allowing any authenticated user to override payment statuses. Apply appropriate NestJS guards (e.g., @UseGuards(RolesGuard) with @Roles('admin')) to both endpoints to restrict access.
🧰 Tools
🪛 ESLint
[error] 672-672: Delete ,
(prettier/prettier)
[error] 675-676: Delete ⏎·····
(prettier/prettier)
[error] 681-681: Replace 'transactions',·'targets' with "transactions",·"targets"
(prettier/prettier)
[error] 686-686: Delete ,
(prettier/prettier)
[error] 697-697: Replace 'Status·manually·overridden' with "Status·manually·overridden"
(prettier/prettier)
[error] 712-712: Delete ,
(prettier/prettier)
[error] 715-715: Delete ,
(prettier/prettier)
[error] 723-723: Replace purpose·===·PaymentPurpose.CERTIFICATE_BUNDLE·&&·intent.targets?.length with ⏎········purpose·===·PaymentPurpose.CERTIFICATE_BUNDLE·&&⏎········intent.targets?.length⏎······
(prettier/prettier)
[error] 729-729: Delete ,
(prettier/prettier)
[error] 733-733: Delete ,
(prettier/prettier)
[error] 743-743: Delete ,
(prettier/prettier)
[error] 747-747: Delete ,
(prettier/prettier)
[error] 756-756: Delete ,
(prettier/prettier)
[error] 761-761: Delete ,
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/payments/services/payment.service.ts` around lines 665 - 768, The
controller endpoints overridePaymentStatus and
overridePaymentStatusByTransactionId are missing auth/role guards allowing any
user to call admin/support actions; add NestJS guards to both controller methods
by decorating them with `@UseGuards`(AuthGuard('jwt'), RolesGuard) (or your
project's equivalent auth guard) and `@Roles`('admin','support') (or the
appropriate role strings), and import the Roles decorator and
RolesGuard/AuthGuard symbols used by your app; ensure the decorators are applied
to the method definitions in PaymentsController so only authorized admin/support
users can invoke overridePaymentStatus and overridePaymentStatusByTransactionId.
| if (status === PaymentIntentStatus.PAID && intent.userId) { | ||
| const purpose = intent.purpose; | ||
| // Run after-payment success steps: certificate generation per target (same as webhook flow) | ||
| if (purpose === PaymentPurpose.CERTIFICATE_BUNDLE && intent.targets?.length) { | ||
| for (const target of intent.targets) { | ||
| this.processPaymentTargets( | ||
| paymentIntentId, | ||
| intent.userId, | ||
| target.contextId, | ||
| purpose, | ||
| ).catch((error) => { | ||
| this.logger.error( | ||
| `Failed to process payment target (certificate) after manual override for ${paymentIntentId}, contextId ${target.contextId}: ${error.message}`, | ||
| error.stack, | ||
| ); | ||
| }); | ||
| } | ||
| } else if (purpose && intent.metadata?.contextId) { | ||
| // Fallback: single context from metadata (e.g. when targets not loaded or legacy) | ||
| this.processPaymentTargets( | ||
| paymentIntentId, | ||
| intent.userId, | ||
| intent.metadata.contextId, | ||
| purpose, | ||
| ).catch((error) => { | ||
| this.logger.error( | ||
| `Failed to process payment targets after manual override for ${paymentIntentId}: ${error.message}`, | ||
| error.stack, | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Potential issue: intent.targets may not be loaded.
The code at line 723 checks intent.targets?.length, but intent is fetched at line 674 using findById which may not load the targets relation. Inside the transaction, intentInTx is fetched with relations ['transactions', 'targets'], but the side-effect logic uses intent (from outside the transaction).
Consider using intentInTx.targets passed out of the transaction, or ensure findById loads the required relations.
Proposed approach
Return the loaded intent with targets from the transaction:
- await this.dataSource.transaction(async (manager: EntityManager) => {
+ const intentWithTargets = await this.dataSource.transaction(async (manager: EntityManager) => {
const intentInTx = await manager.findOne(PaymentIntent, {
where: { id: intent.id },
relations: ['transactions', 'targets'],
});
// ... existing transaction code ...
+ return intentInTx;
});
- if (status === PaymentIntentStatus.PAID && intent.userId) {
- const purpose = intent.purpose;
- if (purpose === PaymentPurpose.CERTIFICATE_BUNDLE && intent.targets?.length) {
- for (const target of intent.targets) {
+ if (status === PaymentIntentStatus.PAID && intentWithTargets?.userId) {
+ const purpose = intentWithTargets.purpose;
+ if (purpose === PaymentPurpose.CERTIFICATE_BUNDLE && intentWithTargets.targets?.length) {
+ for (const target of intentWithTargets.targets) {🧰 Tools
🪛 ESLint
[error] 723-723: Replace purpose·===·PaymentPurpose.CERTIFICATE_BUNDLE·&&·intent.targets?.length with ⏎········purpose·===·PaymentPurpose.CERTIFICATE_BUNDLE·&&⏎········intent.targets?.length⏎······
(prettier/prettier)
[error] 729-729: Delete ,
(prettier/prettier)
[error] 733-733: Delete ,
(prettier/prettier)
[error] 743-743: Delete ,
(prettier/prettier)
[error] 747-747: Delete ,
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/payments/services/payment.service.ts` around lines 720 - 750, The code
checks intent.targets but uses the outer-scope intent (fetched via findById)
which may not have relations loaded; update the flow to use the
transaction-loaded intent (intentInTx) or otherwise re-fetch intent with
relations before running side-effects: ensure the targets array comes from
intentInTx.targets (or call findById with relations ['targets'] into a variable
you then use) and then call processPaymentTargets with that data (referencing
paymentIntentId, intentInTx or the newly fetched intent, processPaymentTargets,
and intent.metadata) so you never assume intent.targets is populated when
running the post-payment logic.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/countries/countries.service.ts`:
- Line 5: The import line for ListCountryDto and COUNTRY_LIST_MAX_LIMIT violates
Prettier; update the import to match project formatting rules (use properly
ordered/named imports and spacing) so the line reads with correctly formatted
braces and spacing for the named symbols (e.g., ensure the import uses the
standard named-import style for ListCountryDto and COUNTRY_LIST_MAX_LIMIT from
'./dto/list-country.dto'); adjust only the import statement to satisfy
lint/Prettier.
In `@src/countries/dto/list-country.dto.ts`:
- Around line 2-4: Run the project formatter (Prettier/your repo's configured
formatter) on src/countries/dto/list-country.dto.ts to normalize quotes and
import wrapping; specifically reformat the import block that includes
IsOptional, IsBoolean, IsString, MaxLength, IsInt, Min, Max and the import block
for Expose, Type and PaginationDto so they follow the repo's import wrapping and
quote style rules, then re-run lint to ensure no Prettier violations remain
(affects the import lines and the surrounding lines mentioned around the file).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
src/countries/countries.service.tssrc/countries/dto/list-country.dto.ts
| import { Country } from './entities/country.entity'; | ||
| import { ListCountryDto } from './dto/list-country.dto'; | ||
| import { MAX_PAGINATION_LIMIT } from '../pathways/common/dto/pagination.dto'; | ||
| import { ListCountryDto, COUNTRY_LIST_MAX_LIMIT } from './dto/list-country.dto'; |
There was a problem hiding this comment.
Fix import formatting on this line to satisfy lint.
This changed import currently violates configured Prettier formatting.
As per coding guidelines, **/*.ts: "Review the JavaScript code for conformity with the Google JavaScript style guide..."
🧰 Tools
🪛 ESLint
[error] 5-5: Replace './dto/list-country.dto' with "./dto/list-country.dto"
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/countries/countries.service.ts` at line 5, The import line for
ListCountryDto and COUNTRY_LIST_MAX_LIMIT violates Prettier; update the import
to match project formatting rules (use properly ordered/named imports and
spacing) so the line reads with correctly formatted braces and spacing for the
named symbols (e.g., ensure the import uses the standard named-import style for
ListCountryDto and COUNTRY_LIST_MAX_LIMIT from './dto/list-country.dto'); adjust
only the import statement to satisfy lint/Prettier.
| import { IsOptional, IsBoolean, IsString, MaxLength, IsInt, Min, Max } from 'class-validator'; | ||
| import { Expose, Type } from 'class-transformer'; | ||
| import { PaginationDto } from '../../pathways/common/dto/pagination.dto'; |
There was a problem hiding this comment.
Run formatter to clear current lint failures in this DTO.
Static analysis already reports Prettier violations (quote style/import wrapping) in this changed block; please normalize formatting to avoid CI/lint blockage.
As per coding guidelines, **/*.ts: "Review the JavaScript code for conformity with the Google JavaScript style guide..."
Also applies to: 11-11, 20-21
🧰 Tools
🪛 ESLint
[error] 2-2: Replace ·IsOptional,·IsBoolean,·IsString,·MaxLength,·IsInt,·Min,·Max·}·from·'class-validator' with ⏎··IsOptional,⏎··IsBoolean,⏎··IsString,⏎··MaxLength,⏎··IsInt,⏎··Min,⏎··Max,⏎}·from·"class-validator"
(prettier/prettier)
[error] 3-3: Replace 'class-transformer' with "class-transformer"
(prettier/prettier)
[error] 4-4: Replace '../../pathways/common/dto/pagination.dto' with "../../pathways/common/dto/pagination.dto"
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/countries/dto/list-country.dto.ts` around lines 2 - 4, Run the project
formatter (Prettier/your repo's configured formatter) on
src/countries/dto/list-country.dto.ts to normalize quotes and import wrapping;
specifically reformat the import block that includes IsOptional, IsBoolean,
IsString, MaxLength, IsInt, Min, Max and the import block for Expose, Type and
PaginationDto so they follow the repo's import wrapping and quote style rules,
then re-run lint to ensure no Prettier violations remain (affects the import
lines and the surrounding lines mentioned around the file).
2 participants
Summary by CodeRabbit
New Features
Changes