[Fix] Add Action By User To Base Entity #8811
Conversation
WalkthroughThis update enhances the entity models by introducing a new interface for user action tracking and by removing legacy user creator properties. In the contracts package, a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant "BaseEntityActionByUser"
Client->>Controller: Request to create entity
Controller->>Service: Validate and process creation
Service->>BaseEntityActionByUser: Instantiate entity with user info
BaseEntityActionByUser-->>Service: Return new entity instance
Service-->>Controller: Persist entity and return result
Controller-->>Client: Respond with created entity
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (9)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/core/src/lib/organization-team/organization-team.entity.ts (1)
165-167: Use caution with eager loading to avoid performance overhead.Marking this relation as
eager: truemeans theImageAssetentity will always be fetched with theOrganizationTeam. For large queries or rarely accessed associations, consider lazy loading (i.e.,eager: false) to minimize performance costs.packages/core/src/lib/core/entities/base.entity.ts (2)
98-98: Potential timezone consistency concern.
return new Date();ensures timestamps use the local server time. Consider forcing UTC to avoid inconsistent date handling in distributed environments.
102-123: Centralized user action tracking is a good pattern.Introducing
BaseEntityActionByUserneatly consolidates the user creation reference. If you plan to track user updates as well, consider extending this pattern for full audit logging.Would you like a follow-up implementation for update-by-user tracking?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
packages/contracts/src/lib/base-entity.model.ts(3 hunks)packages/contracts/src/lib/dashboard.model.ts(1 hunks)packages/contracts/src/lib/deal.model.ts(1 hunks)packages/contracts/src/lib/organization-project-module.model.ts(1 hunks)packages/contracts/src/lib/organization-team.model.ts(1 hunks)packages/contracts/src/lib/payment.model.ts(1 hunks)packages/contracts/src/lib/screening-task.model.ts(1 hunks)packages/contracts/src/lib/task.model.ts(1 hunks)packages/contracts/src/lib/user.model.ts(0 hunks)packages/core/src/lib/core/entities/base.entity.ts(4 hunks)packages/core/src/lib/dashboard/dashboard.entity.ts(1 hunks)packages/core/src/lib/database/migrations/1740727466186-RenameSubscriptionToEntitySubscriptionEntityTable.ts(1 hunks)packages/core/src/lib/deal/deal.entity.ts(1 hunks)packages/core/src/lib/organization-project-module/organization-project-module.entity.ts(2 hunks)packages/core/src/lib/organization-team/organization-team.entity.ts(1 hunks)packages/core/src/lib/payment/payment.entity.ts(1 hunks)packages/core/src/lib/role/role.entity.ts(1 hunks)packages/core/src/lib/tasks/screening-tasks/screening-task.entity.ts(1 hunks)packages/core/src/lib/tasks/task.entity.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/contracts/src/lib/user.model.ts
✅ Files skipped from review due to trivial changes (2)
- packages/core/src/lib/role/role.entity.ts
- packages/core/src/lib/database/migrations/1740727466186-RenameSubscriptionToEntitySubscriptionEntityTable.ts
🧰 Additional context used
🧠 Learnings (1)
packages/core/src/lib/tasks/screening-tasks/screening-task.entity.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: test
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
🔇 Additional comments (19)
packages/contracts/src/lib/deal.model.ts (1)
16-16: Removal of IHasUserCreator is part of refactoring user attributionThe removal of
IHasUserCreatorfrom theIDealinterface aligns with the PR's goal of moving user attribution to a base entity. This simplifies the interface and centralizes user tracking functionality.packages/contracts/src/lib/organization-project-module.model.ts (1)
37-37: IHasUserCreator dependency correctly removedRemoving the
IHasUserCreatorinterface fromIProjectModuleAssociationsis consistent with the architectural changes being made across the codebase to centralize user action tracking.packages/core/src/lib/tasks/task.entity.ts (1)
51-51: Imports updated consistently with user tracking refactoringThe update to imports reflects the removal of user creator properties from the Task entity, consistent with moving user action tracking to a base entity approach.
packages/core/src/lib/dashboard/dashboard.entity.ts (1)
7-8: User import dependencies properly removedThe removal of User-related imports aligns with the architectural changes to centralize user action tracking at the base entity level instead of in each individual entity.
packages/core/src/lib/payment/payment.entity.ts (1)
1-205: LGTM: Removal of user creator properties aligns with broader refactoring.Based on the AI summary, this file has had the
createdByUserandcreatedByUserIdproperties removed from thePaymentclass, which aligns with the broader refactoring being done across the codebase.packages/core/src/lib/tasks/screening-tasks/screening-task.entity.ts (1)
6-8: Verify alignment with future planned changes.The imports have been updated to remove
IUserandMultiORMManyToOne, which aligns with removing the user creator properties. However, based on the retrieved learning, there was a plan to renamecreatorIdtocreatedByUserIdin a future PR, but this PR is removingcreatedByUserIdentirely. Make sure this change is coordinated with those future plans.packages/contracts/src/lib/payment.model.ts (1)
19-19: LGTM: Interface update aligns with entity changes.The removal of
IHasUserCreatorfrom the interface inheritance aligns with the removal of user creator properties from the corresponding entity classes.packages/contracts/src/lib/screening-task.model.ts (1)
18-21: LGTM: Interface simplification aligns with entity changes.The
IScreeningTaskAssociationsinterface no longer extendsIHasUserCreator, which aligns with the removal of user creator properties from the corresponding entity class.packages/contracts/src/lib/dashboard.model.ts (1)
9-9: Interface inheritance updated to remove IHasUserCreator.The
IDashboardinterface no longer directly extendsIHasUserCreator. This change is part of the larger refactoring effort to centralize user action tracking through the base entity hierarchy instead of requiring direct interface implementation.packages/core/src/lib/deal/deal.entity.ts (1)
1-1: Import statements refactored to remove IUser dependency.The removal of the
IUserimport aligns with the elimination of the directcreatedByUserandcreatedByUserIdproperties from theDealentity. This is part of the PR's approach to consolidate user action tracking at the base entity level.Also applies to: 5-5
packages/contracts/src/lib/base-entity.model.ts (2)
37-37: Enhanced IBaseEntityModel to extend from IBaseEntityActionByUserModel.The
IBaseEntityModelnow inherits from bothIBaseEntityActionByUserModelandIBaseSoftDeleteEntityModel, which is a good architectural decision. This change centralizes user action tracking in the inheritance hierarchy rather than having it implemented in individual entity interfaces.
48-51: New interface created for tracking user actions.The introduction of the
IBaseEntityActionByUserModelinterface provides a centralized way to track which user created an entity. This approach improves code organization and maintainability by removing the need for individual entities to implement the same user tracking properties repeatedly.packages/contracts/src/lib/organization-team.model.ts (1)
43-43: Interface inheritance updated to remove IHasUserCreator.The
IOrganizationTeaminterface no longer directly extendsIHasUserCreator. This aligns with the architectural changes in this PR, which centralize user tracking at the base entity level rather than requiring direct interface implementation in each entity type.packages/contracts/src/lib/task.model.ts (1)
24-24: Ensure references to removed interface are updated.By removing
IHasUserCreator, any code or services depending on creator-related properties must be aligned with the new approach (e.g., theBaseEntityActionByUserin the core). Verify that no lingering calls or assumptions about a creator property remain.packages/core/src/lib/organization-project-module/organization-project-module.entity.ts (2)
1-1: New import aligns with advanced relation usage.Adding
RelationIdto the import statement is appropriate for referencing foreign key columns. No concerns here.
33-33: Confirm alignment with multi-tenant architecture.Switching or reaffirming to use
TenantOrganizationBaseEntityis sensible if you need consistent tenant/organization scoping. Verify that all existing behaviors (e.g., user creation tracking, organization scoping) are correct under this new base class.packages/core/src/lib/core/entities/base.entity.ts (3)
5-5: Additional imports for user tracking.The newly added imports (
RelationId,IUser,MultiORMManyToOne, andUser) are consistent with implementing user-linked entity behavior. Implementation looks good.Also applies to: 9-9, 12-12, 13-13
64-64: Documentation improvement.Minor docstring updates clarify the meaning of creation and update timestamps; no functional impact.
Also applies to: 80-80
128-128: Extending the new user-tracking base class.Shifting from
AccessTimestampstoBaseEntityActionByUseris logical. Ensure existing entities expecting only timestamps are updated if they rely on the old inheritance structure. Double-check references and migrations to avoid unexpected breakages.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/src/lib/core/entities/base.entity.ts (1)
102-140: Introduction ofBaseEntityActionByUser.
This abstract class encapsulates user references for creation and update actions, extending the existing timestamp functionality. TheonDelete: 'CASCADE'setting on user relationships can lead to records losing references if a user is deleted. If retaining historical data is critical, consider settingonDeletetoSET NULLor keeping a soft deletion approach for users.Would you like to confirm or test whether cascade deletes on users might cause unintended data loss?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/contracts/src/lib/base-entity.model.ts(3 hunks)packages/core/src/lib/core/entities/base.entity.ts(4 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 (8)
packages/contracts/src/lib/base-entity.model.ts (3)
3-3: Import usage looks consistent.
No concerns here; pulling inIUseris necessary for referencing user-specific properties.
37-37: Confirm expanded inheritance.
By extending bothIBaseEntityActionByUserModelandIBaseSoftDeleteEntityModel, theIBaseEntityModelnow incorporates user action tracking alongside soft-delete functionality. Ensure all existing references toIBaseEntityModelare compatible with this addition.
48-54: New interface for user action tracking.
The newly introducedIBaseEntityActionByUserModelcleanly encapsulatescreatedByUser,createdByUserId,updatedByUser, andupdatedByUserId. This aligns well with typical audit fields. Confirm these fields are optional only when partial creation or updates are valid business cases.packages/core/src/lib/core/entities/base.entity.ts (5)
5-5: Added imports for new user-action functionality.
These imports properly bring in theRelationId,IUser, and custom decorators needed for the new user-related fields. No issues noted.Also applies to: 9-9, 11-11, 13-13
63-63: Minor doc update: creation timestamp description.
This clarifies the field’s usage without altering functionality.
80-80: Minor doc update: update timestamp description.
Again, purely a doc clarification. No further concerns.
98-98: No change in actual behavior.
Usingreturn new Date();remains a valid approach for populating timestamps.
145-145: Inheritance change to incorporate user-tracking.
BaseEntitynow extendsBaseEntityActionByUser, gaining user action fields. Confirm that application logic correctly handles these new properties (e.g., populatingupdatedByUser).
|
View your CI Pipeline Execution ↗ for commit 288418c.
☁️ Nx Cloud last updated this comment at |
rahul-rocket
deleted the
fix/alter-base-entity-add-created-by-user-id-column
branch
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