[Refactor] Rename "Subscription" to "EntitySubscription" #8809
Conversation
|
Warning Rate limit exceeded@rahul-rocket has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 32 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request refactors the subscription-related functionality by replacing the legacy "Subscription" model with the new "EntitySubscription" model. The changes include renaming exports, modules, controllers, services, commands, events, handlers, DTOs, and repositories. Legacy subscription files have been removed and replaced with new implementations that adopt updated naming conventions and adjusted parameters (e.g., replacing Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as EntitySubscriptionController
participant Service as EntitySubscriptionService
participant CommandHandler as EntitySubscriptionCreateHandler
participant Repository as EntitySubscriptionRepository
Client->>Controller: POST /entity-subscription (Create DTO)
Controller->>Service: createEntitySubscription(dto)
Service->>CommandHandler: execute(EntitySubscriptionCreateCommand)
CommandHandler->>Service: invoke create() with employeeId, organizationId
Service->>Repository: Save new EntitySubscription
Repository-->>Service: Return saved entity
Service-->>Controller: Return success response
Controller-->>Client: 201 Created (EntitySubscription details)
Possibly related PRs
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (8)
packages/core/src/lib/entity-subscription/commands/handlers/entity-subscription.create.handler.ts (1)
1-15: Consider renaming variable to match entity name.The handler implementation is correct, but for consistency with the refactoring effort, consider renaming the injected service variable from
subscriptionServicetoentitySubscriptionServiceto fully reflect the new naming convention.- constructor(private readonly subscriptionService: EntitySubscriptionService) {} + constructor(private readonly entitySubscriptionService: EntitySubscriptionService) {} - return await this.subscriptionService.create(input); + return await this.entitySubscriptionService.create(input);packages/core/src/lib/organization-team/organization-team.service.ts (1)
287-287: Update error message to reflect new entity nameThe error message still references the old "CreateSubscriptionEvent" name which should be updated for consistency.
-console.error('Error publishing CreateSubscriptionEvent:', error); +console.error('Error publishing CreateEntitySubscriptionEvent:', error);packages/core/src/lib/tasks/task.service.ts (1)
210-210: Update error message to reflect new entity nameThe error message still references the old "CreateSubscriptionEvent" name which should be updated for consistency.
-console.error('Error publishing CreateSubscriptionEvent:', error); +console.error('Error publishing CreateEntitySubscriptionEvent:', error);packages/core/src/lib/organization-project/organization-project.service.ts (1)
131-140: Consider more robust error handling for subscription creation.
This logic correctly differentiates betweenCREATED_ENTITYandASSIGNMENT, but thetry/catchblock only logs errors without rethrowing or handling them gracefully.packages/core/src/lib/organization-team-employee/organization-team-employee.service.ts (2)
309-309: Wrap task unassignment in a try/catch for reliability.
If_taskService.unassignEmployeeFromTeamTasksfails, it may leave inconsistent data.
339-339: Same suggestion for unassigning tasks here.
Consider adding a try/catch or logging block to ensure errors are surfaced properly.packages/core/src/lib/organization-sprint/organization-sprint.service.ts (1)
125-132: Subscription event dispatch logic is sound but consider more explicit error handling.
The code checks manager vs. member subscriptions, but the catch block is empty. You might wish to log or rethrow.packages/core/src/lib/entity-subscription/entity-subscription.controller.ts (1)
18-23: Constructor injection of EntitySubscriptionService and CommandBus is coherent.
Maintains consistent CQRS approach for creating subscriptions via commands.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
packages/contracts/src/index.ts(1 hunks)packages/contracts/src/lib/entity-subscription.model.ts(1 hunks)packages/contracts/src/lib/subscription.model.ts(0 hunks)packages/core/src/lib/app/app.module.ts(2 hunks)packages/core/src/lib/comment/comment.service.ts(2 hunks)packages/core/src/lib/core/entities/index.ts(2 hunks)packages/core/src/lib/core/entities/internal.ts(1 hunks)packages/core/src/lib/core/seeds/seeder.module.ts(2 hunks)packages/core/src/lib/entity-subscription/commands/entity-subscription.create.command.ts(1 hunks)packages/core/src/lib/entity-subscription/commands/handlers/entity-subscription.create.handler.ts(1 hunks)packages/core/src/lib/entity-subscription/commands/handlers/index.ts(1 hunks)packages/core/src/lib/entity-subscription/commands/index.ts(1 hunks)packages/core/src/lib/entity-subscription/dto/create-entity-subscription.dto.ts(1 hunks)packages/core/src/lib/entity-subscription/dto/entity-subscription-find-input.dto.ts(1 hunks)packages/core/src/lib/entity-subscription/dto/index.ts(1 hunks)packages/core/src/lib/entity-subscription/entity-subscription.controller.ts(5 hunks)packages/core/src/lib/entity-subscription/entity-subscription.entity.ts(1 hunks)packages/core/src/lib/entity-subscription/entity-subscription.module.ts(1 hunks)packages/core/src/lib/entity-subscription/entity-subscription.service.ts(5 hunks)packages/core/src/lib/entity-subscription/events/entity-subscription.create.event.ts(1 hunks)packages/core/src/lib/entity-subscription/events/handlers/entity-subscription.create.handler.ts(2 hunks)packages/core/src/lib/entity-subscription/events/handlers/index.ts(1 hunks)packages/core/src/lib/entity-subscription/events/index.ts(1 hunks)packages/core/src/lib/entity-subscription/repository/mikro-orm-entity-subscription.repository.ts(1 hunks)packages/core/src/lib/entity-subscription/repository/type-orm-entity-subscription.repository.ts(1 hunks)packages/core/src/lib/mention/mention.module.ts(2 hunks)packages/core/src/lib/mention/mention.service.ts(2 hunks)packages/core/src/lib/organization-project/organization-project.service.ts(6 hunks)packages/core/src/lib/organization-sprint/organization-sprint.service.ts(7 hunks)packages/core/src/lib/organization-team-employee/organization-team-employee.service.ts(6 hunks)packages/core/src/lib/organization-team/organization-team.service.ts(3 hunks)packages/core/src/lib/subscription/commands/handlers/index.ts(0 hunks)packages/core/src/lib/subscription/commands/handlers/subscription.create.handler.ts(0 hunks)packages/core/src/lib/subscription/commands/index.ts(0 hunks)packages/core/src/lib/subscription/dto/create-subscription.dto.ts(0 hunks)packages/core/src/lib/subscription/dto/index.ts(0 hunks)packages/core/src/lib/subscription/dto/subscription-find-input.dto.ts(0 hunks)packages/core/src/lib/subscription/events/handlers/index.ts(0 hunks)packages/core/src/lib/subscription/events/index.ts(0 hunks)packages/core/src/lib/subscription/repository/mikro-orm-subscription.repository.ts(0 hunks)packages/core/src/lib/subscription/repository/type-orm-subscription.repository.ts(0 hunks)packages/core/src/lib/subscription/subscription.entity.ts(0 hunks)packages/core/src/lib/subscription/subscription.module.ts(0 hunks)packages/core/src/lib/tasks/commands/handlers/automation-task.sync.handler.ts(7 hunks)packages/core/src/lib/tasks/commands/handlers/task-create.handler.ts(3 hunks)packages/core/src/lib/tasks/screening-tasks/screening-tasks.service.ts(3 hunks)packages/core/src/lib/tasks/task.service.ts(5 hunks)
💤 Files with no reviewable changes (13)
- packages/core/src/lib/subscription/events/index.ts
- packages/core/src/lib/subscription/commands/handlers/index.ts
- packages/core/src/lib/subscription/events/handlers/index.ts
- packages/core/src/lib/subscription/repository/mikro-orm-subscription.repository.ts
- packages/core/src/lib/subscription/dto/subscription-find-input.dto.ts
- packages/core/src/lib/subscription/subscription.entity.ts
- packages/core/src/lib/subscription/dto/create-subscription.dto.ts
- packages/core/src/lib/subscription/dto/index.ts
- packages/contracts/src/lib/subscription.model.ts
- packages/core/src/lib/subscription/subscription.module.ts
- packages/core/src/lib/subscription/commands/index.ts
- packages/core/src/lib/subscription/commands/handlers/subscription.create.handler.ts
- packages/core/src/lib/subscription/repository/type-orm-subscription.repository.ts
✅ Files skipped from review due to trivial changes (8)
- packages/core/src/lib/entity-subscription/events/index.ts
- packages/core/src/lib/entity-subscription/commands/handlers/index.ts
- packages/core/src/lib/entity-subscription/events/entity-subscription.create.event.ts
- packages/core/src/lib/entity-subscription/commands/entity-subscription.create.command.ts
- packages/core/src/lib/entity-subscription/dto/index.ts
- packages/core/src/lib/core/entities/index.ts
- packages/core/src/lib/entity-subscription/commands/index.ts
- packages/core/src/lib/entity-subscription/events/handlers/index.ts
🧰 Additional context used
🧠 Learnings (1)
packages/core/src/lib/tasks/screening-tasks/screening-tasks.service.ts (1)
Learnt from: rahul-rocket
PR: ever-co/ever-gauzy#8800
File: packages/core/src/lib/tasks/screening-tasks/screening-tasks.service.ts:81-81
Timestamp: 2025-02-24T05:25:58.665Z
Learning: The `creatorId` column in the `screening-task` entity will be renamed to `createdByUserId` in a future PR to maintain consistency with the task entity.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (72)
packages/core/src/lib/entity-subscription/repository/mikro-orm-entity-subscription.repository.ts (1)
1-4: Repository implementation looks good!The
MikroOrmEntitySubscriptionRepositoryclass correctly extends the base repository with theEntitySubscriptiontype. This follows the repository pattern used elsewhere in the codebase.packages/core/src/lib/mention/mention.module.ts (1)
7-7: Dependency updated properlyThe module dependency has been correctly updated from
SubscriptionModuletoEntitySubscriptionModuleas part of the renaming refactor.Also applies to: 21-21
packages/core/src/lib/core/entities/internal.ts (1)
134-134: Export correctly updatedThe export for the entity subscription has been properly updated to use the new path and entity name.
packages/core/src/lib/entity-subscription/dto/entity-subscription-find-input.dto.ts (1)
1-7: DTO implementation follows best practicesThe DTO correctly:
- Extends
PartialType(EntitySubscription)to make all entity properties optional- Implements the contract interface
IEntitySubscriptionFindInput- Follows NestJS conventions for DTOs
This approach ensures type safety while maintaining compatibility with the entity structure.
packages/core/src/lib/entity-subscription/dto/create-entity-subscription.dto.ts (1)
1-22: Class structure looks good with proper implementation of the interface.The DTO extends TenantOrganizationBaseDTO and implements IEntitySubscriptionCreateInput correctly. All required properties are defined with appropriate types, and optional properties are properly marked with
?.packages/core/src/lib/entity-subscription/repository/type-orm-entity-subscription.repository.ts (1)
1-11: Repository implementation follows TypeORM best practices.The class correctly extends the TypeORM Repository and uses proper dependency injection. The constructor appropriately initializes the parent class with the repository target, manager, and queryRunner.
packages/core/src/lib/entity-subscription/entity-subscription.module.ts (1)
1-25: Module configuration is correct and follows NestJS best practices.The module is properly marked as @global() and includes all necessary imports and providers. The module exports the service and repository for use in other modules, which maintains compatibility with the previous subscription module structure.
packages/core/src/lib/app/app.module.ts (2)
160-160: Import statement correctly references the new EntitySubscriptionModule.The import has been updated from SubscriptionModule to EntitySubscriptionModule, which aligns with the refactoring efforts.
472-472: Module registration correctly uses EntitySubscriptionModule.The module reference has been properly updated in the imports array from SubscriptionModule to EntitySubscriptionModule.
packages/core/src/lib/tasks/screening-tasks/screening-tasks.service.ts (3)
6-7: All imports updated correctly for the refactoringThe import of
EntitySubscriptionTypeEnumreplaces the formerSubscriptionTypeEnumas part of the subscription model refactoring.
21-21: Import path updated properlyImport statement updated from
CreateSubscriptionEventtoCreateEntitySubscriptionEvent, correctly reflecting the renamed entity.
110-114: Subscription event creation updated with improved semanticsThe event creation has been updated to use:
CreateEntitySubscriptionEventinstead ofCreateSubscriptionEventemployeeId: user?.employeeIdinstead ofuserId: user?.idEntitySubscriptionTypeEnum.CREATED_ENTITYinstead ofSubscriptionTypeEnum.CREATED_ENTITYThese changes are consistent with the refactoring and provide better semantic clarity by using employee-specific identifiers.
packages/contracts/src/lib/entity-subscription.model.ts (1)
1-19: Well-structured new EntitySubscription modelThe newly introduced model properly replaces the old Subscription model with:
- Clear interface definitions that extend appropriate base interfaces
- A well-defined enum with all the required subscription types
- Consistent naming conventions that match the refactoring goals
The model maintains the same functionality while providing more specific and contextual naming.
packages/core/src/lib/core/seeds/seeder.module.ts (2)
10-10: Module import updated correctlyThe import statement has been updated from
SubscriptionModuletoEntitySubscriptionModule, correctly reflecting the renamed module.
45-45: Module usage updated in imports arrayThe module reference in the imports array has been updated from
SubscriptionModuletoEntitySubscriptionModule, ensuring the seeder uses the renamed module.packages/core/src/lib/comment/comment.service.ts (3)
5-7: Import updated correctly for the refactoringThe import of
EntitySubscriptionTypeEnumreplaces the formerSubscriptionTypeEnumas part of the subscription model refactoring.
14-14: Import path updated properlyImport statement updated from
CreateSubscriptionEventtoCreateEntitySubscriptionEvent, correctly reflecting the renamed entity.
86-93: Subscription event creation updated with improved semanticsThe event creation has been updated to use:
CreateEntitySubscriptionEventinstead ofCreateSubscriptionEvent- Explicit
employeeIdparameter (which was likelyuserIdbefore)EntitySubscriptionTypeEnum.COMMENTinstead ofSubscriptionTypeEnum.COMMENTThese changes provide better semantic clarity by using employee-specific identifiers and follow the refactoring pattern consistently.
packages/contracts/src/index.ts (1)
128-128: Correctly added export for the new entity-subscription model.The addition of the entity-subscription model export aligns with the renaming refactor from "Subscription" to "EntitySubscription" while maintaining the alphabetical ordering of exports.
packages/core/src/lib/entity-subscription/entity-subscription.entity.ts (1)
1-58: Well-structured entity implementation.This entity implementation is thorough with proper:
- ORM decorations for both TypeORM and MikroORM
- Validation using class-validator
- Clear comments explaining each property
- Correct relationship mapping with the Employee entity
- Appropriate indexing for performance optimization
The refactoring from Subscription to EntitySubscription is well-executed in this file.
packages/core/src/lib/mention/mention.service.ts (1)
10-10: Correctly updated imports and event references.All Subscription references have been properly updated to EntitySubscription in both imports and usage. The event publishing code now correctly uses the new EntitySubscriptionTypeEnum and CreateEntitySubscriptionEvent.
Also applies to: 16-16, 71-77
packages/core/src/lib/tasks/commands/handlers/task-create.handler.ts (4)
11-12: Properly updated import for EntitySubscriptionTypeEnumThe imports have been correctly updated to use the new EntitySubscriptionTypeEnum which replaces the previous SubscriptionTypeEnum.
20-20: Correctly updated import path for CreateEntitySubscriptionEventThe import path has been properly changed from the old subscription module to the new entity-subscription module.
138-145: Successfully migrated from CreateSubscriptionEvent to CreateEntitySubscriptionEventThe event creation has been correctly updated to use the new EntitySubscriptionTypeEnum.CREATED_ENTITY for the subscription type.
164-171: Properly refactored subscription event for assigneesThe code now correctly uses:
- CreateEntitySubscriptionEvent instead of CreateSubscriptionEvent
- employeeId parameter (which was already in use)
- EntitySubscriptionTypeEnum.ASSIGNMENT enum value
This maintains the same functionality while using the new entity-subscription pattern.
packages/core/src/lib/organization-team/organization-team.service.ts (3)
24-25: Correctly added IEmployee and EntitySubscriptionTypeEnum importsThe imports have been properly added for the interfaces and enums needed in the refactored code.
42-42: Updated import path for CreateEntitySubscriptionEventThe import has been correctly changed from the subscription module to the entity-subscription module.
270-284: Properly refactored subscription event creation for team membersThe changes correctly:
- Use IEmployee interface instead of destructuring
- Use employee.id for assigning the employeeId parameter
- Use appropriate EntitySubscriptionTypeEnum values based on whether the employee is the creator
packages/core/src/lib/entity-subscription/events/handlers/entity-subscription.create.handler.ts (4)
4-5: Updated import paths for entity subscription events and commandsThe imports have been correctly updated to reflect the new module structure and naming convention.
7-8: Properly updated EventsHandler decorator and class implementationThe class definition has been correctly updated to handle CreateEntitySubscriptionEvent.
17-21: Updated method signature and documentationThe method signature and JSDoc comments have been properly updated to reflect the new event type.
24-36: Successfully migrated from userId to employeeIdThe code now correctly:
- Extracts employeeId instead of userId from the event input
- Uses EntitySubscriptionCreateCommand instead of SubscriptionCreateCommand
- Passes employeeId instead of userId to the command
This change properly aligns with the entity-specific subscription model.
packages/core/src/lib/tasks/task.service.ts (6)
29-29: Successfully imported EntitySubscriptionTypeEnumThe import has been correctly updated to use the new EntitySubscriptionTypeEnum.
41-41: Updated import for EntitySubscriptionServiceThe import has been properly changed to use the new EntitySubscriptionService.
45-45: Updated import path for CreateEntitySubscriptionEventThe import path has been correctly changed to the new entity-subscription module.
61-61: Renamed service property to _entitySubscriptionServiceThe dependency injection property has been correctly renamed from _subscriptionService to _entitySubscriptionService.
164-169: Updated unsubscribe logic to use EntitySubscriptionServiceThe code now correctly:
- Uses _entitySubscriptionService.delete method
- Uses employeeId parameter instead of userId
- Uses EntitySubscriptionTypeEnum.ASSIGNMENT
183-191: Updated subscription event creationThe code now correctly uses CreateEntitySubscriptionEvent with the appropriate parameters and EntitySubscriptionTypeEnum.
packages/core/src/lib/organization-project/organization-project.service.ts (3)
18-18: No issues with introducingEntitySubscriptionTypeEnum.
This enum replacement is consistent with the broader subscription refactor.
29-30: New imports for entity subscription event handling look good.
The references toCreateEntitySubscriptionEventandEntitySubscriptionServiceare correctly aligned with the new subscription design.
50-50: Service injection update is consistent with the refactor.
Replacing_subscriptionServicewith_entitySubscriptionServicemaintains clarity for entity-based subscriptions. No concerns here.packages/core/src/lib/organization-team-employee/organization-team-employee.service.ts (4)
6-6:EntitySubscriptionTypeEnumimport is aligned with the subscription refactor.
No issues spotted.
13-13: No concerns regardingRolesEnumusage.
This enum usage is consistent with the rest of the codebase.
20-21: New event and service imports properly replace the old subscription references.
This ensures consistency with the renamed subscription logic.
31-32: Injected_taskServiceand_entitySubscriptionServiceare valid additions.
They unify task unassignment logic and manage entity subscriptions, respectively.packages/core/src/lib/organization-sprint/organization-sprint.service.ts (4)
13-13:EntitySubscriptionTypeEnumis properly introduced.
This aligns well with the new subscription model.
21-22: Replacing the old subscription imports withCreateEntitySubscriptionEventandEntitySubscriptionServiceis correct.
The new approach matches your entity-based subscription refactor.
142-142: Internal activity log injection_activityLogServicelooks fine.
No issues identified.
207-207: Another activity log usage consistent with your approach.
No further concerns here.packages/core/src/lib/entity-subscription/entity-subscription.service.ts (6)
2-2: ImportingDeleteResultis a beneficial step for typed delete operations.
This enhances clarity when performing subscription deletions.
12-13: Repositories and entity imports align with the transition to EntitySubscription.
The new repos are logically consistent with your new naming and structure.Also applies to: 14-14
16-16: Refactor toEntitySubscriptionServicehighlights the new domain-driven approach.
Clear naming better reflects the service’s purpose.
18-20: Constructor injection of repositories and call tosuperis correct.
This ensures the baseTenantAwareCrudServicefunctionalities remain intact.Also applies to: 21-21
31-64: Creating subscriptions withemployeeIdandorganizationIdis a solid approach.
The fallback to context-based tenant and employee data looks consistent, but watch out for calls that omitorganizationId.
81-105:unsubscribemethod logic is cleanly adapted to the new approach.
Again, be mindful that upstream calls should passorganizationIdto match the filter if the subscription was created that way.packages/core/src/lib/tasks/commands/handlers/automation-task.sync.handler.ts (10)
14-15: Renaming to EntitySubscriptionTypeEnum & IEmployee import looks appropriate.
No functional issues spotted here; aligns with the broader rename to entity-based subscriptions.
19-20: Imports from entity-subscription events and service are consistent.
Please verify that any references to the old subscription event/service are fully replaced throughout the codebase.
24-24: TaskService import is valid.
No concerns here; usage seems standard.
33-35: Dependency injection for repositories appears correct.
Matches the custom repositories' declarations and usage.
40-40: Updated subscription service reference is clear.
Dependency name reflects the new entity-based subscription management.
54-54: Nullish coalescing operator is a good choice.
This simplifies the tenant ID fallback logic nicely.
153-157: Publishing the CreateEntitySubscriptionEvent for CREATED_ENTITY is effective.
This aligns with the updated subscription pattern. Double-check if multiple creation events are not triggered unintentionally in edge cases.
173-179: Assigning tasks to employees via subscription events is consistent.
Ensures proper subscription type (ASSIGNMENT) is set. Great approach to keep each member in sync.
264-269: Deleting entity subscriptions for removed task members is well-structured.
Implementation correctly unsubscribes those who are no longer assigned.
281-287: New subscriptions for newly assigned team members look correct.
Consider verifying in tests that duplicates aren’t created for employees who were already on the task.packages/core/src/lib/entity-subscription/entity-subscription.controller.ts (7)
9-12: New imports for EntitySubscription and related DTOs are aligned with rename.
No issues observed with the updated references.
14-17: API tags and controller path renamed to 'EntitySubscriptions' and '/entity-subscription'.
Ensure that any client-side references to the old route or tags are updated.
31-31: Response type updated to EntitySubscription in metadata.
Properly reflects the new entity type.
39-41: findAll with entitySubscriptionService is correct.
The usage of pagination params is aligned with the Crud approach.
56-58: findById references the new method signature.
No obvious issues; ensures typed validations remain in place.
73-75: create method uses EntitySubscriptionCreateCommand.
Promotes consistent usage of the new entity-based model.
91-93: delete method now unsubscribes from entity subscriptions.
Functional flow is straightforward; removing subscription data is logically handled by the service.
packages/core/src/lib/organization-project/organization-project.service.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/organization-project/organization-project.service.ts
Show resolved
Hide resolved
packages/core/src/lib/organization-team-employee/organization-team-employee.service.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/organization-sprint/organization-sprint.service.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/organization-sprint/organization-sprint.service.ts
Show resolved
Hide resolved
|
View your CI Pipeline Execution ↗ for commit cbd1a57.
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/core/src/lib/entity-subscription/dto/create-entity-subscription.dto.ts (1)
1-14: Looks great – consider adding validation decorators.Everything looks correct, and the usage of
IntersectionTypewithPickTypeis a succinct approach to constructing your DTO. If any fields (e.g.,entityId) require strict formats such as UUID, consider applying class-validator decorators (likeIsUUID(),IsString(), etc.) to ensure robust input validation.packages/core/src/lib/entity-subscription/dto/entity-subscription-find-input.dto.ts (1)
1-11: All set – consider stricter validations if needed.The usage of
IntersectionTypeandPartialTypeis correct for building flexible query criteria. If you anticipate needing more robust input validation (e.g., verifying UUID formats in partial fields), consider adding class-validator decorators for the optional properties.packages/core/src/lib/entity-subscription/entity-subscription.service.ts (2)
60-61: Use a logging system instead of console.log for error tracking.Relying on
console.logmakes it harder to centralize or filter logs. Consider using NestJS's built-in logger or a dedicated logging library for better traceability and log management.
96-98: Fix minor grammatical error in throw message.Use “Failed to unsubscribe employee from entity” or similar.
Apply this diff to correct the grammar:
- throw new BadRequestException('Failed to unsubscribing employee from entity', error); + throw new BadRequestException('Failed to unsubscribe employee from entity', error);packages/core/src/lib/entity-subscription/entity-subscription.controller.ts (1)
151-159: Address the spelling to satisfy cspell checks.The pipeline is flagging “unsubscription” as an unknown word. You can either add it to your project's cspell dictionary or replace it with an alternative (e.g., “subscription cancellation” or “unsubscribe operation”) to pass the spelling checks.
Consider the following diff to rewrite relevant descriptions:
- description: 'Optional filters to locate the subscription for unsubscription.' + description: 'Optional filters to locate the subscription for cancellation.'- description: 'Unsubscription request accepted.' + description: 'Unsubscribe request accepted.'🧰 Tools
🪛 GitHub Check: Cspell
[warning] 152-152:
Unknown word (unsubscription)
[warning] 157-157:
Unknown word (Unsubscription)🪛 GitHub Actions: Check Spelling and Typos with cspell
[warning] 152-152: Unknown word (unsubscription)
[warning] 157-157: Unknown word (Unsubscription)
packages/core/src/lib/database/migrations/1740723269734-AlterSubscriptionEntityTable.ts (1)
28-28: Use the standard “new Error” constructor.Throwing errors without the
newkeyword can be confusing and inconsistent with typical JS best practices.- throw Error(`Unsupported database: ${queryRunner.connection.options.type}`); + throw new Error(`Unsupported database: ${queryRunner.connection.options.type}`);Also applies to: 50-50
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/core/src/lib/database/migrations/1740723269734-AlterSubscriptionEntityTable.ts(1 hunks)packages/core/src/lib/entity-subscription/dto/create-entity-subscription.dto.ts(1 hunks)packages/core/src/lib/entity-subscription/dto/entity-subscription-find-input.dto.ts(1 hunks)packages/core/src/lib/entity-subscription/entity-subscription.controller.ts(1 hunks)packages/core/src/lib/entity-subscription/entity-subscription.entity.ts(1 hunks)packages/core/src/lib/entity-subscription/entity-subscription.service.ts(1 hunks)packages/core/src/lib/entity-subscription/events/handlers/entity-subscription.create.handler.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/lib/entity-subscription/events/handlers/entity-subscription.create.handler.ts
- packages/core/src/lib/entity-subscription/entity-subscription.entity.ts
🧰 Additional context used
🪛 GitHub Check: Cspell
packages/core/src/lib/entity-subscription/entity-subscription.controller.ts
[warning] 152-152:
Unknown word (unsubscription)
[warning] 157-157:
Unknown word (Unsubscription)
🪛 GitHub Actions: Check Spelling and Typos with cspell
packages/core/src/lib/entity-subscription/entity-subscription.controller.ts
[warning] 152-152: Unknown word (unsubscription)
[warning] 157-157: Unknown word (Unsubscription)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
🔇 Additional comments (2)
packages/core/src/lib/entity-subscription/entity-subscription.controller.ts (1)
1-150: Code looks solid overall.Your controller methods align well with standard NestJS CRUD patterns, with appropriate guards, input validation, and Swagger annotations. Great job on the thorough inline documentation and typed query/DTO usages.
Also applies to: 160-172
packages/core/src/lib/database/migrations/1740723269734-AlterSubscriptionEntityTable.ts (1)
1-27: No major issues identified.Your migration properly handles PostgreSQL, MySQL, and SQLite paths. The usage of raw queries, index drops, constraint modifications, and table renaming appears consistent with TypeORM migrations. The console log provides helpful status feedback.
Also applies to: 29-49, 51-336
packages/core/src/lib/entity-subscription/entity-subscription.service.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/core/src/lib/organization-team-employee/organization-team-employee.service.ts (1)
92-93: Fix inaccurate error log message.The error message mentions "subscribing" but this code block is handling unsubscription operations.
- console.error('Error subscribing new team members:', error); + console.error('Error unsubscribing team members:', error);packages/core/src/lib/tasks/commands/handlers/automation-task.sync.handler.ts (1)
275-276: Fix inaccurate error log message.The error message mentions "subscribing" but this code block is handling unsubscription operations.
- console.error('Error subscribing new members to the task:', error); + console.error('Error unsubscribing members from the task:', error);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/core/src/lib/organization-project/organization-project.service.ts(6 hunks)packages/core/src/lib/organization-sprint/organization-sprint.service.ts(7 hunks)packages/core/src/lib/organization-team-employee/organization-team-employee.service.ts(6 hunks)packages/core/src/lib/tasks/commands/handlers/automation-task.sync.handler.ts(7 hunks)packages/core/src/lib/tasks/task.service.ts(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/src/lib/organization-sprint/organization-sprint.service.ts
- packages/core/src/lib/organization-project/organization-project.service.ts
- packages/core/src/lib/tasks/task.service.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
🔇 Additional comments (2)
packages/core/src/lib/organization-team-employee/organization-team-employee.service.ts (1)
81-88: Changes successfully address previous review concern.The code now correctly passes both
organizationIdandtenantIdparameters when deleting entity subscriptions, which addresses the previous review comment about ensuringorganizationIdis provided.packages/core/src/lib/tasks/commands/handlers/automation-task.sync.handler.ts (1)
54-54: Verify nullish coalescing operator behavior change.The change from
||to??operator alters how falsy values are handled. The??operator will only useinput.tenantIdwhenRequestContext.currentTenantId()isnullorundefined, not for other falsy values like0or empty string.Make sure this behavior change is intentional and doesn't break existing functionality where falsy (but not null/undefined) tenant IDs might be valid.
- Added migrations to rename the "subscription" table to "entity_subscription" across Postgres, SQLite, and MySQL.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/src/lib/database/migrations/1740727466186-RenameOldSubscriptionToEntitySubscriptionEntityTable.ts (1)
1-26: Migration looks good, with minor suggestions for improvement.The migration correctly implements the renaming of 'subscription' table to 'entity_subscription' with appropriate up and down methods. This aligns well with the overall PR objective of refactoring Subscription to EntitySubscription across the codebase.
A couple of suggestions:
- Consider adding a completion log message after the rename operation completes
- You might want to add more detailed documentation about what this migration is doing and why
public async up(queryRunner: QueryRunner): Promise<void> { console.log(chalk.yellow(this.name + ' start running!')); await queryRunner.renameTable('subscription', 'entity_subscription'); + console.log(chalk.green(this.name + ' completed successfully!')); } /** * Down Migration * * @param queryRunner */ public async down(queryRunner: QueryRunner): Promise<void> { await queryRunner.renameTable('entity_subscription', 'subscription'); + console.log(chalk.green('Reverted ' + this.name + ' successfully!')); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/lib/database/migrations/1740727466186-RenameOldSubscriptionToEntitySubscriptionEntityTable.ts(1 hunks)packages/core/src/lib/entity-subscription/entity-subscription.entity.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/lib/entity-subscription/entity-subscription.entity.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test
- GitHub Check: build
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores