Skip to content

Commit 36def1b

Browse files
Refactor step name generation to use alphabetical indexing (#1646)
<!-- Ensure the title clearly reflects what was changed. Provide a clear and concise description of the changes made. The PR should only contain the changes related to the issue, and no other unrelated changes. --> Fixes OPS-3053.
1 parent 56168ff commit 36def1b

File tree

5 files changed

+271
-200
lines changed

5 files changed

+271
-200
lines changed

packages/react-ui/src/app/features/builder/blocks-selector/block-selector-utils.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import {
1313
Trigger,
1414
TriggerType,
1515
} from '@openops/shared';
16-
1716
import { createDefaultOptionSettings } from '../step-settings/split-settings/utils';
1817

1918
const defaultCode = `export const code = async (inputs) => {
@@ -37,13 +36,7 @@ const getStepName = (block: StepMetadata, flowVersion: FlowVersion) => {
3736
if (block.type === TriggerType.BLOCK) {
3837
return 'trigger';
3938
}
40-
const baseName = 'step_';
41-
let number = 1;
42-
const steps = flowHelper.getAllSteps(flowVersion.trigger);
43-
while (steps.some((step) => step.name === `${baseName}${number}`)) {
44-
number++;
45-
}
46-
return `${baseName}${number}`;
39+
return flowHelper.findAvailableStepName(flowVersion, 'step');
4740
};
4841

4942
const getDefaultStep = ({

packages/react-ui/src/app/features/builder/tests/update-flow-version.test.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ describe('updateFlowVersion', () => {
5252
valid: false,
5353
displayName: 'Select Trigger',
5454
nextAction: {
55-
id: 'step_1',
56-
name: 'step_1',
55+
id: 'step_a',
56+
name: 'step_a',
5757
type: ActionType.BLOCK,
5858
valid: false,
5959
displayName: 'step1',
@@ -68,7 +68,7 @@ describe('updateFlowVersion', () => {
6868
valid: false,
6969
state: FlowVersionState.DRAFT,
7070
},
71-
selectedStep: 'step_1',
71+
selectedStep: 'step_a',
7272
rightSidebar: RightSideBarType.BLOCK_SETTINGS,
7373
readonly: false,
7474
loopsIndexes: {},
@@ -124,7 +124,7 @@ describe('updateFlowVersion', () => {
124124
const operation = {
125125
type: FlowOperationType.UPDATE_ACTION,
126126
request: {
127-
name: 'step_1',
127+
name: 'step_a',
128128
type: 'BLOCK',
129129
valid: false,
130130
settings: { blockVersion: '1.0.0' },
@@ -160,7 +160,7 @@ describe('updateFlowVersion', () => {
160160
valid: false,
161161
displayName: 'Select Trigger',
162162
nextAction: {
163-
name: 'step_1',
163+
name: 'step_a',
164164
valid: false,
165165
type: 'BLOCK',
166166
displayName: 'Google Cloud CLI',
@@ -175,7 +175,7 @@ describe('updateFlowVersion', () => {
175175
it('should handle delete action and clear selection when deleting selected step', async () => {
176176
const operation = {
177177
type: FlowOperationType.DELETE_ACTION,
178-
request: { name: 'step_1' },
178+
request: { name: 'step_a' },
179179
} as FlowOperationRequest;
180180

181181
updateFlowVersion(mockState, operation, mockOnError, mockSet);
@@ -189,26 +189,26 @@ describe('updateFlowVersion', () => {
189189
it('should handle duplicate action selecting new step', async () => {
190190
const operation = {
191191
type: FlowOperationType.DUPLICATE_ACTION,
192-
request: { stepName: 'step_1' },
192+
request: { stepName: 'step_a' },
193193
} as FlowOperationRequest;
194194

195195
updateFlowVersion(mockState, operation, mockOnError, mockSet);
196196

197197
await waitFor(() => expect(mockSet).toHaveBeenCalledWith({ saving: true }));
198198
await waitFor(() =>
199-
expect(mockSet).toHaveBeenCalledWith({ selectedStep: 'step_2' }),
199+
expect(mockSet).toHaveBeenCalledWith({ selectedStep: 'step_b' }),
200200
);
201201
});
202202

203203
it('should call stepTestOutputCache.clearStep when deleting a step', async () => {
204204
const operation = {
205205
type: FlowOperationType.DELETE_ACTION,
206-
request: { name: 'step_1' },
206+
request: { name: 'step_a' },
207207
} as FlowOperationRequest;
208208
const clearStepSpy = jest.spyOn(stepTestOutputCache, 'clearStep');
209209
updateFlowVersion(mockState, operation, mockOnError, mockSet);
210210

211-
expect(clearStepSpy).toHaveBeenCalledWith('step_1');
211+
expect(clearStepSpy).toHaveBeenCalledWith('step_a');
212212
clearStepSpy.mockRestore();
213213
});
214214
});

packages/shared/src/lib/flows/flow-helper.ts

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ type GetStepFromSubFlow = {
4646
const actionSchemaValidator = TypeCompiler.Compile(SingleActionSchema);
4747
const triggerSchemaValidation = TypeCompiler.Compile(TriggerWithOptionalId);
4848

49+
const ALPHABET_LENGTH = 26;
50+
const A_CODE_POINT = 97;
51+
4952
export function buildBlockActionKey(
5053
blockName: string,
5154
actionName: string,
@@ -1096,16 +1099,44 @@ function doesActionHaveChildren(
10961099
return false;
10971100
}
10981101

1102+
/**
1103+
* Converts a zero-based numeric index into a lowercase alphabetical label.
1104+
* Similar to spreadsheet column naming but using lowercase letters:
1105+
* 0 -> "a", 25 -> "z", 26 -> "aa", 27 -> "ab", and so on.
1106+
*
1107+
* @param index - Zero-based integer to convert.
1108+
* @returns Alphabetical label corresponding to the index.
1109+
* @example
1110+
* indexToAlphabetical(0) // "a"
1111+
* indexToAlphabetical(25) // "z"
1112+
* indexToAlphabetical(26) // "aa"
1113+
* indexToAlphabetical(701) // "zz"
1114+
*/
1115+
function indexToAlphabetical(index: number): string {
1116+
let n = index + 1;
1117+
let result = '';
1118+
while (n > 0) {
1119+
n--;
1120+
result =
1121+
String.fromCodePoint(A_CODE_POINT + (n % ALPHABET_LENGTH)) + result;
1122+
n = Math.floor(n / ALPHABET_LENGTH);
1123+
}
1124+
return result;
1125+
}
1126+
10991127
function findUnusedName(names: string[], stepPrefix: string): string {
1100-
let availableNumber = 1;
1101-
let availableName = `${stepPrefix}_${availableNumber}`;
1128+
const prefix = `${stepPrefix}_`;
1129+
const tails = new Set(
1130+
names
1131+
.filter((n) => n.startsWith(prefix))
1132+
.map((n) => n.slice(prefix.length)),
1133+
);
11021134

1103-
while (names.includes(availableName)) {
1104-
availableNumber++;
1105-
availableName = `${stepPrefix}_${availableNumber}`;
1135+
let index = 0;
1136+
while (tails.has(indexToAlphabetical(index))) {
1137+
index++;
11061138
}
1107-
1108-
return availableName;
1139+
return `${stepPrefix}_${indexToAlphabetical(index)}`;
11091140
}
11101141

11111142
function findAvailableStepName(
@@ -1286,4 +1317,6 @@ export const flowHelper = {
12861317
getUsedConnections,
12871318
createTrigger,
12881319
addStepIndices,
1320+
indexToAlphabetical,
1321+
findUnusedName,
12891322
};

packages/shared/src/lib/flows/test/flow-helper.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,51 @@ import { Action } from '../actions/action';
22
import { flowHelper } from '../flow-helper';
33

44
describe('flowHelper', () => {
5+
describe('indexToAlphabetical', () => {
6+
it.each<[number, string]>([
7+
[0, 'a'],
8+
[25, 'z'],
9+
[26, 'aa'],
10+
[27, 'ab'],
11+
[51, 'az'],
12+
[52, 'ba'],
13+
[701, 'zz'],
14+
[702, 'aaa'],
15+
])('converts %d to %s', (input, expected) => {
16+
expect(flowHelper.indexToAlphabetical(input)).toBe(expected);
17+
});
18+
});
19+
20+
describe('findUnusedName', () => {
21+
it('should return the first alphabetical suffix when none exist', () => {
22+
const names: string[] = [];
23+
expect(flowHelper.findUnusedName(names, 'step')).toBe('step_a');
24+
});
25+
26+
it('should fill the first gap in the existing names', () => {
27+
const names = ['step_a', 'step_b', 'step_d'];
28+
expect(flowHelper.findUnusedName(names, 'step')).toBe('step_c');
29+
});
30+
31+
it('should skip unrelated names and prefixes', () => {
32+
const names = ['step_a', 'other_a', 'step_b'];
33+
expect(flowHelper.findUnusedName(names, 'step')).toBe('step_c');
34+
});
35+
36+
it('should roll over to double letters after z', () => {
37+
const names = Array.from(
38+
{ length: 26 },
39+
(_, i) => `step_${flowHelper.indexToAlphabetical(i)}`,
40+
);
41+
expect(flowHelper.findUnusedName(names, 'step')).toBe('step_aa');
42+
});
43+
44+
it('should work with larger gaps and mixed ordering', () => {
45+
const names = ['step_aa', 'step_c', 'step_a', 'step_ab'];
46+
expect(flowHelper.findUnusedName(names, 'step')).toBe('step_b');
47+
});
48+
});
49+
550
describe('truncateFlow', () => {
651
it("should return the same step if it's already the last step", () => {
752
const step = { name: 'A', nextAction: undefined } as Action;

0 commit comments

Comments
 (0)