fix(api-service): fix & simplify email liquid parsing #7519
Conversation
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| try { | ||
| iterableArray = JSON.parse(iterableArrayString.replace(/'/g, '"')); | ||
| } catch (error) { | ||
| throw new Error(`Failed to parse iterable value for "${iterablePath}": ${error.message}`); | ||
| } |
There was a problem hiding this comment.
safeguard since we are calling JSON.parse() on something that is output from liquid, which parses something that user provides etc
| const iterablePath = node.attrs[MailyAttrsEnum.EACH_KEY]; | ||
| const nodeContent = node.content || []; | ||
| const expandedContent: TipTapNode[] = []; | ||
|
|
||
| const iterableArray = this.getValueByPath(variables, iterablePath); | ||
| const iterableArrayString = await parseLiquid(iterablePath, variables); |
There was a problem hiding this comment.
Use Liquid engine to extract the array from payload instead of finding it manually.
✅ Deploy Preview for dashboard-v2-novu-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -31,47 +31,6 @@ export const forSnippet = { | |||
| }, | |||
| ], | |||
| }, | |||
| { | |||
There was a problem hiding this comment.
Removing logic that tested nested for loops - we dont want to support that
| @@ -64,7 +67,7 @@ export class BuildPayloadSchema { | |||
|
|
|||
| for (const [key, value] of Object.entries(controlValue)) { | |||
| if (isStringTipTapNode(value)) { | |||
| processedValue[key] = transformMailyContentToLiquid(JSON.parse(value)); | |||
| processedValue[key] = this.hydrateEmailSchemaUseCase.execute({ emailEditor: value }); | |||
There was a problem hiding this comment.
I realized HydrateEmailSchemaUseCase (I dont understand the naming) does the same as transformMailyContentToLiquid
I think at some point we should merge the HydrateEmailSchemaUseCase and ExpandEmailEditorSchema to single usecase - there is no reason to transform Maily JSON to Liquid in multiple usecases like Maily -> Hydrate -> Expand -> ParseByLiquid, where each usecases does a little bit of transformation. They can't be used standalone anyway.
It would be better to have one usecase which gets Maily JSON and outputs HTML parsed by Liquid.
| function wrapInLiquidOutput( | ||
| variableName: string, | ||
| fallback?: string, | ||
| forLoopVariable?: string, | ||
| ): string { | ||
| const sanitizedVariableName = sanitizeVariableName(variableName); | ||
| const sanitizedForLoopVariable = | ||
| forLoopVariable && sanitizeVariableName(forLoopVariable); | ||
|
|
||
| /* | ||
| * Handle loop variable replacement | ||
| * payload.comments.name => payload.comments[0].name | ||
| */ | ||
| const processedName = | ||
| sanitizedForLoopVariable && | ||
| sanitizedVariableName.startsWith(sanitizedForLoopVariable) | ||
| ? sanitizedVariableName.replace( | ||
| sanitizedForLoopVariable, | ||
| `${sanitizedForLoopVariable}[${MAILY_ITERABLE_MARK}]`, | ||
| ) | ||
| : sanitizedVariableName; | ||
|
|
||
| // Build liquid output syntax | ||
| const fallbackSuffix = fallback ? ` | default: '${fallback}'` : ''; | ||
|
|
||
| return `{{ ${processedName}${fallbackSuffix} }}`; | ||
| } | ||
|
|
||
| function sanitizeVariableName(variableName: string): string { | ||
| return variableName.replace(/{{|}}/g, '').trim(); | ||
| } |
There was a problem hiding this comment.
Looks complicated but I only added payload.comments.name => {{ payload.comments[0].name }}, the "0" is important for us down the line and this is the simplest place to put the transformation for now.
ChmaraX
changed the title
| @@ -64,7 +67,7 @@ export class BuildPayloadSchema { | |||
|
|
|||
| for (const [key, value] of Object.entries(controlValue)) { | |||
| if (isStringTipTapNode(value)) { | |||
| processedValue[key] = transformMailyContentToLiquid(JSON.parse(value)); | |||
| processedValue[key] = this.hydrateEmailSchemaUseCase.execute({ emailEditor: value }); | |||
ChmaraX
deleted the
nv-5182-NV-5194-apiemail-for-use-liquid-for-loop-syntax-parsing-directly
branch
What changed? Why was the change needed?
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer