Skip to content

Commit a3a08a3

Browse files
authored
Fix preview API not considering rules and imputation options (#898)
This commit resolves a bug where the preview API did not account for suppression rules and imputation options. The fix involves passing additional parameters to handle these configurations correctly. Testing: * e2e testing completed. * UT added to cover the new logic. Signed-off-by: Kaituo Li <kaituo@amazon.com>
1 parent 646a08e commit a3a08a3

File tree

6 files changed

+174
-24
lines changed

6 files changed

+174
-24
lines changed

.github/workflows/remote-integ-tests-workflow.yml

+27-15
Original file line numberDiff line numberDiff line change
@@ -73,36 +73,48 @@ jobs:
7373
node-version: ${{ steps.tool-versions.outputs.node_version }}
7474
registry-url: 'https://registry.npmjs.org'
7575

76-
- name: Setup Opensearch Dashboards
76+
- name: Install correct yarn version for OpenSearch Dashboards
77+
id: setup-yarn
7778
run: |
7879
npm uninstall -g yarn
79-
echo "Installing yarn ${{ steps.tool-versions.outputs.yarn_version }}"
80-
npm i -g yarn@${{ steps.tool-versions.outputs.yarn_version }}
81-
yarn cache clean
82-
yarn add sha.js
83-
working-directory: OpenSearch-Dashboards
84-
shell: bash
80+
echo "Installing yarn ${{ steps.versions_step.outputs.yarn_version }}"
81+
npm i -g yarn@${{ steps.versions_step.outputs.yarn_version }}
8582
86-
- name: Boodstrap Opensearch Dashboards
83+
- name: Yarn Cache
84+
uses: actions/cache@v4
85+
with:
86+
path: |
87+
OpenSearch-Dashboards/**/target
88+
OpenSearch-Dashboards/**/node_modules
89+
key: ${{ runner.OS }}-build-${{ hashFiles('OpenSearch-Dashboards/**/yarn.lock') }}
90+
restore-keys: |
91+
${{ runner.OS }}-build-
92+
93+
- name: Bootstrap OpenSearch Dashboards
8794
run: |
95+
cd OpenSearch-Dashboards
8896
yarn osd bootstrap --single-version=loose
89-
working-directory: OpenSearch-Dashboards/plugins/anomaly-detection-dashboards-plugin
9097
98+
- name: Compile OpenSearch Dashboards
99+
run: |
100+
cd OpenSearch-Dashboards
101+
node scripts/build_opensearch_dashboards_platform_plugins --no-examples --workers=10 --verbose
102+
91103
- name: Run Opensearch Dashboards with AD Installed
92104
run: |
93105
nohup yarn start --no-base-path --no-watch --server.host="0.0.0.0" | tee dashboard.log &
94106
working-directory: OpenSearch-Dashboards
95107

96-
- name : Check If OpenSearch Dashboards Is Ready
108+
- name: Check If OpenSearch Dashboards Is Ready
97109
if: ${{ runner.os == 'Linux' }}
98110
run: |
99-
if timeout 600 grep -q "bundles compiled successfully after" <(tail -n0 -f dashboard.log); then
100-
echo "OpenSearch Dashboards compiled successfully."
111+
cd ./OpenSearch-Dashboards
112+
if timeout 60 grep -q "http server running" <(tail -n +1 -f dashboard.log); then
113+
echo "OpenSearch Dashboards started successfully."
101114
else
102-
echo "Timeout for 600 seconds reached. OpenSearch Dashboards did not finish compiling."
115+
echo "Timeout of 60 seconds reached. OpenSearch Dashboards did not start successfully."
103116
exit 1
104-
fi
105-
working-directory: OpenSearch-Dashboards
117+
fi&
106118
107119
- name: Show OpenSearch Dashboards Logs
108120
if: always()

public/pages/ConfigureModel/containers/ConfigureModel.tsx

+2
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,8 @@ export function ConfigureModel(props: ConfigureModelProps) {
456456
categoryFields={formikProps.values.categoryField}
457457
errors={formikProps.errors}
458458
setFieldTouched={formikProps.setFieldTouched}
459+
imputationOption={formikProps.values.imputationOption}
460+
suppressionRules={formikProps.values.suppressionRules}
459461
/>
460462
) : null}
461463
</EuiPageBody>

public/pages/ConfigureModel/containers/SampleAnomalies.tsx

+6-2
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ import {
4444
} from '../../utils/anomalyResultUtils';
4545
import { focusOnFirstWrongFeature } from '../utils/helpers';
4646
import { prepareDetector } from '../utils/helpers';
47-
import { FeaturesFormikValues } from '../models/interfaces';
47+
import { FeaturesFormikValues, ImputationFormikValues, RuleFormikValues} from '../models/interfaces';
4848
import { BASE_DOCS_LINK } from '../../../utils/constants';
4949
import { prettifyErrorMessage } from '../../../../server/utils/helpers';
5050
import { CoreStart } from '../../../../../../src/core/public';
@@ -59,6 +59,8 @@ interface SampleAnomaliesProps {
5959
categoryFields: string[];
6060
errors: any;
6161
setFieldTouched: any;
62+
imputationOption?: ImputationFormikValues;
63+
suppressionRules?: RuleFormikValues[];
6264
}
6365

6466
export function SampleAnomalies(props: SampleAnomaliesProps) {
@@ -183,7 +185,9 @@ export function SampleAnomalies(props: SampleAnomaliesProps) {
183185
props.shingleSize,
184186
props.categoryFields,
185187
newDetector,
186-
true
188+
true,
189+
props.imputationOption,
190+
props.suppressionRules,
187191
);
188192
setPreviewDone(false);
189193
setZoomRange({ ...dateRange });
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
import React from 'react';
4+
import { Provider } from 'react-redux';
5+
import {
6+
HashRouter as Router,
7+
RouteComponentProps,
8+
Route,
9+
Switch,
10+
} from 'react-router-dom';
11+
import { render, fireEvent, waitFor } from '@testing-library/react';
12+
import { SampleAnomalies } from '../SampleAnomalies';
13+
import configureStore from '../../../../redux/configureStore';
14+
import { httpClientMock, coreServicesMock } from '../../../../../test/mocks';
15+
import { mockedStore } from '../../../../redux/utils/testUtils';
16+
import { CoreServicesContext } from '../../../../components/CoreServices/CoreServices';
17+
import { prepareDetector, focusOnFirstWrongFeature } from '../../utils/helpers';
18+
import { createMemoryHistory } from 'history';
19+
import {
20+
FeaturesFormikValues,
21+
ImputationFormikValues,
22+
RuleFormikValues,
23+
} from '../../../../pages/ConfigureModel/models/interfaces';
24+
import { getRandomDetector } from '../../../../redux/reducers/__tests__/utils';
25+
import { FEATURE_TYPE } from '../../../../models/interfaces';
26+
27+
// Mock the helper functions
28+
jest.mock('../../utils/helpers', () => ({
29+
...jest.requireActual('../../utils/helpers'),
30+
prepareDetector: jest.fn(() => ({
31+
// mock a detector object with at least an id, preventing the undefined error when detector.id is accessed in getSampleAdResult.
32+
id: 'test-detector-id',
33+
name: 'test-detector',
34+
description: 'test-detector-description',
35+
})),
36+
focusOnFirstWrongFeature: jest.fn(() => false),
37+
}));
38+
39+
// Mock the Redux actions if necessary
40+
jest.mock('../../../../redux/reducers/previewAnomalies', () => ({
41+
previewDetector: jest.fn(() => async () => Promise.resolve()),
42+
}));
43+
44+
describe('<SampleAnomalies /> spec', () => {
45+
beforeEach(() => {
46+
jest.clearAllMocks();
47+
console.error = jest.fn();
48+
console.warn = jest.fn();
49+
});
50+
51+
test('calls prepareDetector with imputationOption and suppressionRules when previewDetector button is clicked', async () => {
52+
// Set up the mock props
53+
const mockDetector = getRandomDetector();
54+
55+
const mockFeatureList: FeaturesFormikValues[] = [
56+
{
57+
featureId: 'feature1',
58+
featureName: 'Feature 1',
59+
featureEnabled: true,
60+
aggregationBy: 'sum',
61+
aggregationOf: [{ label: 'bytes' }],
62+
featureType: FEATURE_TYPE.SIMPLE,
63+
aggregationQuery: '',
64+
},
65+
];
66+
67+
const mockImputationOption: ImputationFormikValues = {
68+
imputationMethod: 'zero',
69+
};
70+
71+
const mockSuppressionRules: RuleFormikValues[] = [
72+
{
73+
featureName: '',
74+
absoluteThreshold: undefined,
75+
relativeThreshold: undefined,
76+
aboveBelow: 'above',
77+
},
78+
];
79+
80+
const props = {
81+
detector: mockDetector,
82+
featureList: mockFeatureList,
83+
shingleSize: 8,
84+
categoryFields: ['category1'],
85+
errors: {},
86+
setFieldTouched: jest.fn(),
87+
imputationOption: mockImputationOption,
88+
suppressionRules: mockSuppressionRules,
89+
};
90+
91+
// Create a mock history and store
92+
const history = createMemoryHistory();
93+
const initialState = {
94+
anomalies: {
95+
anomaliesResult: {
96+
anomalies: [],
97+
},
98+
},
99+
};
100+
const store = mockedStore();
101+
102+
const { getByText } = render(
103+
<Provider store={store}>
104+
<Router history={history}>
105+
<CoreServicesContext.Provider value={coreServicesMock}>
106+
<SampleAnomalies {...props} />
107+
</CoreServicesContext.Provider>
108+
</Router>
109+
</Provider>
110+
);
111+
112+
// Simulate clicking the previewDetector button
113+
fireEvent.click(getByText('Preview anomalies'));
114+
115+
// Wait for async actions to complete
116+
await waitFor(() => {
117+
expect(prepareDetector).toHaveBeenCalledWith(
118+
props.featureList,
119+
props.shingleSize,
120+
props.categoryFields,
121+
expect.anything(), // newDetector (could be the same as props.detector or modified)
122+
true,
123+
props.imputationOption,
124+
props.suppressionRules
125+
);
126+
});
127+
});
128+
});

public/pages/ConfigureModel/utils/__tests__/helpers.test.tsx

+6-6
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@ describe('featuresToFormik', () => {
3030
aggregationQuery: '',
3131
};
3232
const randomPositiveInt = Math.ceil(Math.random() * 100);
33-
const apiRequest = prepareDetector(
34-
[newFeature],
35-
randomPositiveInt,
36-
randomDetector,
37-
false
38-
);
33+
// const apiRequest = prepareDetector(
34+
// [newFeature],
35+
// randomPositiveInt,
36+
// randomDetector,
37+
// false
38+
// );
3939
// expect(apiRequest.featureAttributes).toEqual([
4040
// {
4141
// featureId: newFeature.featureId,

public/pages/ConfigureModel/utils/helpers.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,9 @@ export function prepareDetector(
340340
shingleSizeValue: number,
341341
categoryFields: string[],
342342
ad: Detector,
343-
forPreview: boolean = false
343+
forPreview: boolean = false,
344+
imputationOption?: ImputationFormikValues,
345+
suppressionRules?: RuleFormikValues[]
344346
): Detector {
345347
const detector = cloneDeep(ad);
346348
const featureAttributes = formikToFeatures(featureValues, forPreview);
@@ -354,6 +356,8 @@ export function prepareDetector(
354356
...detector.uiMetadata,
355357
features: { ...featuresToUIMetadata(featureValues) },
356358
},
359+
imputationOption: formikToImputationOption(imputationOption),
360+
rules: formikToRules(suppressionRules),
357361
};
358362
}
359363

0 commit comments

Comments
 (0)