Skip to content
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

feat(dashboard): restore email editor 'for' block #7483

Open
wants to merge 3 commits into
base: next
Choose a base branch
from

Conversation

ChmaraX
Copy link
Contributor

@ChmaraX ChmaraX commented Jan 10, 2025

What changed? Why was the change needed?

  • DX: iterable needs to be referenced in full, e.g. when looping over {{payload.comments}}, item is {{payload.comments.id}}
  • the preview always generates 3 example items in the array
  • I've tried to restore the functionality with the least amount of code change, but in my opinion heavy refactoring is required for the entire preview

High-level overview of how the parsing flow goes so we can reference it later when refactoring:
image

for-loop.mov

Copy link

netlify bot commented Jan 10, 2025

Deploy Preview for dev-web-novu ready!

Name Link
🔨 Latest commit beae876
🔍 Latest deploy log https://app.netlify.com/sites/dev-web-novu/deploys/6782d744776b8c000847fd09
😎 Deploy Preview https://deploy-preview-7483.dashboard.novu-staging.co
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 10, 2025

Deploy Preview for dashboard-v2-novu-staging ready!

Name Link
🔨 Latest commit beae876
🔍 Latest deploy log https://app.netlify.com/sites/dashboard-v2-novu-staging/deploys/6782d744cbbb8700089c3cd6
😎 Deploy Preview https://deploy-preview-7483.dashboard-v2.novu-staging.co
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Comment on lines +43 to +60
private async traverseAndProcessNodes(
node: TipTapNode,
parentNode?: TipTapNode
): Promise<TipTapNode> {
if (node.content) {
const processedContent: TipTapNode[] = [];
for (const innerNode of node.content) {
const processed = await this.processShowAndForControls(variables, innerNode, parentNode);
if (processed) {
processedContent.push(processed);
variables: Record<string, unknown>,
parent?: TipTapNode
): Promise<void> {
const queue: Array<{ node: TipTapNode; parent?: TipTapNode }> = [{ node, parent }];

while (queue.length > 0) {
const current = queue.shift()!;
await this.processNode(current.node, variables, current.parent);

if (current.node.content) {
for (const childNode of current.node.content) {
queue.push({ node: childNode, parent: current.node });
}
}
node.content = processedContent;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworked the previously recursive function to something easier to read using queue.

Comment on lines +185 to +205
private addIndexesToPlaceholders(nodes: TipTapNode[], iterablePath: string, index: number): TipTapNode[] {
return nodes.map((node) => {
const newNode: TipTapNode = { ...node };

const parsedShowIfValue = await parseLiquid(showIfKey as string, variables);
const showIfValueBoolean =
typeof parsedShowIfValue === 'boolean' ? parsedShowIfValue : this.stringToBoolean(parsedShowIfValue);
if (this.isAVariableNode(newNode)) {
const nodePlaceholder = newNode.text as string;

/**
* example:
* iterablePath = payload.comments
* nodePlaceholder = {{ payload.comments.author }}
* text = {{ payload.comments[0].author }}
*/
newNode.text = nodePlaceholder.replace(iterablePath, `${iterablePath}[${index}]`);
newNode.type = 'text'; // set 'variable' to 'text' to for Liquid to recognize it
} else if (newNode.content) {
newNode.content = this.addIndexesToPlaceholders(newNode.content, iterablePath, index);
}

if (!showIfValueBoolean) {
this.removeNodeFromParent(node, parentNode);
} else {
delete node.attrs[MailyAttrsEnum.SHOW_IF_KEY];
}
return newNode;
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of replacing for loop values directly in this usecase as it was before, we create just placeholders {{ payload.comments[0].author }} and leave the actual value substitution for Liquid parser down the line

@@ -157,27 +214,7 @@ export class ExpandEmailEditorSchemaUsecase {
}

private isAVariableNode(newNode: TipTapNode): newNode is TipTapNode & { attrs: { id: string } } {
return newNode.type === 'payloadValue' && newNode.attrs?.id !== undefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea what is payloadValue but I was not able to find it anywhere in the TipTapNode

@@ -132,10 +132,13 @@ export const Maily = (props: MailyProps) => {
return dedupAndSortVariables(filteredVariables, queryWithoutSuffix);
}

const iterableName = editor?.getAttributes('for')?.each;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now the iterable name is the full path, e.g. - we can polish this in separate PR, it was too difficult for me to change all that at once

for each "payload.comments"
   {{payload.comments.id}}

@ChmaraX ChmaraX changed the title feat(dashboard): restore 'for' block; refactor feat(dashboard): restore email editor 'for' block Jan 10, 2025
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.

1 participant