Skip to content

Added coderabbit file #37

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Tusharmahajan12 wants to merge 2 commits into
base: aspire-leaders
Choose a base branch
from
Open

Added coderabbit file #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
215e608
Added coderabbit file
Tusharmahajan12 Jul 22, 2025
528cd0c
Update API created
Tusharmahajan12 Sep 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions .coderabbit.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
language: 'en'

early_access: false

reviews:
request_changes_workflow: true
high_level_summary: true
poem: false
review_status: true
collapse_walkthrough: false
path_filters:
- '!**/.xml'
Comment on lines +11 to +12
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jul 22, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

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'.

path_instructions:
- path: '**/*.js'
instructions: 'Review the JavaScript code for conformity with the Google JavaScript style guide, highlighting any deviations.'
- path: '**/*.ts'
instructions: |
"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."
auto_review:
enabled: true
ignore_title_keywords:
- 'WIP'
- 'DO NOT MERGE'
drafts: false
base_branches:
- 'master'
- 'main'
- 'aspire-leaders'

chat:
auto_reply: true
123 changes: 123 additions & 0 deletions src/modules/notification_events/dto/updateNotificationAction.dto.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import { ApiProperty } from "@nestjs/swagger";
import {
IsEnum,
IsNotEmpty,
IsOptional,
IsString,
IsUUID,
ValidateNested,
} from "class-validator";
import { Type } from "class-transformer";

export class EmailUpdateDto {
@ApiProperty({ example: "Updated email subject", description: "Email subject" })
@IsString()
@IsNotEmpty()
@IsOptional()
subject: string;

@ApiProperty({ example: "Updated email body content", description: "Email body" })
@IsString()
@IsNotEmpty()
@IsOptional()
body: string;

@ApiProperty({ example: "Aspire Leaders", description: "Email from name" })
@IsString()
@IsNotEmpty()
@IsOptional()
emailFromName: string;

@ApiProperty({ example: "noreply@aspireleaders.org", description: "Email from address" })
@IsString()
@IsNotEmpty()
@IsOptional()
emailFrom: string;
}

export class SmsUpdateDto {
@ApiProperty({ example: "Updated SMS subject", description: "SMS subject" })
@IsString()
@IsNotEmpty()
@IsOptional()
subject: string;

@ApiProperty({ example: "Updated SMS body content", description: "SMS body" })
@IsString()
@IsNotEmpty()
@IsOptional()
body: string;
}

export class PushUpdateDto {
@ApiProperty({ example: "Updated push subject", description: "Push subject" })
@IsString()
@IsNotEmpty()
@IsOptional()
subject: string;

@ApiProperty({ example: "Updated push body content", description: "Push body" })
@IsString()
@IsNotEmpty()
@IsOptional()
body: string;

@ApiProperty({ example: "https://images.unsplash.com/photo-1519389950473-47ba0277781c", description: "Image URL" })
@IsString()
@IsOptional()
image: string;

@ApiProperty({ example: "https://aspireleaders.org/learn", description: "Link URL" })
@IsString()
@IsOptional()
link: string;
}

export class UpdateNotificationActionDto {
@ApiProperty({ example: "Updated notification title", description: "Notification title" })
@IsString()
@IsNotEmpty()
@IsOptional()
title: string;

@ApiProperty({ example: "published", description: "Status for NotificationActions" })
@IsEnum(['published', 'unpublished'], {
message: 'Status must be one of: published, unpublished',
})
@IsString()
@IsNotEmpty()
@IsOptional()
status: string;
Comment on lines +87 to +90
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Sep 12, 2025

Choose a reason for hiding this comment

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

🛠️ 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.

Suggested change
@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.


@ApiProperty({ example: "published", description: "Status for NotificationActionTemplates" })
@IsEnum(['published', 'unpublished'], {
message: 'Template status must be one of: published, unpublished',
})
@IsString()
@IsNotEmpty()
@IsOptional()
templateStatus: string;
Comment on lines +96 to +99
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Sep 12, 2025

Choose a reason for hiding this comment

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

🛠️ 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.

Suggested change
@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.


@ApiProperty({ type: EmailUpdateDto, description: "Email update details" })
@ValidateNested()
@Type(() => EmailUpdateDto)
@IsOptional()
email?: EmailUpdateDto;

@ApiProperty({ type: SmsUpdateDto, description: "SMS update details" })
@ValidateNested()
@Type(() => SmsUpdateDto)
@IsOptional()
sms?: SmsUpdateDto;

@ApiProperty({ type: PushUpdateDto, description: "Push notification update details" })
@ValidateNested()
@Type(() => PushUpdateDto)
@IsOptional()
push?: PushUpdateDto;

@ApiProperty({ example: "uuid-string", description: "Updated by user ID" })
@IsUUID()
@IsOptional()
updatedBy: string;
}
6 changes: 6 additions & 0 deletions src/modules/notification_events/entity/notificationActionTemplates.entity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ export class NotificationActionTemplates {
@Column({ nullable: true })
link: string;

@Column({ nullable: true })
emailFromName: string;

@Column({ nullable: true })
emailFrom: string;

@Column({ type: 'uuid', nullable: true })
updatedBy: string;

Expand Down
25 changes: 25 additions & 0 deletions src/modules/notification_events/notification_events.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { SearchFilterDto } from './dto/searchTemplateType.dto';
import { Response } from 'express';
import { CreateEventDto } from './dto/createTemplate.dto';
import { UpdateEventDto } from './dto/updateEventTemplate.dto';
import { UpdateNotificationActionDto } from './dto/updateNotificationAction.dto';
import { AllExceptionsFilter } from 'src/common/filters/exception.filter';
import { APIID } from 'src/common/utils/api-id.config';
import { ERROR_MESSAGES, SUCCESS_MESSAGES } from 'src/common/utils/constant.util';
Expand Down Expand Up @@ -75,6 +76,30 @@ export class NotificationEventsController {
);
}

@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
);
}
Comment on lines +79 to +101
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Sep 12, 2025

Choose a reason for hiding this comment

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

🛠️ 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.


@UseFilters(new AllExceptionsFilter(APIID.TEMPLATE_DELETE))
@Delete("/:id")
@UsePipes(new ValidationPipe({ transform: true }))
Expand Down
Loading