Skip to content

Commit 7c2bb16

Browse files
davetsaydavetsay
authored
Bugfix/7873 time conductor input validation (#7886)
* validate on change because input is too aggressive * validate logical bounds on submit * perfection
1 parent 890ddca commit 7c2bb16

File tree

4 files changed

+296
-203
lines changed

4 files changed

+296
-203
lines changed

‎e2e/appActions.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,10 @@ async function setTimeConductorBounds(page, { submitChanges = true, ...bounds })
510510
// Open the time conductor popup
511511
await page.getByRole('button', { name: 'Time Conductor Mode', exact: true }).click();
512512

513+
// FIXME: https://github.com/nasa/openmct/pull/7818
514+
// eslint-disable-next-line playwright/no-wait-for-timeout
515+
await page.waitForTimeout(500);
516+
513517
if (startDate) {
514518
await page.getByLabel('Start date').fill(startDate);
515519
}

‎e2e/tests/functional/plugins/telemetryTable/telemetryTable.e2e.spec.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,8 @@ test.describe('Telemetry Table', () => {
117117

118118
endTimeStamp.setUTCMinutes(endTimeStamp.getUTCMinutes() - 5);
119119
const endDate = endTimeStamp.toISOString().split('T')[0];
120-
const endTime = endTimeStamp.toISOString().split('T')[1];
120+
const milliseconds = endTimeStamp.getMilliseconds();
121+
const endTime = endTimeStamp.toISOString().split('T')[1].replace(`.${milliseconds}Z`, '');
121122

122123
await setTimeConductorBounds(page, { endDate, endTime });
123124

‎e2e/tests/functional/plugins/timeConductor/timeConductor.e2e.spec.js‎

Lines changed: 189 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -24,65 +24,210 @@ import {
2424
setEndOffset,
2525
setFixedTimeMode,
2626
setRealTimeMode,
27-
setStartOffset,
28-
setTimeConductorBounds
27+
setStartOffset
2928
} from '../../../../appActions.js';
3029
import { expect, test } from '../../../../pluginFixtures.js';
3130

3231
test.describe('Time conductor operations', () => {
33-
test('validate start time does not exceed end time', async ({ page }) => {
32+
const DAY = '2024-01-01';
33+
const DAY_AFTER = '2024-01-02';
34+
const ONE_O_CLOCK = '01:00:00';
35+
const TWO_O_CLOCK = '02:00:00';
36+
37+
test.beforeEach(async ({ page }) => {
3438
// Go to baseURL
3539
await page.goto('./', { waitUntil: 'domcontentloaded' });
36-
const year = new Date().getFullYear();
40+
});
41+
42+
test('validate date and time inputs are validated on input event', async ({ page }) => {
43+
const submitButtonLocator = page.getByLabel('Submit time bounds');
44+
45+
// Open the time conductor popup
46+
await page.getByRole('button', { name: 'Time Conductor Mode', exact: true }).click();
47+
48+
await test.step('invalid start date disables submit button', async () => {
49+
const initialStartDate = await page.getByLabel('Start date').inputValue();
50+
const invalidStartDate = `${initialStartDate.substring(0, 5)}${initialStartDate.substring(6)}`;
51+
52+
await page.getByLabel('Start date').fill(invalidStartDate);
53+
await expect(submitButtonLocator).toBeDisabled();
54+
await page.getByLabel('Start date').fill(initialStartDate);
55+
await expect(submitButtonLocator).toBeEnabled();
56+
});
57+
58+
await test.step('invalid start time disables submit button', async () => {
59+
const initialStartTime = await page.getByLabel('Start time').inputValue();
60+
const invalidStartTime = `${initialStartTime.substring(0, 5)}${initialStartTime.substring(6)}`;
61+
62+
await page.getByLabel('Start time').fill(invalidStartTime);
63+
await expect(submitButtonLocator).toBeDisabled();
64+
await page.getByLabel('Start time').fill(initialStartTime);
65+
await expect(submitButtonLocator).toBeEnabled();
66+
});
67+
68+
await test.step('disable/enable submit button also works with multiple invalid inputs', async () => {
69+
const initialEndDate = await page.getByLabel('End date').inputValue();
70+
const invalidEndDate = `${initialEndDate.substring(0, 5)}${initialEndDate.substring(6)}`;
71+
const initialStartTime = await page.getByLabel('Start time').inputValue();
72+
const invalidStartTime = `${initialStartTime.substring(0, 5)}${initialStartTime.substring(6)}`;
73+
74+
await page.getByLabel('Start time').fill(invalidStartTime);
75+
await expect(submitButtonLocator).toBeDisabled();
76+
await page.getByLabel('End date').fill(invalidEndDate);
77+
await expect(submitButtonLocator).toBeDisabled();
78+
await page.getByLabel('End date').fill(initialEndDate);
79+
await expect(submitButtonLocator).toBeDisabled();
80+
await page.getByLabel('Start time').fill(initialStartTime);
81+
await expect(submitButtonLocator).toBeEnabled();
82+
});
83+
});
84+
85+
test('validate date and time inputs validation is reported on change event', async ({ page }) => {
86+
// Open the time conductor popup
87+
await page.getByRole('button', { name: 'Time Conductor Mode', exact: true }).click();
88+
89+
await test.step('invalid start date is reported on change event, not on input event', async () => {
90+
const initialStartDate = await page.getByLabel('Start date').inputValue();
91+
const invalidStartDate = `${initialStartDate.substring(0, 5)}${initialStartDate.substring(6)}`;
92+
93+
await page.getByLabel('Start date').fill(invalidStartDate);
94+
await expect(page.getByLabel('Start date')).not.toHaveAttribute('title', 'Invalid Date');
95+
await page.getByLabel('Start date').press('Tab');
96+
await expect(page.getByLabel('Start date')).toHaveAttribute('title', 'Invalid Date');
97+
await page.getByLabel('Start date').fill(initialStartDate);
98+
await expect(page.getByLabel('Start date')).not.toHaveAttribute('title', 'Invalid Date');
99+
});
100+
101+
await test.step('invalid start time is reported on change event, not on input event', async () => {
102+
const initialStartTime = await page.getByLabel('Start time').inputValue();
103+
const invalidStartTime = `${initialStartTime.substring(0, 5)}${initialStartTime.substring(6)}`;
104+
105+
await page.getByLabel('Start time').fill(invalidStartTime);
106+
await expect(page.getByLabel('Start time')).not.toHaveAttribute('title', 'Invalid Time');
107+
await page.getByLabel('Start time').press('Tab');
108+
await expect(page.getByLabel('Start time')).toHaveAttribute('title', 'Invalid Time');
109+
await page.getByLabel('Start time').fill(initialStartTime);
110+
await expect(page.getByLabel('Start time')).not.toHaveAttribute('title', 'Invalid Time');
111+
});
112+
113+
await test.step('invalid end date is reported on change event, not on input event', async () => {
114+
const initialEndDate = await page.getByLabel('End date').inputValue();
115+
const invalidEndDate = `${initialEndDate.substring(0, 5)}${initialEndDate.substring(6)}`;
116+
117+
await page.getByLabel('End date').fill(invalidEndDate);
118+
await expect(page.getByLabel('End date')).not.toHaveAttribute('title', 'Invalid Date');
119+
await page.getByLabel('End date').press('Tab');
120+
await expect(page.getByLabel('End date')).toHaveAttribute('title', 'Invalid Date');
121+
await page.getByLabel('End date').fill(initialEndDate);
122+
await expect(page.getByLabel('End date')).not.toHaveAttribute('title', 'Invalid Date');
123+
});
124+
125+
await test.step('invalid end time is reported on change event, not on input event', async () => {
126+
const initialEndTime = await page.getByLabel('End time').inputValue();
127+
const invalidEndTime = `${initialEndTime.substring(0, 5)}${initialEndTime.substring(6)}`;
128+
129+
await page.getByLabel('End time').fill(invalidEndTime);
130+
await expect(page.getByLabel('End time')).not.toHaveAttribute('title', 'Invalid Time');
131+
await page.getByLabel('End time').press('Tab');
132+
await expect(page.getByLabel('End time')).toHaveAttribute('title', 'Invalid Time');
133+
await page.getByLabel('End time').fill(initialEndTime);
134+
await expect(page.getByLabel('End time')).not.toHaveAttribute('title', 'Invalid Time');
135+
});
136+
});
137+
138+
test('validate start time does not exceed end time on submit', async ({ page }) => {
139+
// Open the time conductor popup
140+
await page.getByRole('button', { name: 'Time Conductor Mode', exact: true }).click();
141+
142+
// FIXME: https://github.com/nasa/openmct/pull/7818
143+
// eslint-disable-next-line playwright/no-wait-for-timeout
144+
await page.waitForTimeout(500);
145+
146+
await page.getByLabel('Start date').fill(DAY);
147+
await page.getByLabel('Start time').fill(TWO_O_CLOCK);
148+
await page.getByLabel('End date').fill(DAY);
149+
await page.getByLabel('End time').fill(ONE_O_CLOCK);
150+
await page.getByLabel('Submit time bounds').click();
37151

38-
// Set initial valid time bounds
39-
const startDate = `${year}-01-01`;
40-
const startTime = '01:00:00';
41-
const endDate = `${year}-01-01`;
42-
const endTime = '02:00:00';
43-
await setTimeConductorBounds(page, { startDate, startTime, endDate, endTime });
152+
await expect(page.getByLabel('Start date')).toHaveAttribute(
153+
'title',
154+
'Specified start date exceeds end bound'
155+
);
156+
await expect(page.getByLabel('Start bounds')).not.toHaveText(`${DAY} ${TWO_O_CLOCK}.000Z`);
157+
await expect(page.getByLabel('End bounds')).not.toHaveText(`${DAY} ${ONE_O_CLOCK}.000Z`);
158+
159+
await page.getByLabel('Start date').fill(DAY);
160+
await page.getByLabel('Start time').fill(ONE_O_CLOCK);
161+
await page.getByLabel('End date').fill(DAY);
162+
await page.getByLabel('End time').fill(TWO_O_CLOCK);
163+
await page.getByLabel('Submit time bounds').click();
164+
165+
await expect(page.getByLabel('Start bounds')).toHaveText(`${DAY} ${ONE_O_CLOCK}.000Z`);
166+
await expect(page.getByLabel('End bounds')).toHaveText(`${DAY} ${TWO_O_CLOCK}.000Z`);
167+
});
44168

169+
test('validate start datetime does not exceed end datetime on submit', async ({ page }) => {
45170
// Open the time conductor popup
46171
await page.getByRole('button', { name: 'Time Conductor Mode', exact: true }).click();
47172

48-
// Test invalid start date
49-
const invalidStartDate = `${year}-01-02`;
50-
await page.getByLabel('Start date').fill(invalidStartDate);
51-
await expect(page.getByLabel('Submit time bounds')).toBeDisabled();
52-
await page.getByLabel('Start date').fill(startDate);
53-
await expect(page.getByLabel('Submit time bounds')).toBeEnabled();
54-
55-
// Test invalid end date
56-
const invalidEndDate = `${year - 1}-12-31`;
57-
await page.getByLabel('End date').fill(invalidEndDate);
58-
await expect(page.getByLabel('Submit time bounds')).toBeDisabled();
59-
await page.getByLabel('End date').fill(endDate);
60-
await expect(page.getByLabel('Submit time bounds')).toBeEnabled();
61-
62-
// Test invalid start time
63-
const invalidStartTime = '42:00:00';
64-
await page.getByLabel('Start time').fill(invalidStartTime);
65-
await expect(page.getByLabel('Submit time bounds')).toBeDisabled();
66-
await page.getByLabel('Start time').fill(startTime);
67-
await expect(page.getByLabel('Submit time bounds')).toBeEnabled();
68-
69-
// Test invalid end time
70-
const invalidEndTime = '43:00:00';
71-
await page.getByLabel('End time').fill(invalidEndTime);
72-
await expect(page.getByLabel('Submit time bounds')).toBeDisabled();
73-
await page.getByLabel('End time').fill(endTime);
74-
await expect(page.getByLabel('Submit time bounds')).toBeEnabled();
75-
76-
// Submit valid time bounds
173+
// FIXME: https://github.com/nasa/openmct/pull/7818
174+
// eslint-disable-next-line playwright/no-wait-for-timeout
175+
await page.waitForTimeout(500);
176+
177+
await page.getByLabel('Start date').fill(DAY_AFTER);
178+
await page.getByLabel('Start time').fill(ONE_O_CLOCK);
179+
await page.getByLabel('End date').fill(DAY);
180+
await page.getByLabel('End time').fill(ONE_O_CLOCK);
77181
await page.getByLabel('Submit time bounds').click();
78182

79-
// Verify the submitted time bounds
80-
await expect(page.getByLabel('Start bounds')).toHaveText(
81-
new RegExp(`${startDate} ${startTime}.000Z`)
183+
await expect(page.getByLabel('Start date')).toHaveAttribute(
184+
'title',
185+
'Specified start date exceeds end bound'
82186
);
83-
await expect(page.getByLabel('End bounds')).toHaveText(
84-
new RegExp(`${endDate} ${endTime}.000Z`)
187+
await expect(page.getByLabel('Start bounds')).not.toHaveText(
188+
`${DAY_AFTER} ${ONE_O_CLOCK}.000Z`
85189
);
190+
await expect(page.getByLabel('End bounds')).not.toHaveText(`${DAY} ${ONE_O_CLOCK}.000Z`);
191+
192+
await page.getByLabel('Start date').fill(DAY);
193+
await page.getByLabel('Start time').fill(ONE_O_CLOCK);
194+
await page.getByLabel('End date').fill(DAY_AFTER);
195+
await page.getByLabel('End time').fill(ONE_O_CLOCK);
196+
await page.getByLabel('Submit time bounds').click();
197+
198+
await expect(page.getByLabel('Start bounds')).toHaveText(`${DAY} ${ONE_O_CLOCK}.000Z`);
199+
await expect(page.getByLabel('End bounds')).toHaveText(`${DAY_AFTER} ${ONE_O_CLOCK}.000Z`);
200+
});
201+
202+
test('cancelling form does not set bounds', async ({ page }) => {
203+
test.info().annotations.push({
204+
type: 'issue',
205+
description: 'https://github.com/nasa/openmct/issues/7791'
206+
});
207+
208+
// Open the time conductor popup
209+
await page.getByRole('button', { name: 'Time Conductor Mode', exact: true }).click();
210+
211+
await page.getByLabel('Start date').fill(DAY);
212+
await page.getByLabel('Start time').fill(ONE_O_CLOCK);
213+
await page.getByLabel('End date').fill(DAY_AFTER);
214+
await page.getByLabel('End time').fill(ONE_O_CLOCK);
215+
await page.getByLabel('Discard changes and close time popup').click();
216+
217+
await expect(page.getByLabel('Start bounds')).not.toHaveText(`${DAY} ${ONE_O_CLOCK}.000Z`);
218+
await expect(page.getByLabel('End bounds')).not.toHaveText(`${DAY_AFTER} ${ONE_O_CLOCK}.000Z`);
219+
220+
// Open the time conductor popup
221+
await page.getByRole('button', { name: 'Time Conductor Mode', exact: true }).click();
222+
223+
await page.getByLabel('Start date').fill(DAY);
224+
await page.getByLabel('Start time').fill(ONE_O_CLOCK);
225+
await page.getByLabel('End date').fill(DAY_AFTER);
226+
await page.getByLabel('End time').fill(ONE_O_CLOCK);
227+
await page.getByLabel('Submit time bounds').click();
228+
229+
await expect(page.getByLabel('Start bounds')).toHaveText(`${DAY} ${ONE_O_CLOCK}.000Z`);
230+
await expect(page.getByLabel('End bounds')).toHaveText(`${DAY_AFTER} ${ONE_O_CLOCK}.000Z`);
86231
});
87232
});
88233

@@ -131,77 +276,6 @@ test.describe('Global Time Conductor', () => {
131276
await expect(page.getByLabel('End offset: 01:30:31')).toBeVisible();
132277
});
133278

134-
test('Input field validation: fixed time mode', async ({ page }) => {
135-
test.info().annotations.push({
136-
type: 'issue',
137-
description: 'https://github.com/nasa/openmct/issues/7791'
138-
});
139-
// Switch to fixed time mode
140-
await setFixedTimeMode(page);
141-
142-
// Define valid time bounds for testing
143-
const validBounds = {
144-
startDate: '2024-04-20',
145-
startTime: '00:04:20',
146-
endDate: '2024-04-20',
147-
endTime: '16:04:20'
148-
};
149-
// Set valid time conductor bounds ✌️
150-
await setTimeConductorBounds(page, validBounds);
151-
152-
// Verify that the time bounds are set correctly
153-
await expect(page.getByLabel(`Start bounds: 2024-04-20 00:04:20.000Z`)).toBeVisible();
154-
await expect(page.getByLabel(`End bounds: 2024-04-20 16:04:20.000Z`)).toBeVisible();
155-
156-
// Open the Time Conductor Mode popup
157-
await page.getByLabel('Time Conductor Mode').click();
158-
159-
// Test invalid start date
160-
const invalidStartDate = '2024-04-21';
161-
await page.getByLabel('Start date').fill(invalidStartDate);
162-
await expect(page.getByLabel('Submit time bounds')).toBeDisabled();
163-
await page.getByLabel('Start date').fill(validBounds.startDate);
164-
await expect(page.getByLabel('Submit time bounds')).toBeEnabled();
165-
166-
// Test invalid end date
167-
const invalidEndDate = '2024-04-19';
168-
await page.getByLabel('End date').fill(invalidEndDate);
169-
await expect(page.getByLabel('Submit time bounds')).toBeDisabled();
170-
await page.getByLabel('End date').fill(validBounds.endDate);
171-
await expect(page.getByLabel('Submit time bounds')).toBeEnabled();
172-
173-
// Test invalid start time
174-
const invalidStartTime = '16:04:21';
175-
await page.getByLabel('Start time').fill(invalidStartTime);
176-
await expect(page.getByLabel('Submit time bounds')).toBeDisabled();
177-
await page.getByLabel('Start time').fill(validBounds.startTime);
178-
await expect(page.getByLabel('Submit time bounds')).toBeEnabled();
179-
180-
// Test invalid end time
181-
const invalidEndTime = '00:04:19';
182-
await page.getByLabel('End time').fill(invalidEndTime);
183-
await expect(page.getByLabel('Submit time bounds')).toBeDisabled();
184-
await page.getByLabel('End time').fill(validBounds.endTime);
185-
await expect(page.getByLabel('Submit time bounds')).toBeEnabled();
186-
187-
// Verify that the time bounds remain unchanged after invalid inputs
188-
await expect(page.getByLabel(`Start bounds: 2024-04-20 00:04:20.000Z`)).toBeVisible();
189-
await expect(page.getByLabel(`End bounds: 2024-04-20 16:04:20.000Z`)).toBeVisible();
190-
191-
// Discard changes and verify that bounds remain unchanged
192-
await setTimeConductorBounds(page, {
193-
startDate: validBounds.startDate,
194-
startTime: '04:20:00',
195-
endDate: validBounds.endDate,
196-
endTime: '04:20:20',
197-
submitChanges: false
198-
});
199-
200-
// Verify that the original time bounds are still displayed after discarding changes
201-
await expect(page.getByLabel(`Start bounds: 2024-04-20 00:04:20.000Z`)).toBeVisible();
202-
await expect(page.getByLabel(`End bounds: 2024-04-20 16:04:20.000Z`)).toBeVisible();
203-
});
204-
205279
/**
206280
* Verify that offsets and url params are preserved when switching
207281
* between fixed timespan and real-time mode.

0 commit comments

Comments
 (0)