Skip to content

Commit c51fab0

Browse files
feat: Use cron input in start workflow form (#1040)
* Create cron schedule input Signed-off-by: Assem Hafez <[email protected]> * use cron input in start form Signed-off-by: Assem Hafez <[email protected]> * add trigger to test cases Signed-off-by: Assem Hafez <[email protected]> * fix test cases Signed-off-by: Assem Hafez <[email protected]> * refactor popover and other cron input components * address comments Signed-off-by: Assem Hafez <[email protected]> * fix ranges to match backend * fix test cases * fix typecheck * fix unit test * fix unit tests --------- Signed-off-by: Assem Hafez <[email protected]> Co-authored-by: Adhitya Mamallan <[email protected]>
1 parent b344d57 commit c51fab0

File tree

20 files changed

+308
-71
lines changed

20 files changed

+308
-71
lines changed

src/components/cron-schedule-input/cron-schedule-input-popover/__tests__/cron-schedule-input-popover.test.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,14 @@ describe(CronScheduleInputPopover.name, () => {
2828
setup({ fieldType: 'daysOfMonth' });
2929

3030
expect(screen.getByText('Day of Month')).toBeInTheDocument();
31-
expect(screen.getByText('0-31')).toBeInTheDocument();
31+
expect(screen.getByText('1-31')).toBeInTheDocument();
3232
});
3333

3434
it('should render with months field type and show month aliases', () => {
3535
setup({ fieldType: 'months' });
3636

3737
expect(screen.getByText('Month')).toBeInTheDocument();
38-
expect(screen.getByText('0-12')).toBeInTheDocument();
38+
expect(screen.getByText('1-12')).toBeInTheDocument();
3939
expect(screen.getByText('JAN-DEC')).toBeInTheDocument();
4040
expect(screen.getByText('alternative single values')).toBeInTheDocument();
4141
});
@@ -82,8 +82,8 @@ describe(CronScheduleInputPopover.name, () => {
8282
const testCases = [
8383
{ fieldType: 'minutes' as const, expectedRange: '0-59' },
8484
{ fieldType: 'hours' as const, expectedRange: '0-23' },
85-
{ fieldType: 'daysOfMonth' as const, expectedRange: '0-31' },
86-
{ fieldType: 'months' as const, expectedRange: '0-12' },
85+
{ fieldType: 'daysOfMonth' as const, expectedRange: '1-31' },
86+
{ fieldType: 'months' as const, expectedRange: '1-12' },
8787
{ fieldType: 'daysOfWeek' as const, expectedRange: '0-6' },
8888
];
8989

src/utils/cron-validate/__tests__/cron-validate.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,11 @@ describe('cronValidate', () => {
4848
maxValue: 23,
4949
});
5050
expect(CRON_VALIDATE_CADENCE_PRESET.daysOfMonth).toEqual({
51-
minValue: 0,
51+
minValue: 1,
5252
maxValue: 31,
5353
});
5454
expect(CRON_VALIDATE_CADENCE_PRESET.months).toEqual({
55-
minValue: 0,
55+
minValue: 1,
5656
maxValue: 12,
5757
});
5858
expect(CRON_VALIDATE_CADENCE_PRESET.daysOfWeek).toEqual({

src/utils/cron-validate/cron-validate.constants.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ export const CRON_VALIDATE_CADENCE_PRESET = {
2626
maxValue: 23,
2727
},
2828
daysOfMonth: {
29-
minValue: 0,
29+
minValue: 1, // starting from 1 instead of 0, 0 is not standard
3030
maxValue: 31,
3131
},
3232
months: {
33-
minValue: 0,
33+
minValue: 1, // starting from 1 instead of 0, 0 is not standard
3434
maxValue: 12,
3535
},
3636
daysOfWeek: {

src/views/workflow-actions/workflow-action-reset-form/__tests__/workflow-action-reset-form.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ function TestWrapper({ formErrors, formData }: TestProps) {
192192

193193
return (
194194
<WorkflowActionResetForm
195+
trigger={methods.trigger}
195196
control={methods.control}
196197
clearErrors={methods.clearErrors}
197198
fieldErrors={formErrors}

src/views/workflow-actions/workflow-action-signal-form/__tests__/workflow-action-signal-form.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ function TestWrapper({ formErrors, formData }: TestProps) {
8080

8181
return (
8282
<WorkflowActionSignalForm
83+
trigger={methods.trigger}
8384
control={methods.control}
8485
clearErrors={methods.clearErrors}
8586
fieldErrors={formErrors}

src/views/workflow-actions/workflow-action-start-form/__tests__/workflow-action-start-form.test.tsx

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,26 @@ describe('WorkflowActionStartForm', () => {
100100
);
101101

102102
await user.click(screen.getByRole('radio', { name: 'Cron' }));
103-
expect(
104-
screen.getByRole('textbox', { name: 'Cron Schedule (UTC)' })
105-
).toHaveAttribute('aria-invalid', 'true');
103+
expect(screen.getByLabelText('Minute')).toHaveAttribute(
104+
'aria-invalid',
105+
'true'
106+
);
107+
expect(screen.getByLabelText('Hour')).toHaveAttribute(
108+
'aria-invalid',
109+
'true'
110+
);
111+
expect(screen.getByLabelText('Day of Month')).toHaveAttribute(
112+
'aria-invalid',
113+
'true'
114+
);
115+
expect(screen.getByLabelText('Month')).toHaveAttribute(
116+
'aria-invalid',
117+
'true'
118+
);
119+
expect(screen.getByLabelText('Day of Week')).toHaveAttribute(
120+
'aria-invalid',
121+
'true'
122+
);
106123
});
107124

108125
it('handles input changes correctly', async () => {
@@ -201,6 +218,7 @@ function TestWrapper({ formErrors, formData }: TestProps) {
201218

202219
return (
203220
<WorkflowActionStartForm
221+
trigger={methods.trigger}
204222
control={methods.control}
205223
clearErrors={methods.clearErrors}
206224
fieldErrors={formErrors}
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import { cronValidate } from '@/utils/cron-validate/cron-validate';
2+
3+
import { getCronFieldsError } from '../get-cron-fields-error';
4+
5+
jest.mock('@/utils/cron-validate/cron-validate');
6+
7+
const mockCronValidate = cronValidate as jest.MockedFunction<
8+
typeof cronValidate
9+
>;
10+
11+
type CronValidateResult = ReturnType<typeof cronValidate>;
12+
13+
describe('getCronFieldsError', () => {
14+
beforeEach(() => {
15+
jest.clearAllMocks();
16+
});
17+
18+
it('should return null for valid cron expression', () => {
19+
const { result } = setup({
20+
isValid: true,
21+
});
22+
23+
expect(result).toBeNull();
24+
});
25+
26+
it('should handle field-specific errors correctly', () => {
27+
const { result } = setup({
28+
isValid: false,
29+
errors: [
30+
'Invalid value for minutes field: x',
31+
'Invalid value for hours field: x',
32+
'Invalid value for daysOfMonth field: x',
33+
'Invalid value for months field: x',
34+
'Invalid value for daysOfWeek field: x',
35+
],
36+
});
37+
38+
expect(result).toEqual({
39+
minutes: 'Invalid value for minutes field: x',
40+
hours: 'Invalid value for hours field: x',
41+
daysOfWeek: 'Invalid value for daysOfWeek field: x',
42+
daysOfMonth: 'Invalid value for daysOfMonth field: x',
43+
months: 'Invalid value for months field: x',
44+
});
45+
});
46+
47+
it('should return general error when errors array has no field-specific matches', () => {
48+
const { result } = setup({
49+
isValid: false,
50+
errors: ['Malformed cron expression', 'Cannot parse input'],
51+
});
52+
53+
expect(result).toEqual({
54+
general: 'Malformed cron expression',
55+
});
56+
});
57+
58+
it('should handle mixed field-specific and general errors', () => {
59+
const { result } = setup({
60+
isValid: false,
61+
errors: [
62+
'Invalid value for minutes field: abc',
63+
'Syntax error in expression',
64+
'Invalid value for hours field: xyz',
65+
],
66+
});
67+
68+
// Should prioritize field-specific errors over general ones
69+
expect(result).toEqual({
70+
minutes: 'Invalid value for minutes field: abc',
71+
hours: 'Invalid value for hours field: xyz',
72+
});
73+
});
74+
});
75+
76+
function setup({
77+
cronText = '* * * * *',
78+
isValid = true,
79+
errors = [],
80+
}: {
81+
cronText?: string;
82+
isValid?: boolean;
83+
errors?: string[];
84+
}) {
85+
const mockCronResult: Partial<CronValidateResult> = {
86+
isValid: jest.fn(() => isValid),
87+
getError: jest.fn(() => errors),
88+
};
89+
90+
mockCronValidate.mockReturnValue(mockCronResult as CronValidateResult);
91+
92+
const result = getCronFieldsError(cronText);
93+
94+
return {
95+
result,
96+
mockCronResult,
97+
mockIsValid: mockCronResult.isValid,
98+
mockGetError: mockCronResult.getError,
99+
};
100+
}

src/views/workflow-actions/workflow-action-start-form/helpers/__tests__/transform-start-workflow-form-to-submission.test.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,13 @@ describe('transformStartWorkflowFormToSubmission', () => {
3737
const formData: StartWorkflowFormData = {
3838
...baseFormData,
3939
scheduleType: 'CRON',
40-
cronSchedule: '0 9 * * 1-5',
40+
cronSchedule: {
41+
minutes: '0',
42+
hours: '9',
43+
daysOfMonth: '*',
44+
months: '*',
45+
daysOfWeek: '1-5',
46+
},
4147
};
4248

4349
const result = transformStartWorkflowFormToSubmission(formData);
@@ -50,7 +56,13 @@ describe('transformStartWorkflowFormToSubmission', () => {
5056
...baseFormData,
5157
scheduleType: 'NOW',
5258
firstRunAt: '2024-01-01T10:00:00Z',
53-
cronSchedule: '0 9 * * 1-5',
59+
cronSchedule: {
60+
minutes: '0',
61+
hours: '9',
62+
daysOfMonth: '*',
63+
months: '*',
64+
daysOfWeek: '1-5',
65+
},
5466
};
5567

5668
const result = transformStartWorkflowFormToSubmission(formData);
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { CRON_FIELD_ORDER } from '@/components/cron-schedule-input/cron-schedule-input.constants';
2+
import { cronValidate } from '@/utils/cron-validate/cron-validate';
3+
import { type CronData } from '@/utils/cron-validate/cron-validate.types';
4+
5+
import { type CronFieldsError } from '../workflow-action-start-form.types';
6+
7+
export const getCronFieldsError = (cronString: string): CronFieldsError => {
8+
const cronObj = cronValidate(cronString);
9+
10+
if (!cronObj.isValid()) {
11+
const errors = cronObj.getError();
12+
const errorFieldsKeys = CRON_FIELD_ORDER;
13+
const fieldsErrors: Partial<Record<keyof CronData, string>> = {};
14+
errors.forEach((e) => {
15+
const errorKey = errorFieldsKeys.find((key) =>
16+
e.includes(`${key} field`)
17+
);
18+
if (errorKey) fieldsErrors[errorKey] = e;
19+
});
20+
21+
if (!Object.keys(fieldsErrors).length) return { general: errors[0] };
22+
else return fieldsErrors;
23+
}
24+
25+
return null;
26+
};

src/views/workflow-actions/workflow-action-start-form/helpers/transform-start-workflow-form-to-submission.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { CRON_FIELD_ORDER } from '@/components/cron-schedule-input/cron-schedule-input.constants';
12
import { type Json } from '@/route-handlers/start-workflow/start-workflow.types';
23

34
import {
@@ -22,7 +23,9 @@ export default function transformStartWorkflowFormToSubmission(
2223
firstRunAt: formData.firstRunAt,
2324
}),
2425
...(formData.scheduleType === 'CRON' && {
25-
cronSchedule: formData.cronSchedule,
26+
cronSchedule: CRON_FIELD_ORDER.map(
27+
(key) => formData.cronSchedule?.[key]
28+
).join(' '),
2629
}),
2730
...(formData.enableRetryPolicy && {
2831
retryPolicy: {

0 commit comments

Comments
 (0)