-
Notifications
You must be signed in to change notification settings - Fork 5k
Fix the rendering of notes from workflow triggering #17210
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?
Fix the rendering of notes from workflow triggering #17210
Conversation
...wenty-server/src/engine/core-modules/record-transformer/utils/transform-rich-text-v2.util.ts
Outdated
Show resolved
Hide resolved
Greptile SummaryThis PR fixes markdown rendering issues when workflow templates use notes/tasks by normalizing redundant escape sequences generated during BlockNote-to-markdown conversion. Key Changes:
Issues Found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant WF as Workflow Engine
participant RT as RecordInputTransformer
participant TRTV2 as transformRichTextV2Value
participant BN as BlockNote ServerEditor
participant DB as Database
WF->>RT: Create note with markdown from template
RT->>TRTV2: Transform rich text value
TRTV2->>TRTV2: Parse input (string/object)
TRTV2->>BN: blocksToMarkdownLossy(blocknote)
BN-->>TRTV2: Markdown with redundant escapes
Note over TRTV2: NEW: Normalize escapes<br/>Remove \\+([ntr"'\n])|\\+
TRTV2->>TRTV2: Apply regex normalization
TRTV2->>BN: tryParseMarkdownToBlocks(normalizedMarkdown)
BN-->>TRTV2: Clean BlockNote structure
TRTV2-->>RT: {markdown, blocknote}
RT->>DB: Save note with normalized content
DB-->>WF: Note created successfully
|
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.
1 file reviewed, 4 comments
| convertedMarkdown = (convertedMarkdown as string)?.replace( | ||
| /\\+([ntr"'\n])|\\+/g, | ||
| (_: string, char: string) => (char ? (char === '\n' ? char : '\\' + char) : ''), | ||
| ) |
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: The regex pattern /\\+([ntr"'\n])|\\+/g may have unintended side effects. The second alternative \\+ matches and removes all backslashes not followed by [ntr"'\n]. This could corrupt legitimate backslashes in content like file paths (C:\\Users\\), regex patterns in code blocks, or backslashes before other characters (e.g., \\x becomes x).
Test case: if markdown contains text\\more, it would become textmore.
Consider if this is the intended behavior or if backslashes before characters outside [ntr"'\n] should be preserved. Are there test cases with file paths, code blocks with regex, or other content containing backslashes that should be preserved?
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-rich-text-v2.util.ts
Line: 52:55
Comment:
**logic:** The regex pattern `/\\+([ntr"'\n])|\\+/g` may have unintended side effects. The second alternative `\\+` matches and **removes all backslashes** not followed by `[ntr"'\n]`. This could corrupt legitimate backslashes in content like file paths (`C:\\Users\\`), regex patterns in code blocks, or backslashes before other characters (e.g., `\\x` becomes `x`).
Test case: if markdown contains `text\\more`, it would become `textmore`.
Consider if this is the intended behavior or if backslashes before characters outside `[ntr"'\n]` should be preserved. Are there test cases with file paths, code blocks with regex, or other content containing backslashes that should be preserved?
How can I resolve this? If you propose a fix, please make it concise.| convertedMarkdown = (convertedMarkdown as string)?.replace( | ||
| /\\+([ntr"'\n])|\\+/g, | ||
| (_: string, char: string) => (char ? (char === '\n' ? char : '\\' + char) : ''), | ||
| ) |
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.
syntax: Missing semicolon at end of statement
| ) | |
| ); | |
| <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> | |
| <details><summary>Prompt To Fix With AI</summary> | |
| `````markdown | |
| This is a comment left during a code review. | |
| Path: packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-rich-text-v2.util.ts | |
| Line: 55:55 | |
| Comment: | |
| **syntax:** Missing semicolon at end of statement | |
| ```suggestion | |
| ); | |
| <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> | |
| How can I resolve this? If you propose a fix, please make it concise. |
| convertedMarkdown = (convertedMarkdown as string)?.replace( | ||
| /\\+([ntr"'\n])|\\+/g, | ||
| (_: string, char: string) => (char ? (char === '\n' ? char : '\\' + char) : ''), | ||
| ) |
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 adding tests for the escape normalization logic to ensure it handles edge cases correctly (file paths, code blocks, multiple backslashes, special characters)
Context Used: Context from dashboard - Always consider adding tests for new functionality, especially for edge cases like empty responses. (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-rich-text-v2.util.ts
Line: 52:55
Comment:
**style:** Consider adding tests for the escape normalization logic to ensure it handles edge cases correctly (file paths, code blocks, multiple backslashes, special characters)
**Context Used:** Context from `dashboard` - Always consider adding tests for new functionality, especially for edge cases like empty responses. ([source](https://app.greptile.com/review/custom-context?memory=458c2737-e49d-49c4-a233-2605d1516703))
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| markdown: parsedValue.markdown || convertedMarkdown, | ||
| blocknote: parsedValue.blocknote || convertedBlocknote, | ||
| markdown: convertedMarkdown || parsedValue.markdown, | ||
| blocknote: convertedBlocknote || parsedValue.blocknote |
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.
syntax: Missing comma at end of object property
| blocknote: convertedBlocknote || parsedValue.blocknote | |
| blocknote: convertedBlocknote || parsedValue.blocknote, | |
| <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> | |
| <details><summary>Prompt To Fix With AI</summary> | |
| `````markdown | |
| This is a comment left during a code review. | |
| Path: packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-rich-text-v2.util.ts | |
| Line: 78:78 | |
| Comment: | |
| **syntax:** Missing comma at end of object property | |
| ```suggestion | |
| blocknote: convertedBlocknote || parsedValue.blocknote, | |
| <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> | |
| How can I resolve this? If you propose a fix, please make it concise. |
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.
2 issues found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-rich-text-v2.util.ts">
<violation number="1" location="packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-rich-text-v2.util.ts:53">
P1: Backslash normalization strips valid markdown content: any backslash not before n/t/r/"/'/newline is removed, corrupting escaped characters, paths, and code blocks.</violation>
<violation number="2" location="packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-rich-text-v2.util.ts:77">
P1: Conversion failures/lossy conversions now override original markdown/blocknote, risking corruption (catch assigns blocknote JSON to markdown and return prioritizes converted values).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
...wenty-server/src/engine/core-modules/record-transformer/utils/transform-rich-text-v2.util.ts
Show resolved
Hide resolved
...wenty-server/src/engine/core-modules/record-transformer/utils/transform-rich-text-v2.util.ts
Outdated
Show resolved
Hide resolved
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:12391 This environment will automatically shut down when the PR is closed or after 5 hours. |
...wenty-server/src/engine/core-modules/record-transformer/utils/transform-rich-text-v2.util.ts
Show resolved
Hide resolved
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.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-rich-text-v2.util.ts">
<violation number="1" location="packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-rich-text-v2.util.ts:59">
P2: Slash count includes following char, leaving extra backslashes for odd-length escape runs</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Problem:
Using the markdown from a saved note/task as a template in workflow to create a new note does not show the created note/task correctly.
Root cause
The function where blocknote to markdown conversion fails for certain block types (custom/code blocks)
convertedMarkdown = isDefined(parsedValue.blocknote) ? await serverBlockNoteEditor.blocksToMarkdownLossy( JSON.parse(parsedValue.blocknote), ) : null;This adds some extra slashes which were required to normalize redundant escapes: strips all slashes before newlines and collapses multi-escapes to a single slash at first place.
Changes
Normalized the redundant escapes generated in converted markdown so that converted block note can be generated correctly.
Fixes: #17171