-
Notifications
You must be signed in to change notification settings - Fork 4.5k
TimelineActivity migration to morph #15652
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:53566 This environment will automatically shut down when the PR is closed or after 5 hours. |
…ion-timeline-activities
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR implements a migration strategy to transition timeline activities from direct foreign key relations to morph relations, enabling polymorphic associations across multiple entity types.
Key Changes:
- Added new
timelineActivity{Object}Idfields alongside existing{object}Idfields inTimelineActivityWorkspaceEntityfor Company, Person, Opportunity, Note, Task, Workflow, WorkflowVersion, WorkflowRun, and Dashboard - Created corresponding
timelineActivities2relations on all target entities with properinverseSideFieldKeyconfiguration - Implemented feature flag
IS_TIMELINE_ACTIVITY_MIGRATEDto control gradual rollout - Migration command copies data from old fields to new fields and enables the feature flag per workspace
- Runtime services/repositories switch field names based on feature flag status
- Frontend hook checks feature flag and adjusts field name queries accordingly
Implementation Strategy:
The PR uses a dual-field approach during transition: old and new fields coexist, allowing the system to operate with either schema based on the feature flag. The migration command populates new fields from old ones, then the feature flag controls which fields are used at runtime.
Issues Found:
- Console.log statement left in production code (timeline-activity.repository.ts:211-213)
Confidence Score: 4/5
- Safe to merge after removing console.log statement
- The PR implements a well-structured gradual migration strategy with feature flags. The approach of maintaining both old and new fields during transition minimizes risk. However, the console.log statement in production code needs to be removed before merging. The consistent typo in variable naming (isFeatureFlagTimelineActivtyMigrated) is present throughout but doesn't affect functionality.
- packages/twenty-server/src/modules/timeline/repositories/timeline-activity.repository.ts requires removing console.log statement before merge
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| packages/twenty-server/src/database/commands/upgrade-version-command/1-11/1-11-migrate-timeline-activity-to-morph-relations.command.ts | 4/5 | Migration command that copies old field values to new morph relation fields and sets feature flag. Contains console.log statement that should be removed. |
| packages/twenty-server/src/modules/timeline/standard-objects/timeline-activity.workspace-entity.ts | 5/5 | Adds new morph relation fields (timelineActivity*) alongside existing fields for gradual migration. Properly configured with standardIds and metadata. |
| packages/twenty-server/src/modules/timeline/repositories/timeline-activity.repository.ts | 3/5 | Switches between old and new field names based on feature flag. Contains console.log that needs removal and consistent typo in variable name. |
Sequence Diagram
sequenceDiagram
participant MigCmd as Migration Command
participant FeatureFlag as Feature Flag Service
participant DB as Database
participant Timeline as Timeline Activity Service
participant Repo as Timeline Activity Repository
Note over MigCmd,DB: Migration Phase (1-11 Upgrade)
MigCmd->>FeatureFlag: Check IS_TIMELINE_ACTIVITY_MIGRATED
alt Already Migrated
FeatureFlag-->>MigCmd: true (skip)
else Not Migrated
FeatureFlag-->>MigCmd: false
loop For each field mapping
MigCmd->>DB: UPDATE timelineActivity SET new field = old field
DB-->>MigCmd: rows updated
end
MigCmd->>FeatureFlag: Enable IS_TIMELINE_ACTIVITY_MIGRATED
end
Note over Timeline,Repo: Runtime Timeline Activity Creation
Timeline->>FeatureFlag: Check IS_TIMELINE_ACTIVITY_MIGRATED
FeatureFlag-->>Timeline: feature flag status
Timeline->>Repo: upsertTimelineActivities(with flag)
alt Feature Flag Enabled (New Morph Fields)
Repo->>Repo: Use timelineActivity{Object}Id fields
Repo->>DB: Query/Insert with new field names
else Feature Flag Disabled (Old Fields)
Repo->>Repo: Use {object}Id fields
Repo->>DB: Query/Insert with old field names
end
Repo-->>Timeline: activities created/updated
31 files reviewed, 1 comment
packages/twenty-server/src/modules/timeline/repositories/timeline-activity.repository.ts
Outdated
Show resolved
Hide resolved
…ion-timeline-activities
…ion-timeline-activities
packages/twenty-server/src/modules/timeline/repositories/timeline-activity.repository.ts
Outdated
Show resolved
Hide resolved
...es/twenty-server/src/modules/timeline/standard-objects/timeline-activity.workspace-entity.ts
Outdated
Show resolved
Hide resolved
charlesBochet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also handle custom case
...er/src/database/commands/upgrade-version-command/1-11/1-11-upgrade-version-command.module.ts
Outdated
Show resolved
Hide resolved
|
IMO we are good to merge this @charlesBochet . just waiting for your last approval before merging and duplicating the logic on attachments. EDIT: it's not finished actually |
TimelineActivity migration to morph
timelineActivities2relations on Company, Dashboard, Note, Opportunity, Person, Task, Workflow, WorkflowRun, and WorkflowVersion entities with proper metadata and cascade delete behavior.It was required to create standard fields as well since the mapObjectMetadataByUniqueIdentifier needs it. otherwise the fields won't be considered
Feature Flag
IS_TIMELINE_ACTIVITY_MIGRATEDnecessary to have the two states in parallel. It is used as a stamp once the migration has been runMigration is done using the coreDataSource. Why ?
even though is unsafe to use, the first implementation of the migration took forever on each workspace. See this commit
The plan for this complex migration is as follows :

Note: we will need to rename fields in the release 1.12 (there is no easy way to do all this in one release)