fix(plugin-workflow): address v2 workflow regressions (#9903)

2026-08-28 17:43:07 +08:00 · 2026-06-25 16:50:29 +08:00
parent eb564591f9
commit 456654a418
12 changed files with 321 additions and 31 deletions
@@ -530,12 +530,14 @@ export function TypedVariableInput(props: TypedVariableInputProps) {
  );

  const onClearVariable = useCallback(() => {
-    if (nullable) {
-      onChange?.(null);
+    const first = normalizedTypes[0];
+    if (first) {
+      onChange?.(defaultValueFor(first.type));
      return;
    }
-    const first = normalizedTypes[0];
-    if (first) onChange?.(defaultValueFor(first.type));
+    if (nullable) {
+      onChange?.(null);
+    }
  }, [nullable, normalizedTypes, onChange]);

  const constantTypeForRendering: TypedConstantType = useMemo(() => {
@@ -619,6 +621,42 @@ export function TypedVariableInput(props: TypedVariableInputProps) {
    if (constantTypeForRendering !== 'object') setJsonError(null);
  }, [constantTypeForRendering]);

+  const variableValueClassName = useMemo(
+    () => css`
+      &:hover .clear-button,
+      &:focus-within .clear-button {
+        visibility: visible;
+        pointer-events: auto;
+        opacity: 1;
+      }
+
+      .clear-button {
+        display: inline-flex;
+        align-items: center;
+        justify-content: center;
+        width: ${token.fontSizeIcon}px;
+        height: ${token.fontSizeIcon}px;
+        padding: 0;
+        color: ${token.colorTextQuaternary};
+        background: transparent;
+        cursor: pointer;
+        visibility: hidden;
+        pointer-events: none;
+        opacity: 0;
+        transition:
+          visibility ${token.motionDurationMid} ease,
+          color ${token.motionDurationMid} ease,
+          opacity ${token.motionDurationSlow} ease;
+
+        &:hover {
+          color: ${token.colorTextTertiary};
+          background: transparent;
+        }
+      }
+    `,
+    [token],
+  );
+
  return (
    <div style={{ width: '100%' }}>
      {/* Default `Space.Compact` (align-items: stretch) so the switcher button
@@ -629,6 +667,7 @@ export function TypedVariableInput(props: TypedVariableInputProps) {
        <div style={{ flex: 1, minWidth: 0, overflow: 'hidden' }}>
          {isVariable ? (
            <div
+              className={variableValueClassName}
              role="button"
              aria-label="variable-tag"
              style={{
@@ -666,9 +705,10 @@ export function TypedVariableInput(props: TypedVariableInputProps) {
                <Button
                  type="text"
                  size="small"
+                  className="clear-button"
                  aria-label="icon-close"
                  onClick={onClearVariable}
-                  icon={<CloseCircleFilled style={{ color: token.colorTextTertiary }} />}
+                  icon={<CloseCircleFilled />}
                />
              ) : null}
            </div>
@@ -161,10 +161,10 @@ describe('TypedVariableInput - variable rendering', () => {
    });
  });

-  it('clears back to null when the close button is clicked (nullable=true)', async () => {
+  it('clears back to default-of-first-type when the close button is clicked (nullable=true)', async () => {
    const ctx = createContextWithEnv();
    const handleChange = vi.fn();
-    renderWithCtx(
+    const { container } = renderWithCtx(
      ctx,
      <TypedVariableInput
        value="{{$env.SMTP_PORT}}"
@@ -174,15 +174,17 @@ describe('TypedVariableInput - variable rendering', () => {
        onChange={handleChange}
      />,
    );
-    const clear = await screen.findByRole('button', { name: 'icon-close' });
-    fireEvent.click(clear);
-    expect(handleChange).toHaveBeenCalledWith(null);
+    const clear = container.querySelector('button.clear-button') as HTMLButtonElement | null;
+    expect(clear).not.toBeNull();
+    expect(clear).toHaveClass('clear-button');
+    fireEvent.click(clear as HTMLButtonElement);
+    expect(handleChange).toHaveBeenCalledWith(0);
  });

  it('clears back to default-of-first-type when nullable=false', async () => {
    const ctx = createContextWithEnv();
    const handleChange = vi.fn();
-    renderWithCtx(
+    const { container } = renderWithCtx(
      ctx,
      <TypedVariableInput
        value="{{$env.SMTP_PORT}}"
@@ -192,8 +194,9 @@ describe('TypedVariableInput - variable rendering', () => {
        onChange={handleChange}
      />,
    );
-    const clear = await screen.findByRole('button', { name: 'icon-close' });
-    fireEvent.click(clear);
+    const clear = container.querySelector('button.clear-button') as HTMLButtonElement | null;
+    expect(clear).not.toBeNull();
+    fireEvent.click(clear as HTMLButtonElement);
    expect(handleChange).toHaveBeenCalledWith(0);
  });
 });
@@ -205,10 +205,10 @@ function NodeConfigForm({
              // `justify-content: space-between` pushes the node-key tag to the drawer's far right (mirrors v1's flex
              // title), the native close X sitting just left of the title.
              <div style={{ display: 'flex', alignItems: 'center', justifyContent: 'space-between', width: '100%' }}>
-                <strong>{data.title ?? typeTitle}</strong>
+                <strong style={{ fontWeight: 'bold' }}>{data.title ?? typeTitle}</strong>
                <Tooltip title={t('Variable key of node')}>
                  <Tag style={{ marginInlineEnd: 0 }}>
-                    <code>{data.key}</code>
+                    <code style={{ fontWeight: 'normal' }}>{data.key}</code>
                  </Tag>
                </Tooltip>
              </div>
@@ -96,4 +96,30 @@ describe('openNodeConfigDrawer', () => {

    expect(await screen.findByTestId('legacy-workflow-collection')).toHaveTextContent('main.posts');
  });
+
+  it('keeps the drawer title weight aligned with the v1 title layout', () => {
+    const drawer = vi.fn();
+
+    holder.ctx = {
+      api: {
+        resource: () => ({ update: vi.fn() }),
+      },
+    };
+
+    openNodeConfigDrawer({
+      ctx: { viewer: { drawer } },
+      data: { id: 9, key: 'node_9', title: 'Webhook response', type: 'update', config: {} },
+      instruction: {
+        title: 'Webhook response',
+      },
+      t: (key: string) => key,
+      workflow: { id: 1, config: {} },
+    });
+
+    const content = drawer.mock.calls[0][0].content as () => React.ReactElement;
+    renderWithApp(content());
+
+    expect(screen.getByText('Webhook response')).toHaveStyle({ fontWeight: 'bold' });
+    expect(screen.getByText('node_9')).toHaveStyle({ fontWeight: 'normal' });
+  });
 });
@@ -99,7 +99,12 @@ function useOperandMetaTree(): MetaTreeNode[] {

 function Calculation({ calculator, operands = [], onChange }: any) {
  const compile = useT();
-  const metaTree = useOperandMetaTree();
+  // Keep the left/right operands on separate meta-tree instances. `TypedVariableInput`
+  // lazily resolves relation children by mutating its `metaTree` in place; sharing one
+  // tree between both sides can leave the other cascader stuck on a stale loading column
+  // when both operands walk the same workflow-variable branch.
+  const leftMetaTree = useOperandMetaTree();
+  const rightMetaTree = useOperandMetaTree();
  const leftOperandOnChange = useCallback(
    (v: unknown) => onChange({ calculator, operands: [v, operands[1]] }),
    [calculator, onChange, operands],
@@ -127,7 +132,7 @@ function Calculation({ calculator, operands = [], onChange }: any) {
          matches v1's single-row [operand · operator · operand] layout. */}
      <TypedVariableInput
        types={OPERAND_TYPES}
-        metaTree={metaTree}
+        metaTree={leftMetaTree}
        value={operands[0]}
        onChange={leftOperandOnChange}
        style={{ flex: 1, minWidth: 0 }}
@@ -158,7 +163,7 @@ function Calculation({ calculator, operands = [], onChange }: any) {
      </Select>
      <TypedVariableInput
        types={OPERAND_TYPES}
-        metaTree={metaTree}
+        metaTree={rightMetaTree}
        value={operands[1]}
        onChange={rightOperandOnChange}
        style={{ flex: 1, minWidth: 0 }}
@@ -277,10 +277,17 @@ export function FilterDynamicComponent({
  collection,
  value,
  onChange,
+  rightAsVariable = true,
 }: {
  collection?: string;
  value?: Record<string, unknown> | null;
  onChange?: (value: Record<string, unknown> | null) => void;
+  /**
+   * Controls whether the filter row's right-hand value editor allows workflow variables.
+   * - `true`: render the RHS as a variable-aware input (constant or workflow variable)
+   * - `false`: render the RHS as a pure typed static input with no variable picker
+   */
+  rightAsVariable?: boolean;
 }) {
  const flowEngine = useFlowEngine();
  const t = useT();
@@ -365,12 +372,17 @@ export function FilterDynamicComponent({

    const Component = ({ value }: { value: VariableFilterItemValue }) => (
      <FlowModelProvider model={filterModel}>
-        <VariableFilterItem value={value} model={filterModel} rightAsVariable rightMetaTree={rightMetaTree} />
+        <VariableFilterItem
+          value={value}
+          model={filterModel}
+          rightAsVariable={rightAsVariable}
+          rightMetaTree={rightMetaTree}
+        />
      </FlowModelProvider>
    );
    Component.displayName = 'WorkflowVariableFilterItem';
    return Component;
-  }, [filterModel, rightMetaTree]);
+  }, [filterModel, rightAsVariable, rightMetaTree]);

  return <FilterGroup value={filterRef.current} FilterItem={FilterItemComponent ?? undefined} />;
 }
@@ -13,7 +13,7 @@ import { render, screen } from '@testing-library/react';
 import { AppendsSelect } from '../collection/AppendsSelect';

 const treeSelectState = vi.hoisted(() => ({
-  props: null as null | { treeData?: Array<{ title: string; value: string }> },
+  props: null as null | Record<string, any>,
 }));

 vi.mock('antd', () => {
@@ -22,7 +22,9 @@ vi.mock('antd', () => {
    return <div data-testid="tree-select" />;
  };
  TreeSelect.SHOW_PARENT = 'SHOW_PARENT';
-  return { TreeSelect };
+  TreeSelect.SHOW_ALL = 'SHOW_ALL';
+  const Tag = ({ children }: any) => <span>{children}</span>;
+  return { TreeSelect, Tag };
 });

 vi.mock('@nocobase/flow-engine', () => ({
@@ -41,6 +43,14 @@ vi.mock('@nocobase/flow-engine', () => ({
                    uiSchema: { title: '{{t("Created by")}}' },
                  },
                },
+                {
+                  options: {
+                    name: 'role',
+                    type: 'belongsTo',
+                    target: 'roles',
+                    uiSchema: { title: '{{t("Role")}}' },
+                  },
+                },
              ],
            }),
          },
@@ -68,6 +78,42 @@ describe('AppendsSelect', () => {
        title: 'Created by',
        value: 'createdBy',
      }),
+      expect.objectContaining({
+        title: 'Role',
+        value: 'role',
+      }),
    ]);
  });
+
+  it('uses strict tree checking and full-path tag rendering to match v1 appends behavior', () => {
+    render(<AppendsSelect collection="users" value={['createdBy.role']} />);
+
+    expect(treeSelectState.props?.treeCheckStrictly).toBe(true);
+    expect(treeSelectState.props?.showCheckedStrategy).toBe('SHOW_ALL');
+    expect(treeSelectState.props?.value).toEqual([{ value: 'createdBy.role', label: 'createdBy.role' }]);
+
+    const tag = treeSelectState.props?.tagRender?.({
+      value: 'createdBy.role',
+      closable: true,
+      onClose: vi.fn(),
+    });
+    const { container } = render(tag);
+    expect(container).toHaveTextContent('Created by / Role');
+  });
+
+  it('adds parent paths when a child node is selected', () => {
+    const onChange = vi.fn();
+    render(<AppendsSelect collection="users" value={[]} onChange={onChange} />);
+
+    treeSelectState.props?.onChange?.([{ value: 'createdBy.role', label: 'createdBy.role' }]);
+    expect(onChange).toHaveBeenCalledWith(['createdBy.role', 'createdBy']);
+  });
+
+  it('removes descendants when a selected parent node is unselected', () => {
+    const onChange = vi.fn();
+    render(<AppendsSelect collection="users" value={['createdBy', 'createdBy.role']} onChange={onChange} />);
+
+    treeSelectState.props?.onChange?.([{ value: 'createdBy.role', label: 'createdBy.role' }]);
+    expect(onChange).toHaveBeenCalledWith([]);
+  });
 });
@@ -0,0 +1,66 @@
+/**
+ * This file is part of the NocoBase (R) project.
+ * Copyright (c) 2020-2024 NocoBase Co., Ltd.
+ * Authors: NocoBase Team.
+ *
+ * This project is dual-licensed under AGPL-3.0 and NocoBase Commercial License.
+ * For more information, please refer to: https://www.nocobase.com/agreement.
+ */
+
+import React from 'react';
+import { render, screen } from '@testing-library/react';
+import { describe, expect, it, vi } from 'vitest';
+import { CalculationConfig } from '../Calculation';
+
+const holder = vi.hoisted(() => ({
+  typedVariableInputProps: [] as Array<{ metaTree: unknown; value: unknown }>,
+}));
+
+vi.mock('../../locale', () => ({
+  NAMESPACE: 'workflow',
+  useT: () => (key: string) => key,
+  useWorkflowTranslation: () => ({ t: (key: string) => key }),
+}));
+
+vi.mock('@nocobase/client-v2', () => ({
+  TypedVariableInput: (props: any) => {
+    holder.typedVariableInputProps.push({
+      metaTree: props.metaTree,
+      value: props.value,
+    });
+    return <div data-testid="typed-variable-input" />;
+  },
+}));
+
+vi.mock('@nocobase/evaluators/client', () => ({
+  evaluators: {
+    getEntities: () => [],
+  },
+}));
+
+describe('CalculationConfig', () => {
+  it('creates separate workflow variable trees for left and right operands', () => {
+    holder.typedVariableInputProps = [];
+    const useVariableHook = vi
+      .fn()
+      .mockImplementation(() => [{ name: '$context', title: 'Trigger variables', paths: ['$context'], type: '' }]);
+
+    render(
+      <CalculationConfig
+        useVariableHook={useVariableHook}
+        value={{
+          group: {
+            type: 'and',
+            calculations: [{ calculator: 'equal', operands: ['{{$context.data.id}}', '{{$context.data.id}}'] }],
+          },
+        }}
+        onChange={() => undefined}
+      />,
+    );
+
+    expect(screen.getAllByTestId('typed-variable-input')).toHaveLength(2);
+    expect(useVariableHook).toHaveBeenCalledTimes(2);
+    expect(holder.typedVariableInputProps).toHaveLength(2);
+    expect(holder.typedVariableInputProps[0].metaTree).not.toBe(holder.typedVariableInputProps[1].metaTree);
+  });
+});
@@ -251,4 +251,19 @@ describe('FilterDynamicComponent', () => {
      expect(onChange).toHaveBeenCalledWith({ $and: [{ title: { $eq: '{{$jobsMapByNodeKey.n1.body}}' } }] });
    });
  });
+
+  it('can disable right-side variable input for trigger-only filter usage', () => {
+    const { engine } = setupEngine();
+
+    render(
+      <FlowEngineProvider engine={engine}>
+        <FilterDynamicComponent collection="posts" value={{}} onChange={() => undefined} rightAsVariable={false} />
+      </FlowEngineProvider>,
+    );
+
+    fireEvent.click(screen.getByText('Add condition'));
+
+    expect(testState.variableFilterItems).toHaveLength(1);
+    expect(testState.variableFilterItems[0].rightAsVariable).toBe(false);
+  });
 });
@@ -8,7 +8,8 @@
 */

 import { useFlowEngine } from '@nocobase/flow-engine';
-import { TreeSelect } from 'antd';
+import { useMemoizedFn } from 'ahooks';
+import { Tag, TreeSelect } from 'antd';
 import React, { useMemo } from 'react';
 import { useT } from '../../locale';
 import {
@@ -19,7 +20,13 @@ import {
  type CollectionTriggerField,
 } from './utils';

-type AppendsTreeNode = { title: string; value: string; key: string; children?: AppendsTreeNode[] };
+type AppendsTreeNode = {
+  title: string;
+  value: string;
+  key: string;
+  fullTitle: string[];
+  children?: AppendsTreeNode[];
+};
 type CollectionDataSourceManager = Parameters<typeof getCollectionFields>[0];

 function buildAssociationTree(
@@ -28,6 +35,7 @@ function buildAssociationTree(
  collectionValue?: string,
  prefix = '',
  depth = 2,
+  parentTitles: string[] = [],
 ): AppendsTreeNode[] {
  const fields = getCollectionFields(dataSourceManager, collectionValue);
  return fields
@@ -35,23 +43,38 @@ function buildAssociationTree(
    .filter(isAssociationField)
    .map((field: CollectionTriggerField & { name: string }) => {
      const value = prefix ? `${prefix}.${field.name}` : field.name;
+      const title = field.uiSchema?.title ? compile(field.uiSchema.title) : field.name;
+      const fullTitle = [...parentTitles, title];
      const [dataSourceKey] = parseCollectionName(collectionValue) as [string, string];
      const targetCollection = field.target
        ? `${dataSourceKey && dataSourceKey !== 'main' ? `${dataSourceKey}:` : ''}${field.target}`
        : undefined;
      const children =
        depth > 1 && targetCollection
-          ? buildAssociationTree(dataSourceManager, compile, targetCollection, value, depth - 1)
+          ? buildAssociationTree(dataSourceManager, compile, targetCollection, value, depth - 1, fullTitle)
          : [];
      return {
-        title: field.uiSchema?.title ? compile(field.uiSchema.title) : field.name,
+        title,
        value,
        key: value,
+        fullTitle,
        children: children.length ? children : undefined,
      };
    });
 }

+type TreeSelectValue = { value: string; label?: React.ReactNode };
+
+function flattenTree(treeData: AppendsTreeNode[]): Record<string, AppendsTreeNode> {
+  return treeData.reduce<Record<string, AppendsTreeNode>>((result, node) => {
+    result[node.value] = node;
+    if (node.children?.length) {
+      Object.assign(result, flattenTree(node.children));
+    }
+    return result;
+  }, {});
+}
+
 export function AppendsSelect({
  collection,
  value,
@@ -67,16 +90,62 @@ export function AppendsSelect({
    () => buildAssociationTree(flowEngine.context.dataSourceManager, t, collection),
    [flowEngine, t, collection],
  );
+  const optionsMap = useMemo(() => flattenTree(treeData), [treeData]);
+  const treeValue = useMemo<TreeSelectValue[]>(
+    () => (value ?? []).map((item) => ({ value: item, label: item })).filter((item) => item.value in optionsMap),
+    [optionsMap, value],
+  );
+
+  const handleChange = useMemoizedFn((next: TreeSelectValue | TreeSelectValue[] | undefined) => {
+    const nextItems = Array.isArray(next) ? next : next ? [next] : [];
+    const nextValues = nextItems.map((item) => item.value).filter(Boolean) as string[];
+    const valueSet = new Set(nextValues);
+    const removedValue = (value ?? []).find((item) => !valueSet.has(item));
+
+    if (removedValue) {
+      const prefix = `${removedValue}.`;
+      Object.keys(optionsMap).forEach((key) => {
+        if (key.startsWith(prefix)) {
+          valueSet.delete(key);
+        }
+      });
+    } else {
+      nextValues.forEach((item) => {
+        const paths = item.split('.');
+        for (let i = 1; i <= paths.length; i++) {
+          valueSet.add(paths.slice(0, i).join('.'));
+        }
+      });
+    }
+
+    onChange?.(Array.from(valueSet));
+  });
+
+  const tagRender = useMemoizedFn(
+    (props: { value?: string; closable?: boolean; onClose?: (event?: React.MouseEvent<HTMLElement>) => void }) => {
+      const node = props.value ? optionsMap[props.value] : undefined;
+      if (!node) {
+        return null;
+      }
+      return (
+        <Tag closable={props.closable} onClose={props.onClose}>
+          {node.fullTitle.join(' / ')}
+        </Tag>
+      );
+    },
+  );

  return (
    <TreeSelect
      treeData={treeData}
-      value={value}
-      onChange={onChange}
+      value={treeValue}
+      onChange={handleChange}
      treeCheckable
-      showCheckedStrategy={TreeSelect.SHOW_PARENT}
+      treeCheckStrictly
+      showCheckedStrategy={TreeSelect.SHOW_ALL}
      placeholder={t('Preload associations')}
      treeNodeFilterProp="title"
+      tagRender={tagRender}
      allowClear
    />
  );
@@ -97,7 +97,7 @@ export function CollectionTriggerConfig() {

      {collection && !hasCollectionTriggerMode(mode, COLLECTION_TRIGGER_MODE.DELETED) ? (
        <Form.Item name={['config', 'condition']} label={t('Only triggers when match conditions')}>
-          <ConditionField collection={collection} />
+          <ConditionField collection={collection} rightAsVariable={false} />
        </Form.Item>
      ) : null}

@@ -15,6 +15,9 @@ import { FlowEngine, FlowEngineProvider } from '@nocobase/flow-engine';
 import CollectionTriggerConfig from '../CollectionConfig';

 const workflowState = vi.hoisted(() => ({ sync: true }));
+const conditionFieldState = vi.hoisted(() => ({
+  propsList: [] as any[],
+}));

 vi.mock('../../../locale', () => ({
  NAMESPACE: 'workflow',
@@ -28,7 +31,10 @@ vi.mock('../../../components/collection', () => ({
 }));

 vi.mock('../../../components/FilterDynamicComponent', () => ({
-  ConditionField: () => <div data-testid="condition-field" />,
+  ConditionField: (props: any) => {
+    conditionFieldState.propsList.push(props);
+    return <div data-testid="condition-field" />;
+  },
 }));

 vi.mock('../../../canvas/contexts', () => ({
@@ -38,6 +44,7 @@ vi.mock('../../../canvas/contexts', () => ({
 describe('CollectionTriggerConfig', () => {
  it('consumes extracted shared collection components', () => {
    workflowState.sync = true;
+    conditionFieldState.propsList = [];
    const engine = new FlowEngine();

    render(
@@ -52,6 +59,7 @@ describe('CollectionTriggerConfig', () => {
    expect(screen.getByTestId('fields-select')).toBeInTheDocument();
    expect(screen.getByTestId('condition-field')).toBeInTheDocument();
    expect(screen.getByTestId('appends-select')).toBeInTheDocument();
+    expect(conditionFieldState.propsList.at(-1)?.rightAsVariable).toBe(false);
    expect(
      screen.getByText(
        'Synchronous collection event workflows run within the trigger transaction by default. Related data operations automatically use this transaction.',