Skip to content

Commit

Permalink
fixed end date issue in pipelines scheduling
Browse files Browse the repository at this point in the history
  • Loading branch information
rsun19 committed Jun 3, 2024
1 parent f72473b commit db445ad
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import EndDateBeforeStartDateError from '~/concepts/pipelines/content/createRun/
import CatchUp from '~/concepts/pipelines/content/createRun/contentSections/CatchUp';
import MaxConcurrencyField from '~/concepts/pipelines/content/createRun/contentSections/MaxConcurrencyField';
import TriggerTypeField from '~/concepts/pipelines/content/createRun/contentSections/TriggerTypeField';
import { convertToTwentyFourHourTime } from '~/utilities/time';

type RunTypeSectionScheduledProps = {
data: RunTypeScheduledData;
Expand Down Expand Up @@ -39,7 +40,8 @@ const RunTypeSectionScheduled: React.FC<RunTypeSectionScheduledProps> = ({ data,
onChange={(end) => onChange({ ...data, end })}
adjustNow={(now) => {
if (data.start) {
const start = new Date(`${data.start.date} ${data.start.time}`);
const convertedTime = convertToTwentyFourHourTime(data.start.time);
const start = new Date(`${data.start.date}T${convertedTime}:00.000`);
start.setDate(start.getDate() + 7);
return start;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
getInputDefinitionParams,
isFilledRunFormData,
} from '~/concepts/pipelines/content/createRun/utils';
import { convertPeriodicTimeToSeconds } from '~/utilities/time';
import { convertPeriodicTimeToSeconds, convertToTwentyFourHourTime } from '~/utilities/time';

const createRun = async (
formData: SafeRunFormData,
Expand All @@ -46,12 +46,13 @@ const createRun = async (
return createPipelineRun({}, data);
};

const convertDateDataToKFDateTime = (dateData?: RunDateTime): DateTimeKF | null => {
export const convertDateDataToKFDateTime = (dateData?: RunDateTime): DateTimeKF | null => {
if (!dateData) {
return null;
}

const date = new Date(`${dateData.date} ${dateData.time}`);
const convertedTime = convertToTwentyFourHourTime(dateData.time);
const date = new Date(`${dateData.date}T${convertedTime}:00.000`);
return date.toISOString();
};

Expand Down
14 changes: 7 additions & 7 deletions frontend/src/concepts/pipelines/content/createRun/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import {
ScheduledType,
} from '~/concepts/pipelines/content/createRun/types';
import { ParametersKF, PipelineVersionKFv2 } from '~/concepts/pipelines/kfTypes';

import { getCorePipelineSpec } from '~/concepts/pipelines/getCorePipelineSpec';
import { convertToTwentyFourHourTime } from '~/utilities/time';

const runTypeSafeData = (runType: RunFormData['runType']): boolean =>
runType.type !== RunTypeOption.SCHEDULED ||
Expand All @@ -18,19 +18,19 @@ export const isStartBeforeEnd = (start?: RunDateTime, end?: RunDateTime): boolea
if (!start || !end) {
return true;
}

const startDate = new Date(`${start.date} ${start.time}`);
const endDate = new Date(`${end.date} ${end.time}`);

const startDateConvertedTime = convertToTwentyFourHourTime(start.time);
const startDate = new Date(`${start.date}T${startDateConvertedTime}:00.000`);
const endDateConvertedTime = convertToTwentyFourHourTime(end.time);
const endDate = new Date(`${end.date}T${endDateConvertedTime}:00.000`);
return endDate.getTime() - startDate.getTime() > 0;
};

const isValidDate = (value?: RunDateTime): boolean => {
if (!value) {
return true;
}

const date = new Date(`${value.date} ${value.time}`);
const convertedTime = convertToTwentyFourHourTime(value.time);
const date = new Date(`${value.date}T${convertedTime}:00.000`);
return date.toString() !== 'Invalid Date';
};

Expand Down
28 changes: 28 additions & 0 deletions frontend/src/utilities/__tests__/time.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
ensureTimeFormat,
printSeconds,
relativeTime,
convertToTwentyFourHourTime,
} from '~/utilities/time';

describe('relativeDuration', () => {
Expand Down Expand Up @@ -51,6 +52,33 @@ describe('convertDateToTimeString', () => {
});
});

describe('ConvertToTwentyFourHourTime', () => {
it('should convert 12 hour time to 24 hour time', () => {
const time = '1:30 AM';
expect(convertToTwentyFourHourTime(time)).toBe('01:30');
});

it('should convert past 12', () => {
const time = '2:55 PM';
expect(convertToTwentyFourHourTime(time)).toBe('14:55');
});

it('should convert from 10 AM <= x < 12PM to 24 hour time', () => {
const time = '11:24 AM';
expect(convertToTwentyFourHourTime(time)).toBe('11:24');
});

it('should convert from 12:30 PM to 12:30', () => {
const time = '12:30 PM';
expect(convertToTwentyFourHourTime(time)).toBe('12:30');
});

it('should convert from 12:30 AM to 00:30', () => {
const time = '12:30 AM';
expect(convertToTwentyFourHourTime(time)).toBe('00:30');
});
});

describe('ensureTimeFormat', () => {
it('should return time value', () => {
expect(ensureTimeFormat('3:30 PM')).toBe('3:30 PM');
Expand Down
12 changes: 12 additions & 0 deletions frontend/src/utilities/time.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,18 @@ export const convertDateToTimeString = (date?: Date): string | null => {
return `${hoursIn12}:${leadZero(date.getMinutes())} ${hours >= 12 ? 'PM' : 'AM'}`;
};

/* Return HH:MM from HH:MM *M */
export const convertToTwentyFourHourTime = (time: string): string => {
const timeArray = time.trim().split(' ');
const hourMinutesArray = timeArray[0].trim().split(':');
const hour = Number(hourMinutesArray[0]);
const minutes = hourMinutesArray[1];
if (timeArray[1] === 'AM') {
return `${hour === 12 ? 0 : hour}:${minutes}`.padStart(5, '0');
}
return `${hour === 12 ? hour : hour + 12}:${minutes}`;
};

/** The TimeField component can sometimes cause '6:00PM' instead of '6:00 PM' if the user edits directly */
export const ensureTimeFormat = (time: string): string | null => {
if (/\s[AP]M/.test(time)) {
Expand Down

0 comments on commit db445ad

Please sign in to comment.