-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fix 0.32 #8319
Fix 0.32 #8319
Conversation
charlesBochet
commented
Nov 4, 2024
•
edited
Loading
edited
- Removing unique constraint creation in 0.32 as we have a dependency on sync-metadata and sync-metadata has a dependency on it if we keep it.
- sync-metadata does not support well the deletion of activity + activityTarget, silently swallow the exception for now
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.
PR Summary
This PR implements changes to enforce unique constraints for person emails in version 0.32, with related adjustments to foreign key handling and entity definitions.
- Removes optional flag for person email uniqueness in
/packages/twenty-server/src/database/commands/upgrade-version/0-32/0-32-enforce-unique-constraints.command.ts
, making it mandatory - Adds temporary workaround in
workspace-migration-runner.service.ts
to handle missing 'activityId' foreign key during 0.32 upgrade - Shifts from decorator-based to database-level uniqueness by removing
@WorkspaceIsUnique()
fromperson.workspace-entity.ts
- Handles duplicate emails by appending incremental numbers while preserving the original email structure
- Includes migration script to safely update existing duplicate records without data loss
3 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
// Todo: Remove this temporary hack tied to 0.32 upgrade | ||
if (migrationColumn.columnName === 'activityId') { | ||
return; | ||
} |
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.
style: This temporary hack silently ignores missing foreign key constraints for activityId columns. Consider adding logging to track these occurrences.
// @WorkspaceIsUnique() | ||
[EMAILS_FIELD_NAME]: EmailsMetadata; |
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.
style: Consider removing the commented-out decorator entirely instead of leaving it as a comment, since the uniqueness is now handled by the migration script
// @WorkspaceIsUnique() | ||
[EMAILS_FIELD_NAME]: EmailsMetadata; |
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.
logic: Removing uniqueness constraint on emails could allow duplicate contacts to be created - ensure proper validation is implemented at the application level