Skip to content
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

Fixes #38059 - job wizard limit starts at to future only #932

Merged
merged 1 commit into from
Jan 2, 2025
Merged
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
54 changes: 38 additions & 16 deletions webpack/JobWizard/JobWizard.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { useValidation } from './validation';
import { useAutoFill } from './autofill';
import { submit } from './submit';
import { generateDefaultDescription } from './JobWizardHelpers';
import { StartsBeforeErrorAlert } from './StartsBeforeErrorAlert';
import { StartsBeforeErrorAlert, StartsAtErrorAlert } from './StartsErrorAlert';
import { Footer } from './Footer';
import './JobWizard.scss';

Expand Down Expand Up @@ -203,22 +203,37 @@ export const JobWizard = ({ rerunData }) => {
}, [rerunData, jobTemplateID, dispatch]);

const [isStartsBeforeError, setIsStartsBeforeError] = useState(false);
const [isStartsAtError, setIsStartsAtError] = useState(false);
useEffect(() => {
const updateStartsBeforeError = () => {
setIsStartsBeforeError(
scheduleValue.scheduleType === SCHEDULE_TYPES.FUTURE &&
new Date().getTime() >= new Date(scheduleValue.startsBefore).getTime()
);
const updateStartsError = () => {
if (scheduleValue.scheduleType === SCHEDULE_TYPES.FUTURE) {
setIsStartsAtError(
!!scheduleValue?.startsAt?.length &&
new Date().getTime() >= new Date(scheduleValue.startsAt).getTime()
);
setIsStartsBeforeError(
!!scheduleValue?.startsBefore?.length &&
new Date().getTime() >=
new Date(scheduleValue.startsBefore).getTime()
);
} else if (scheduleValue.scheduleType === SCHEDULE_TYPES.RECURRING) {
setIsStartsAtError(
!!scheduleValue?.startsAt?.length &&
new Date().getTime() >= new Date(scheduleValue.startsAt).getTime()
);
setIsStartsBeforeError(false);
} else {
setIsStartsAtError(false);
setIsStartsBeforeError(false);
}
};
let interval;
if (scheduleValue.scheduleType === SCHEDULE_TYPES.FUTURE) {
updateStartsBeforeError();
interval = setInterval(updateStartsBeforeError, 5000);
}
updateStartsError();
const interval = setInterval(updateStartsError, 5000);

return () => {
interval && clearInterval(interval);
};
}, [scheduleValue.scheduleType, scheduleValue.startsBefore]);
}, [scheduleValue]);

const [valid, setValid] = useValidation({
advancedValues,
Expand Down Expand Up @@ -369,7 +384,9 @@ export const JobWizard = ({ rerunData }) => {
valid.hostsAndInputs &&
areHostsSelected &&
valid.advanced &&
valid.schedule,
valid.schedule &&
!isStartsBeforeError &&
!isStartsAtError,
},
]
: []),
Expand Down Expand Up @@ -399,7 +416,8 @@ export const JobWizard = ({ rerunData }) => {
valid.hostsAndInputs &&
areHostsSelected &&
valid.advanced &&
valid.schedule,
valid.schedule &&
!isStartsAtError,
},
]
: []),
Expand All @@ -424,15 +442,18 @@ export const JobWizard = ({ rerunData }) => {
valid.advanced &&
valid.hostsAndInputs &&
areHostsSelected &&
valid.schedule,
valid.schedule &&
!isStartsBeforeError &&
!isStartsAtError,
enableNext:
isTemplate &&
valid.hostsAndInputs &&
areHostsSelected &&
valid.advanced &&
valid.schedule &&
!isSubmitting &&
!isStartsBeforeError,
!isStartsBeforeError &&
!isStartsAtError,
adamruzicka marked this conversation as resolved.
Show resolved Hide resolved
},
];
const location = useForemanLocation();
Expand All @@ -456,6 +477,7 @@ export const JobWizard = ({ rerunData }) => {
return (
<>
{isStartsBeforeError && <StartsBeforeErrorAlert />}
{isStartsAtError && <StartsAtErrorAlert />}
<Wizard
onClose={() => history.goBack()}
navAriaLabel="Run Job steps"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export const StartsBeforeErrorAlert = () => (
<Alert
ouiaId="starts-before-error-alert"
variant="danger"
title={__("'Starts before' date must in the future")}
title={__("'Starts before' date must be in the future")}
>
{__(
'Please go back to "Schedule" - "Future execution" step to fix the error'
Expand All @@ -16,3 +16,18 @@ export const StartsBeforeErrorAlert = () => (
<Divider component="div" />
</>
);

export const StartsAtErrorAlert = () => (
<>
<Alert
ouiaId="starts-at-error-alert"
variant="danger"
title={__("'Starts at' date must be in the future")}
>
{__(
'Please go back to "Schedule" - "Future execution" or "Recurring execution" step to fix the error'
)}
</Alert>
<Divider component="div" />
</>
);
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ describe('AdvancedFields', () => {
target: { value: 'some search' },
});

jest.runAllTimers();
jest.advanceTimersByTime(10000);
});
expect(newStore.getActions()).toMatchSnapshot('resource search');
});
Expand Down
2 changes: 1 addition & 1 deletion webpack/JobWizard/steps/Schedule/ScheduleFuture.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const ScheduleFuture = ({
setError(__("'Starts before' date must be after 'Starts at' date"));
} else if (new Date().getTime() >= new Date(startsBefore).getTime()) {
wrappedSetValid(false);
setError(__("'Starts before' date must in the future"));
setError(__("'Starts before' date must be in the future"));
} else {
wrappedSetValid(true);
setError(null);
Expand Down
8 changes: 5 additions & 3 deletions webpack/JobWizard/steps/Schedule/ScheduleRecurring.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const ScheduleRecurring = ({
if (isNeverEnds) setValidEnd(true);
else if (!ends) setValidEnd(true);
else if (
!startsAt.length &&
!startsAt?.length &&
new Date().getTime() <= new Date(ends).getTime()
)
setValidEnd(true);
Expand All @@ -63,7 +63,7 @@ export const ScheduleRecurring = ({

if (!validEnd || !repeatValid) {
wrappedSetValid(false);
} else if (isFuture && startsAt.length) {
} else if (isFuture && startsAt?.length) {
wrappedSetValid(true);
} else if (!isFuture) {
wrappedSetValid(true);
Expand Down Expand Up @@ -111,7 +111,9 @@ export const ScheduleRecurring = ({
onChange={() =>
setScheduleValue(current => ({
...current,
startsAt: new Date().toISOString(),
startsAt: new Date(
new Date().getTime() + 60000
).toISOString(), // 1 minute in the future
isFuture: true,
}))
}
Expand Down
2 changes: 1 addition & 1 deletion webpack/JobWizard/steps/Schedule/ScheduleType.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export const ScheduleType = ({
onChange={() => {
setScheduleValue(current => ({
...current,
startsAt: new Date().toISOString(),
startsAt: new Date(new Date().getTime() + 60000).toISOString(), // 1 minute in the future
scheduleType: SCHEDULE_TYPES.FUTURE,
repeatType: repeatTypes.noRepeat,
}));
Expand Down
6 changes: 3 additions & 3 deletions webpack/JobWizard/steps/Schedule/__tests__/Schedule.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ describe('Schedule', () => {
).toHaveLength(1);

expect(
screen.queryAllByText("'Starts before' date must in the future")
screen.queryAllByText("'Starts before' date must be in the future")
).toHaveLength(0);
await act(async () => {
await fireEvent.change(startsBeforeDateField(), {
Expand All @@ -182,7 +182,7 @@ describe('Schedule', () => {

expect(startsBeforeDateField().value).toBe('2019/03/11');
expect(
screen.getAllByText("'Starts before' date must in the future")
screen.getAllByText("'Starts before' date must be in the future")
).toHaveLength(2);
});

Expand Down Expand Up @@ -329,7 +329,7 @@ describe('Schedule', () => {
fireEvent.click(
screen.getByRole('button', { name: 'Recurring execution' })
);
jest.runAllTimers();
jest.advanceTimersByTime(1000);
});
expect(screen.queryAllByText('Recurring execution')).toHaveLength(3);

Expand Down
13 changes: 13 additions & 0 deletions webpack/JobWizard/steps/form/DateTimePicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const DateTimePicker = ({
ariaLabel,
allowEmpty,
includeSeconds,
isFutureOnly,
}) => {
const [validated, setValidated] = useState();
const dateFormat = date =>
Expand Down Expand Up @@ -87,6 +88,15 @@ export const DateTimePicker = ({
setValidated(ValidatedOptions.error);
}
};
const validateFuture = date => {
if (
isFutureOnly &&
date.setHours(1, 0, 0, 0) < new Date().setHours(0, 0, 0, 0)
) {
return __('Date must be in the future');
}
return '';
};
return (
<>
<DatePicker
Expand All @@ -105,6 +115,7 @@ export const DateTimePicker = ({
validated === ValidatedOptions.error ? __('Invalid date') : ''
}
inputProps={{ validated }}
validators={[validateFuture]}
/>
<TimePicker
aria-label={`${ariaLabel} timepicker`}
Expand Down Expand Up @@ -132,11 +143,13 @@ DateTimePicker.propTypes = {
ariaLabel: PropTypes.string,
allowEmpty: PropTypes.bool,
includeSeconds: PropTypes.bool,
isFutureOnly: PropTypes.bool,
};
DateTimePicker.defaultProps = {
dateTime: null,
isDisabled: false,
ariaLabel: '',
allowEmpty: true,
includeSeconds: false,
isFutureOnly: true,
};
1 change: 1 addition & 0 deletions webpack/JobWizard/steps/form/Formatter.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ export const formatter = (input, values, setValue) => {
isRequired={required}
>
<DateTimePicker
isFutureOnly={false}
ariaLabel={name}
className={hidden ? 'masked-input' : null}
id={id}
Expand Down
Loading