Skip to content

Conversation

@guillim
Copy link
Contributor

@guillim guillim commented Nov 5, 2025

TimelineActivity migration to morph

  • Creates timelineActivities2 relations 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_MIGRATED necessary to have the two states in parallel. It is used as a stamp once the migration has been run

  • Migration 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 :
plan
Note: we will need to rename fields in the release 1.12 (there is no easy way to do all this in one release)

@guillim guillim self-assigned this Nov 5, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

🚀 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.

@guillim guillim changed the title relation decorator TimelineActivity migration to morph Nov 6, 2025
@guillim guillim marked this pull request as ready for review November 6, 2025 14:22
Copy link
Contributor

@greptile-apps greptile-apps bot left a 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}Id fields alongside existing {object}Id fields in TimelineActivityWorkspaceEntity for Company, Person, Opportunity, Note, Task, Workflow, WorkflowVersion, WorkflowRun, and Dashboard
  • Created corresponding timelineActivities2 relations on all target entities with proper inverseSideFieldKey configuration
  • Implemented feature flag IS_TIMELINE_ACTIVITY_MIGRATED to 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

Loading

31 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link
Member

@charlesBochet charlesBochet left a 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

@guillim guillim requested a review from charlesBochet November 7, 2025 15:31
@guillim
Copy link
Contributor Author

guillim commented Nov 7, 2025

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
I need to implement the morph in the WorkspaceDynamicRelation just like i did in the WorkspaceRelation. i will come back to you when finished

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants