From f6b3413a1173d2687a4d9be3cd2d8344bcc82ce1 Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Fri, 26 Aug 2022 16:00:08 -0700 Subject: [PATCH 1/9] add schedule exemptions to form --- .../Schedule/shared/ScheduleFormFields.js | 73 ++--- .../Schedule/shared/buildRuleSet.js | 14 +- .../Schedule/shared/buildRuleSet.test.js | 253 ++++++++++++++++++ .../Schedule/shared/parseRuleObj.js | 67 ++++- .../Schedule/shared/parseRuleObj.test.js | 47 ++++ 5 files changed, 408 insertions(+), 46 deletions(-) diff --git a/awx/ui/src/components/Schedule/shared/ScheduleFormFields.js b/awx/ui/src/components/Schedule/shared/ScheduleFormFields.js index 272ed187dc8d..5b61eb54a65e 100644 --- a/awx/ui/src/components/Schedule/shared/ScheduleFormFields.js +++ b/awx/ui/src/components/Schedule/shared/ScheduleFormFields.js @@ -3,6 +3,7 @@ import { useField } from 'formik'; import { FormGroup, Title } from '@patternfly/react-core'; import { t } from '@lingui/macro'; import styled from 'styled-components'; +import 'styled-components/macro'; import FormField from 'components/FormField'; import { required } from 'util/validators'; import { useConfig } from 'contexts/Config'; @@ -53,11 +54,11 @@ export default function ScheduleFormFields({ } const config = useConfig(); - // const [exceptionFrequency, exceptionFrequencyMeta, exceptionFrequencyHelper] = - // useField({ - // name: 'exceptionFrequency', - // validate: required(t`Select a value for this field`), - // }); + const [exceptionFrequency, exceptionFrequencyMeta, exceptionFrequencyHelper] = + useField({ + name: 'exceptionFrequency', + validate: required(t`Select a value for this field`), + }); const updateFrequency = (setFrequency) => (values) => { setFrequency(values.sort(sortFrequencies)); @@ -151,34 +152,40 @@ export default function ScheduleFormFields({ /> ))} - {/* {t`Exceptions`} - - {t`Exceptions`} + + - {t`None`} - {t`Minute`} - {t`Hour`} - {t`Day`} - {t`Week`} - {t`Month`} - {t`Year`} - - + + {t`None`} + {t`Minute`} + {t`Hour`} + {t`Day`} + {t`Week`} + {t`Month`} + {t`Year`} + + + {exceptionFrequency.value.map((val) => ( - ))} */} + ))} ) : null} diff --git a/awx/ui/src/components/Schedule/shared/buildRuleSet.js b/awx/ui/src/components/Schedule/shared/buildRuleSet.js index c15594e0c628..0f8182bc01b6 100644 --- a/awx/ui/src/components/Schedule/shared/buildRuleSet.js +++ b/awx/ui/src/components/Schedule/shared/buildRuleSet.js @@ -39,7 +39,19 @@ export default function buildRuleSet(values) { set.rrule(new RRule(rule)); }); - // TODO: exclusions + frequencies.forEach((frequency) => { + if (!values.exceptionFrequency?.includes(frequency)) { + return; + } + const rule = buildRuleObj({ + startDate: values.startDate, + startTime: values.startTime, + timezone: values.timezone, + frequency, + ...values.exceptionOptions[frequency], + }); + set.exrule(new RRule(rule)); + }); return set; } diff --git a/awx/ui/src/components/Schedule/shared/buildRuleSet.test.js b/awx/ui/src/components/Schedule/shared/buildRuleSet.test.js index 3d7831507cfe..42fde679812f 100644 --- a/awx/ui/src/components/Schedule/shared/buildRuleSet.test.js +++ b/awx/ui/src/components/Schedule/shared/buildRuleSet.test.js @@ -243,4 +243,257 @@ RRULE:INTERVAL=1;FREQ=MONTHLY;BYSETPOS=2;BYDAY=MO;UNTIL=20260602T170000Z`); expect(ruleSet.toString()).toEqual(`DTSTART:20220613T123000Z RRULE:INTERVAL=1;COUNT=1;FREQ=MINUTELY`); }); + + test('should build minutely exception', () => { + const values = { + startDate: '2022-06-13', + startTime: '12:30 PM', + frequency: ['minute'], + frequencyOptions: { + minute: { + interval: 1, + end: 'never', + }, + }, + exceptionFrequency: ['minute'], + exceptionOptions: { + minute: { + interval: 3, + end: 'never', + }, + }, + }; + + const ruleSet = buildRuleSet(values); + expect(ruleSet.toString()).toEqual( + [ + 'DTSTART:20220613T123000Z', + 'RRULE:INTERVAL=1;FREQ=MINUTELY', + 'EXRULE:INTERVAL=3;FREQ=MINUTELY', + ].join('\n') + ); + }); + + test('should build hourly exception', () => { + const values = { + startDate: '2022-06-13', + startTime: '12:30 PM', + frequency: ['minute'], + frequencyOptions: { + minute: { + interval: 1, + end: 'never', + }, + }, + exceptionFrequency: ['hour'], + exceptionOptions: { + hour: { + interval: 3, + end: 'never', + }, + }, + }; + + const ruleSet = buildRuleSet(values); + expect(ruleSet.toString()).toEqual( + [ + 'DTSTART:20220613T123000Z', + 'RRULE:INTERVAL=1;FREQ=MINUTELY', + 'EXRULE:INTERVAL=3;FREQ=HOURLY', + ].join('\n') + ); + }); + + test('should build daily exception', () => { + const values = { + startDate: '2022-06-13', + startTime: '12:30 PM', + frequency: ['minute'], + frequencyOptions: { + minute: { + interval: 1, + end: 'never', + }, + }, + exceptionFrequency: ['day'], + exceptionOptions: { + day: { + interval: 3, + end: 'never', + }, + }, + }; + + const ruleSet = buildRuleSet(values); + expect(ruleSet.toString()).toEqual( + [ + 'DTSTART:20220613T123000Z', + 'RRULE:INTERVAL=1;FREQ=MINUTELY', + 'EXRULE:INTERVAL=3;FREQ=DAILY', + ].join('\n') + ); + }); + + test('should build weekly exception', () => { + const values = { + startDate: '2022-06-13', + startTime: '12:30 PM', + frequency: ['minute'], + frequencyOptions: { + minute: { + interval: 1, + end: 'never', + }, + }, + exceptionFrequency: ['week'], + exceptionOptions: { + week: { + interval: 3, + end: 'never', + daysOfWeek: [RRule.SU], + }, + }, + }; + + const ruleSet = buildRuleSet(values); + expect(ruleSet.toString()).toEqual( + [ + 'DTSTART:20220613T123000Z', + 'RRULE:INTERVAL=1;FREQ=MINUTELY', + 'EXRULE:INTERVAL=3;FREQ=WEEKLY;BYDAY=SU', + ].join('\n') + ); + }); + + test('should build monthly exception by day', () => { + const values = { + startDate: '2022-06-13', + startTime: '12:30 PM', + frequency: ['minute'], + frequencyOptions: { + minute: { + interval: 1, + end: 'never', + }, + }, + exceptionFrequency: ['month'], + exceptionOptions: { + month: { + interval: 3, + end: 'never', + runOn: 'day', + runOnDayNumber: 15, + }, + }, + }; + + const ruleSet = buildRuleSet(values); + expect(ruleSet.toString()).toEqual( + [ + 'DTSTART:20220613T123000Z', + 'RRULE:INTERVAL=1;FREQ=MINUTELY', + 'EXRULE:INTERVAL=3;FREQ=MONTHLY;BYMONTHDAY=15', + ].join('\n') + ); + }); + + test('should build monthly exception by weekday', () => { + const values = { + startDate: '2022-06-13', + startTime: '12:30 PM', + frequency: ['minute'], + frequencyOptions: { + minute: { + interval: 1, + end: 'never', + }, + }, + exceptionFrequency: ['month'], + exceptionOptions: { + month: { + interval: 3, + end: 'never', + runOn: 'the', + runOnTheOccurrence: 2, + runOnTheDay: 'monday', + }, + }, + }; + + const ruleSet = buildRuleSet(values); + expect(ruleSet.toString()).toEqual( + [ + 'DTSTART:20220613T123000Z', + 'RRULE:INTERVAL=1;FREQ=MINUTELY', + 'EXRULE:INTERVAL=3;FREQ=MONTHLY;BYSETPOS=2;BYDAY=MO', + ].join('\n') + ); + }); + + test('should build annual exception by day', () => { + const values = { + startDate: '2022-06-13', + startTime: '12:30 PM', + frequency: ['minute'], + frequencyOptions: { + minute: { + interval: 1, + end: 'never', + }, + }, + exceptionFrequency: ['year'], + exceptionOptions: { + year: { + interval: 1, + end: 'never', + runOn: 'day', + runOnDayMonth: 3, + runOnDayNumber: 15, + }, + }, + }; + + const ruleSet = buildRuleSet(values); + expect(ruleSet.toString()).toEqual( + [ + 'DTSTART:20220613T123000Z', + 'RRULE:INTERVAL=1;FREQ=MINUTELY', + 'EXRULE:INTERVAL=1;FREQ=YEARLY;BYMONTH=3;BYMONTHDAY=15', + ].join('\n') + ); + }); + + test('should build annual exception by weekday', () => { + const values = { + startDate: '2022-06-13', + startTime: '12:30 PM', + frequency: ['minute'], + frequencyOptions: { + minute: { + interval: 1, + end: 'never', + }, + }, + exceptionFrequency: ['year'], + exceptionOptions: { + year: { + interval: 1, + end: 'never', + runOn: 'the', + runOnTheOccurrence: 4, + runOnTheDay: 'monday', + runOnTheMonth: 6, + }, + }, + }; + + const ruleSet = buildRuleSet(values); + expect(ruleSet.toString()).toEqual( + [ + 'DTSTART:20220613T123000Z', + 'RRULE:INTERVAL=1;FREQ=MINUTELY', + 'EXRULE:INTERVAL=1;FREQ=YEARLY;BYSETPOS=4;BYDAY=MO;BYMONTH=6', + ].join('\n') + ); + }); }); diff --git a/awx/ui/src/components/Schedule/shared/parseRuleObj.js b/awx/ui/src/components/Schedule/shared/parseRuleObj.js index 9699a6a51afa..a67623119306 100644 --- a/awx/ui/src/components/Schedule/shared/parseRuleObj.js +++ b/awx/ui/src/components/Schedule/shared/parseRuleObj.js @@ -32,6 +32,9 @@ export default function parseRuleObj(schedule) { case 'RRULE': values = parseRrule(ruleString, schedule, values); break; + case 'EXRULE': + values = parseExRule(ruleString, schedule, values); + break; default: throw new UnsupportedRRuleError(`Unsupported rrule type: ${type}`); } @@ -79,6 +82,54 @@ const frequencyTypes = { }; function parseRrule(rruleString, schedule, values) { + const { frequency, options } = parseRule( + rruleString, + schedule, + values.exceptionFrequency + ); + + if (values.frequencyOptions[frequency]) { + throw new UnsupportedRRuleError( + 'Duplicate exception frequency types not supported' + ); + } + + return { + ...values, + frequency: [...values.frequency, frequency].sort(sortFrequencies), + frequencyOptions: { + ...values.frequencyOptions, + [frequency]: options, + }, + }; +} + +function parseExRule(exruleString, schedule, values) { + const { frequency, options } = parseRule( + exruleString, + schedule, + values.exceptionFrequency + ); + + if (values.exceptionOptions[frequency]) { + throw new UnsupportedRRuleError( + 'Duplicate exception frequency types not supported' + ); + } + + return { + ...values, + exceptionFrequency: [...values.exceptionFrequency, frequency].sort( + sortFrequencies + ), + exceptionOptions: { + ...values.exceptionOptions, + [frequency]: options, + }, + }; +} + +function parseRule(ruleString, schedule, frequencies) { const { origOptions: { bymonth, @@ -90,7 +141,7 @@ function parseRrule(rruleString, schedule, values) { interval, until, }, - } = RRule.fromString(rruleString); + } = RRule.fromString(ruleString); const now = DateTime.now(); const closestQuarterHour = DateTime.fromMillis( @@ -127,7 +178,7 @@ function parseRrule(rruleString, schedule, values) { throw new Error(`Unexpected rrule frequency: ${freq}`); } const frequency = frequencyTypes[freq]; - if (values.frequency.includes(frequency)) { + if (frequencies.includes(frequency)) { throw new Error(`Duplicate frequency types not supported (${frequency})`); } @@ -171,17 +222,9 @@ function parseRrule(rruleString, schedule, values) { } } - if (values.frequencyOptions.frequency) { - throw new UnsupportedRRuleError('Duplicate frequency types not supported'); - } - return { - ...values, - frequency: [...values.frequency, frequency].sort(sortFrequencies), - frequencyOptions: { - ...values.frequencyOptions, - [frequency]: options, - }, + frequency, + options, }; } diff --git a/awx/ui/src/components/Schedule/shared/parseRuleObj.test.js b/awx/ui/src/components/Schedule/shared/parseRuleObj.test.js index 9f0fcba1b680..426998bcc28f 100644 --- a/awx/ui/src/components/Schedule/shared/parseRuleObj.test.js +++ b/awx/ui/src/components/Schedule/shared/parseRuleObj.test.js @@ -241,4 +241,51 @@ RRULE:INTERVAL=1;FREQ=MONTHLY;BYSETPOS=2;BYDAY=MO;UNTIL=20260602T170000Z`; expect(parsed).toEqual(values); }); + + test('should parse exemptions', () => { + const schedule = { + rrule: [ + 'DTSTART;TZID=US/Eastern:20220608T123000', + 'RRULE:INTERVAL=1;FREQ=WEEKLY;BYDAY=MO', + 'EXRULE:INTERVAL=1;FREQ=MONTHLY;BYSETPOS=1;BYDAY=MO', + ].join(' '), + dtstart: '2022-06-13T16:30:00Z', + timezone: 'US/Eastern', + until: '', + dtend: null, + }; + + const parsed = parseRuleObj(schedule); + + expect(parsed).toEqual({ + startDate: '2022-06-13', + startTime: '12:30 PM', + timezone: 'US/Eastern', + frequency: ['week'], + frequencyOptions: { + week: { + interval: 1, + end: 'never', + occurrences: 1, + endDate: '2022-06-02', + endTime: '1:00 PM', + daysOfWeek: [RRule.MO], + }, + }, + exceptionFrequency: ['month'], + exceptionOptions: { + month: { + interval: 1, + end: 'never', + endDate: '2022-06-02', + endTime: '1:00 PM', + occurrences: 1, + runOn: 'the', + runOnDayNumber: 1, + runOnTheOccurrence: 1, + runOnTheDay: 'monday', + }, + }, + }); + }); }); From 4a92fcfc627f4d74c724d0578fe1813a48d8b6c5 Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Mon, 29 Aug 2022 11:55:32 -0700 Subject: [PATCH 2/9] add schedule exceptions to details --- .../ScheduleDetail/FrequencyDetails.js | 13 ++++++-- .../Schedule/ScheduleDetail/ScheduleDetail.js | 32 +++++++++++++++++-- .../Schedule/shared/FrequencyDetailSubform.js | 4 +-- .../Schedule/shared/ScheduleFormFields.js | 1 + 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/awx/ui/src/components/Schedule/ScheduleDetail/FrequencyDetails.js b/awx/ui/src/components/Schedule/ScheduleDetail/FrequencyDetails.js index 48f55601edf9..bb54aa6f62d6 100644 --- a/awx/ui/src/components/Schedule/ScheduleDetail/FrequencyDetails.js +++ b/awx/ui/src/components/Schedule/ScheduleDetail/FrequencyDetails.js @@ -10,7 +10,13 @@ const Label = styled.div` font-weight: var(--pf-global--FontWeight--bold); `; -export default function FrequencyDetails({ type, label, options, timezone }) { +export default function FrequencyDetails({ + type, + label, + options, + timezone, + isException, +}) { const getRunEveryLabel = () => { const { interval } = options; switch (type) { @@ -81,7 +87,10 @@ export default function FrequencyDetails({ type, label, options, timezone }) {
- + {type === 'week' ? ( frequencies[f]).join(', ') : t`None (Run Once)`; + const exceptionRepeatFrequency = exceptionFrequency.length + ? exceptionFrequency.map((f) => frequencies[f]).join(', ') + : t`None (Run Once)`; const { ask_credential_on_launch, @@ -288,6 +295,10 @@ function ScheduleDetail({ hasDaysToKeepField, schedule, surveyConfig }) { helpText={helpText.localTimeZone(config)} /> + {frequency.length ? ( @@ -305,6 +316,23 @@ function ScheduleDetail({ hasDaysToKeepField, schedule, surveyConfig }) { ))} ) : null} + {exceptionFrequency.length ? ( + +

+ {t`Frequency Exception Details`} +

+ {exceptionFrequency.map((freq) => ( + + ))} +
+ ) : null} {hasDaysToKeepField ? ( diff --git a/awx/ui/src/components/Schedule/shared/FrequencyDetailSubform.js b/awx/ui/src/components/Schedule/shared/FrequencyDetailSubform.js index 69b54ab154d8..615d814b8d5b 100644 --- a/awx/ui/src/components/Schedule/shared/FrequencyDetailSubform.js +++ b/awx/ui/src/components/Schedule/shared/FrequencyDetailSubform.js @@ -45,7 +45,7 @@ const Checkbox = styled(_Checkbox)` } `; -const FrequencyDetailSubform = ({ frequency, prefix }) => { +const FrequencyDetailSubform = ({ frequency, prefix, isException }) => { const id = prefix.replace('.', '-'); const [runOnDayMonth] = useField({ name: `${prefix}.runOnDayMonth`, @@ -220,7 +220,7 @@ const FrequencyDetailSubform = ({ frequency, prefix }) => { validated={ !intervalMeta.touched || !intervalMeta.error ? 'default' : 'error' } - label={t`Run every`} + label={isException ? t`Skip every` : t`Run every`} >
))} From dda2931e600c4c0f97d5f505ad220ee1b06c1177 Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Mon, 29 Aug 2022 13:43:49 -0700 Subject: [PATCH 3/9] fix exception frequency placeholder text --- awx/ui/src/components/Schedule/shared/ScheduleFormFields.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/awx/ui/src/components/Schedule/shared/ScheduleFormFields.js b/awx/ui/src/components/Schedule/shared/ScheduleFormFields.js index 63b79478aab0..887e6f7535bc 100644 --- a/awx/ui/src/components/Schedule/shared/ScheduleFormFields.js +++ b/awx/ui/src/components/Schedule/shared/ScheduleFormFields.js @@ -173,7 +173,11 @@ export default function ScheduleFormFields({ id="exception-frequency" onChange={exceptionFrequencyHelper.setValue} value={exceptionFrequency.value} - placeholderText={t`None`} + placeholderText={ + exceptionFrequency.value.length + ? t`Select frequency` + : t`None` + } onBlur={exceptionFrequencyHelper.setTouched} > {t`None`} From 0005d249c049a27a7f1fa53e3284ff52a53507d5 Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Tue, 30 Aug 2022 15:44:52 -0700 Subject: [PATCH 4/9] update tests --- .../Schedule/shared/ScheduleForm.test.js | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/awx/ui/src/components/Schedule/shared/ScheduleForm.test.js b/awx/ui/src/components/Schedule/shared/ScheduleForm.test.js index 54d47c0de3e3..47936fc314d6 100644 --- a/awx/ui/src/components/Schedule/shared/ScheduleForm.test.js +++ b/awx/ui/src/components/Schedule/shared/ScheduleForm.test.js @@ -86,7 +86,7 @@ const mockSchedule = { let wrapper; -const defaultFieldsVisible = () => { +const defaultFieldsVisible = (isExceptionsVisible) => { expect(wrapper.find('FormGroup[label="Name"]').length).toBe(1); expect(wrapper.find('FormGroup[label="Description"]').length).toBe(1); expect(wrapper.find('FormGroup[label="Start date/time"]').length).toBe(1); @@ -94,7 +94,11 @@ const defaultFieldsVisible = () => { expect( wrapper.find('FormGroup[label="Local time zone"]').find('HelpIcon').length ).toBe(1); - expect(wrapper.find('FrequencySelect').length).toBe(1); + if (isExceptionsVisible) { + expect(wrapper.find('FrequencySelect').length).toBe(2); + } else { + expect(wrapper.find('FrequencySelect').length).toBe(1); + } }; const nonRRuleValuesMatch = () => { @@ -513,7 +517,7 @@ describe('', () => { runFrequencySelect.invoke('onChange')(['minute']); }); wrapper.update(); - defaultFieldsVisible(); + defaultFieldsVisible(true); expect(wrapper.find('FormGroup[label="Run every"]').length).toBe(1); expect(wrapper.find('FormGroup[label="End"]').length).toBe(1); expect(wrapper.find('FormGroup[label="On days"]').length).toBe(0); @@ -547,7 +551,7 @@ describe('', () => { runFrequencySelect.invoke('onChange')(['hour']); }); wrapper.update(); - defaultFieldsVisible(); + defaultFieldsVisible(true); expect(wrapper.find('FormGroup[label="Run every"]').length).toBe(1); expect(wrapper.find('FormGroup[label="End"]').length).toBe(1); expect(wrapper.find('FormGroup[label="On days"]').length).toBe(0); @@ -579,7 +583,7 @@ describe('', () => { runFrequencySelect.invoke('onChange')(['day']); }); wrapper.update(); - defaultFieldsVisible(); + defaultFieldsVisible(true); expect(wrapper.find('FormGroup[label="Run every"]').length).toBe(1); expect(wrapper.find('FormGroup[label="End"]').length).toBe(1); expect(wrapper.find('FormGroup[label="On days"]').length).toBe(0); @@ -611,7 +615,7 @@ describe('', () => { runFrequencySelect.invoke('onChange')(['week']); }); wrapper.update(); - defaultFieldsVisible(); + defaultFieldsVisible(true); expect(wrapper.find('FormGroup[label="Run every"]').length).toBe(1); expect(wrapper.find('FormGroup[label="End"]').length).toBe(1); expect(wrapper.find('FormGroup[label="On days"]').length).toBe(1); @@ -643,7 +647,7 @@ describe('', () => { runFrequencySelect.invoke('onChange')(['month']); }); wrapper.update(); - defaultFieldsVisible(); + defaultFieldsVisible(true); expect(wrapper.find('FormGroup[label="Run every"]').length).toBe(1); expect(wrapper.find('FormGroup[label="End"]').length).toBe(1); expect(wrapper.find('FormGroup[label="On days"]').length).toBe(0); @@ -692,7 +696,7 @@ describe('', () => { runFrequencySelect.invoke('onChange')(['year']); }); wrapper.update(); - defaultFieldsVisible(); + defaultFieldsVisible(true); expect(wrapper.find('FormGroup[label="Run every"]').length).toBe(1); expect(wrapper.find('FormGroup[label="End"]').length).toBe(1); expect(wrapper.find('FormGroup[label="On days"]').length).toBe(0); @@ -1058,7 +1062,7 @@ describe('', () => { wrapper.update(); expect(wrapper.find('ScheduleForm').length).toBe(1); - defaultFieldsVisible(); + defaultFieldsVisible(true); expect(wrapper.find('FormGroup[label="End"]').length).toBe(1); expect(wrapper.find('FormGroup[label="Run every"]').length).toBe(1); expect(wrapper.find('FormGroup[label="Occurrences"]').length).toBe(0); @@ -1113,7 +1117,7 @@ describe('', () => { wrapper.update(); expect(wrapper.find('ScheduleForm').length).toBe(1); - defaultFieldsVisible(); + defaultFieldsVisible(true); expect(wrapper.find('FormGroup[label="End"]').length).toBe(1); expect(wrapper.find('FormGroup[label="Run every"]').length).toBe(1); expect(wrapper.find('FormGroup[label="Occurrences"]').length).toBe(1); @@ -1171,7 +1175,7 @@ describe('', () => { wrapper.update(); expect(wrapper.find('ScheduleForm').length).toBe(1); - defaultFieldsVisible(); + defaultFieldsVisible(true); expect(wrapper.find('FormGroup[label="Run every"]').length).toBe(1); expect(wrapper.find('FormGroup[label="End"]').length).toBe(1); expect(wrapper.find('FormGroup[label="On days"]').length).toBe(0); @@ -1224,7 +1228,7 @@ describe('', () => { wrapper.update(); expect(wrapper.find('ScheduleForm').length).toBe(1); - defaultFieldsVisible(); + defaultFieldsVisible(true); expect(wrapper.find('FormGroup[label="End"]').length).toBe(1); expect(wrapper.find('FormGroup[label="Run every"]').length).toBe(1); expect(wrapper.find('FormGroup[label="End date/time"]').length).toBe(1); @@ -1318,10 +1322,7 @@ describe('', () => { wrapper.update(); expect(wrapper.find('ScheduleForm').length).toBe(1); - defaultFieldsVisible(); - - expect(wrapper.find('ScheduleForm').length).toBe(1); - defaultFieldsVisible(); + defaultFieldsVisible(true); expect(wrapper.find('FormGroup[label="End"]').length).toBe(1); expect(wrapper.find('FormGroup[label="Run every"]').length).toBe(1); expect(wrapper.find('FormGroup[label="Run on"]').length).toBe(1); @@ -1394,7 +1395,7 @@ describe('', () => { wrapper.update(); expect(wrapper.find('ScheduleForm').length).toBe(1); - defaultFieldsVisible(); + defaultFieldsVisible(true); expect(wrapper.find('FormGroup[label="End"]').length).toBe(1); expect(wrapper.find('FormGroup[label="Run every"]').length).toBe(1); expect(wrapper.find('FormGroup[label="Run on"]').length).toBe(1); From 078c3ae6d805371fffc8ce543b65d7ed32be44d0 Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Wed, 7 Sep 2022 10:19:25 -0700 Subject: [PATCH 5/9] add schedule form validation to ensure at least one occurrence --- .../Schedule/shared/ScheduleForm.js | 21 ++++++++ .../Schedule/shared/buildRuleObj.js | 38 +++++++++---- .../Schedule/shared/buildRuleSet.js | 53 +++++++++++-------- 3 files changed, 79 insertions(+), 33 deletions(-) diff --git a/awx/ui/src/components/Schedule/shared/ScheduleForm.js b/awx/ui/src/components/Schedule/shared/ScheduleForm.js index c29a927868dd..a41bb9195fe9 100644 --- a/awx/ui/src/components/Schedule/shared/ScheduleForm.js +++ b/awx/ui/src/components/Schedule/shared/ScheduleForm.js @@ -20,6 +20,7 @@ import ScheduleFormFields from './ScheduleFormFields'; import UnsupportedScheduleForm from './UnsupportedScheduleForm'; import parseRuleObj, { UnsupportedRRuleError } from './parseRuleObj'; import buildRuleObj from './buildRuleObj'; +import buildRuleSet from './buildRuleSet'; const NUM_DAYS_PER_FREQUENCY = { week: 7, @@ -411,6 +412,26 @@ function ScheduleForm({ } }); + if (values.exceptionFrequency.length > 0) { + let rangeToCheck = 1; + values.frequency.forEach((freq) => { + if (NUM_DAYS_PER_FREQUENCY[freq] > rangeToCheck) { + rangeToCheck = NUM_DAYS_PER_FREQUENCY[freq]; + } + }); + const ruleSet = buildRuleSet(values, true); + const startDate = DateTime.fromISO(values.startDate); + const endDate = startDate.plus({ days: rangeToCheck }); + const instances = ruleSet.between( + startDate.toJSDate(), + endDate.toJSDate(), + true + ); + if (instances.length === 0) { + errors.exceptionFrequency = t`This schedule has no occurrences due to the selected exceptions.`; + } + } + return errors; }; diff --git a/awx/ui/src/components/Schedule/shared/buildRuleObj.js b/awx/ui/src/components/Schedule/shared/buildRuleObj.js index 3fcba27240b2..c631ed959d18 100644 --- a/awx/ui/src/components/Schedule/shared/buildRuleObj.js +++ b/awx/ui/src/components/Schedule/shared/buildRuleObj.js @@ -36,11 +36,19 @@ function pad(num) { return num < 10 ? `0${num}` : num; } -export default function buildRuleObj(values) { +export default function buildRuleObj(values, includeStart) { const ruleObj = { interval: values.interval, }; + if (includeStart) { + ruleObj.dtstart = buildDateTime( + values.startDate, + values.startTime, + values.timezone + ); + } + switch (values.frequency) { case 'none': ruleObj.count = 1; @@ -91,16 +99,11 @@ export default function buildRuleObj(values) { ruleObj.count = values.occurrences; break; case 'onDate': { - const [endHour, endMinute] = parseTime(values.endTime); - const localEndDate = DateTime.fromISO(`${values.endDate}T000000`, { - zone: values.timezone, - }); - const localEndTime = localEndDate.set({ - hour: endHour, - minute: endMinute, - second: 0, - }); - ruleObj.until = localEndTime.toJSDate(); + ruleObj.until = buildDateTime( + values.endDate, + values.endTime, + values.timezone + ); break; } default: @@ -110,3 +113,16 @@ export default function buildRuleObj(values) { return ruleObj; } + +function buildDateTime(dateString, timeString, timezone) { + const localDate = DateTime.fromISO(`${dateString}T000000`, { + zone: timezone, + }); + const [hour, minute] = parseTime(timeString); + const localTime = localDate.set({ + hour, + minute, + second: 0, + }); + return localTime.toJSDate(); +} diff --git a/awx/ui/src/components/Schedule/shared/buildRuleSet.js b/awx/ui/src/components/Schedule/shared/buildRuleSet.js index 0f8182bc01b6..0304b368874d 100644 --- a/awx/ui/src/components/Schedule/shared/buildRuleSet.js +++ b/awx/ui/src/components/Schedule/shared/buildRuleSet.js @@ -4,7 +4,7 @@ import buildRuleObj, { buildDtStartObj } from './buildRuleObj'; window.RRuleSet = RRuleSet; const frequencies = ['minute', 'hour', 'day', 'week', 'month', 'year']; -export default function buildRuleSet(values) { +export default function buildRuleSet(values, includeStart) { const set = new RRuleSet(); const startRule = buildDtStartObj({ @@ -15,13 +15,16 @@ export default function buildRuleSet(values) { set.rrule(startRule); if (values.frequency.length === 0) { - const rule = buildRuleObj({ - startDate: values.startDate, - startTime: values.startTime, - timezone: values.timezone, - frequency: 'none', - interval: 1, - }); + const rule = buildRuleObj( + { + startDate: values.startDate, + startTime: values.startTime, + timezone: values.timezone, + frequency: 'none', + interval: 1, + }, + includeStart + ); set.rrule(new RRule(rule)); } @@ -29,13 +32,16 @@ export default function buildRuleSet(values) { if (!values.frequency.includes(frequency)) { return; } - const rule = buildRuleObj({ - startDate: values.startDate, - startTime: values.startTime, - timezone: values.timezone, - frequency, - ...values.frequencyOptions[frequency], - }); + const rule = buildRuleObj( + { + startDate: values.startDate, + startTime: values.startTime, + timezone: values.timezone, + frequency, + ...values.frequencyOptions[frequency], + }, + includeStart + ); set.rrule(new RRule(rule)); }); @@ -43,13 +49,16 @@ export default function buildRuleSet(values) { if (!values.exceptionFrequency?.includes(frequency)) { return; } - const rule = buildRuleObj({ - startDate: values.startDate, - startTime: values.startTime, - timezone: values.timezone, - frequency, - ...values.exceptionOptions[frequency], - }); + const rule = buildRuleObj( + { + startDate: values.startDate, + startTime: values.startTime, + timezone: values.timezone, + frequency, + ...values.exceptionOptions[frequency], + }, + includeStart + ); set.exrule(new RRule(rule)); }); From 1e952bab9548887f0e9ea22dc19e62a658b4eeb9 Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Mon, 12 Sep 2022 12:58:25 -0700 Subject: [PATCH 6/9] fix error message on new schedules with no instances --- .../Schedule/shared/buildRuleSet.js | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/awx/ui/src/components/Schedule/shared/buildRuleSet.js b/awx/ui/src/components/Schedule/shared/buildRuleSet.js index 0304b368874d..070a0ef66315 100644 --- a/awx/ui/src/components/Schedule/shared/buildRuleSet.js +++ b/awx/ui/src/components/Schedule/shared/buildRuleSet.js @@ -4,15 +4,17 @@ import buildRuleObj, { buildDtStartObj } from './buildRuleObj'; window.RRuleSet = RRuleSet; const frequencies = ['minute', 'hour', 'day', 'week', 'month', 'year']; -export default function buildRuleSet(values, includeStart) { +export default function buildRuleSet(values, useUTCStart) { const set = new RRuleSet(); - const startRule = buildDtStartObj({ - startDate: values.startDate, - startTime: values.startTime, - timezone: values.timezone, - }); - set.rrule(startRule); + if (!useUTCStart) { + const startRule = buildDtStartObj({ + startDate: values.startDate, + startTime: values.startTime, + timezone: values.timezone, + }); + set.rrule(startRule); + } if (values.frequency.length === 0) { const rule = buildRuleObj( @@ -23,7 +25,7 @@ export default function buildRuleSet(values, includeStart) { frequency: 'none', interval: 1, }, - includeStart + useUTCStart ); set.rrule(new RRule(rule)); } @@ -40,7 +42,7 @@ export default function buildRuleSet(values, includeStart) { frequency, ...values.frequencyOptions[frequency], }, - includeStart + useUTCStart ); set.rrule(new RRule(rule)); }); @@ -57,7 +59,7 @@ export default function buildRuleSet(values, includeStart) { frequency, ...values.exceptionOptions[frequency], }, - includeStart + useUTCStart ); set.exrule(new RRule(rule)); }); From 16da9b784a3fbb4a9c4b62103c0044fa96b5a74c Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Mon, 12 Sep 2022 13:54:00 -0700 Subject: [PATCH 7/9] add schedule integration test locators --- .../ScheduleDetail/FrequencyDetails.js | 23 +++- .../Schedule/ScheduleDetail/ScheduleDetail.js | 105 ++++++++++++------ 2 files changed, 91 insertions(+), 37 deletions(-) diff --git a/awx/ui/src/components/Schedule/ScheduleDetail/FrequencyDetails.js b/awx/ui/src/components/Schedule/ScheduleDetail/FrequencyDetails.js index bb54aa6f62d6..4bde844499ae 100644 --- a/awx/ui/src/components/Schedule/ScheduleDetail/FrequencyDetails.js +++ b/awx/ui/src/components/Schedule/ScheduleDetail/FrequencyDetails.js @@ -83,6 +83,8 @@ export default function FrequencyDetails({ 6: t`Sunday`, }; + const prefix = isException ? `exception-${type}` : `frequency-${type}`; + return (
@@ -90,6 +92,7 @@ export default function FrequencyDetails({ {type === 'week' ? ( weekdays[d.weekday]) .join(', ')} + dataCy={`${prefix}-days-of-week`} /> ) : null} - - + +
); @@ -113,11 +121,15 @@ function sortWeekday(a, b) { return a.weekday - b.weekday; } -function RunOnDetail({ type, options }) { +function RunOnDetail({ type, options, prefix }) { if (type === 'month') { if (options.runOn === 'day') { return ( - + ); } const dayOfWeek = options.runOnTheDay; @@ -138,6 +150,7 @@ function RunOnDetail({ type, options }) { /> ) } + dataCy={`${prefix}-run-on-day`} /> ); } @@ -161,6 +174,7 @@ function RunOnDetail({ type, options }) { ); } @@ -195,6 +209,7 @@ function RunOnDetail({ type, options }) { /> ) } + dataCy={`${prefix}-run-on-day`} /> ); } diff --git a/awx/ui/src/components/Schedule/ScheduleDetail/ScheduleDetail.js b/awx/ui/src/components/Schedule/ScheduleDetail/ScheduleDetail.js index 7abca5bb59f9..228e0e026383 100644 --- a/awx/ui/src/components/Schedule/ScheduleDetail/ScheduleDetail.js +++ b/awx/ui/src/components/Schedule/ScheduleDetail/ScheduleDetail.js @@ -278,64 +278,84 @@ function ScheduleDetail({ hasDaysToKeepField, schedule, surveyConfig }) { isDisabled={isDisabled} /> - - + + + - {frequency.length ? ( -

- {t`Frequency Details`} -

- {frequency.map((freq) => ( - - ))} +
+

+ {t`Frequency Details`} +

+ {frequency.map((freq) => ( + + ))} +
) : null} {exceptionFrequency.length ? ( -

- {t`Frequency Exception Details`} -

- {exceptionFrequency.map((freq) => ( - - ))} +
+

+ {t`Frequency Exception Details`} +

+ {exceptionFrequency.map((freq) => ( + + ))} +
) : null} {hasDaysToKeepField ? ( - + ) : null} {ask_job_type_on_launch && ( - + )} {showInventoryDetail && ( )} {ask_verbosity_on_launch && ( - + )} {ask_scm_branch_on_launch && ( - + + )} + {ask_limit_on_launch && ( + )} - {ask_limit_on_launch && } {showDiffModeDetail && ( )} {showCredentialsDetail && ( @@ -410,6 +446,7 @@ function ScheduleDetail({ hasDaysToKeepField, schedule, surveyConfig }) { ))} } + dataCy="schedule-credentials" /> )} {showTagsDetail && ( @@ -433,6 +470,7 @@ function ScheduleDetail({ hasDaysToKeepField, schedule, surveyConfig }) { ))} } + dataCy="schedule-job-tags" /> )} {showSkipTagsDetail && ( @@ -456,6 +494,7 @@ function ScheduleDetail({ hasDaysToKeepField, schedule, surveyConfig }) { ))} } + dataCy="schedule-skip-tags" /> )} {showVariablesDetail && ( From 35e9d00beb4e48cc206038a23252cd7ca750f2f1 Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Wed, 14 Sep 2022 15:33:00 -0700 Subject: [PATCH 8/9] improve frequency validation performance --- .../Schedule/shared/ScheduleForm.js | 41 +++++++++++-------- .../Schedule/shared/ScheduleFormFields.js | 2 +- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/awx/ui/src/components/Schedule/shared/ScheduleForm.js b/awx/ui/src/components/Schedule/shared/ScheduleForm.js index a41bb9195fe9..143cb3b44581 100644 --- a/awx/ui/src/components/Schedule/shared/ScheduleForm.js +++ b/awx/ui/src/components/Schedule/shared/ScheduleForm.js @@ -412,24 +412,8 @@ function ScheduleForm({ } }); - if (values.exceptionFrequency.length > 0) { - let rangeToCheck = 1; - values.frequency.forEach((freq) => { - if (NUM_DAYS_PER_FREQUENCY[freq] > rangeToCheck) { - rangeToCheck = NUM_DAYS_PER_FREQUENCY[freq]; - } - }); - const ruleSet = buildRuleSet(values, true); - const startDate = DateTime.fromISO(values.startDate); - const endDate = startDate.plus({ days: rangeToCheck }); - const instances = ruleSet.between( - startDate.toJSDate(), - endDate.toJSDate(), - true - ); - if (instances.length === 0) { - errors.exceptionFrequency = t`This schedule has no occurrences due to the selected exceptions.`; - } + if (values.exceptionFrequency.length > 0 && !scheduleHasInstances(values)) { + errors.exceptionFrequency = t`This schedule has no occurrences due to the selected exceptions.`; } return errors; @@ -539,3 +523,24 @@ ScheduleForm.defaultProps = { }; export default ScheduleForm; + +function scheduleHasInstances(values) { + let rangeToCheck = 1; + values.frequency.forEach((freq) => { + if (NUM_DAYS_PER_FREQUENCY[freq] > rangeToCheck) { + rangeToCheck = NUM_DAYS_PER_FREQUENCY[freq]; + } + }); + + const ruleSet = buildRuleSet(values, true); + const startDate = DateTime.fromISO(values.startDate); + const endDate = startDate.plus({ days: rangeToCheck }); + const instances = ruleSet.between( + startDate.toJSDate(), + endDate.toJSDate(), + true, + (date, i) => i === 0 + ); + + return instances.length > 0; +} diff --git a/awx/ui/src/components/Schedule/shared/ScheduleFormFields.js b/awx/ui/src/components/Schedule/shared/ScheduleFormFields.js index 887e6f7535bc..6bf6e65a8548 100644 --- a/awx/ui/src/components/Schedule/shared/ScheduleFormFields.js +++ b/awx/ui/src/components/Schedule/shared/ScheduleFormFields.js @@ -171,7 +171,7 @@ export default function ScheduleFormFields({ > Date: Thu, 15 Sep 2022 09:37:03 -0700 Subject: [PATCH 9/9] adjust DetailList spacing when two appear in succession --- awx/ui/src/components/DetailList/DetailList.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/awx/ui/src/components/DetailList/DetailList.js b/awx/ui/src/components/DetailList/DetailList.js index 78bc160f6468..dceaa1cba947 100644 --- a/awx/ui/src/components/DetailList/DetailList.js +++ b/awx/ui/src/components/DetailList/DetailList.js @@ -29,4 +29,8 @@ export default styled(DetailList)` --column-count: 3; } `} + + & + & { + margin-top: 20px; + } `;