Conversation
WalkthroughAdds configuration file Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as NotificationEventsController
participant Service as NotificationEventsService
participant RepoAction as NotificationActionsRepo
participant RepoTemplate as NotificationActionTemplatesRepo
Client->>Controller: PATCH /action/:id (UpdateNotificationActionDto)
Controller->>Service: updateNotificationAction(id, dto, userId, res)
Service->>RepoAction: find action by id
alt action not found
RepoAction-->>Service: null
Service-->>Controller: throws 404
else action found
RepoAction-->>Service: action
Service->>RepoAction: update action fields (title, status, updatedBy)
loop per template type (email/sms/push)
alt dto contains template data
Service->>RepoTemplate: find template by actionId & type
alt template exists
RepoTemplate-->>Service: template
Service->>RepoTemplate: update template fields (subject, body, image, link, emailFrom*, etc.)
else create new template
Service->>RepoTemplate: create template (language='en', createdBy/updatedBy)
end
end
end
alt dto.templateStatus provided
Service->>RepoTemplate: bulk update templates status for action
end
Service->>RepoTemplate: fetch templates for action
RepoTemplate-->>Service: templates[]
Service-->>Controller: 200 OK with action + templates organized by type
end
Controller-->>Client: HTTP 200 response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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: 1
🧹 Nitpick comments (1)
.coderabbit.yaml (1)
17-22: Instruction text references “JavaScript” for a TypeScript ruleWithin the TypeScript‐specific
path_instructions, the narrative still says “Review the JavaScript code…”.
This may confuse contributors reading the config or downstream tooling that surfaces the text. Consider re-phrasing to explicitly call out TypeScript.
📜 Review details
Configuration used: .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 (1)
.coderabbit.yaml(1 hunks)
| path_filters: | ||
| - '!**/.xml' |
There was a problem hiding this comment.
Incorrect glob pattern prevents XML files from being excluded
'!**/.xml' matches only files literally named .xml.
To exclude every XML file regardless of directory, the pattern needs the * wildcard before the dot.
- - '!**/.xml'
+ - '!**/*.xml'🤖 Prompt for AI Agents
In .coderabbit.yaml around lines 11 to 12, the glob pattern '!**/.xml'
incorrectly matches only files named exactly '.xml'. To exclude all XML files
regardless of their directory, update the pattern to '!**/*.xml' by adding the
'*' wildcard before the dot to correctly match any filename ending with '.xml'.
|
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/modules/notification_events/notification_events.service.ts (1)
206-211: Fix async forEach antipattern in updateNotificationTemplate method.Same issue as identified in the updateNotificationAction method - using forEach with async callbacks doesn't wait for completion.
-existingConfig.forEach(async (config) => { - if (updateEventDto.status) { - config.status = updateEventDto.status; - await this.notificationTemplateConfigRepository.save(config); - } -}); +await Promise.all( + existingConfig.map(async (config) => { + if (updateEventDto.status) { + config.status = updateEventDto.status; + return this.notificationTemplateConfigRepository.save(config); + } + }) +);
🧹 Nitpick comments (1)
src/modules/notification_events/notification_events.service.ts (1)
415-419: Improve error message specificity.The generic error message doesn't provide useful debugging information. Consider including the action ID for better error tracking.
-throw new BadRequestException( - 'Failed to fetch updated notification action' -); +throw new BadRequestException( + `Failed to fetch updated notification action with ID: ${id}` +);
📜 Review details
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 (4)
src/modules/notification_events/dto/updateNotificationAction.dto.ts(1 hunks)src/modules/notification_events/entity/notificationActionTemplates.entity.ts(1 hunks)src/modules/notification_events/notification_events.controller.ts(2 hunks)src/modules/notification_events/notification_events.service.ts(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
⚙️ CodeRabbit configuration file
**/*.ts: "Review the JavaScript code for conformity with the Google JavaScript style guide, highlighting any deviations. Ensure that:
- The code adheres to best practices associated with nodejs.
- The code adheres to best practices associated with nestjs framework.
- The code adheres to best practices recommended for performance.
- The code adheres to similar naming conventions for controllers, models, services, methods, variables."
Files:
src/modules/notification_events/entity/notificationActionTemplates.entity.tssrc/modules/notification_events/dto/updateNotificationAction.dto.tssrc/modules/notification_events/notification_events.service.tssrc/modules/notification_events/notification_events.controller.ts
🔇 Additional comments (3)
src/modules/notification_events/entity/notificationActionTemplates.entity.ts (1)
37-42: LGTM! Entity fields properly defined for email configuration.The new nullable fields
emailFromNameandemailFromare correctly added to support email sender configuration. The placement and nullable constraint are appropriate.src/modules/notification_events/dto/updateNotificationAction.dto.ts (2)
54-56: Remove redundant@IsNotEmpty()decorator for push notification fields.The
@IsNotEmpty()decorator on lines 55 and 61 is redundant since these fields are already marked as@IsOptional(). This combination can cause validation conflicts.@ApiProperty({ example: "Updated push subject", description: "Push subject" }) @IsString() -@IsNotEmpty() @IsOptional() subject: string;Likely an incorrect or invalid review comment.
60-62: Remove redundant@IsNotEmpty()decorator.Same issue as above -
@IsNotEmpty()should be removed for the optional body field.@ApiProperty({ example: "Updated push body content", description: "Push body" }) @IsString() -@IsNotEmpty() @IsOptional() body: string;Likely an incorrect or invalid review comment.
| @IsString() | ||
| @IsNotEmpty() | ||
| @IsOptional() | ||
| status: string; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant decorators from enum fields.
The @IsString() and @IsNotEmpty() decorators are unnecessary when using @IsEnum(). The enum validator already ensures the value is valid.
@IsEnum(['published', 'unpublished'], {
message: 'Status must be one of: published, unpublished',
})
-@IsString()
-@IsNotEmpty()
@IsOptional()
status: string;📝 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.
| @IsString() | |
| @IsNotEmpty() | |
| @IsOptional() | |
| status: string; | |
| @IsEnum(['published', 'unpublished'], { | |
| message: 'Status must be one of: published, unpublished', | |
| }) | |
| @IsOptional() | |
| status: string; |
🤖 Prompt for AI Agents
In src/modules/notification_events/dto/updateNotificationAction.dto.ts around
lines 87 to 90, the enum field is decorated with redundant validators; remove
the @IsString() and @IsNotEmpty() decorators and keep @IsOptional() plus an
appropriate @IsEnum(...) validator for the status field so that the enum
validation alone enforces allowed values.
| @IsString() | ||
| @IsNotEmpty() | ||
| @IsOptional() | ||
| templateStatus: string; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant decorators from enum field.
Same as above - remove unnecessary decorators for the templateStatus enum field.
@IsEnum(['published', 'unpublished'], {
message: 'Template status must be one of: published, unpublished',
})
-@IsString()
-@IsNotEmpty()
@IsOptional()
templateStatus: string;📝 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.
| @IsString() | |
| @IsNotEmpty() | |
| @IsOptional() | |
| templateStatus: string; | |
| @IsOptional() | |
| templateStatus: string; |
🤖 Prompt for AI Agents
In src/modules/notification_events/dto/updateNotificationAction.dto.ts around
lines 96 to 99, the templateStatus enum field currently uses redundant
decorators (@IsString, @IsNotEmpty, @IsOptional); remove the @IsString and
@IsNotEmpty decorators and instead validate the field as an optional enum by
keeping @IsOptional and adding @IsEnum(<YourEnumName>) and set the property type
to that enum type (e.g., TemplateStatus) so the DTO validates against the enum
values correctly.
| @UseFilters(new AllExceptionsFilter(APIID.TEMPLATE_UPDATE)) | ||
| @Patch("/action/:id") | ||
| @ApiBody({ type: UpdateNotificationActionDto }) | ||
| @ApiResponse({ | ||
| status: 200, | ||
| description: "Notification action updated successfully", | ||
| }) | ||
| @ApiResponse({ status: 400, description: ERROR_MESSAGES.BAD_REQUEST }) | ||
| @ApiResponse({ status: 404, description: ERROR_MESSAGES.TEMPLATE_NOTFOUND }) | ||
| @UsePipes(new ValidationPipe({ transform: true })) | ||
| updateNotificationAction( | ||
| @Param("id") id: number, | ||
| @Body() updateNotificationActionDto: UpdateNotificationActionDto, | ||
| @Res() response: Response, | ||
| @GetUserId() userId: string, | ||
| ) { | ||
| return this.notificationeventsService.updateNotificationAction( | ||
| id, | ||
| updateNotificationActionDto, | ||
| userId, | ||
| response | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding transaction support for atomic updates.
The new endpoint updates both NotificationActions and NotificationActionTemplates tables. Without transactions, partial updates could occur if one operation fails, leading to data inconsistency.
Consider wrapping the service method operations in a database transaction to ensure atomicity. This would prevent partial updates if any operation fails during the update process.
🤖 Prompt for AI Agents
In src/modules/notification_events/notification_events.controller.ts around
lines 79 to 101, the update endpoint calls the service to modify both
NotificationActions and NotificationActionTemplates without transaction
protection; modify the service method invoked by this controller to run both
update operations inside a single database transaction using your project's ORM
transaction API (start a transaction / obtain a transactional entity manager or
query runner), perform both updates via that transactional context, commit on
success and rollback on any error, and ensure any thrown errors propagate back
to the controller so the AllExceptionsFilter can handle them.
| async updateNotificationAction( | ||
| id: number, | ||
| updateNotificationActionDto: UpdateNotificationActionDto, | ||
| userId: string, | ||
| response: Response | ||
| ) { | ||
| const apiId = APIID.TEMPLATE_UPDATE; | ||
|
|
||
| // Check if NotificationActions record exists | ||
| const existingAction = await this.notificationTemplatesRepository.findOne({ | ||
| where: { actionId: id }, | ||
| }); | ||
|
|
||
| if (!existingAction) { | ||
| LoggerUtil.error( | ||
| ERROR_MESSAGES.TEMPLATE_NOT_EXIST, | ||
| ERROR_MESSAGES.NOT_FOUND, | ||
| apiId, | ||
| userId | ||
| ); | ||
| throw new BadRequestException(ERROR_MESSAGES.TEMPLATE_NOT_EXIST); | ||
| } | ||
|
|
||
| // Update NotificationActions table fields | ||
| const actionUpdates: any = {}; | ||
| if (updateNotificationActionDto.title) { | ||
| actionUpdates.title = updateNotificationActionDto.title; | ||
| } | ||
| if (updateNotificationActionDto.status) { | ||
| actionUpdates.status = updateNotificationActionDto.status; | ||
| } | ||
| actionUpdates.updatedBy = userId; | ||
|
|
||
| if (Object.keys(actionUpdates).length > 0) { | ||
| await this.notificationTemplatesRepository.update(id, actionUpdates); | ||
| } | ||
|
|
||
| // Helper function to update template by type | ||
| const updateTemplateByType = async (type: string, configData: any) => { | ||
| if (configData && Object.keys(configData).length > 0) { | ||
| let existingConfig = | ||
| await this.notificationTemplateConfigRepository.findOne({ | ||
| where: { actionId: id, type: type }, | ||
| }); | ||
|
|
||
| if (existingConfig) { | ||
| // Update existing template | ||
| Object.assign(existingConfig, configData); | ||
| existingConfig.updatedBy = userId; | ||
| if (updateNotificationActionDto.templateStatus) { | ||
| existingConfig.status = updateNotificationActionDto.templateStatus; | ||
| } | ||
| return await this.notificationTemplateConfigRepository.save( | ||
| existingConfig | ||
| ); | ||
| } else { | ||
| // Create new template if it doesn't exist | ||
| if (!configData.subject || !configData.body) { | ||
| throw new BadRequestException( | ||
| ERROR_MESSAGES.NOT_EMPTY_SUBJECT_OR_BODY | ||
| ); | ||
| } | ||
| const newConfig = this.notificationTemplateConfigRepository.create({ | ||
| actionId: id, | ||
| type: type, | ||
| subject: configData.subject, | ||
| body: configData.body, | ||
| status: | ||
| updateNotificationActionDto.templateStatus || | ||
| existingAction.status, | ||
| language: 'en', | ||
| updatedBy: userId, | ||
| createdBy: userId, | ||
| image: configData.image || null, | ||
| link: configData.link || null, | ||
| emailFromName: configData.emailFromName || null, | ||
| emailFrom: configData.emailFrom || null, | ||
| }); | ||
| return await this.notificationTemplateConfigRepository.save( | ||
| newConfig | ||
| ); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| // Update templates based on type | ||
| if ( | ||
| updateNotificationActionDto.email && | ||
| Object.keys(updateNotificationActionDto.email).length > 0 | ||
| ) { | ||
| await updateTemplateByType('email', updateNotificationActionDto.email); | ||
| } | ||
|
|
||
| if ( | ||
| updateNotificationActionDto.sms && | ||
| Object.keys(updateNotificationActionDto.sms).length > 0 | ||
| ) { | ||
| await updateTemplateByType('sms', updateNotificationActionDto.sms); | ||
| } | ||
|
|
||
| if ( | ||
| updateNotificationActionDto.push && | ||
| Object.keys(updateNotificationActionDto.push).length > 0 | ||
| ) { | ||
| await updateTemplateByType('push', updateNotificationActionDto.push); | ||
| } | ||
|
|
||
| // Update template status for all existing templates if templateStatus is provided | ||
| if (updateNotificationActionDto.templateStatus) { | ||
| let existingConfigs = | ||
| await this.notificationTemplateConfigRepository.find({ | ||
| where: { actionId: id }, | ||
| }); | ||
| existingConfigs.forEach(async (config) => { | ||
| config.status = updateNotificationActionDto.templateStatus; | ||
| config.updatedBy = userId; | ||
| await this.notificationTemplateConfigRepository.save(config); | ||
| }); | ||
| } | ||
|
|
||
| // Fetch the updated data from both tables | ||
| const updatedAction = await this.notificationTemplatesRepository.findOne({ | ||
| where: { actionId: id }, | ||
| relations: ['templateconfig'], | ||
| }); | ||
|
|
||
| if (!updatedAction) { | ||
| throw new BadRequestException( | ||
| 'Failed to fetch updated notification action' | ||
| ); | ||
| } | ||
|
|
||
| // Format the response to include both tables data | ||
| const { templateconfig, ...actionData } = updatedAction; | ||
| const formattedTemplateConfig = templateconfig.reduce((acc, config) => { | ||
| const { | ||
| type, | ||
| language, | ||
| subject, | ||
| body, | ||
| createdOn, | ||
| image, | ||
| link, | ||
| emailFromName, | ||
| emailFrom, | ||
| status, | ||
| } = config; | ||
| acc[type] = { | ||
| language, | ||
| subject, | ||
| body, | ||
| createdOn, | ||
| image, | ||
| link, | ||
| emailFromName, | ||
| emailFrom, | ||
| status, | ||
| }; | ||
| return acc; | ||
| }, {}); | ||
|
|
||
| const responseData = { | ||
| actionId: id, | ||
| ...actionData, | ||
| templates: formattedTemplateConfig, | ||
| }; | ||
|
|
||
| LoggerUtil.log( | ||
| `Notification action updated successfully by userId: ${userId}`, | ||
| apiId, | ||
| userId | ||
| ); | ||
|
|
||
| return response | ||
| .status(HttpStatus.OK) | ||
| .json( | ||
| APIResponse.success( | ||
| apiId, | ||
| responseData, | ||
| 'Notification action updated successfully' | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Add database transaction support for data consistency.
The updateNotificationAction method performs multiple database operations without transactions. If any operation fails after some updates are committed, the data will be in an inconsistent state.
Wrap all database operations in a transaction using TypeORM's transaction manager:
async updateNotificationAction(
id: number,
updateNotificationActionDto: UpdateNotificationActionDto,
userId: string,
response: Response
) {
const apiId = APIID.TEMPLATE_UPDATE;
return await this.notificationTemplatesRepository.manager.transaction(
async (transactionalEntityManager) => {
// Use transactionalEntityManager for all database operations
// This ensures atomicity
}
);
}🤖 Prompt for AI Agents
In src/modules/notification_events/notification_events.service.ts around lines
289 to 471, the method performs multiple repository operations without a
transaction; wrap all DB reads/writes in a single TypeORM transaction to ensure
atomicity. Start a transaction via
this.notificationTemplatesRepository.manager.transaction(async
transactionalEntityManager => { ... }) and replace all direct repository calls
(findOne, find, update, create, save) with
transactionalEntityManager.getRepository(YourEntity).<method>() so every lookup,
update, create and save for both NotificationTemplates and
NotificationTemplateConfig uses the transactionalEntityManager; perform the
existence check, all template updates/creates, and the final fetch inside the
transaction, and return or throw inside the transaction so failures roll back.
Ensure templateStatus mass-update uses transactional saves and adjust any early
throws to occur within the transaction block.
| const updateTemplateByType = async (type: string, configData: any) => { | ||
| if (configData && Object.keys(configData).length > 0) { | ||
| let existingConfig = | ||
| await this.notificationTemplateConfigRepository.findOne({ | ||
| where: { actionId: id, type: type }, | ||
| }); | ||
|
|
||
| if (existingConfig) { | ||
| // Update existing template | ||
| Object.assign(existingConfig, configData); | ||
| existingConfig.updatedBy = userId; | ||
| if (updateNotificationActionDto.templateStatus) { | ||
| existingConfig.status = updateNotificationActionDto.templateStatus; | ||
| } | ||
| return await this.notificationTemplateConfigRepository.save( | ||
| existingConfig | ||
| ); | ||
| } else { | ||
| // Create new template if it doesn't exist | ||
| if (!configData.subject || !configData.body) { | ||
| throw new BadRequestException( | ||
| ERROR_MESSAGES.NOT_EMPTY_SUBJECT_OR_BODY | ||
| ); | ||
| } | ||
| const newConfig = this.notificationTemplateConfigRepository.create({ | ||
| actionId: id, | ||
| type: type, | ||
| subject: configData.subject, | ||
| body: configData.body, | ||
| status: | ||
| updateNotificationActionDto.templateStatus || | ||
| existingAction.status, | ||
| language: 'en', | ||
| updatedBy: userId, | ||
| createdBy: userId, | ||
| image: configData.image || null, | ||
| link: configData.link || null, | ||
| emailFromName: configData.emailFromName || null, | ||
| emailFrom: configData.emailFrom || null, | ||
| }); | ||
| return await this.notificationTemplateConfigRepository.save( | ||
| newConfig | ||
| ); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract duplicate template update logic into a shared method.
The updateTemplateByType function is duplicated in both updateNotificationTemplate and updateNotificationAction methods with slight variations. This violates the DRY principle.
Consider extracting this logic into a private method that both functions can use:
private async updateOrCreateTemplate(
actionId: number,
type: string,
configData: any,
userId: string,
templateStatus?: string,
actionStatus?: string
) {
// Shared implementation here
}🤖 Prompt for AI Agents
In src/modules/notification_events/notification_events.service.ts around lines
327 to 372, the template create/update logic in updateTemplateByType is
duplicated elsewhere; extract it into a private helper (e.g.,
updateOrCreateTemplate(actionId: number, type: string, configData: any, userId:
string, templateStatus?: string, actionStatus?: string)) that performs: validate
configData (subject/body) for creation, find existing config by actionId and
type, merge and set updatedBy/status for updates, or build and save a new entity
with createdBy/updatedBy/status/language and optional fields
(image/link/emailFromName/emailFrom); replace both callers to invoke this helper
passing id, type, configData, userId,
updateNotificationActionDto.templateStatus, and existingAction.status as needed.
| emailFromName: configData.emailFromName || null, | ||
| emailFrom: configData.emailFrom || null, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate email fields for proper format.
The emailFromName and emailFrom fields are being saved without validation. The emailFrom field should be validated as a proper email address.
Add email validation in the DTO:
// In EmailUpdateDto
@ApiProperty({ example: "noreply@aspireleaders.org", description: "Email from address" })
@IsString()
@IsNotEmpty()
+@IsEmail({}, { message: 'Please provide a valid email address' })
@IsOptional()
emailFrom: string;Don't forget to import IsEmail from class-validator.
📝 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.
| emailFromName: configData.emailFromName || null, | |
| emailFrom: configData.emailFrom || null, | |
| import { IsString, IsNotEmpty, IsOptional, IsEmail } from 'class-validator'; | |
| import { ApiProperty } from '@nestjs/swagger'; | |
| @ApiProperty({ example: "noreply@aspireleaders.org", description: "Email from address" }) | |
| @IsString() | |
| @IsNotEmpty() | |
| @IsEmail({}, { message: 'Please provide a valid email address' }) | |
| @IsOptional() | |
| emailFrom: string; |
🤖 Prompt for AI Agents
In src/modules/notification_events/notification_events.service.ts around lines
364-365, the emailFromName and emailFrom are being persisted without validation;
update the DTO that represents these fields (the create/update notification
event DTO used by this service) to import IsEmail from class-validator and add
the @IsEmail() decorator to the emailFrom property (leave emailFromName as a
plain string or add @IsString()/length checks if desired), then ensure the
service uses that DTO for input validation before saving.
| existingConfigs.forEach(async (config) => { | ||
| config.status = updateNotificationActionDto.templateStatus; | ||
| config.updatedBy = userId; | ||
| await this.notificationTemplateConfigRepository.save(config); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Fix async forEach antipattern - use Promise.all for concurrent updates.
Using forEach with async callbacks doesn't wait for the promises to resolve, which could lead to race conditions or incomplete updates before the response is sent.
-existingConfigs.forEach(async (config) => {
- config.status = updateNotificationActionDto.templateStatus;
- config.updatedBy = userId;
- await this.notificationTemplateConfigRepository.save(config);
-});
+await Promise.all(
+ existingConfigs.map(async (config) => {
+ config.status = updateNotificationActionDto.templateStatus;
+ config.updatedBy = userId;
+ return this.notificationTemplateConfigRepository.save(config);
+ })
+);📝 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.
| existingConfigs.forEach(async (config) => { | |
| config.status = updateNotificationActionDto.templateStatus; | |
| config.updatedBy = userId; | |
| await this.notificationTemplateConfigRepository.save(config); | |
| }); | |
| } | |
| await Promise.all( | |
| existingConfigs.map(async (config) => { | |
| config.status = updateNotificationActionDto.templateStatus; | |
| config.updatedBy = userId; | |
| return this.notificationTemplateConfigRepository.save(config); | |
| }) | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/modules/notification_events/notification_events.service.ts around lines
402 to 407, the code uses existingConfigs.forEach with an async callback which
does not await saves; replace the forEach with a mapped array of promises
(existingConfigs.map(config => { update fields; return
this.notificationTemplateConfigRepository.save(config); })) and then await
Promise.all(promises) so all repository.save operations complete before
proceeding; ensure you still set config.status =
updateNotificationActionDto.templateStatus and config.updatedBy = userId before
returning the promise.
1 participant
Summary by CodeRabbit
New Features
Chores