[Feat] Employee weekly schedule planning #8741
Conversation
WalkthroughThis pull request implements a comprehensive employee availability functionality across various packages. It introduces models, data transfer objects (DTOs), services, repositories, and controllers to manage employee availability records. Key features include support for bulk creation, retrieval, and updating of availability data, with robust validation and multi-ORM compatibility. Additionally, new command handlers facilitate the processing of employee availability commands, enhancing the overall structure and usability of the application. Changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
View your CI Pipeline Execution ↗ for commit d8ece61.
☁️ Nx Cloud last updated this comment at |
...ges/core/src/lib/employee-availability/commands/employee-availability.bulk.create.command.ts
Outdated
Show resolved
Hide resolved
...core/src/lib/employee-availability/commands/handlers/employee-availability.create.handler.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/employee-availability/employee-availability.controller.ts
Outdated
Show resolved
Hide resolved
| import { EmployeeAvailabilityBulkCreateCommand, EmployeeAvailabilityCreateCommand } from './commands'; | ||
|
|
||
| @ApiTags('EmployeeAvailability') | ||
| @UseGuards(TenantPermissionGuard) |
There was a problem hiding this comment.
@samuelmbabhazi Permission Gurad is missing and also permissions missing for API routes.
packages/core/src/lib/employee-availability/employee-availability.controller.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/shared/pipes/employee-availability-status.pipe.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/employee-availability/employee-availability.service.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/employee-availability/employee-availability.entity.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/employee-availability/employee-availability.entity.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (4)
packages/core/src/lib/employee-availability/employee-availability.controller.ts (4)
39-42:⚠️ Potential issueAdd missing
@Permissionsdecorator to securecreateBulkendpointThe
createBulkmethod is missing the@Permissionsdecorator, which specifies the required permissions for this endpoint. Adding the appropriate permission ensures that only authorized users can create employee availability records in bulk.Apply this diff to add the
@Permissionsdecorator:+ @Permissions('EMPLOYEE_AVAILABILITY_CREATE') @HttpCode(HttpStatus.CREATED) @Post('/bulk') async createBulk(@Body() entities: IEmployeeAvailabilityCreateInput[]): Promise<IEmployeeAvailability[]> {
59-64:⚠️ Potential issueAdd missing
@Permissionsdecorator to securefindAllendpointThe
findAllmethod is missing the@Permissionsdecorator, which specifies the required permissions for this endpoint. Adding the appropriate permission ensures that only authorized users can retrieve employee availability records.Apply this diff to add the
@Permissionsdecorator:+ @Permissions('EMPLOYEE_AVAILABILITY_VIEW') @Get() async findAll( @Query() filter: PaginationParams<EmployeeAvailability> ): Promise<IPagination<IEmployeeAvailability>> { return this.availabilityService.findAll(filter); }
82-85:⚠️ Potential issueAdd missing
@Permissionsdecorator to securecreateendpointThe
createmethod is missing the@Permissionsdecorator, which specifies the required permissions for this endpoint. Adding the appropriate permission ensures that only authorized users can create employee availability records.Apply this diff to add the
@Permissionsdecorator:+ @Permissions('EMPLOYEE_AVAILABILITY_CREATE') @HttpCode(HttpStatus.CREATED) @Post() async create(@Body() entity: IEmployeeAvailabilityCreateInput): Promise<IEmployeeAvailability> { return await this.commandBus.execute(new EmployeeAvailabilityCreateCommand(entity)); }
104-110:⚠️ Potential issueAdd missing
@Permissionsdecorator to secureupdateendpointThe
updatemethod is missing the@Permissionsdecorator, which specifies the required permissions for this endpoint. Adding the appropriate permission ensures that only authorized users can update employee availability records.Apply this diff to add the
@Permissionsdecorator:+ @Permissions('EMPLOYEE_AVAILABILITY_EDIT') @HttpCode(HttpStatus.ACCEPTED) @Put(':id') async update( @Param('id', UUIDValidationPipe) id: ID, @Body() entity: IEmployeeAvailability ): Promise<IEmployeeAvailability | UpdateResult> { return this.availabilityService.update(id, { ...entity }); }
🧹 Nitpick comments (9)
packages/core/src/lib/employee-availability/employee-availability.controller.ts (1)
61-63: Ensure proper type handling infindAllmethodThe
filterparameter in thefindAllmethod usesPaginationParams<EmployeeAvailability>, which may not have the necessary validation. Consider using a DTO with validation decorators to ensure query parameters are properly validated.Example:
Create a
QueryEmployeeAvailabilityDTOclass:import { PaginationParams } from '../core'; import { IsOptional, IsString } from 'class-validator'; export class QueryEmployeeAvailabilityDTO extends PaginationParams<EmployeeAvailability> { @IsOptional() @IsString() employeeId?: string; // Add other query parameters as needed with appropriate validation decorators }Update the method signature:
@Get() - async findAll( - @Query() filter: PaginationParams<EmployeeAvailability> - ): Promise<IPagination<IEmployeeAvailability>> { + async findAll( + @Query() filter: QueryEmployeeAvailabilityDTO + ): Promise<IPagination<IEmployeeAvailability>> { return this.availabilityService.findAll(filter); }packages/core/src/lib/employee-availability/commands/employee-availability.bulk.create.command.ts (2)
5-5: Maintain consistent command type naming format.The command type string should follow a consistent format. Consider using
[Employee Availability] Bulk Createto match the entity name format.- static readonly type = '[Employee Bulk Availability ] Create'; + static readonly type = '[Employee Availability] Bulk Create';
7-7: Consider adding input validation.The input array should be validated to ensure data integrity. Consider adding class-validator decorators.
+ @ValidateNested({ each: true }) + @Type(() => EmployeeAvailabilityCreateDto) constructor(public readonly input: IEmployeeAvailabilityCreateInput[]) {}packages/core/src/lib/employee-availability/repository/type-orm-employee-availability.repository.ts (1)
7-11: Add common query methods for availability lookups.Consider adding useful query methods that will likely be needed:
- Find overlapping availabilities
- Find availabilities by date range
- Find availabilities by employee and status
async findOverlapping( employeeId: string, startDate: Date, endDate: Date ): Promise<EmployeeAvailability[]> { return this.createQueryBuilder('availability') .where('availability.employeeId = :employeeId', { employeeId }) .andWhere( '(availability.startDate <= :endDate AND availability.endDate >= :startDate)', { startDate, endDate } ) .getMany(); } async findByDateRange( startDate: Date, endDate: Date ): Promise<EmployeeAvailability[]> { return this.createQueryBuilder('availability') .where( '(availability.startDate <= :endDate AND availability.endDate >= :startDate)', { startDate, endDate } ) .getMany(); }packages/core/src/lib/employee-availability/employee-availability.service.ts (1)
4-4: Use absolute imports for better maintainability.Replace relative import with absolute import path.
-import { TenantAwareCrudService } from './../core/crud'; +import { TenantAwareCrudService } from '@gauzy/core';packages/contracts/src/lib/employee-availability.model.ts (2)
18-19: Convert dayOfWeek to enum for type safety.Using a number with comments for days of the week is error-prone. Consider using an enum instead.
+/** + * Enum representing days of the week. + */ +export enum DayOfWeek { + Sunday = 0, + Monday = 1, + Tuesday = 2, + Wednesday = 3, + Thursday = 4, + Friday = 5, + Saturday = 6 +} export interface IEmployeeAvailability extends IBasePerTenantAndOrganizationEntityModel { employee: IEmployee; employeeId: ID; startDate: Date; endDate: Date; - dayOfWeek: number; // 0 = Sunday, 6 = Saturday + dayOfWeek: DayOfWeek; availabilityStatus: AvailabilityStatusEnum;
48-55: Consider using Partial utility type for update interface.The update interface could be more precisely typed using TypeScript's Partial utility type.
-export interface IEmployeeAvailabilityUpdateInput extends IBasePerTenantAndOrganizationEntityModel { - employeeId?: ID; - startDate?: Date; - endDate?: Date; - dayOfWeek?: number; - availabilityStatus?: AvailabilityStatusEnum; - availabilityNotes?: string; -} +export type IEmployeeAvailabilityUpdateInput = Partial<IEmployeeAvailabilityCreateInput>;packages/core/src/lib/employee-availability/employee-availability.entity.ts (2)
26-27: Consider adding validation for dayOfWeek range.While the database constraint ensures values between 0 and 6, add a validator to enforce this at the application level.
@IsInt() @IsNotEmpty() +@Min(0) +@Max(6) @MultiORMColumn({ check: 'day_of_week BETWEEN 0 AND 6' }) dayOfWeek: number;
32-36: Consider adding a default status.The
availabilityStatusfield might benefit from a default value to ensure consistent behavior.@MultiORMColumn({ type: 'int', - transformer: new AvailabilityStatusTransformer() + transformer: new AvailabilityStatusTransformer(), + default: AvailabilityStatusEnum.Available })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
packages/contracts/src/index.ts(1 hunks)packages/contracts/src/lib/employee-availability.model.ts(1 hunks)packages/core/src/lib/core/entities/internal.ts(1 hunks)packages/core/src/lib/employee-availability/commands/employee-availability.bulk.create.command.ts(1 hunks)packages/core/src/lib/employee-availability/commands/employee-availability.create.command.ts(1 hunks)packages/core/src/lib/employee-availability/commands/handlers/employee-availability.bulk.create.handler.ts(1 hunks)packages/core/src/lib/employee-availability/commands/handlers/employee-availability.create.handler.ts(1 hunks)packages/core/src/lib/employee-availability/commands/handlers/index.ts(1 hunks)packages/core/src/lib/employee-availability/commands/index.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.controller.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.entity.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.module.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.service.ts(1 hunks)packages/core/src/lib/employee-availability/index.ts(1 hunks)packages/core/src/lib/employee-availability/repository/micro-orm-employee-availability.repository.ts(1 hunks)packages/core/src/lib/employee-availability/repository/type-orm-employee-availability.repository.ts(1 hunks)packages/core/src/lib/employee/employee.entity.ts(3 hunks)packages/core/src/lib/shared/pipes/employee-availability-status.pipe.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/core/src/lib/employee-availability/index.ts
- packages/core/src/lib/employee-availability/commands/handlers/index.ts
- packages/core/src/lib/employee-availability/commands/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/core/src/lib/employee-availability/employee-availability.service.ts
[error] 9-14: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (6)
packages/core/src/lib/employee-availability/repository/micro-orm-employee-availability.repository.ts (1)
1-4: LGTMThe repository class is appropriately defined, extending the base repository for
EmployeeAvailabilityentities. This sets up the groundwork for any future custom repository methods specific toEmployeeAvailability.packages/core/src/lib/employee-availability/commands/employee-availability.create.command.ts (1)
1-8: LGTMThe command class is correctly implemented for creating employee availability records, adhering to the CQRS pattern.
packages/core/src/lib/employee-availability/employee-availability.entity.ts (1)
57-61: LGTM! Proper cascade configuration.The many-to-one relationship with Employee is correctly configured with cascade delete, which will automatically remove availability records when an employee is deleted.
packages/contracts/src/index.ts (1)
43-43: LGTM! Export added in correct order.The new export for employee-availability model is properly placed in alphabetical order among other exports.
packages/core/src/lib/core/entities/internal.ts (1)
39-39: LGTM! Export added in correct order.The new export for employee-availability entity is properly placed in alphabetical order among other entity exports.
packages/core/src/lib/employee/employee.entity.ts (1)
546-554: LGTM! Well-documented relationship.The one-to-many relationship with EmployeeAvailability is properly configured with:
- Clear JSDoc documentation
- Correct cascade configuration
- Follows the pattern used by other similar relationships in the file
packages/core/src/lib/employee-availability/employee-availability.controller.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/employee-availability/employee-availability.controller.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/employee-availability/employee-availability.controller.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/employee-availability/employee-availability.module.ts
Outdated
Show resolved
Hide resolved
| if (!startDate || !endDate) { | ||
| throw new BadRequestException('Start date and end date are required.'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add date range validation.
Validate that the end date is after the start date to ensure a valid date range.
if (!startDate || !endDate) {
throw new BadRequestException('Start date and end date are required.');
}
+if (new Date(endDate) <= new Date(startDate)) {
+ throw new BadRequestException('End date must be after start date.');
+}📝 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.
| if (!startDate || !endDate) { | |
| throw new BadRequestException('Start date and end date are required.'); | |
| } | |
| if (!startDate || !endDate) { | |
| throw new BadRequestException('Start date and end date are required.'); | |
| } | |
| if (new Date(endDate) <= new Date(startDate)) { | |
| throw new BadRequestException('End date must be after start date.'); | |
| } |
...core/src/lib/employee-availability/commands/handlers/employee-availability.create.handler.ts
Outdated
Show resolved
Hide resolved
...src/lib/employee-availability/commands/handlers/employee-availability.bulk.create.handler.ts
Outdated
Show resolved
Hide resolved
...src/lib/employee-availability/commands/handlers/employee-availability.bulk.create.handler.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/shared/pipes/employee-availability-status.pipe.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/shared/pipes/employee-availability-status.pipe.ts
Outdated
Show resolved
Hide resolved
| @@ -23,15 +23,13 @@ export class EmployeeAvailability extends TenantOrganizationBaseEntity implement | |||
| @ApiProperty({ type: () => Number, description: 'Day of the week (0 = Sunday, 6 = Saturday)' }) | |||
| @IsInt() | |||
| @IsNotEmpty() | |||
| @MultiORMColumn({ type: 'int', check: 'day_of_week BETWEEN 0 AND 6' }) | |||
| @MultiORMColumn({ check: 'day_of_week BETWEEN 0 AND 6' }) | |||
There was a problem hiding this comment.
@samuelmbabhazi Is this property supporting into Mikro-ORM?
There was a problem hiding this comment.
I'm not sure about micro-orm, so we can do it in the migration
packages/core/src/lib/employee-availability/employee-availability.entity.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
packages/core/src/lib/employee-availability/dto/create-employee-availability.dto.ts (2)
10-16: Add date range validation.Consider adding a custom validator to ensure startDate is before endDate.
@ValidateIf((o) => o.startDate && o.endDate) @Validate((startDate, { object }) => { return new Date(startDate) < new Date(object.endDate); }, { message: 'startDate must be before endDate' }) startDate: Date;
18-20: Add range validation for dayOfWeek.The dayOfWeek property should be validated to ensure it's between 0 and 6.
@ApiProperty({ type: () => Number }) @IsInt() +@Min(0) +@Max(6) dayOfWeek: number;packages/core/src/lib/employee-availability/commands/handlers/employee-availability.bulk.create.handler.ts (1)
34-36: Improve error handling specificity.The current error handling combines all errors into a generic BadRequestException. Consider handling specific error types differently.
-throw new BadRequestException('Failed to create some availability records: ' + error.message); +if (error instanceof QueryFailedError) { + throw new BadRequestException('Database error while creating availability records: ' + error.message); +} else if (error instanceof ValidationError) { + throw new BadRequestException('Validation failed for some availability records: ' + error.message); +} else { + throw new InternalServerErrorException('Unexpected error while creating availability records'); +}packages/contracts/src/lib/employee-availability.model.ts (2)
24-24: Consider adding JSDoc for dayOfWeek property.The inline comment about day of week values would be more useful as a JSDoc comment for better IDE integration.
+/** + * Day of the week where: + * - 0 = Sunday + * - 1 = Monday + * - ... + * - 6 = Saturday + */ dayOfWeek: number; // 0 = Sunday, 6 = Saturday
42-49: Consider adding validation constraints in the create input interface.The interface could benefit from validation constraints to ensure data integrity at the contract level.
export interface IEmployeeAvailabilityCreateInput extends IBasePerTenantAndOrganizationEntityModel { + /** Employee ID must be a valid UUID */ employeeId: ID; + /** Start date must be a valid date string */ startDate: Date; + /** End date must be a valid date string and after start date */ endDate: Date; + /** Day of week must be between 0 and 6 */ dayOfWeek: number; + /** Status must be one of the defined enum values */ availabilityStatus: AvailabilityStatusEnum; + /** Optional notes for additional context */ availabilityNotes?: string; }packages/core/src/lib/employee-availability/commands/handlers/employee-availability.create.handler.ts (1)
13-43: Consider implementing retry logic for concurrent creation attempts.The create operation might fail due to concurrent requests. Consider implementing retry logic with exponential backoff.
+import { retry } from '../../../core/utils'; public async execute(command: EmployeeAvailabilityCreateCommand): Promise<IEmployeeAvailability> { const { input } = command; // ... validation logic ... const tenantId = RequestContext.currentTenantId(); const availability = new EmployeeAvailability({ ...input, tenantId }); - return await this.availabilityService.create(availability); + return await retry( + () => this.availabilityService.create(availability), + { + maxAttempts: 3, + backoff: 'exponential', + initialDelay: 100 + } + ); }packages/core/src/lib/employee-availability/employee-availability.entity.ts (1)
9-10: Consider adding indexes for frequently queried fields.Add indexes for fields that will be frequently used in queries to improve performance.
@MultiORMEntity('employee_availability') +@Index(['employeeId', 'startDate', 'endDate']) +@Index(['dayOfWeek']) export class EmployeeAvailability extends TenantOrganizationBaseEntity implements IEmployeeAvailability {packages/core/src/lib/employee-availability/employee-availability.controller.ts (1)
107-112: Consider implementing optimistic locking for updates.The update operation could benefit from optimistic locking to handle concurrent modifications.
async update( @Param('id', UUIDValidationPipe) id: ID, - @Body() entity: UpdateEmployeeAvailabilityDTO + @Body() entity: UpdateEmployeeAvailabilityDTO, + @Headers('If-Match') version: string ): Promise<IEmployeeAvailability | UpdateResult> { - return this.availabilityService.update(id, { ...entity }); + return this.availabilityService.update(id, { ...entity }, version); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/contracts/src/lib/employee-availability.model.ts(1 hunks)packages/core/src/lib/employee-availability/commands/handlers/employee-availability.bulk.create.handler.ts(1 hunks)packages/core/src/lib/employee-availability/commands/handlers/employee-availability.create.handler.ts(1 hunks)packages/core/src/lib/employee-availability/dto/create-employee-availability.dto.ts(1 hunks)packages/core/src/lib/employee-availability/dto/update-employee-availability.dto.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.controller.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.entity.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.module.ts(1 hunks)packages/core/src/lib/shared/pipes/employee-availability-status.pipe.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (5)
packages/core/src/lib/employee-availability/dto/update-employee-availability.dto.ts (1)
5-7: LGTM! Clean implementation following NestJS best practices.The DTO correctly extends PartialType to make all fields optional and implements the appropriate interface.
packages/core/src/lib/employee-availability/employee-availability.module.ts (1)
11-21: LGTM! Module configuration is complete and well-structured.The module includes all necessary providers, repositories, and handlers for both TypeORM and MikroORM.
packages/core/src/lib/shared/pipes/employee-availability-status.pipe.ts (1)
1-48: LGTM! Well-structured transformer implementation.The implementation is clean, well-documented, and includes proper error handling for invalid values.
packages/core/src/lib/employee-availability/commands/handlers/employee-availability.create.handler.ts (1)
37-40: Consider validating tenant ID before creating availability.The handler should verify that the tenant ID exists before proceeding with the creation.
packages/core/src/lib/employee-availability/employee-availability.entity.ts (1)
52-64: Consider adding cascade options for the employee relation.The current implementation only specifies
onDelete: 'CASCADE'. Consider adding other cascade options if needed.
| try { | ||
| // Process items in parallel with Promise.all | ||
| const results = await Promise.all( | ||
| input.map(async (item) => { | ||
| const availability = new EmployeeAvailability({ | ||
| ...item, | ||
| tenantId | ||
| }); | ||
| return this.commandBus.execute(new EmployeeAvailabilityCreateCommand(availability)); | ||
| }) | ||
| ); | ||
| allAvailability.push(...results); | ||
| } catch (error) { | ||
| throw new BadRequestException('Failed to create some availability records: ' + error.message); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding transaction support and input size limits.
While parallel processing is implemented, consider these improvements:
- Wrap the bulk operation in a transaction to ensure atomicity
- Add a limit to the input array size to prevent performance issues
try {
+ const MAX_BULK_SIZE = 100;
+ if (input.length > MAX_BULK_SIZE) {
+ throw new BadRequestException(`Cannot process more than ${MAX_BULK_SIZE} records at once`);
+ }
+ // Use TypeORM's transaction
+ const queryRunner = this.connection.createQueryRunner();
+ await queryRunner.connect();
+ await queryRunner.startTransaction();
try {
const results = await Promise.all(
input.map(async (item) => {
const availability = new EmployeeAvailability({
...item,
tenantId
});
return this.commandBus.execute(new EmployeeAvailabilityCreateCommand(availability));
})
);
allAvailability.push(...results);
+ await queryRunner.commitTransaction();
} catch (error) {
+ await queryRunner.rollbackTransaction();
throw new BadRequestException('Failed to create some availability records: ' + error.message);
+ } finally {
+ await queryRunner.release();
}Committable suggestion skipped: line range outside the PR's diff.
| @Post('/bulk') | ||
| async createBulk(@Body() entities: CreateEmployeeAvailabilityDTO[]): Promise<IEmployeeAvailability[]> { | ||
| return await this.commandBus.execute(new EmployeeAvailabilityBulkCreateCommand(entities)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding rate limiting for bulk operations.
The bulk creation endpoint could be vulnerable to abuse. Consider adding rate limiting.
@HttpCode(HttpStatus.CREATED)
+@UseGuards(ThrottlerGuard)
+@Throttle(10, 60) // 10 requests per minute
@Post('/bulk')
async createBulk(@Body() entities: CreateEmployeeAvailabilityDTO[]): Promise<IEmployeeAvailability[]> {📝 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.
| @Post('/bulk') | |
| async createBulk(@Body() entities: CreateEmployeeAvailabilityDTO[]): Promise<IEmployeeAvailability[]> { | |
| return await this.commandBus.execute(new EmployeeAvailabilityBulkCreateCommand(entities)); | |
| } | |
| @HttpCode(HttpStatus.CREATED) | |
| @UseGuards(ThrottlerGuard) | |
| @Throttle(10, 60) // 10 requests per minute | |
| @Post('/bulk') | |
| async createBulk(@Body() entities: CreateEmployeeAvailabilityDTO[]): Promise<IEmployeeAvailability[]> { | |
| return await this.commandBus.execute(new EmployeeAvailabilityBulkCreateCommand(entities)); | |
| } |
| import { UpdateEmployeeAvailabilityDTO } from './dto/update-employee-availability.dto'; | ||
|
|
||
| @ApiTags('EmployeeAvailability') | ||
| @UseGuards(TenantPermissionGuard) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding role-based access control.
The controller only uses TenantPermissionGuard. Consider adding role-based access control for finer-grained permissions.
+@UseGuards(TenantPermissionGuard, RolesGuard)
+@Roles(RolesEnum.ADMIN, RolesEnum.MANAGER)
@UseGuards(TenantPermissionGuard)Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/core/src/lib/employee-availability/employee-availability.controller.ts (2)
14-16: 🛠️ Refactor suggestionAdd role-based access control.
The controller only uses
TenantPermissionGuard. Consider adding role-based access control for better security.@ApiTags('EmployeeAvailability') -@UseGuards(TenantPermissionGuard) +@UseGuards(TenantPermissionGuard, RolesGuard) +@Roles(RolesEnum.ADMIN, RolesEnum.MANAGER) @Controller('/employee-availability')
41-44: 🛠️ Refactor suggestionAdd rate limiting for bulk operations.
The bulk creation endpoint should be protected against abuse with rate limiting.
@HttpCode(HttpStatus.CREATED) +@UseGuards(ThrottlerGuard) +@Throttle(10, 60) // 10 requests per minute @Post('/bulk') async createBulk(@Body() entities: CreateEmployeeAvailabilityDTO[]): Promise<IEmployeeAvailability[]> {
🧹 Nitpick comments (1)
packages/core/src/lib/employee-availability/employee-availability.controller.ts (1)
107-112: Consider making the return type more specific.The method returns either
IEmployeeAvailabilityorUpdateResult. Consider handling theUpdateResultcase in the service layer and always returnIEmployeeAvailabilityfor a more consistent API.async update( @Param('id', UUIDValidationPipe) id: ID, @Body() entity: UpdateEmployeeAvailabilityDTO -): Promise<IEmployeeAvailability | UpdateResult> { +): Promise<IEmployeeAvailability> { - return this.availabilityService.update(id, { ...entity }); + const result = await this.availabilityService.update(id, { ...entity }); + return this.availabilityService.findOne(id); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/src/lib/employee-availability/employee-availability.controller.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (3)
packages/core/src/lib/employee-availability/employee-availability.controller.ts (3)
1-12: Organize imports by grouping them logically.Consider organizing imports into groups:
- External libraries (@nestjs, typeorm)
- Internal shared modules (@gauzy/contracts)
- Local imports (./*)
import { UpdateResult } from 'typeorm'; import { Body, Controller, Get, HttpCode, HttpStatus, Param, Post, Put, Query, UseGuards } from '@nestjs/common'; import { CommandBus } from '@nestjs/cqrs'; import { ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger'; + import { ID, IEmployeeAvailability, IPagination } from '@gauzy/contracts'; + import { EmployeeAvailabilityService } from './employee-availability.service'; import { EmployeeAvailability } from './employee-availability.entity'; import { CrudController, PaginationParams } from '../core';
62-66: LGTM! Well-implemented pagination with proper documentation.The implementation correctly uses
PaginationParamsand includes comprehensive API documentation.
85-87: LGTM! Good use of Command pattern with proper validation.The implementation correctly uses DTO for validation and follows the CQRS pattern.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
packages/core/src/lib/employee-availability/commands/handlers/index.ts (1)
4-7: Enhance the documentation while keeping the clean implementation.The implementation follows the command pattern best practices and provides a clean way to register handlers. Consider enhancing the documentation to provide more context.
/** - * Exports all command handlers for EmployeeAvailability.` + * Exports command handlers for managing employee availability. + * + * Includes handlers for: + * - Single availability record creation + * - Bulk availability records creation + * + * These handlers are used by the EmployeeAvailabilityController to process + * creation requests while ensuring proper validation and business rules. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/contracts/src/lib/employee-availability.model.ts(1 hunks)packages/core/src/lib/employee-availability/commands/employee-availability.bulk.create.command.ts(1 hunks)packages/core/src/lib/employee-availability/commands/handlers/employee-availability.create.handler.ts(1 hunks)packages/core/src/lib/employee-availability/commands/handlers/index.ts(1 hunks)packages/core/src/lib/employee-availability/commands/index.ts(1 hunks)packages/core/src/lib/employee-availability/dto/create-employee-availability.dto.ts(1 hunks)packages/core/src/lib/employee-availability/dto/update-employee-availability.dto.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.controller.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.entity.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.module.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/core/src/lib/employee-availability/commands/employee-availability.bulk.create.command.ts
- packages/core/src/lib/employee-availability/dto/update-employee-availability.dto.ts
- packages/core/src/lib/employee-availability/commands/handlers/employee-availability.create.handler.ts
- packages/core/src/lib/employee-availability/employee-availability.controller.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/core/src/lib/employee-availability/employee-availability.service.ts
[error] 9-14: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (8)
packages/core/src/lib/employee-availability/commands/handlers/index.ts (2)
1-2: LGTM! Clean and well-structured imports.The imports follow a consistent naming pattern and are properly organized for command handlers.
1-7: Verify command pattern implementation and controller integration.Let's ensure the command pattern is consistently implemented and properly integrated with the controller.
✅ Verification successful
Command pattern implementation and controller integration verified successfully
The command handlers are properly integrated with the controller through the command bus pattern, with consistent implementation across both single and bulk operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify command pattern implementation and controller integration # Check if the command pattern structure is consistent echo "Checking command pattern structure..." rg -g "*.ts" -l "ICommandHandler|CommandHandler" packages/core/src/lib/employee-availability/ # Verify controller's usage of these handlers echo "Verifying controller integration..." rg -g "*.ts" "EmployeeAvailabilityBulkCreateCommand|EmployeeAvailabilityCreateCommand" packages/core/src/lib/employee-availability/Length of output: 3790
packages/core/src/lib/employee-availability/commands/index.ts (1)
1-2: LGTM!Clean and standard barrel file exports for command modules.
packages/core/src/lib/employee-availability/employee-availability.module.ts (1)
12-26: LGTM!The module is well-structured with all necessary providers and imports:
- Both ORM features are properly registered
- Command handlers are included
- Repository and service providers are present
packages/core/src/lib/employee-availability/employee-availability.entity.ts (4)
41-52: LGTM! Status field implementation looks good.The
availabilityStatusfield correctly:
- Uses enum validation
- Includes type specification (
type: 'int')- Implements the status transformer
53-65: LGTM! Notes field implementation looks good.The
availabilityNotesfield correctly:
- Uses proper validation decorators
- Implements nullable text column
- Includes descriptive API documentation
66-93: LGTM! Employee relation implementation looks good.The employee relation correctly:
- Uses proper relation decorators
- Implements cascade delete
- Uses RelationId for the foreign key
1-10: Verify MultiORM compatibility with MikroORM.Based on the past review comments, there was a concern about MikroORM support. Let's verify the compatibility of the MultiORM decorators with MikroORM.
✅ Verification successful
MikroORM compatibility is properly implemented
The MultiORM decorators in the codebase are explicitly designed to support both MikroORM and TypeORM, with proper mappings for entities, columns, and relations. The implementation includes complete support for MikroORM's features, making the
EmployeeAvailabilityentity fully compatible with MikroORM.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if MikroORM is configured in the project and verify decorator usage # Check if MikroORM is a dependency grep -r "@mikro-orm" package.json # Look for similar entity patterns in other files rg -l "MultiORMEntity|MultiORMColumn|MultiORMManyToOne" "packages/core/src"Length of output: 10695
Script:
#!/bin/bash # Check the implementation of MultiORM decorators cat packages/core/src/lib/core/decorators/entity/entity.decorator.ts cat packages/core/src/lib/core/decorators/entity/column.decorator.ts cat packages/core/src/lib/core/decorators/entity/relations/many-to-one.decorator.tsLength of output: 9117
| export class EmployeeAvailabilityService extends TenantAwareCrudService<EmployeeAvailability> { | ||
| constructor( | ||
| typeOrmEmployeeAvailabilityRepository: TypeOrmEmployeeAvailabilityRepository, | ||
| mikroOrmEmployeeAvailabilityRepository: MikroOrmEmployeeAvailabilityRepository | ||
| ) { | ||
| super(typeOrmEmployeeAvailabilityRepository, mikroOrmEmployeeAvailabilityRepository); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add essential business logic methods for availability management.
The service should include methods for:
- Validating scheduling conflicts
- Checking employee availability for a given time period
- Handling overlapping availability records
Consider adding these methods:
async validateAvailabilityConflicts(
employeeId: string,
startDate: Date,
endDate: Date,
excludeId?: string
): Promise<boolean> {
const conflicts = await this.repository.find({
where: {
employeeId,
startDate: LessThanOrEqual(endDate),
endDate: MoreThanOrEqual(startDate),
...(excludeId && { id: Not(excludeId) })
}
});
return conflicts.length > 0;
}
async getEmployeeAvailability(
employeeId: string,
date: Date
): Promise<EmployeeAvailability | null> {
const dayOfWeek = date.getDay();
return this.repository.findOne({
where: {
employeeId,
dayOfWeek,
startDate: LessThanOrEqual(date),
endDate: MoreThanOrEqual(date)
}
});
}🧰 Tools
🪛 Biome (1.9.4)
[error] 9-14: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
| @ApiProperty({ type: () => Date }) | ||
| @IsDateString() | ||
| startDate: Date; | ||
|
|
||
| @ApiProperty({ type: () => Date }) | ||
| @IsDateString() | ||
| endDate: Date; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add date range validation.
Add validation to ensure startDate is before endDate.
@IsDateString()
@ValidateIf((o) => o.endDate && new Date(o.startDate) >= new Date(o.endDate))
@ApiProperty({ type: () => Date })
startDate: Date;There was a problem hiding this comment.
@samuelmbabhazi Yes. Please add date range validation here. We have some custom validator for it. Can you please check?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| @ApiProperty({ type: () => Number }) | ||
| @IsInt() | ||
| dayOfWeek: number; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add range validation for dayOfWeek.
The dayOfWeek property should be validated to ensure it's between 0 and 6 (Sunday to Saturday).
@ApiProperty({ type: () => Number, description: '0 = Sunday, 6 = Saturday' })
@IsInt()
@Min(0)
@Max(6)
dayOfWeek: number;| export interface IEmployeeAvailability extends IBaseEmployeeAvailability { | ||
| employeeId: ID; | ||
| employee: IEmployee; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicate employeeId property.
The employeeId property is already inherited from IBaseEmployeeAvailability and doesn't need to be redeclared.
export interface IEmployeeAvailability extends IBaseEmployeeAvailability {
- employeeId: ID;
employee: IEmployee;
}📝 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.
| export interface IEmployeeAvailability extends IBaseEmployeeAvailability { | |
| employeeId: ID; | |
| employee: IEmployee; | |
| } | |
| export interface IEmployeeAvailability extends IBaseEmployeeAvailability { | |
| employee: IEmployee; | |
| } |
| /** | ||
| * The day of the week corresponding to the availability. | ||
| * Values range from `0` (Sunday) to `6` (Saturday). | ||
| */ | ||
| @ApiProperty({ type: () => Number, description: 'Day of the week (0 = Sunday, 6 = Saturday)' }) | ||
| @IsInt() | ||
| @IsNotEmpty() | ||
| @MultiORMColumn() | ||
| dayOfWeek: number; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add range validation for dayOfWeek field.
The dayOfWeek field should be constrained to values between 0 and 6 (inclusive).
@ApiProperty({ type: () => Number, description: 'Day of the week (0 = Sunday, 6 = Saturday)' })
@IsInt()
@IsNotEmpty()
+@Min(0, { message: 'Day of week must be between 0 and 6' })
+@Max(6, { message: 'Day of week must be between 0 and 6' })
@MultiORMColumn()
dayOfWeek: number;📝 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.
| /** | |
| * The day of the week corresponding to the availability. | |
| * Values range from `0` (Sunday) to `6` (Saturday). | |
| */ | |
| @ApiProperty({ type: () => Number, description: 'Day of the week (0 = Sunday, 6 = Saturday)' }) | |
| @IsInt() | |
| @IsNotEmpty() | |
| @MultiORMColumn() | |
| dayOfWeek: number; | |
| /** | |
| * The day of the week corresponding to the availability. | |
| * Values range from `0` (Sunday) to `6` (Saturday). | |
| */ | |
| @ApiProperty({ type: () => Number, description: 'Day of the week (0 = Sunday, 6 = Saturday)' }) | |
| @IsInt() | |
| @IsNotEmpty() | |
| @Min(0, { message: 'Day of week must be between 0 and 6' }) | |
| @Max(6, { message: 'Day of week must be between 0 and 6' }) | |
| @MultiORMColumn() | |
| dayOfWeek: number; |
| /** | ||
| * Represents the start date of an employee's availability period. | ||
| * This marks when the availability status takes effect. | ||
| */ | ||
| @ApiProperty({ type: () => Date }) | ||
| @IsDate() | ||
| @IsNotEmpty() | ||
| @MultiORMColumn() | ||
| startDate: Date; | ||
|
|
||
| /** | ||
| * Represents the end date of an employee's availability period. | ||
| * This marks when the availability status expires. | ||
| */ | ||
| @ApiProperty({ type: () => Date }) | ||
| @IsDate() | ||
| @IsNotEmpty() | ||
| @MultiORMColumn() | ||
| endDate: Date; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation to ensure endDate is after startDate.
Consider adding a custom validator to ensure that endDate occurs after startDate to prevent invalid date ranges.
+import { ValidateIf } from 'class-validator';
+
@MultiORMEntity('employee_availability')
export class EmployeeAvailability extends TenantOrganizationBaseEntity implements IEmployeeAvailability {
@ApiProperty({ type: () => Date })
@IsDate()
@IsNotEmpty()
@MultiORMColumn()
startDate: Date;
@ApiProperty({ type: () => Date })
@IsDate()
@IsNotEmpty()
+ @ValidateIf((o) => o.startDate && o.endDate)
+ @IsDateAfter('startDate', {
+ message: 'End date must be after start date'
+ })
@MultiORMColumn()
endDate: Date;Committable suggestion skipped: line range outside the PR's diff.
| ...item, | ||
| tenantId | ||
| }); | ||
| return this.commandBus.execute(new EmployeeAvailabilityCreateCommand(availability)); |
There was a problem hiding this comment.
@samuelmbabhazi Why we creating EmployeeAvailability in loop instead of bulk?
| export class CreateEmployeeAvailabilityDTO | ||
| extends TenantOrganizationBaseDTO | ||
| implements IEmployeeAvailabilityCreateInput | ||
| { |
There was a problem hiding this comment.
@samuelmbabhazi Why we not used EmployeeAvailability entity here?? Why we duplicated property which already existed into EmployeeAvailability
Note: Please try to reuse as much as code.
| * Transformer to handle the conversion between the enum values | ||
| * (used in the application) and integer values (stored in the database). | ||
| */ | ||
| export class AvailabilityStatusTransformer implements ValueTransformer { |
There was a problem hiding this comment.
@samuelmbabhazi And please move this pipe to feature folder. Its not a common pipe.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/core/src/lib/employee-availability/commands/handlers/employee-availability.bulk.create.handler.ts (1)
13-17: Validate additional attributes to prevent data integrity issues.
You correctly check ifinputis a non-empty array. Consider also validating each element’s required properties (e.g.,startDate,endDate) before creation to avoid downstream errors.packages/core/src/lib/employee-availability/pipes/employee-availability-status.pipe.ts (1)
9-27: Potential optimization of switch logic.
Consider using a map or object literal key-value structure to map enum values to integer values, which may simplify maintenance if new statuses are added.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/contracts/src/lib/employee-availability.model.ts(1 hunks)packages/core/src/lib/employee-availability/commands/handlers/employee-availability.bulk.create.handler.ts(1 hunks)packages/core/src/lib/employee-availability/dto/create-employee-availability.dto.ts(1 hunks)packages/core/src/lib/employee-availability/dto/update-employee-availability.dto.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.entity.ts(1 hunks)packages/core/src/lib/employee-availability/pipes/employee-availability-status.pipe.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/src/lib/employee-availability/dto/update-employee-availability.dto.ts
- packages/core/src/lib/employee-availability/dto/create-employee-availability.dto.ts
- packages/contracts/src/lib/employee-availability.model.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (13)
packages/core/src/lib/employee-availability/commands/handlers/employee-availability.bulk.create.handler.ts (4)
1-2: Imports are well-structured and appropriate.
No issues with the imports here.
19-21: Solid approach for initializing local variables.
DeclaringallAvailabilityand retrievingtenantIdhere is clear and concise.
22-29: Consider handling partial failures with transactions or parallel processing.
This loop-based creation can cause partial inserts if an error occurs mid-iteration, leaving some records created and others not. Using a transaction or parallel processing strategy can ensure atomicity or improved performance.
30-32: Good final return statement.
ReturningallAvailabilityhere cleanly wraps up the creation logic.packages/core/src/lib/employee-availability/pipes/employee-availability-status.pipe.ts (2)
1-2: Imports look good.
No concerns with how you’re importing required modules and types.
29-47: Consistent approach in thefrommethod.
Your mapping logic mirrors thetomethod, which is consistent. If you implement a map-based approach into, consider doing the same here for maintainability.packages/core/src/lib/employee-availability/employee-availability.entity.ts (7)
12-23: Proper validation for start date.
Using@IsBeforeDate(...)is a good approach to ensurestartDateprecedesendDate.
25-33: endDate property is well-defined.
No further issues here; validation is consistent withstartDate.
35-45: Clear day-of-week validations.
Requiring a value between 0 (Sunday) and 6 (Saturday) looks correct. The min/max constraints align perfectly with this range.
47-57: Availability status property is clearly defined.
The transformer usage is appropriate, ensuring consistent enum ↔ integer conversions.
59-70: Optional notes are handled cleanly.
Marking this column as nullable with@IsOptional()is well-aligned with the domain requirements for notes.
90-98: Good use ofemployeeIdas a foreign key.
Use of@RelationIdensures correct linkage. Just ensure that usinguuidaligns with the employee entity ID type across ORMs.
78-88: Consider verifying multi-ORM usage.
The many-to-one relationship withEmployeemight differ between TypeORM and MikroORM usage. Ensure both ORMs handle cascading and relation definitions consistently.✅ Verification successful
Multi-ORM implementation verified and working as expected
The@MultiORMManyToOnedecorator properly handles both TypeORM and MikroORM, with correct cascade behavior and relationship mappings for the Employee relationship.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Confirm that both TypeORM and MikroORM configurations handle the cascade and relationship definitions. rg -A 5 '@MultiORMManyToOne'Length of output: 68462
Script:
#!/bin/bash # Find the implementation of MultiORMManyToOne decorator rg -l "export.*MultiORMManyToOne"Length of output: 115
Script:
#!/bin/bash # View the implementation of MultiORMManyToOne decorator cat packages/core/src/lib/core/decorators/entity/relations/many-to-one.decorator.tsLength of output: 5301
| const allAvailability: IEmployeeAvailability[] = []; | ||
| const tenantId = RequestContext.currentTenantId(); | ||
|
|
||
| for (const item of input) { |
There was a problem hiding this comment.
@samuelmbabhazi Again same, we don't want to insert EmployeeAvailability one by one using EmployeeAvailabilityCreateCommand command.
|
@samuelmbabhazi please review suggestions and close / fix etc |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 11719210 | Triggered | Nx Cloud Token | b1dd009 | nx.json | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/core/src/lib/core/crud/crud.service.ts (1)
22-22: Add 'iquery' to the cspell dictionary.The word 'iquery' in 'IQueryBuilder' follows TypeScript interface naming conventions. Add it to the cspell dictionary to resolve the pipeline failure.
Add the following to your cspell configuration:
{ "words": [ "iquery" ] }🧰 Tools
🪛 GitHub Check: Cspell
[warning] 22-22:
Unknown word (iquery)🪛 GitHub Actions: Check Spelling and Typos with cspell
[warning] 22-22: Unknown word 'iquery' found in the code
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/core/src/lib/core/crud/crud.service.ts(5 hunks)packages/core/src/lib/core/crud/tenant-aware-crud.service.ts(3 hunks)packages/core/src/lib/employee-availability/commands/employee-availability.bulk.create.command.ts(1 hunks)packages/core/src/lib/employee-availability/commands/handlers/employee-availability.bulk.create.handler.ts(1 hunks)packages/core/src/lib/employee-availability/commands/handlers/employee-availability.create.handler.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.entity.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/core/src/lib/employee-availability/commands/employee-availability.bulk.create.command.ts
- packages/core/src/lib/employee-availability/employee-availability.entity.ts
- packages/core/src/lib/employee-availability/commands/handlers/employee-availability.create.handler.ts
- packages/core/src/lib/employee-availability/employee-availability.service.ts
🧰 Additional context used
🪛 GitHub Actions: Check Spelling and Typos with cspell
packages/core/src/lib/core/crud/crud.service.ts
[warning] 22-22: Unknown word 'iquery' found in the code
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (8)
packages/core/src/lib/employee-availability/commands/handlers/employee-availability.bulk.create.handler.ts (3)
1-12: LGTM! Well-structured class declaration with proper imports.The command handler is correctly set up with appropriate decorator and interface implementation.
13-13: LGTM! Clean dependency injection.The service is properly injected with appropriate access modifier.
21-35: 🛠️ Refactor suggestionAdd input validation, error handling, and transaction support.
While the implementation now correctly uses bulk operations instead of individual creates, there are several improvements needed:
- Input validation is missing
- No size limits on bulk operations
- Error handling is not implemented
- Transaction support is missing
Here's the suggested implementation:
public async execute(command: EmployeeAvailabilityBulkCreateCommand): Promise<IEmployeeAvailability[]> { const { input } = command; + + // Input validation + if (!Array.isArray(input) || input.length === 0) { + throw new BadRequestException('Input must be a non-empty array of availability records.'); + } + + const MAX_BULK_SIZE = 100; + if (input.length > MAX_BULK_SIZE) { + throw new BadRequestException(`Cannot process more than ${MAX_BULK_SIZE} records at once`); + } + const tenantId = RequestContext.currentTenantId(); - // Prepare employee availability records with tenantId - const employeeAvailabilities = input.map(item => - new EmployeeAvailability({ - ...item, - tenantId - }) - ); + try { + // Prepare employee availability records with tenantId + const employeeAvailabilities = input.map(item => + new EmployeeAvailability({ + ...item, + tenantId + }) + ); - // Perform bulk insert using the availability service - return await this._availabilityService.bulkCreate(employeeAvailabilities); + // Perform bulk insert using the availability service within a transaction + return await this._availabilityService.bulkCreate(employeeAvailabilities); + } catch (error) { + throw new BadRequestException(`Failed to create availability records: ${error.message}`); + } }Note: Ensure that
_availabilityService.bulkCreateinternally uses transactions for atomicity.Likely invalid or redundant comment.
packages/core/src/lib/core/crud/tenant-aware-crud.service.ts (2)
242-242: LGTM! Type safety improvement.The change from
T['id']toIDtype enhances type safety and maintains consistency with theCrudServiceclass.
285-285: LGTM! Type safety improvement.The change from
T['id']toIDtype enhances type safety and maintains consistency with theCrudServiceclass.packages/core/src/lib/core/crud/crud.service.ts (3)
19-19: LGTM! Import aligns with type changes.The import of
IDtype is necessary for the method signature changes and is correctly placed.
349-349: LGTM! Type safety improvement.The change from
T['id']toIDtype enhances type safety and maintains consistency across the codebase.
720-720: LGTM! Type safety improvement.The change from
T['id']toIDtype enhances type safety and maintains consistency across the codebase. The error handling and implementation in both methods are correct.Also applies to: 756-756
| /** | ||
| * Bulk creates new entities or updates existing ones based on the provided entity data. | ||
| * | ||
| * @param entities The array of partial entity data for creation or update. | ||
| * @param createOptions Options for the creation of the entity in MikroORM. | ||
| * @param assignOptions Options for assigning existing entities in MikroORM. | ||
| * @returns The created or updated entities. | ||
| */ | ||
| public async bulkCreate( | ||
| partialEntities: IPartialEntity<T>[], | ||
| createOptions: CreateOptions<boolean> = { | ||
| partial: true, | ||
| managed: true | ||
| }, | ||
| assignOptions: AssignOptions<boolean> = { | ||
| updateNestedEntities: false, | ||
| onlyOwnProperties: true | ||
| } | ||
| ): Promise<T[]> { | ||
| try { | ||
| switch (this.ormType) { | ||
| case MultiORMEnum.MikroORM: | ||
| try { | ||
| const newEntities: T[] = []; | ||
|
|
||
| for (const partialEntity of partialEntities) { | ||
| if (partialEntity['id']) { | ||
| // Try to find the existing entity | ||
| const existingEntity = await this.mikroOrmRepository.findOne(partialEntity['id']); | ||
| if (existingEntity) { | ||
| // If found, perform an update (upsert) | ||
| this.mikroOrmRepository.assign(existingEntity, partialEntity as any, assignOptions); | ||
| newEntities.push(existingEntity); | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| // Create a new entity using MikroORM | ||
| const newEntity = this.mikroOrmRepository.create( | ||
| partialEntity as RequiredEntityData<T>, | ||
| createOptions | ||
| ); | ||
| newEntities.push(newEntity); | ||
| } | ||
|
|
||
| // Persist all new entities and flush | ||
| await this.mikroOrmRepository.persistAndFlush(newEntities); | ||
| return newEntities.map(entity => this.serialize(entity)); | ||
| } catch (error) { | ||
| console.error('Error during MikroORM bulk create transaction:', error); | ||
| } | ||
| break; | ||
|
|
||
| case MultiORMEnum.TypeORM: | ||
| try { | ||
| // Bulk insert using TypeORM's `save` method for better performance | ||
| const newEntities = this.typeOrmRepository.create(partialEntities as DeepPartial<T>[]); | ||
| return await this.typeOrmRepository.save(newEntities); | ||
| } catch (error) { | ||
| console.error('Error during TypeORM bulk create transaction:', error); | ||
| } | ||
| break; | ||
|
|
||
| default: | ||
| throw new Error(`Not implemented for ${this.ormType}`); | ||
| } | ||
| } catch (error) { | ||
| console.error('Error in CRUD service bulk create method:', error); | ||
| throw new BadRequestException(error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Improve error handling in bulkCreate method.
The method has several issues that need to be addressed:
- Error handling in MikroORM and TypeORM blocks doesn't rethrow errors, causing undefined return.
- Missing return type annotation for error cases.
- Nested try-catch blocks add unnecessary complexity.
Apply this diff to fix the issues:
public async bulkCreate(
partialEntities: IPartialEntity<T>[],
createOptions: CreateOptions<boolean> = {
partial: true,
managed: true
},
assignOptions: AssignOptions<boolean> = {
updateNestedEntities: false,
onlyOwnProperties: true
}
): Promise<T[]> {
try {
switch (this.ormType) {
case MultiORMEnum.MikroORM: {
- try {
const newEntities: T[] = [];
for (const partialEntity of partialEntities) {
if (partialEntity['id']) {
// Try to find the existing entity
const existingEntity = await this.mikroOrmRepository.findOne(partialEntity['id']);
if (existingEntity) {
// If found, perform an update (upsert)
this.mikroOrmRepository.assign(existingEntity, partialEntity as any, assignOptions);
newEntities.push(existingEntity);
continue;
}
}
// Create a new entity using MikroORM
const newEntity = this.mikroOrmRepository.create(
partialEntity as RequiredEntityData<T>,
createOptions
);
newEntities.push(newEntity);
}
// Persist all new entities and flush
await this.mikroOrmRepository.persistAndFlush(newEntities);
return newEntities.map(entity => this.serialize(entity));
- } catch (error) {
- console.error('Error during MikroORM bulk create transaction:', error);
- }
- break;
+ }
case MultiORMEnum.TypeORM: {
- try {
// Bulk insert using TypeORM's `save` method for better performance
const newEntities = this.typeOrmRepository.create(partialEntities as DeepPartial<T>[]);
return await this.typeOrmRepository.save(newEntities);
- } catch (error) {
- console.error('Error during TypeORM bulk create transaction:', error);
- }
- break;
+ }
default:
throw new Error(`Not implemented for ${this.ormType}`);
}
} catch (error) {
console.error('Error in CRUD service bulk create method:', error);
throw new BadRequestException(error);
}
}📝 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.
| /** | |
| * Bulk creates new entities or updates existing ones based on the provided entity data. | |
| * | |
| * @param entities The array of partial entity data for creation or update. | |
| * @param createOptions Options for the creation of the entity in MikroORM. | |
| * @param assignOptions Options for assigning existing entities in MikroORM. | |
| * @returns The created or updated entities. | |
| */ | |
| public async bulkCreate( | |
| partialEntities: IPartialEntity<T>[], | |
| createOptions: CreateOptions<boolean> = { | |
| partial: true, | |
| managed: true | |
| }, | |
| assignOptions: AssignOptions<boolean> = { | |
| updateNestedEntities: false, | |
| onlyOwnProperties: true | |
| } | |
| ): Promise<T[]> { | |
| try { | |
| switch (this.ormType) { | |
| case MultiORMEnum.MikroORM: | |
| try { | |
| const newEntities: T[] = []; | |
| for (const partialEntity of partialEntities) { | |
| if (partialEntity['id']) { | |
| // Try to find the existing entity | |
| const existingEntity = await this.mikroOrmRepository.findOne(partialEntity['id']); | |
| if (existingEntity) { | |
| // If found, perform an update (upsert) | |
| this.mikroOrmRepository.assign(existingEntity, partialEntity as any, assignOptions); | |
| newEntities.push(existingEntity); | |
| continue; | |
| } | |
| } | |
| // Create a new entity using MikroORM | |
| const newEntity = this.mikroOrmRepository.create( | |
| partialEntity as RequiredEntityData<T>, | |
| createOptions | |
| ); | |
| newEntities.push(newEntity); | |
| } | |
| // Persist all new entities and flush | |
| await this.mikroOrmRepository.persistAndFlush(newEntities); | |
| return newEntities.map(entity => this.serialize(entity)); | |
| } catch (error) { | |
| console.error('Error during MikroORM bulk create transaction:', error); | |
| } | |
| break; | |
| case MultiORMEnum.TypeORM: | |
| try { | |
| // Bulk insert using TypeORM's `save` method for better performance | |
| const newEntities = this.typeOrmRepository.create(partialEntities as DeepPartial<T>[]); | |
| return await this.typeOrmRepository.save(newEntities); | |
| } catch (error) { | |
| console.error('Error during TypeORM bulk create transaction:', error); | |
| } | |
| break; | |
| default: | |
| throw new Error(`Not implemented for ${this.ormType}`); | |
| } | |
| } catch (error) { | |
| console.error('Error in CRUD service bulk create method:', error); | |
| throw new BadRequestException(error); | |
| } | |
| } | |
| /** | |
| * Bulk creates new entities or updates existing ones based on the provided entity data. | |
| * | |
| * @param entities The array of partial entity data for creation or update. | |
| * @param createOptions Options for the creation of the entity in MikroORM. | |
| * @param assignOptions Options for assigning existing entities in MikroORM. | |
| * @returns The created or updated entities. | |
| */ | |
| public async bulkCreate( | |
| partialEntities: IPartialEntity<T>[], | |
| createOptions: CreateOptions<boolean> = { | |
| partial: true, | |
| managed: true | |
| }, | |
| assignOptions: AssignOptions<boolean> = { | |
| updateNestedEntities: false, | |
| onlyOwnProperties: true | |
| } | |
| ): Promise<T[]> { | |
| try { | |
| switch (this.ormType) { | |
| case MultiORMEnum.MikroORM: { | |
| const newEntities: T[] = []; | |
| for (const partialEntity of partialEntities) { | |
| if (partialEntity['id']) { | |
| // Try to find the existing entity | |
| const existingEntity = await this.mikroOrmRepository.findOne(partialEntity['id']); | |
| if (existingEntity) { | |
| // If found, perform an update (upsert) | |
| this.mikroOrmRepository.assign(existingEntity, partialEntity as any, assignOptions); | |
| newEntities.push(existingEntity); | |
| continue; | |
| } | |
| } | |
| // Create a new entity using MikroORM | |
| const newEntity = this.mikroOrmRepository.create( | |
| partialEntity as RequiredEntityData<T>, | |
| createOptions | |
| ); | |
| newEntities.push(newEntity); | |
| } | |
| // Persist all new entities and flush | |
| await this.mikroOrmRepository.persistAndFlush(newEntities); | |
| return newEntities.map(entity => this.serialize(entity)); | |
| } | |
| case MultiORMEnum.TypeORM: { | |
| // Bulk insert using TypeORM's `save` method for better performance | |
| const newEntities = this.typeOrmRepository.create(partialEntities as DeepPartial<T>[]); | |
| return await this.typeOrmRepository.save(newEntities); | |
| } | |
| default: | |
| throw new Error(`Not implemented for ${this.ormType}`); | |
| } | |
| } catch (error) { | |
| console.error('Error in CRUD service bulk create method:', error); | |
| throw new BadRequestException(error); | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/core/src/lib/employee-availability/commands/handlers/employee-availability.create.handler.ts (1)
19-27:⚠️ Potential issueAdd comprehensive input validation.
The handler should validate all required fields before creating the availability record.
Apply this diff to add input validation:
public async execute(command: EmployeeAvailabilityCreateCommand): Promise<IEmployeeAvailability> { const { input } = command; + const { startDate, endDate, employeeId, dayOfWeek, availabilityStatus } = input; const tenantId = RequestContext.currentTenantId() ?? input.tenantId; + if (!startDate || !endDate) { + throw new BadRequestException('Start date and end date are required.'); + } + if (new Date(endDate) <= new Date(startDate)) { + throw new BadRequestException('End date must be after start date.'); + } + if (!employeeId) { + throw new BadRequestException('Employee ID is required.'); + } + if (typeof dayOfWeek !== 'number' || dayOfWeek < 0 || dayOfWeek > 6) { + throw new BadRequestException('Day of week must be a number between 0 and 6.'); + } + if (!availabilityStatus) { + throw new BadRequestException('Availability status is required.'); + } + return await this._availabilityService.create(new EmployeeAvailability({ ...input, tenantId })); }packages/core/src/lib/employee-availability/commands/handlers/employee-availability.bulk.create.handler.ts (1)
21-35:⚠️ Potential issueAdd input validation and size limits.
The bulk create handler should validate the input array and implement size limits to prevent performance issues.
Apply this diff to add input validation and size limits:
public async execute(command: EmployeeAvailabilityBulkCreateCommand): Promise<IEmployeeAvailability[]> { const { input } = command; + if (!Array.isArray(input) || input.length === 0) { + throw new BadRequestException('Input must be a non-empty array of availability records.'); + } + + const MAX_BULK_SIZE = 100; + if (input.length > MAX_BULK_SIZE) { + throw new BadRequestException(`Cannot process more than ${MAX_BULK_SIZE} records at once`); + } + const tenantId = RequestContext.currentTenantId() ?? input.tenantId; // Prepare employee availability records with tenantId const employeeAvailabilities = input.map(item => new EmployeeAvailability({ ...item, tenantId }) ); // Perform bulk insert using the availability service return await this._availabilityService.bulkCreate(employeeAvailabilities); }packages/core/src/lib/employee-availability/employee-availability.service.ts (1)
11-80: 🛠️ Refactor suggestionAdd essential business logic methods for availability management.
The service should include methods for validating scheduling conflicts and checking employee availability.
Add these essential methods to the service:
/** * Validates if there are any scheduling conflicts for an employee. */ async validateAvailabilityConflicts( employeeId: string, startDate: Date, endDate: Date, excludeId?: string ): Promise<boolean> { const conflicts = await this.repository.find({ where: { employeeId, startDate: LessThanOrEqual(endDate), endDate: MoreThanOrEqual(startDate), ...(excludeId && { id: Not(excludeId) }) } }); return conflicts.length > 0; } /** * Gets an employee's availability for a specific date. */ async getEmployeeAvailability( employeeId: string, date: Date ): Promise<EmployeeAvailability | null> { const dayOfWeek = date.getDay(); return this.repository.findOne({ where: { employeeId, dayOfWeek, startDate: LessThanOrEqual(date), endDate: MoreThanOrEqual(date) } }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/core/src/lib/core/crud/crud.service.ts(4 hunks)packages/core/src/lib/employee-availability/commands/handlers/employee-availability.bulk.create.handler.ts(1 hunks)packages/core/src/lib/employee-availability/commands/handlers/employee-availability.create.handler.ts(1 hunks)packages/core/src/lib/employee-availability/employee-availability.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/lib/core/crud/crud.service.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test
- GitHub Check: build
| public async bulkCreate(entities: Partial<EmployeeAvailability>[]): Promise<IEmployeeAvailability[]> { | ||
| const tenantId = RequestContext.currentTenantId(); | ||
|
|
||
| // Prepare entities ensuring `tenantId` is assigned | ||
| const items = entities.map(entity => | ||
| new EmployeeAvailability({ | ||
| ...entity, | ||
| tenantId | ||
| }) | ||
| ); | ||
|
|
||
| try { | ||
| switch (this.ormType) { | ||
| case MultiORMEnum.MikroORM: | ||
| try { | ||
| // Convert input entities to MikroORM format | ||
| const mikroEntities = items.map((item: EmployeeAvailability) => | ||
| this.mikroOrmRepository.create(item, { | ||
| managed: true | ||
| }) | ||
| ); | ||
|
|
||
| // Bulk insert using MikroORM | ||
| await this.mikroOrmRepository.persistAndFlush(mikroEntities); | ||
| return mikroEntities; | ||
| } catch (error) { | ||
| throw new HttpException( | ||
| `Error during MikroORM bulk create transaction : ${error.message}`, | ||
| HttpStatus.INTERNAL_SERVER_ERROR | ||
| ); | ||
| } | ||
|
|
||
| case MultiORMEnum.TypeORM: | ||
| try { | ||
| // Bulk insert using TypeORM's `save` method for optimized inserts | ||
| return await this.typeOrmRepository.save(items as DeepPartial<EmployeeAvailability>[]); | ||
| } catch (error) { | ||
| throw new HttpException( | ||
| `Error during TypeORM bulk create transaction : ${error.message}`, | ||
| HttpStatus.INTERNAL_SERVER_ERROR | ||
| ); | ||
| } | ||
|
|
||
| default: | ||
| throw new Error(`Not implemented for ${this.ormType}`); | ||
| } | ||
| } catch (error) { | ||
| throw new HttpException( | ||
| `Error in bulkCreate method of employee availability: ${error.message}`, | ||
| HttpStatus.INTERNAL_SERVER_ERROR | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation to bulkCreate method.
The bulkCreate method should validate the input entities before processing.
Apply this diff to add input validation:
public async bulkCreate(entities: Partial<EmployeeAvailability>[]): Promise<IEmployeeAvailability[]> {
+ if (!Array.isArray(entities) || entities.length === 0) {
+ throw new BadRequestException('Input must be a non-empty array of availability records.');
+ }
+
+ // Validate each entity
+ for (const entity of entities) {
+ const { startDate, endDate, employeeId, dayOfWeek } = entity;
+ if (!startDate || !endDate) {
+ throw new BadRequestException('Start date and end date are required for all records.');
+ }
+ if (new Date(endDate) <= new Date(startDate)) {
+ throw new BadRequestException('End date must be after start date for all records.');
+ }
+ if (!employeeId) {
+ throw new BadRequestException('Employee ID is required for all records.');
+ }
+ if (typeof dayOfWeek !== 'number' || dayOfWeek < 0 || dayOfWeek > 6) {
+ throw new BadRequestException('Day of week must be a number between 0 and 6 for all records.');
+ }
+ }
+
const tenantId = RequestContext.currentTenantId();
// ... rest of the method📝 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.
| public async bulkCreate(entities: Partial<EmployeeAvailability>[]): Promise<IEmployeeAvailability[]> { | |
| const tenantId = RequestContext.currentTenantId(); | |
| // Prepare entities ensuring `tenantId` is assigned | |
| const items = entities.map(entity => | |
| new EmployeeAvailability({ | |
| ...entity, | |
| tenantId | |
| }) | |
| ); | |
| try { | |
| switch (this.ormType) { | |
| case MultiORMEnum.MikroORM: | |
| try { | |
| // Convert input entities to MikroORM format | |
| const mikroEntities = items.map((item: EmployeeAvailability) => | |
| this.mikroOrmRepository.create(item, { | |
| managed: true | |
| }) | |
| ); | |
| // Bulk insert using MikroORM | |
| await this.mikroOrmRepository.persistAndFlush(mikroEntities); | |
| return mikroEntities; | |
| } catch (error) { | |
| throw new HttpException( | |
| `Error during MikroORM bulk create transaction : ${error.message}`, | |
| HttpStatus.INTERNAL_SERVER_ERROR | |
| ); | |
| } | |
| case MultiORMEnum.TypeORM: | |
| try { | |
| // Bulk insert using TypeORM's `save` method for optimized inserts | |
| return await this.typeOrmRepository.save(items as DeepPartial<EmployeeAvailability>[]); | |
| } catch (error) { | |
| throw new HttpException( | |
| `Error during TypeORM bulk create transaction : ${error.message}`, | |
| HttpStatus.INTERNAL_SERVER_ERROR | |
| ); | |
| } | |
| default: | |
| throw new Error(`Not implemented for ${this.ormType}`); | |
| } | |
| } catch (error) { | |
| throw new HttpException( | |
| `Error in bulkCreate method of employee availability: ${error.message}`, | |
| HttpStatus.INTERNAL_SERVER_ERROR | |
| ); | |
| } | |
| } | |
| public async bulkCreate(entities: Partial<EmployeeAvailability>[]): Promise<IEmployeeAvailability[]> { | |
| if (!Array.isArray(entities) || entities.length === 0) { | |
| throw new BadRequestException('Input must be a non-empty array of availability records.'); | |
| } | |
| // Validate each entity | |
| for (const entity of entities) { | |
| const { startDate, endDate, employeeId, dayOfWeek } = entity; | |
| if (!startDate || !endDate) { | |
| throw new BadRequestException('Start date and end date are required for all records.'); | |
| } | |
| if (new Date(endDate) <= new Date(startDate)) { | |
| throw new BadRequestException('End date must be after start date for all records.'); | |
| } | |
| if (!employeeId) { | |
| throw new BadRequestException('Employee ID is required for all records.'); | |
| } | |
| if (typeof dayOfWeek !== 'number' || dayOfWeek < 0 || dayOfWeek > 6) { | |
| throw new BadRequestException('Day of week must be a number between 0 and 6 for all records.'); | |
| } | |
| } | |
| const tenantId = RequestContext.currentTenantId(); | |
| // Prepare entities ensuring `tenantId` is assigned | |
| const items = entities.map(entity => | |
| new EmployeeAvailability({ | |
| ...entity, | |
| tenantId | |
| }) | |
| ); | |
| try { | |
| switch (this.ormType) { | |
| case MultiORMEnum.MikroORM: | |
| try { | |
| // Convert input entities to MikroORM format | |
| const mikroEntities = items.map((item: EmployeeAvailability) => | |
| this.mikroOrmRepository.create(item, { | |
| managed: true | |
| }) | |
| ); | |
| // Bulk insert using MikroORM | |
| await this.mikroOrmRepository.persistAndFlush(mikroEntities); | |
| return mikroEntities; | |
| } catch (error) { | |
| throw new HttpException( | |
| `Error during MikroORM bulk create transaction : ${error.message}`, | |
| HttpStatus.INTERNAL_SERVER_ERROR | |
| ); | |
| } | |
| case MultiORMEnum.TypeORM: | |
| try { | |
| // Bulk insert using TypeORM's `save` method for optimized inserts | |
| return await this.typeOrmRepository.save(items as DeepPartial<EmployeeAvailability>[]); | |
| } catch (error) { | |
| throw new HttpException( | |
| `Error during TypeORM bulk create transaction : ${error.message}`, | |
| HttpStatus.INTERNAL_SERVER_ERROR | |
| ); | |
| } | |
| default: | |
| throw new Error(`Not implemented for ${this.ormType}`); | |
| } | |
| } catch (error) { | |
| throw new HttpException( | |
| `Error in bulkCreate method of employee availability: ${error.message}`, | |
| HttpStatus.INTERNAL_SERVER_ERROR | |
| ); | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/core/src/lib/database/migrations/1737958313399-CreateEmployeeAvailabilityTable.ts (3)
14-14: Avoid console.log in migrations for consistent logging practices
Consider using the project's logger or removing this log statement to keep migrations clean and consistent with production logging standards.
60-61: Consider adding a check constraint or domain for dayOfWeek
If dayOfWeek is limited to values 0–6, you could consider adding a check constraint in each DB environment for stricter data validation.Also applies to: 108-147, 192-194
192-194: MySQL ID column type considerations
Currently, you’re usingvarchar(36)for the UUID-based primary key. You might consider switching tochar(36)for a more standardized approach in MySQL, or rely on native UUID functionality if supported.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/src/lib/database/migrations/1737958313399-CreateEmployeeAvailabilityTable.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (2)
packages/core/src/lib/database/migrations/1737958313399-CreateEmployeeAvailabilityTable.ts (2)
60-61: Ensure Postgres supports gen_random_uuid()
The functiongen_random_uuid()requires thepgcryptoextension in Postgres. Please confirm that the extension is enabled in your production environment, or switch to an alternative UUID generation method that doesn’t rely on this extension.
60-84: Validate foreign key references to the "employee" table
Make sure theemployeetable exists before applying this migration. If the table is missing or named differently, this foreign key constraint will fail.
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
#5293
Summary by CodeRabbit
Release Notes: Employee Availability Feature
New Features
Improvements
Permissions
Technical Enhancements