Skip to content

feat(dashboard): Invalid variable pill improvements #8079

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

Open
wants to merge 7 commits into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/dashboard/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
"launchdarkly-react-client-sdk": "^3.3.2",
"liquidjs": "^10.20.0",
"lodash.debounce": "^4.0.8",
"lodash.isempty": "^4.4.0",
"lodash.isequal": "^4.5.0",
"lodash.merge": "^4.6.2",
"lucide-react": "^0.439.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { FieldSelector } from '@/components/conditions-editor/field-selector';
import { OperatorSelector } from '@/components/conditions-editor/operator-selector';
import { RuleActions } from '@/components/conditions-editor/rule-actions';
import { ValueEditor } from '@/components/conditions-editor/value-editor';
import { IsAllowedVariable, LiquidVariable } from '@/utils/parseStepVariables';
import { IsAllowedVariable, LiquidVariable } from '@/components/variable/parseStepVariables';

const ruleActionsClassName = `[&>[data-actions="true"]]:opacity-0 [&:hover>[data-actions="true"]]:opacity-100 [&>[data-actions="true"]:has(~[data-radix-popper-content-wrapper])]:opacity-100`;
const groupActionsClassName = `[&_.ruleGroup-header>[data-actions="true"]]:opacity-0 [&_.ruleGroup-header:hover>[data-actions="true"]]:opacity-100 [&_.ruleGroup-header>[data-actions="true"]:has(~[data-radix-popper-content-wrapper])]:opacity-100`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useFormContext } from 'react-hook-form';
import { useValueEditor, ValueEditorProps } from 'react-querybuilder';

import { InputRoot } from '@/components/primitives/input';
import { IsAllowedVariable, LiquidVariable } from '@/utils/parseStepVariables';
import { IsAllowedVariable, LiquidVariable } from '@/components/variable/parseStepVariables';
import { ControlInput } from '../primitives/control-input/control-input';

export const ValueEditor = (props: ValueEditorProps) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { useCallback, useMemo, useRef } from 'react';

import { Editor } from '@/components/primitives/editor';
import { EditVariablePopover } from '@/components/variable/edit-variable-popover';
import { createAutocompleteSource } from '@/utils/liquid-autocomplete';
import { IsAllowedVariable, LiquidVariable } from '@/utils/parseStepVariables';
import { createAutocompleteSource } from '@/components/variable/utils/codemirror-autocomplete';
import { IsAllowedVariable, LiquidVariable } from '@/components/variable/parseStepVariables';
import { useVariables } from './hooks/use-variables';
import { createVariableExtension } from './variable-plugin';
import { variablePillTheme } from './variable-plugin/variable-theme';
Expand Down Expand Up @@ -91,7 +91,7 @@ export function ControlInput({
onSelect: handleVariableSelect,
isAllowedVariable,
}),
[handleVariableSelect]
[handleVariableSelect, isAllowedVariable]
);

const extensions = useMemo(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IsAllowedVariable } from '@/utils/parseStepVariables';
import { IsAllowedVariable } from '@/components/variable/parseStepVariables';
import { Decoration, DecorationSet, EditorView, Range } from '@uiw/react-codemirror';
import { MutableRefObject } from 'react';
import { VARIABLE_REGEX_STRING } from './';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IsAllowedVariable } from '@/utils/parseStepVariables';
import { IsAllowedVariable } from '@/components/variable/parseStepVariables';
import { EditorView } from '@uiw/react-codemirror';
import { MutableRefObject } from 'react';

Expand Down
4 changes: 2 additions & 2 deletions apps/dashboard/src/components/primitives/form/form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,9 @@ FormMessagePure.displayName = 'FormMessagePure';
const FormMessage = React.forwardRef<
HTMLParagraphElement,
React.HTMLAttributes<HTMLParagraphElement> & { suppressError?: boolean }
>(({ children, suppressError, ...rest }, ref) => {
>(({ children, ...rest }, ref) => {
const { error, formMessageId } = useFormField();
const content = !suppressError && error ? String(error.message) : children;
const content = error ? String(error.message) : children;
const icon = error ? RiErrorWarningFill : RiInformationLine;

const isFirstMount = useRef(true);
Expand Down
6 changes: 4 additions & 2 deletions apps/dashboard/src/components/variable/constants.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { Filters } from './types';

const FILTERS: Filters[] = [
// Text Transformations
export const FIXED_NAMESPACES_WITH_UNKNOWN_KEYS = ['subscriber.data', 'payload'];
export const DYNAMIC_NAMESPACES_WITH_UNKNOWN_KEYS_REGEX = /^steps\.(.+?)\.events\[\d+\]/;

export const FILTERS: Filters[] = [
{
label: 'Uppercase',
value: 'upcase',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Popover, PopoverTrigger } from '@/components/primitives/popover';
import { EditVariablePopoverContent } from '@/components/variable/edit-variable-popover-content';
import { IsAllowedVariable } from '@/utils/parseStepVariables';
import { IsAllowedVariable } from '@/components/variable/parseStepVariables';
import { ReactNode } from 'react';

type EditVariablePopoverProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ export interface ParsedVariables {
*/
export function parseStepVariables(schema: JSONSchemaDefinition, isEnhancedDigestEnabled: boolean): ParsedVariables {
const result: ParsedVariables = {
/**
* deprecated: use variables instead
*/
primitives: [],
arrays: [],
variables: [],
Expand All @@ -34,17 +37,14 @@ export function parseStepVariables(schema: JSONSchemaDefinition, isEnhancedDiges
if (typeof obj === 'boolean') return;

if (obj.type === 'object') {
// Handle object with additionalProperties
if (obj.additionalProperties === true) {
result.namespaces.push({
type: 'variable',
label: path,
});
}

if (!obj.properties) return;

for (const [key, value] of Object.entries(obj.properties)) {
for (const [key, value] of Object.entries(obj.properties || {})) {
const fullPath = path ? `${path}.${key}` : key;

if (typeof value === 'object') {
Expand Down Expand Up @@ -91,68 +91,37 @@ export function parseStepVariables(schema: JSONSchemaDefinition, isEnhancedDiges

extractProperties(schema);

function parseVariablePath(path: string): string[] | null {
const parts = path
.split(/\.|\[(\d+)\]/)
.filter(Boolean)
.map((part): string | null => {
const num = parseInt(part);

if (!isNaN(num)) {
if (num < 0) return null;
return num.toString();
}
function isAllowedVariable(value: string): boolean {
// Validate is variable is valid against the parsed array of variables coming from the server
const isValidFromServerResponse = !!result.variables.find((v) => v.label === value);

return part;
});

return parts.includes(null) ? null : (parts as string[]);
}

function isAllowedVariable(path: string): boolean {
if (typeof schema === 'boolean') return false;

if (result.primitives.some((primitive) => primitive.label === path)) {
if (isValidFromServerResponse) {
return true;
}

const parts = parseVariablePath(path);
if (!parts) return false;

let currentObj: JSONSchemaDefinition = schema;

for (let i = 0; i < parts.length; i++) {
const part = parts[i];
// Handle array variables and validate them against the parsed array variables.
// For example: steps.digest.events[1].payload.name is validated against steps.digest.events that is returned by the server
const isValidFromArrayNameSpace = result.arrays.some((v) => value.startsWith(v.label));

if (typeof currentObj === 'boolean' || !('type' in currentObj)) return false;

if (currentObj.type === 'array') {
const items = Array.isArray(currentObj.items) ? currentObj.items[0] : currentObj.items;
currentObj = items as JSONSchemaDefinition;
continue;
}

if (currentObj.type !== 'object') return false;

if (currentObj.additionalProperties === true) {
return true;
}
if (isValidFromArrayNameSpace) {
return true;
}

if (!currentObj.properties || !(part in currentObj.properties)) {
return false;
}
// Handle variable for payload and subscriber.data namespace such as payload.name or subscriber.data.name
const isValidFromNamespaceWithUnknownKeys = result.namespaces.some(
(v) => value.startsWith(v.label) && value !== v.label
);

currentObj = currentObj.properties[part];
if (isValidFromNamespaceWithUnknownKeys) {
return true;
}

return true;
return false;
}

return {
...result,
variables: isEnhancedDigestEnabled
? [...result.primitives, ...result.arrays, ...result.namespaces]
: [...result.primitives, ...result.namespaces],
variables: [...result.primitives, ...result.arrays],
isAllowedVariable,
};
}
Original file line number Diff line number Diff line change
@@ -1,29 +1,22 @@
import { getFilters } from '@/components/variable/constants';
import { LiquidVariable } from '@/utils/parseStepVariables';
import type { LiquidVariable } from '@/components/variable/parseStepVariables';

import { Completion, CompletionContext, CompletionResult } from '@codemirror/autocomplete';
import { EditorView } from '@uiw/react-codemirror';
import { FIXED_NAMESPACES_WITH_UNKNOWN_KEYS, DYNAMIC_NAMESPACES_WITH_UNKNOWN_KEYS_REGEX } from '../constants';

interface CompletionOption {
label: string;
type: string;
boost?: number;
}

const ROOT_PREFIXES = {
subscriber: 'subscriber.',
payload: 'payload.',
steps: 'steps.',
} as const;

const VALID_DYNAMIC_PATHS = ['subscriber.data.', 'payload.', /^steps\.[^.]+\.events\[\d+\]\.payload\./] as const;

/**
* Liquid variable autocomplete for the following patterns:
*
* 1. Payload Variables:
* Valid:
* - payload.
* - payload.user
* - payload.userId
* - payload.anyNewField (allows any new field)
* - payload.deeply.nested.field
* Invalid:
Expand Down Expand Up @@ -99,27 +92,24 @@ export const completions =
};
}

const matchingVariables = getMatchingVariables(searchText, variables);
const matchingVariables = getVariableCompletions(searchText, variables);

const options = matchingVariables.map((v) => createCompletionOption(v.label, 'variable'));

// If we have matches or we're in a valid context, show them
if (matchingVariables.length > 0 || isInsideLiquidBlock(beforeCursor)) {
return {
from: lastOpenBrace + 2,
to: pos,
options:
matchingVariables.length > 0
? matchingVariables.map((v) => createCompletionOption(v.label, 'variable'))
: variables.map((v) => createCompletionOption(v.label, 'variable')),
options,
};
}

return null;
};

function isInsideLiquidBlock(beforeCursor: string): boolean {
const lastOpenBrace = beforeCursor.lastIndexOf('{{');

return lastOpenBrace !== -1;
return beforeCursor.lastIndexOf('{{') !== -1;
}

function getContentAfterPipe(content: string): string | null {
Expand All @@ -139,73 +129,43 @@ function getFilterCompletions(afterPipe: string, isEnhancedDigestEnabled: boolea
.map((f) => createCompletionOption(f.value, 'function'));
}

function isValidDynamicPath(searchText: string): boolean {
return VALID_DYNAMIC_PATHS.some((path) =>
typeof path === 'string' ? searchText.startsWith(path) : path.test(searchText)
);
}

function validateSubscriberField(searchText: string, matches: LiquidVariable[]): LiquidVariable[] {
const parts = searchText.split('.');

if (parts.length === 2 && parts[0] === 'subscriber') {
if (!matches.some((v) => v.label === searchText)) {
return [];
}
}

return matches;
}

function validateStepId(searchText: string, variables: LiquidVariable[]): boolean {
if (!searchText.startsWith('steps.')) return true;

const stepMatch = searchText.match(/^steps\.([^.]+)/);
if (!stepMatch) return true;

const stepId = stepMatch[1];
return variables.some((v) => v.label.startsWith(`steps.${stepId}.`));
}

function getMatchingVariables(searchText: string, variables: LiquidVariable[]): LiquidVariable[] {
function getVariableCompletions(searchText: string, variables: LiquidVariable[]): LiquidVariable[] {
if (!searchText) return variables;

const searchLower = searchText.toLowerCase();

// Handle root prefixes and their partials
for (const [root, prefix] of Object.entries(ROOT_PREFIXES)) {
if (searchLower.startsWith(root) || root.startsWith(searchLower)) {
let matches = variables.filter((v) => v.label.startsWith(prefix));

// Special handling for subscriber fields
if (prefix === 'subscriber.') {
matches = validateSubscriberField(searchText, matches);
}
const dynamicNamespacesWithUnknownKeys = variables.reduce<string[]>((acc, entry) => {
const match = entry.label.match(DYNAMIC_NAMESPACES_WITH_UNKNOWN_KEYS_REGEX);

// Allow new paths for dynamic paths
if (isValidDynamicPath(searchText)) {
if (!matches.some((v) => v.label === searchText)) {
matches.push({ label: searchText, type: 'variable' } as LiquidVariable);
}
}
if (match) {
acc.push(`${match[0]}.payload`);
}

return matches;
return acc;
}, []);

const dynamicVariables = [...FIXED_NAMESPACES_WITH_UNKNOWN_KEYS, ...dynamicNamespacesWithUnknownKeys].reduce<
LiquidVariable[]
>((acc, namespace) => {
if (searchText.startsWith(namespace) && searchText !== namespace) {
// Ensure that if the user types payload.foo the first suggestion is payload.foo
acc.push({ label: searchText, type: 'variable' });
} else if (!searchText.startsWith(namespace)) {
// For all other values, suggest payload.whatever, subscriber.data.whatever
acc.push({
label: `${namespace}.${searchLower.trim()}`,
type: 'variable',
});
}
}

// Handle dot endings
if (searchText.endsWith('.')) {
const prefix = searchText.slice(0, -1);
return variables.filter((v) => v.label.startsWith(prefix));
}
return acc;
}, []);

// Validate step ID exists
if (!validateStepId(searchText, variables)) {
return [];
}
const uniqueVariables = Array.from(
new Map([...variables, ...dynamicVariables].map((item) => [item.label, item])).values()
);

// Default case: show any variables containing the search text
return variables.filter((v) => v.label.toLowerCase().includes(searchLower));
return uniqueVariables.filter((v) => v.label.toLowerCase().includes(searchLower));
}

export function createAutocompleteSource(variables: LiquidVariable[], isEnhancedDigestEnabled: boolean) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Editor } from '@maily-to/core';

import type { Editor as TiptapEditor } from '@tiptap/core';
import { HTMLAttributes, useCallback, useMemo, useState } from 'react';
import { FeatureFlagsKeysEnum } from '@novu/shared';
Expand All @@ -23,6 +22,7 @@ export const Maily = ({ value, onChange, className, ...rest }: MailyProps) => {
const { step } = useWorkflow();
const isEnhancedDigestEnabled = useFeatureFlag(FeatureFlagsKeysEnum.IS_ENHANCED_DIGEST_ENABLED);
const parsedVariables = useParseVariables(step?.variables);

const primitives = useMemo(
() => parsedVariables.primitives.map((v) => ({ name: v.label, required: false })),
[parsedVariables.primitives]
Expand Down
Loading
Loading