Skip to content

Commit cbeec45

Browse files
authored
fix: detect nested variable references and hide tracked properties on triggers (#8804)
- Fix hasVariableReference regex to detect variables() calls nested inside other functions like substring(), length(), concat(), etc. Previously the regex required @ directly before variables, missing patterns like @{substring(variables('x'), ...)}. - Prevent tracked properties from showing on triggers. The backend never supports tracked properties on triggers regardless of what the manifest declares. - Added unit tests for nested variable reference patterns in both designer v1 and v2. Changes mirrored in both libs/designer and libs/designer-v2.
1 parent 52a9e4c commit cbeec45

File tree

6 files changed

+120
-10
lines changed

6 files changed

+120
-10
lines changed

libs/designer-v2/src/lib/core/actions/bjsworkflow/settings.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -848,12 +848,18 @@ const getTrackedProperties = (isTrigger: boolean, manifest?: OperationManifest,
848848
};
849849

850850
const areTrackedPropertiesSupported = (isTrigger: boolean, manifest?: OperationManifest): boolean => {
851+
// Tracked properties are never supported for triggers (backend limitation),
852+
// regardless of what the manifest declares. See Azure/logicapps#798.
853+
if (isTrigger) {
854+
return false;
855+
}
856+
851857
if (manifest) {
852858
const setting = getOperationSettingFromManifest(manifest, 'trackedProperties') as OperationManifestSetting<void> | undefined;
853859
return isSettingSupportedFromOperationManifest(setting, isTrigger);
854860
}
855861

856-
return !isTrigger;
862+
return true;
857863
};
858864

859865
const getSecureInputsSetting = (definition?: LogicAppsV2.OperationDefinition): boolean => {

libs/designer-v2/src/lib/core/parsers/ParseReduxAction.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,13 +208,17 @@ export const detectSequentialInitializeVariables = (definition: LogicAppsV2.Work
208208
};
209209

210210
/**
211-
* Checks if a variable value contains references to other variables
212-
* using patterns: @{variable({name})} or @variable({name})
211+
* Checks if a variable value contains references to other variables.
212+
* Detects variables() calls in any context including:
213+
* - Direct references: @variables('name') or @{variables('name')}
214+
* - Nested in expressions: @{substring(variables('name'), ...)}
215+
* - Inside other functions: @{length(variables('name'))}
213216
*/
214217
export const hasVariableReference = (value: any): boolean => {
215218
if (typeof value === 'string') {
216-
// Check for @{variable(...)} or @variable(...) patterns
217-
const variableReferencePattern = /@\{?variables?\s*\([^)]*\)\}?/i;
219+
// Check for variable(s)(...) anywhere in the string, including nested in expressions.
220+
// This catches @variable('x'), @variables('x'), @{variables('x')}, and @{substring(variables('x'), ...)} etc.
221+
const variableReferencePattern = /\bvariables?\s*\(/i;
218222
return variableReferencePattern.test(value);
219223
}
220224
if (Array.isArray(value)) {

libs/designer-v2/src/lib/core/parsers/__test__/combineSequentialInitializeVariables.spec.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,26 @@ describe('combineSequentialInitializeVariables', () => {
8181
const value = "prefix @{variables('test')} suffix";
8282
expect(hasVariableReference(value)).toBe(true);
8383
});
84+
85+
it('should detect variables() nested inside other functions (issue #1447)', () => {
86+
const value = "@{substring(variables('alert-resource-name'), 0, sub(length(variables('alert-resource-name')), 4)) }";
87+
expect(hasVariableReference(value)).toBe(true);
88+
});
89+
90+
it('should detect variables() inside length()', () => {
91+
const value = "@{length(variables('myList'))}";
92+
expect(hasVariableReference(value)).toBe(true);
93+
});
94+
95+
it('should detect variables() inside concat()', () => {
96+
const value = "@{concat('prefix-', variables('name'), '-suffix')}";
97+
expect(hasVariableReference(value)).toBe(true);
98+
});
99+
100+
it('should detect variables() inside toLower()', () => {
101+
const value = "@{toLower(variables('myVar'))}";
102+
expect(hasVariableReference(value)).toBe(true);
103+
});
84104
});
85105

86106
describe('sequential variable combining', () => {
@@ -345,6 +365,31 @@ describe('combineSequentialInitializeVariables', () => {
345365
expect(result.actions!.Initialize_var2).toBeDefined();
346366
});
347367

368+
it('should NOT combine when variable references another variable inside nested functions (issue #1447)', () => {
369+
const definition = createBaseWorkflowDefinition();
370+
definition.actions = {
371+
Initialize_var1: createInitializeVariableAction([{ name: 'alert-resource-name', type: 'string', value: 'thhhhhhhhhhhhhhhhhhhhhhhh' }]),
372+
Initialize_var2: createInitializeVariableAction(
373+
[
374+
{
375+
name: 'alert-logic-app-name',
376+
type: 'string',
377+
value: "@{substring(variables('alert-resource-name'), 0, sub(length(variables('alert-resource-name')), 4)) }",
378+
},
379+
],
380+
{
381+
Initialize_var1: ['SUCCEEDED'],
382+
}
383+
),
384+
};
385+
386+
const result = combineSequentialInitializeVariables(definition);
387+
expect(result.actions).toBeDefined();
388+
expect(Object.keys(result.actions!)).toHaveLength(2);
389+
expect(result.actions!.Initialize_var1).toBeDefined();
390+
expect(result.actions!.Initialize_var2).toBeDefined();
391+
});
392+
348393
it('should handle multiple parallel branches from same parent', () => {
349394
const definition = createBaseWorkflowDefinition();
350395
definition.actions = {

libs/designer/src/lib/core/actions/bjsworkflow/settings.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -848,12 +848,18 @@ const getTrackedProperties = (isTrigger: boolean, manifest?: OperationManifest,
848848
};
849849

850850
const areTrackedPropertiesSupported = (isTrigger: boolean, manifest?: OperationManifest): boolean => {
851+
// Tracked properties are never supported for triggers (backend limitation),
852+
// regardless of what the manifest declares. See Azure/logicapps#798.
853+
if (isTrigger) {
854+
return false;
855+
}
856+
851857
if (manifest) {
852858
const setting = getOperationSettingFromManifest(manifest, 'trackedProperties') as OperationManifestSetting<void> | undefined;
853859
return isSettingSupportedFromOperationManifest(setting, isTrigger);
854860
}
855861

856-
return !isTrigger;
862+
return true;
857863
};
858864

859865
const getSecureInputsSetting = (definition?: LogicAppsV2.OperationDefinition): boolean => {

libs/designer/src/lib/core/parsers/ParseReduxAction.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,17 @@ export const detectSequentialInitializeVariables = (definition: LogicAppsV2.Work
205205
};
206206

207207
/**
208-
* Checks if a variable value contains references to other variables
209-
* using patterns: @{variable({name})} or @variable({name})
208+
* Checks if a variable value contains references to other variables.
209+
* Detects variables() calls in any context including:
210+
* - Direct references: @variables('name') or @{variables('name')}
211+
* - Nested in expressions: @{substring(variables('name'), ...)}
212+
* - Inside other functions: @{length(variables('name'))}
210213
*/
211214
export const hasVariableReference = (value: any): boolean => {
212215
if (typeof value === 'string') {
213-
// Check for @{variable(...)} or @variable(...) patterns
214-
const variableReferencePattern = /@\{?variables?\s*\([^)]*\)\}?/i;
216+
// Check for variable(s)(...) anywhere in the string, including nested in expressions.
217+
// This catches @variable('x'), @variables('x'), @{variables('x')}, and @{substring(variables('x'), ...)} etc.
218+
const variableReferencePattern = /\bvariables?\s*\(/i;
215219
return variableReferencePattern.test(value);
216220
}
217221
if (Array.isArray(value)) {

libs/designer/src/lib/core/parsers/__test__/combineSequentialInitializeVariables.spec.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,26 @@ describe('combineSequentialInitializeVariables', () => {
8181
const value = "prefix @{variables('test')} suffix";
8282
expect(hasVariableReference(value)).toBe(true);
8383
});
84+
85+
it('should detect variables() nested inside other functions (issue #1447)', () => {
86+
const value = "@{substring(variables('alert-resource-name'), 0, sub(length(variables('alert-resource-name')), 4)) }";
87+
expect(hasVariableReference(value)).toBe(true);
88+
});
89+
90+
it('should detect variables() inside length()', () => {
91+
const value = "@{length(variables('myList'))}";
92+
expect(hasVariableReference(value)).toBe(true);
93+
});
94+
95+
it('should detect variables() inside concat()', () => {
96+
const value = "@{concat('prefix-', variables('name'), '-suffix')}";
97+
expect(hasVariableReference(value)).toBe(true);
98+
});
99+
100+
it('should detect variables() inside toLower()', () => {
101+
const value = "@{toLower(variables('myVar'))}";
102+
expect(hasVariableReference(value)).toBe(true);
103+
});
84104
});
85105

86106
describe('sequential variable combining', () => {
@@ -345,6 +365,31 @@ describe('combineSequentialInitializeVariables', () => {
345365
expect(result.actions!.Initialize_var2).toBeDefined();
346366
});
347367

368+
it('should NOT combine when variable references another variable inside nested functions (issue #1447)', () => {
369+
const definition = createBaseWorkflowDefinition();
370+
definition.actions = {
371+
Initialize_var1: createInitializeVariableAction([{ name: 'alert-resource-name', type: 'string', value: 'thhhhhhhhhhhhhhhhhhhhhhhh' }]),
372+
Initialize_var2: createInitializeVariableAction(
373+
[
374+
{
375+
name: 'alert-logic-app-name',
376+
type: 'string',
377+
value: "@{substring(variables('alert-resource-name'), 0, sub(length(variables('alert-resource-name')), 4)) }",
378+
},
379+
],
380+
{
381+
Initialize_var1: ['SUCCEEDED'],
382+
}
383+
),
384+
};
385+
386+
const result = combineSequentialInitializeVariables(definition);
387+
expect(result.actions).toBeDefined();
388+
expect(Object.keys(result.actions!)).toHaveLength(2);
389+
expect(result.actions!.Initialize_var1).toBeDefined();
390+
expect(result.actions!.Initialize_var2).toBeDefined();
391+
});
392+
348393
it('should handle multiple parallel branches from same parent', () => {
349394
const definition = createBaseWorkflowDefinition();
350395
definition.actions = {

0 commit comments

Comments
 (0)