-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
RICH_TEXT_V2 upgrade command #9704
RICH_TEXT_V2 upgrade command #9704
Conversation
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 introduces a migration command to upgrade RICH_TEXT fields to a new RICH_TEXT_V2 composite structure, which stores both BlockNote JSON and Markdown representations of rich text content.
- Added
MigrateRichTextFieldCommand
in/packages/twenty-server/src/database/commands/upgrade-version/0-41/0-41-migrate-rich-text-field.command.ts
to handle field migration with BlockNote to Markdown conversion - Defined new
RICH_TEXT_V2
composite type in/packages/twenty-server/src/engine/metadata-modules/field-metadata/composite-types/rich-text-v2.composite-type.ts
with blocknote and markdown properties - Missing transaction wrapping and rollback mechanisms in migration process which could lead to data inconsistency if failures occur
- No validation or cleanup of old fields after migration is completed
- Row-by-row processing could impact performance with large datasets
8 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
async executeActiveWorkspacesCommand( | ||
passedParam: string[], | ||
options: BaseCommandOptions, | ||
workspaceIds: string[], | ||
): Promise<void> { |
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: passedParam is unused in this implementation but passed through to migrateRichTextFieldCommand. Consider documenting the expected parameters or removing if not needed.
for (const row of rows) { | ||
const blocknoteFieldValue = row[richTextField.name]; | ||
const markdownFieldValue = blocknoteFieldValue | ||
? await serverBlockNoteEditor.blocksToMarkdownLossy( | ||
JSON.parse(blocknoteFieldValue), | ||
) | ||
: null; |
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: No error handling for JSON.parse() - could throw exception and halt migration if blocknoteFieldValue contains invalid JSON
@eliasylonen I'm closing this PR for now as we have to merge first the backend PR, let's only open one PR at a time. |
What the upgrade command does:
RICH_TEXT_V2
fieldsRICH_TEXT
fields toRICH_TEXT_V2
bodyV2
view field position to match that of thebody
field for each workspacePR #3 of this comment