refactor(api): centrelize lifecycle-hook-manager types #1108
Conversation
yassinedorbozgithub
added
the
🪄 Investigate
WalkthroughThe codebase refactors all lifecycle hook method signatures in repository classes to use unified argument and return types imported from a new Changes
Sequence Diagram(s)sequenceDiagram
participant Repository
participant LifecycleTypes
Repository->>LifecycleTypes: Import Args, preCreate, preUpdate, etc.
Repository->>Repository: Define hook (e.g., preCreate(...[doc]: Args<preCreate<T>>))
Note right of Repository: Use tuple destructuring for arguments
Repository->>Repository: Execute hook logic as before
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
yassinedorbozgithub
force-pushed
the
1107-issue---refactor-lifecycle-hook-manager-types
branch
from
d8200ff to
3e22921
Compare
marrouchi
changed the base branch from
1105-issue---botstats-is-missed-echo-type
to
main
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
api/src/utils/types/lifecycle-hook-manager.types.ts (1)
61-64: Consider stronger typing for the updated parameter.The
_updated: anyparameter inpostUpdateManycould benefit from more specific typing for better type safety.export type postUpdateMany<T> = ( _query: Query<Document<T>, Document<T>, unknown, T, 'updateMany'>, - _updated: any, + _updated: { acknowledged: boolean; modifiedCount: number; upsertedId?: any; upsertedCount: number; matchedCount: number }, ) => Promise<void>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
api/src/chat/repositories/block.repository.ts(7 hunks)api/src/chat/repositories/category.repository.ts(2 hunks)api/src/chat/repositories/context-var.repository.ts(2 hunks)api/src/chat/repositories/label.repository.ts(3 hunks)api/src/chat/repositories/subscriber.repository.ts(3 hunks)api/src/cms/repositories/content-type.repository.ts(2 hunks)api/src/cms/repositories/content.repository.ts(3 hunks)api/src/nlp/repositories/nlp-entity.repository.ts(4 hunks)api/src/nlp/repositories/nlp-sample.repository.ts(2 hunks)api/src/nlp/repositories/nlp-value.repository.ts(4 hunks)api/src/setting/repositories/setting.repository.ts(3 hunks)api/src/user/repositories/user.repository.ts(3 hunks)api/src/utils/generics/base-repository.ts(3 hunks)api/src/utils/types/lifecycle-hook-manager.types.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
api/src/nlp/repositories/nlp-sample.repository.ts (5)
api/src/utils/types/lifecycle-hook-manager.types.ts (2)
Args(21-21)preDelete(71-80)api/src/chat/repositories/label.repository.ts (1)
preDelete(74-83)api/src/nlp/repositories/nlp-entity.repository.ts (1)
preDelete(85-101)api/src/nlp/repositories/nlp-value.repository.ts (1)
preDelete(84-101)api/src/utils/generics/base-repository.ts (1)
preDelete(631-633)
api/src/chat/repositories/category.repository.ts (9)
api/src/utils/types/lifecycle-hook-manager.types.ts (2)
Args(21-21)preDelete(71-80)api/src/chat/repositories/label.repository.ts (1)
preDelete(74-83)api/src/chat/repositories/context-var.repository.ts (1)
preDelete(50-72)api/src/chat/repositories/block.repository.ts (1)
preDelete(224-247)api/src/cms/repositories/content-type.repository.ts (1)
preDelete(45-60)api/src/nlp/repositories/nlp-sample.repository.ts (1)
preDelete(46-56)api/src/nlp/repositories/nlp-entity.repository.ts (1)
preDelete(85-101)api/src/nlp/repositories/nlp-value.repository.ts (1)
preDelete(84-101)api/src/utils/generics/base-repository.ts (1)
preDelete(631-633)
api/src/cms/repositories/content-type.repository.ts (5)
api/src/utils/types/lifecycle-hook-manager.types.ts (2)
Args(21-21)preDelete(71-80)api/src/chat/repositories/category.repository.ts (1)
preDelete(44-63)api/src/chat/repositories/context-var.repository.ts (1)
preDelete(50-72)api/src/nlp/repositories/nlp-sample.repository.ts (1)
preDelete(46-56)api/src/utils/generics/base-repository.ts (1)
preDelete(631-633)
api/src/chat/repositories/label.repository.ts (10)
api/src/utils/types/lifecycle-hook-manager.types.ts (3)
Args(21-21)postCreate(47-47)preDelete(71-80)api/src/chat/repositories/subscriber.repository.ts (1)
postCreate(45-52)api/src/nlp/repositories/nlp-entity.repository.ts (2)
postCreate(54-59)preDelete(85-101)api/src/nlp/repositories/nlp-value.repository.ts (2)
postCreate(57-62)preDelete(84-101)api/src/utils/generics/base-repository.ts (2)
postCreate(611-613)preDelete(631-633)api/src/chat/repositories/category.repository.ts (1)
preDelete(44-63)api/src/chat/repositories/context-var.repository.ts (1)
preDelete(50-72)api/src/chat/repositories/block.repository.ts (1)
preDelete(224-247)api/src/cms/repositories/content-type.repository.ts (1)
preDelete(45-60)api/src/nlp/repositories/nlp-sample.repository.ts (1)
preDelete(46-56)
api/src/setting/repositories/setting.repository.ts (4)
api/src/utils/types/lifecycle-hook-manager.types.ts (4)
Args(21-21)preCreateValidate(25-29)preUpdateValidate(35-38)postUpdate(66-69)api/src/utils/generics/base-repository.ts (3)
preCreateValidate(583-587)preUpdateValidate(595-599)postUpdate(627-629)api/src/nlp/repositories/nlp-entity.repository.ts (1)
postUpdate(69-74)api/src/nlp/repositories/nlp-value.repository.ts (1)
postUpdate(70-75)
api/src/utils/types/lifecycle-hook-manager.types.ts (2)
api/src/utils/generics/base-repository.ts (13)
preCreateValidate(583-587)postCreateValidate(589-593)preUpdateValidate(595-599)postUpdateValidate(601-605)preCreate(607-609)postCreate(611-613)preUpdate(615-617)preUpdateMany(619-621)postUpdateMany(623-625)postUpdate(627-629)preDelete(631-633)DeleteResult(58-61)postDelete(635-637)api/src/utils/types/filter.types.ts (1)
TFilterQuery(134-142)
api/src/cms/repositories/content.repository.ts (5)
api/src/utils/types/lifecycle-hook-manager.types.ts (3)
Args(21-21)preCreate(45-45)preUpdate(49-53)api/src/user/repositories/user.repository.ts (2)
preCreate(46-53)preUpdate(62-83)api/src/utils/generics/base-repository.ts (2)
preCreate(607-609)preUpdate(615-617)api/src/chat/repositories/message.repository.ts (1)
preCreate(47-53)api/src/user/repositories/invitation.repository.ts (1)
preCreate(38-44)
api/src/user/repositories/user.repository.ts (4)
api/src/utils/types/lifecycle-hook-manager.types.ts (3)
Args(21-21)preCreate(45-45)preUpdate(49-53)api/src/cms/repositories/content.repository.ts (2)
preCreate(45-47)preUpdate(57-63)api/src/utils/generics/base-repository.ts (2)
preCreate(607-609)preUpdate(615-617)api/src/user/repositories/invitation.repository.ts (1)
preCreate(38-44)
api/src/nlp/repositories/nlp-value.repository.ts (4)
api/src/utils/types/lifecycle-hook-manager.types.ts (4)
Args(21-21)postCreate(47-47)postUpdate(66-69)preDelete(71-80)api/src/nlp/repositories/nlp-entity.repository.ts (3)
postCreate(54-59)postUpdate(69-74)preDelete(85-101)api/src/utils/generics/base-repository.ts (3)
postCreate(611-613)postUpdate(627-629)preDelete(631-633)api/src/nlp/repositories/nlp-sample.repository.ts (1)
preDelete(46-56)
🔇 Additional comments (30)
api/src/chat/repositories/context-var.repository.ts (2)
16-19: Import refactoring looks good.The import changes correctly remove unused Mongoose types and add the centralized lifecycle hook types, improving consistency across the codebase.
50-50: Method signature refactoring is consistent.The
preDeletemethod signature correctly adopts the newArgs<preDelete<ContextVar>>pattern with rest parameter destructuring, maintaining type safety while following the centralized approach.api/src/nlp/repositories/nlp-sample.repository.ts (2)
11-14: Import statements properly updated.The imports are correctly simplified and the centralized lifecycle hook types are properly imported, maintaining consistency with the refactoring pattern.
46-46: Lifecycle hook signature correctly updated.The method signature follows the established pattern using
Args<preDelete<NlpSample>>with proper parameter destructuring.api/src/chat/repositories/category.repository.ts (2)
11-14: Import refactoring implemented correctly.The import changes align with the centralization effort, removing unused Mongoose types and adding the required lifecycle hook types.
44-45: Method signature follows the new pattern.The
preDeletemethod correctly uses theArgs<preDelete<Category>>pattern. Note that thecriteriaparameter is immediately reassigned fromquery.getQuery(), which suggests the original implementation pattern is preserved where the query object's criteria takes precedence over the passed criteria parameter.api/src/cms/repositories/content-type.repository.ts (3)
11-15: Clean import refactoring.The imports are properly updated to use the centralized lifecycle hook types, removing dependency on explicit Mongoose types in the method signatures.
45-46: Method signature and parameter handling improved.The new signature using
Args<preDelete<ContentType>>with destructuring is consistent with the pattern, and the direct assignment ofcriteria._idtoentityIdis cleaner than the previous approach.
53-54: Logic preservation with cleaner structure.The entity ID validation and deletion logic is preserved while benefiting from the simplified parameter handling.
api/src/utils/types/lifecycle-hook-manager.types.ts (4)
21-23: LGTM: Clean utility types for parameter and return type extraction.The
Args<T>andR<T>utility types provide a clean way to extract function parameters and return types, enabling consistent lifecycle hook signatures across repositories.
25-44: LGTM: Comprehensive validation hook types.The validation hook types cover both pre and post validation scenarios for create and update operations with appropriate parameter types and async return signatures.
45-70: LGTM: Well-structured CRUD operation hook types.The create and update hook types provide comprehensive coverage with proper Mongoose query typing and generic document support.
71-91: LGTM: Properly typed delete operation hooks.The delete hook types correctly use the
DeleteResulttype and support both single and bulk delete operations with appropriate Mongoose query typing.api/src/chat/repositories/label.repository.ts (3)
11-11: LGTM: Clean import refactoring.The imports have been properly updated to remove unused Mongoose types and add the necessary lifecycle hook manager types.
Also applies to: 14-18
46-46: LGTM: Consistent destructuring pattern for postCreate.The method signature correctly uses the new
Args<postCreate<Label>>pattern with proper destructuring to extract thecreatedparameter.
74-74: LGTM: Proper parameter extraction in preDelete.The method correctly destructures the arguments, ignoring the query parameter and extracting the criteria for deletion logic.
api/src/chat/repositories/subscriber.repository.ts (3)
11-11: LGTM: Consistent import refactoring.The imports follow the same clean pattern as other repositories, removing unused Mongoose types and adding lifecycle hook manager types.
Also applies to: 15-19
45-45: LGTM: Proper postCreate refactoring.The method signature correctly implements the new pattern while preserving the event emission logic.
62-64: LGTM: Correct preUpdate parameter extraction.The method properly destructures the
Args<preUpdate<Subscriber>>tuple, extracting the criteria and updates parameters while ignoring the query parameter.api/src/user/repositories/user.repository.ts (3)
11-11: LGTM: Import cleanup follows established pattern.The imports are properly updated to use the centralized lifecycle hook manager types.
Also applies to: 14-18
46-46: LGTM: Preserved security logic in preCreate.The method signature is correctly updated while maintaining the critical password hashing functionality.
62-62: LGTM: Proper parameter extraction in preUpdate.The method correctly extracts the query and updates parameters from the lifecycle hook arguments while preserving the password and resetToken hashing logic.
api/src/cms/repositories/content.repository.ts (1)
11-18: LGTM! Well-executed type refactoring that improves consistency.The refactoring successfully centralizes lifecycle hook types while maintaining the existing logic. The parameter destructuring is correctly implemented:
preCreateproperly extracts_docfromArgs<preCreate<Content>>preUpdatecorrectly destructures to get_queryand_updateswhile skipping the middle parameterImport cleanup removes unused Mongoose types, and the new pattern aligns with the broader codebase standardization.
Also applies to: 45-63
api/src/setting/repositories/setting.repository.ts (1)
12-20: LGTM! Clean refactoring with improved parameter naming.The lifecycle hook method signatures have been successfully updated to use the centralized types:
- Proper destructuring in
preCreateValidateandpreUpdateValidate- Good parameter renaming in
postUpdatefromsettingtoupdatedfor better clarity- Import cleanup removes unused Mongoose types while adding the new lifecycle hook types
The refactoring maintains all existing logic while improving type consistency across the codebase.
Also applies to: 31-37, 67-73
api/src/nlp/repositories/nlp-entity.repository.ts (1)
11-19: LGTM! Consistent refactoring maintains all business logic.The lifecycle hook methods have been properly updated to use the centralized type system:
- Correct parameter destructuring in all three methods (
postCreate,postUpdate,preDelete)- Import cleanup removes unused Mongoose types
- All existing business logic preserved, including event emission for non-builtin entities and cascade deletion handling
The refactoring aligns perfectly with the codebase-wide standardization effort.
Also applies to: 54-59, 69-74, 85-101
api/src/nlp/repositories/nlp-value.repository.ts (1)
12-23: LGTM! Well-executed refactoring with appropriate import management.The lifecycle hook methods have been correctly updated to use the centralized type system:
- Proper parameter destructuring in all methods (
postCreate,postUpdate,preDelete)- Selective import cleanup that retains necessary Mongoose types while removing unused ones
- All existing business logic preserved, including event emission for non-builtin entities and cascade deletion handling
The refactoring maintains consistency with the broader codebase standardization while preserving all functionality.
Also applies to: 57-62, 70-75, 84-101
api/src/chat/repositories/block.repository.ts (3)
13-21: Well-structured imports for centralized lifecycle hook types.The refactoring to import lifecycle hook types from a centralized location improves maintainability and follows the PR's objective effectively.
66-68: Good defensive programming with null check.The addition of the null check before calling
checkDeprecatedAttachmentUrlprevents potential runtime errors. The new signature pattern improves type safety.
77-104: Correct handling of update extraction and validation.The method properly extracts updates from the
$setoperator and includes appropriate null checks before processing. The refactored signature maintains the same functionality with improved type safety.api/src/utils/generics/base-repository.ts (1)
583-637: Excellent consistency in lifecycle hook refactoring.The uniform application of the
Args<>andR<>type patterns across all lifecycle hooks improves type safety and maintainability. The use of rest parameters with underscore for unused arguments follows TypeScript best practices.
| async postDelete(...[, result]: Args<postDelete<Block>>) { | ||
| if (result.deletedCount > 0) { | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove empty conditional block or implement the missing logic.
The method contains an empty conditional block that serves no purpose. Either implement the intended logic or remove the condition entirely.
- async postDelete(...[, result]: Args<postDelete<Block>>) {
- if (result.deletedCount > 0) {
- }
- }
+ async postDelete(...[, result]: Args<postDelete<Block>>) {
+ // Implementation can be added when needed
+ }📝 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.
| async postDelete(...[, result]: Args<postDelete<Block>>) { | |
| if (result.deletedCount > 0) { | |
| } | |
| } | |
| async postDelete(...[, result]: Args<postDelete<Block>>) { | |
| // Implementation can be added when needed | |
| } |
🤖 Prompt for AI Agents
In api/src/chat/repositories/block.repository.ts around lines 212 to 215, there
is an empty if statement checking if result.deletedCount is greater than 0 with
no logic inside. You should either implement the intended logic that should run
when deletions occur or remove the entire if condition and its block to avoid
having an empty conditional statement.
| // Nothing ... | ||
| } | ||
|
|
||
| async preCreate(_doc: HydratedDocument<T>): Promise<void> { | ||
| async preCreate(..._: Args<preCreate<T>>): R<postCreate<T>> { |
There was a problem hiding this comment.
Fix incorrect return types in lifecycle hook methods.
Two lifecycle hook methods have incorrect return types that don't match their method names. This breaks type safety and could lead to runtime issues.
- async preCreate(..._: Args<preCreate<T>>): R<postCreate<T>> {
+ async preCreate(..._: Args<preCreate<T>>): R<preCreate<T>> {- async postUpdate(..._: Args<postUpdate<T>>): R<preDelete<T>> {
+ async postUpdate(..._: Args<postUpdate<T>>): R<postUpdate<T>> {Also applies to: 627-627
🤖 Prompt for AI Agents
In api/src/utils/generics/base-repository.ts at lines 607 and 627, the lifecycle
hook methods preCreate and the other method at line 627 have incorrect return
types that do not align with their method names. Review the expected return
types for these lifecycle hooks and update the method signatures to use the
correct generic return types that match their lifecycle stage, ensuring type
safety and consistency.
|
|
||
| export type Args<T extends (...args: any) => unknown> = Parameters<T>; | ||
|
|
||
| export type R<T extends (...args: any) => unknown> = ReturnType<T>; |
There was a problem hiding this comment.
Is the R type alias unused ?
There was a problem hiding this comment.
The R type is already used
There was a problem hiding this comment.
I tested the following hooks/scenarios to make sure the new implementation works properly :
- BlockRepository:
- preDelete: Create two blocks, A and B. Attach block B to block A. Then, delete block B
- preUpdate: Create two blocks, A and B. Attach block B to block A. Then, update block B to move it to a different category.
- CategoryRepository
- preDelete: Prevent the deletion of categories that are not empty.
- ContentTypeRepository
- preDelete: ensures that content types associated with blocks cannot be deleted.
- NlpEntityRepository
- preDelete: Deletes to related NLP values and samples.
I didn't observe any issues during testing.
Motivation
The main motivation of this PR is to centralize lifecycle-hook-manager types.
Fixes #1107
Checklist:
Summary by CodeRabbit