Skip to content

Commit 1f55ed4

Browse files
KishoreKicha14KishoreKicha14
authored
3.1 Group by selector on Configuration page always shows None after refresh (#409)
Signed-off-by: Kishore Kumaar Natarajan <[email protected]>
1 parent 06f5a62 commit 1f55ed4

File tree

7 files changed

+430
-16
lines changed

7 files changed

+430
-16
lines changed

‎common/constants.ts‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,7 @@ export const GROUP_BY = 'Group by';
1818
export const AVERAGE_LATENCY = 'Average Latency';
1919
export const AVERAGE_CPU_TIME = 'Average CPU Time';
2020
export const AVERAGE_MEMORY_USAGE = 'Average Memory Usage';
21-
/*
22-
* Copyright OpenSearch Contributors
23-
* SPDX-License-Identifier: Apache-2.0
24-
*/
21+
2522
export const MetricType = {
2623
LATENCY: 'latency',
2724
CPU: 'cpu',
@@ -69,3 +66,41 @@ export const DEFAULT_TIME_UNIT = TIME_UNIT.MINUTES;
6966
export const DEFAULT_GROUP_BY = 'none';
7067
export const DEFAULT_EXPORTER_TYPE = EXPORTER_TYPE.localIndex;
7168
export const DEFAULT_DELETE_AFTER_DAYS = '7';
69+
export const DEFAULT_REFRESH_INTERVAL = 30000; // default 30s
70+
export const TOP_N_DISPLAY_LIMIT = 9;
71+
export const WLM_GROUP_ID_PARAM = 'wlmGroupId';
72+
export const ALL_WORKLOAD_GROUPS_TEXT = 'All workload groups';
73+
export const CHART_COLORS = [
74+
'#1f77b4',
75+
'#ff7f0e',
76+
'#2ca02c',
77+
'#d62728',
78+
'#9467bd',
79+
'#8c564b',
80+
'#e377c2',
81+
'#7f7f7f',
82+
'#bcbd22',
83+
'#17becf',
84+
];
85+
export const REFRESH_INTERVAL_OPTIONS = [
86+
{ value: '5000', text: '5 seconds' },
87+
{ value: '10000', text: '10 seconds' },
88+
{ value: '30000', text: '30 seconds' },
89+
{ value: '60000', text: '1 minute' },
90+
];
91+
92+
// Validation constants
93+
export const VALIDATION_LIMITS = {
94+
TOP_N_SIZE: {
95+
MIN: 1,
96+
MAX: 100,
97+
},
98+
WINDOW_SIZE_HOURS: {
99+
MIN: 1,
100+
MAX: 24,
101+
},
102+
DELETE_AFTER_DAYS: {
103+
MIN: 1,
104+
MAX: 180,
105+
},
106+
};

‎public/pages/Configuration/Configuration.test.tsx‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ import '@testing-library/jest-dom/extend-expect';
99
import { MemoryRouter } from 'react-router-dom';
1010
import Configuration from './Configuration';
1111
import { DataSourceContext } from '../TopNQueries/TopNQueries';
12+
import { TIME_UNITS_TEXT, EXPORTER_TYPE } from '../../../common/constants';
13+
import {
14+
validateTopNSize,
15+
validateWindowSize,
16+
validateDeleteAfterDays,
17+
validateConfiguration,
18+
} from './configurationValidation';
1219

1320
const mockConfigInfo = jest.fn();
1421
const mockCoreStart = {
@@ -145,4 +152,183 @@ describe('Configuration Component', () => {
145152
fireEvent.click(screen.getByText('Cancel'));
146153
expect(getTopNSizeConfiguration()[0]).toHaveValue(5); // Resets to initial value
147154
});
155+
156+
describe('Validation Logic Tests', () => {
157+
describe('TopN Size Validation', () => {
158+
it('should hide Save button when topN size is less than 1', () => {
159+
renderConfiguration();
160+
fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '0' } });
161+
expect(screen.queryByText('Save')).not.toBeInTheDocument();
162+
});
163+
164+
it('should hide Save button when topN size is greater than 100', () => {
165+
renderConfiguration();
166+
fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '101' } });
167+
expect(screen.queryByText('Save')).not.toBeInTheDocument();
168+
});
169+
170+
it('should show Save button when topN size is within valid range (1-100)', () => {
171+
renderConfiguration();
172+
fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '50' } });
173+
expect(screen.getByText('Save')).toBeInTheDocument();
174+
});
175+
});
176+
177+
describe('Window Size Validation - Minutes', () => {
178+
it('should hide Save button when window size is empty for minutes', () => {
179+
renderConfiguration();
180+
fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } });
181+
fireEvent.change(getWindowSizeConfigurations()[2], { target: { value: 'MINUTES' } });
182+
fireEvent.change(getWindowSizeConfigurations()[1], { target: { value: '' } });
183+
expect(screen.queryByText('Save')).not.toBeInTheDocument();
184+
});
185+
186+
it('should hide Save button when window size is NaN for minutes', () => {
187+
renderConfiguration();
188+
fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } });
189+
fireEvent.change(getWindowSizeConfigurations()[2], { target: { value: 'MINUTES' } });
190+
fireEvent.change(getWindowSizeConfigurations()[1], { target: { value: 'invalid' } });
191+
expect(screen.queryByText('Save')).not.toBeInTheDocument();
192+
});
193+
194+
it('should show Save button when window size is valid for minutes', () => {
195+
renderConfiguration();
196+
fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } });
197+
fireEvent.change(getWindowSizeConfigurations()[2], { target: { value: 'MINUTES' } });
198+
fireEvent.change(getWindowSizeConfigurations()[1], { target: { value: '5' } });
199+
expect(screen.getByText('Save')).toBeInTheDocument();
200+
});
201+
});
202+
203+
describe('Window Size Validation - Hours', () => {
204+
it('should show Save button when window size is within valid range (1-24) for hours', () => {
205+
renderConfiguration();
206+
fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } });
207+
fireEvent.change(getWindowSizeConfigurations()[2], { target: { value: 'HOURS' } });
208+
fireEvent.change(getWindowSizeConfigurations()[1], { target: { value: '12' } });
209+
expect(screen.getByText('Save')).toBeInTheDocument();
210+
});
211+
});
212+
213+
describe('Delete After Days Validation', () => {
214+
it('should hide Save button when delete after days is less than 1 for local index', () => {
215+
renderConfiguration();
216+
fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } });
217+
const deleteAfterField = getTopNSizeConfiguration()[1];
218+
fireEvent.change(deleteAfterField, { target: { value: '0' } });
219+
expect(screen.queryByText('Save')).not.toBeInTheDocument();
220+
});
221+
222+
it('should hide Save button when delete after days is greater than 180 for local index', () => {
223+
renderConfiguration();
224+
fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } });
225+
const deleteAfterField = getTopNSizeConfiguration()[1];
226+
fireEvent.change(deleteAfterField, { target: { value: '181' } });
227+
expect(screen.queryByText('Save')).not.toBeInTheDocument();
228+
});
229+
230+
it('should show Save button when delete after days is within valid range (1-180) for local index', () => {
231+
renderConfiguration();
232+
fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } });
233+
const deleteAfterField = getTopNSizeConfiguration()[1];
234+
fireEvent.change(deleteAfterField, { target: { value: '90' } });
235+
expect(screen.getByText('Save')).toBeInTheDocument();
236+
});
237+
238+
it('should show Save button when exporter is changed to none', () => {
239+
renderConfiguration();
240+
fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } });
241+
fireEvent.change(screen.getByDisplayValue('Local Index'), { target: { value: 'none' } });
242+
expect(screen.getByText('Save')).toBeInTheDocument();
243+
});
244+
});
245+
246+
describe('Combined Validation Scenarios', () => {
247+
it('should hide Save button when multiple validation rules fail', () => {
248+
renderConfiguration();
249+
fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '101' } });
250+
expect(screen.queryByText('Save')).not.toBeInTheDocument();
251+
});
252+
253+
it('should show Save button when all validation rules pass', () => {
254+
renderConfiguration();
255+
fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '25' } });
256+
expect(screen.getByText('Save')).toBeInTheDocument();
257+
});
258+
});
259+
});
260+
261+
describe('Validation Utility Functions', () => {
262+
describe('validateTopNSize', () => {
263+
it('should return false for values less than 1', () => {
264+
expect(validateTopNSize('0')).toBe(false);
265+
expect(validateTopNSize('-1')).toBe(false);
266+
});
267+
268+
it('should return false for values greater than 100', () => {
269+
expect(validateTopNSize('101')).toBe(false);
270+
expect(validateTopNSize('200')).toBe(false);
271+
});
272+
273+
it('should return true for valid values (1-100)', () => {
274+
expect(validateTopNSize('1')).toBe(true);
275+
expect(validateTopNSize('50')).toBe(true);
276+
expect(validateTopNSize('100')).toBe(true);
277+
});
278+
279+
it('should return false for non-numeric strings', () => {
280+
expect(validateTopNSize('abc')).toBe(false);
281+
expect(validateTopNSize('')).toBe(false);
282+
expect(validateTopNSize('10.5')).toBe(false);
283+
});
284+
});
285+
286+
describe('validateWindowSize', () => {
287+
it('should validate minutes correctly', () => {
288+
const minutesUnit = TIME_UNITS_TEXT[0].value;
289+
expect(validateWindowSize('', minutesUnit)).toBe(false);
290+
expect(validateWindowSize('abc', minutesUnit)).toBe(false);
291+
expect(validateWindowSize('1', minutesUnit)).toBe(true);
292+
expect(validateWindowSize('30', minutesUnit)).toBe(true);
293+
});
294+
295+
it('should validate hours correctly', () => {
296+
const hoursUnit = TIME_UNITS_TEXT[1].value;
297+
expect(validateWindowSize('0', hoursUnit)).toBe(false);
298+
expect(validateWindowSize('25', hoursUnit)).toBe(false);
299+
expect(validateWindowSize('1', hoursUnit)).toBe(true);
300+
expect(validateWindowSize('24', hoursUnit)).toBe(true);
301+
});
302+
});
303+
304+
describe('validateDeleteAfterDays', () => {
305+
it('should validate local index correctly', () => {
306+
const localIndexType = EXPORTER_TYPE.localIndex;
307+
expect(validateDeleteAfterDays('0', localIndexType)).toBe(false);
308+
expect(validateDeleteAfterDays('181', localIndexType)).toBe(false);
309+
expect(validateDeleteAfterDays('1', localIndexType)).toBe(true);
310+
expect(validateDeleteAfterDays('180', localIndexType)).toBe(true);
311+
});
312+
313+
it('should return true for non-local index', () => {
314+
const noneType = EXPORTER_TYPE.none;
315+
expect(validateDeleteAfterDays('0', noneType)).toBe(true);
316+
expect(validateDeleteAfterDays('abc', noneType)).toBe(true);
317+
});
318+
});
319+
320+
describe('validateConfiguration', () => {
321+
it('should return false when any validation fails', () => {
322+
expect(validateConfiguration('101', '5', 'MINUTES', '30', EXPORTER_TYPE.localIndex)).toBe(
323+
false
324+
);
325+
expect(validateConfiguration('50', '', 'MINUTES', '30', EXPORTER_TYPE.localIndex)).toBe(
326+
false
327+
);
328+
expect(validateConfiguration('50', '5', 'MINUTES', '181', EXPORTER_TYPE.localIndex)).toBe(
329+
false
330+
);
331+
});
332+
});
333+
});
148334
});

‎public/pages/Configuration/Configuration.tsx‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
} from '../../../common/constants';
4444
import { QueryInsightsDataSourceMenu } from '../../components/DataSourcePicker';
4545
import { QueryInsightsDashboardsPluginStartDependencies } from '../../types';
46+
import { validateConfiguration } from './configurationValidation';
4647

4748
const Configuration = ({
4849
latencySettings,
@@ -198,6 +199,9 @@ const Configuration = ({
198199
);
199200

200201
const WindowChoice = time === TIME_UNITS_TEXT[0].value ? MinutesBox : HoursBox;
202+
const isLocalIndex = exporterType === EXPORTER_TYPE.localIndex;
203+
const parsedDeleteAfter = parseInt(deleteAfterDays, 10);
204+
const isDeleteAfterValid = !isLocalIndex || (parsedDeleteAfter >= 1 && parsedDeleteAfter <= 180);
201205

202206
const isChanged =
203207
isEnabled !== metricSettingsMap[metric].isEnabled ||
@@ -208,13 +212,7 @@ const Configuration = ({
208212
exporterType !== dataRetentionSettingMap.dataRetention.exporterType ||
209213
deleteAfterDays !== dataRetentionSettingMap.dataRetention.deleteAfterDays;
210214

211-
const isValid = (() => {
212-
const nVal = parseInt(topNSize, 10);
213-
if (nVal < 1 || nVal > 100) return false;
214-
if (time === TIME_UNITS_TEXT[0].value) return true;
215-
const windowVal = parseInt(windowSize, 10);
216-
return windowVal >= 1 && windowVal <= 24;
217-
})();
215+
const isValid = validateConfiguration(topNSize, windowSize, time, deleteAfterDays, exporterType);
218216

219217
const formRowPadding = { padding: '0px 0px 20px' };
220218
const enabledSymb = <EuiHealth color="primary">Enabled</EuiHealth>;
@@ -505,13 +503,25 @@ const Configuration = ({
505503
/>
506504
</EuiFlexItem>
507505
<EuiFlexItem>
508-
<EuiFormRow style={formRowPadding}>
506+
<EuiFormRow
507+
style={formRowPadding}
508+
helpText="Max allowed limit 180."
509+
isInvalid={isLocalIndex && !isDeleteAfterValid}
510+
error={
511+
isLocalIndex && !isDeleteAfterValid
512+
? 'Please enter a value between 1 and 180.'
513+
: undefined
514+
}
515+
>
509516
<EuiFieldNumber
510-
disabled={exporterType !== EXPORTER_TYPE.localIndex}
517+
disabled={!isLocalIndex}
511518
min={1}
512519
max={180}
513-
value={exporterType !== EXPORTER_TYPE.localIndex ? '' : deleteAfterDays}
520+
value={
521+
!isLocalIndex ? '' : deleteAfterDays === '' ? '' : Number(deleteAfterDays)
522+
}
514523
onChange={onDeleteAfterDaysChange}
524+
isInvalid={isLocalIndex && !isDeleteAfterValid}
515525
/>
516526
</EuiFormRow>
517527
</EuiFlexItem>

‎public/pages/Configuration/__snapshots__/Configuration.test.tsx.snap‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -962,6 +962,7 @@ exports[`Configuration Component renders with default settings: should match def
962962
class="euiFormControlLayout__childrenWrapper"
963963
>
964964
<input
965+
aria-describedby="random_html_id-help-0"
965966
class="euiFieldNumber"
966967
id="random_html_id"
967968
max="180"
@@ -971,6 +972,12 @@ exports[`Configuration Component renders with default settings: should match def
971972
/>
972973
</div>
973974
</div>
975+
<div
976+
class="euiFormHelpText euiFormRow__text"
977+
id="random_html_id-help-0"
978+
>
979+
Max allowed limit 180.
980+
</div>
974981
</div>
975982
</div>
976983
</div>
@@ -1991,6 +1998,7 @@ exports[`Configuration Component updates state when toggling metrics and enables
19911998
class="euiFormControlLayout__childrenWrapper"
19921999
>
19932000
<input
2001+
aria-describedby="random_html_id-help-0"
19942002
class="euiFieldNumber"
19952003
id="random_html_id"
19962004
max="180"
@@ -2000,6 +2008,12 @@ exports[`Configuration Component updates state when toggling metrics and enables
20002008
/>
20012009
</div>
20022010
</div>
2011+
<div
2012+
class="euiFormHelpText euiFormRow__text"
2013+
id="random_html_id-help-0"
2014+
>
2015+
Max allowed limit 180.
2016+
</div>
20032017
</div>
20042018
</div>
20052019
</div>

0 commit comments

Comments
 (0)