Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Add 'No data' state for historical detectors #364

Merged
merged 5 commits into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions public/pages/DetectorsList/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,18 @@ export const getURLQueryParams = (location: {
};
};

// For realtime detectors: cannot have 'Completed' state
// For realtime detectors: cannot have 'Completed' nor 'No data' states
export const getDetectorStateOptions = () => {
return Object.values(DETECTOR_STATE)
.map((detectorState) => ({
label: detectorState,
text: detectorState,
}))
.filter((option) => option.label !== DETECTOR_STATE.FINISHED);
.filter(
(option) =>
option.label !== DETECTOR_STATE.FINISHED &&
option.label !== DETECTOR_STATE.NO_DATA
);
};

export const getDetectorsForAction = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ export const HistoricalDetectorDetail = (
isOpen: false,
action: undefined,
});
const callout = getCallout(detector, isStoppingDetector);

useEffect(() => {
if (
Expand Down Expand Up @@ -299,6 +298,8 @@ export const HistoricalDetectorDetail = (
backgroundColor: '#FFF',
};

const callout = getCallout(detector, isStoppingDetector, handleEditAction);

return (
<React.Fragment>
{!isEmpty(detector) ? getHistoricalDetectorDetailModal() : null}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ const ACTIONS_BUTTON_TEXT = 'Actions';
const STOPPED_CALLOUT_TEXT = 'The historical detector is stopped';
const INIT_CALLOUT_TEXT = 'Initializing the historical detector';
const RUNNING_CALLOUT_TEXT = 'Running the historical detector';
const NO_DATA_CALLOUT_TEXT =
'No data available in the selected detection date range.';
const START_DETECTOR_BUTTON_TEXT = 'Start historical detector';
const STOP_DETECTOR_BUTTON_TEXT = 'Stop historical detector';

Expand Down Expand Up @@ -167,4 +169,17 @@ describe('<HistoricalDetectorDetail /> spec', () => {
getByText('Finished');
getByText(START_DETECTOR_BUTTON_TEXT);
});
test('shows correct callout when detector has no data', async () => {
httpClientMock.get = jest.fn().mockResolvedValue({
ok: true,
response: {
...TEST_DETECTOR,
curState: DETECTOR_STATE.NO_DATA,
},
});
const { getByText } = renderWithRouter();
await wait();
getByText(NO_DATA_CALLOUT_TEXT);
getByText(START_DETECTOR_BUTTON_TEXT);
});
});
23 changes: 22 additions & 1 deletion public/pages/HistoricalDetectorDetail/utils/helpers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,19 @@ import {
EuiText,
EuiFlexItem,
EuiProgress,
EuiButton,
} from '@elastic/eui';
import { Detector } from '../../../models/interfaces';
import { DETECTOR_STATE } from '../../../../server/utils/constants';

export const waitForMs = (ms: number) =>
new Promise((res) => setTimeout(res, ms));

export const getCallout = (detector: Detector, isStoppingDetector: boolean) => {
export const getCallout = (
detector: Detector,
isStoppingDetector: boolean,
onEdit: () => any
) => {
if (!detector || !detector.curState) {
return null;
}
Expand Down Expand Up @@ -120,6 +125,22 @@ export const getCallout = (detector: Detector, isStoppingDetector: boolean) => {
color="primary"
/>
);
case DETECTOR_STATE.NO_DATA:
return (
<EuiCallOut
title={
<EuiText>
<p>No data available in the selected detection date range.</p>
</EuiText>
}
iconType="alert"
color="danger"
>
<EuiButton color="danger" onClick={() => onEdit()}>
Edit historical detector
</EuiButton>
</EuiCallOut>
);
default:
return null;
}
Expand Down
4 changes: 3 additions & 1 deletion public/pages/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export enum DETECTOR_STATE_COLOR {
FEATURE_REQUIRED = '#98A2B3',
INIT_FAILURE = '#F66',
UNEXPECTED_FAILURE = '#F66',
NO_DATA = '#F66',
}

export const stateToColorMap = new Map<DETECTOR_STATE, DETECTOR_STATE_COLOR>()
Expand All @@ -41,7 +42,8 @@ export const stateToColorMap = new Map<DETECTOR_STATE, DETECTOR_STATE_COLOR>()
.set(
DETECTOR_STATE.UNEXPECTED_FAILURE,
DETECTOR_STATE_COLOR.UNEXPECTED_FAILURE
);
)
.set(DETECTOR_STATE.NO_DATA, DETECTOR_STATE_COLOR.NO_DATA);

export const ALL_DETECTOR_STATES = [];
export const ALL_INDICES = [];
Expand Down
15 changes: 9 additions & 6 deletions server/routes/utils/adHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { get, omit, cloneDeep, isEmpty } from 'lodash';
import { AnomalyResults } from '../../models/interfaces';
import { GetDetectorsQueryParams, Detector } from '../../models/types';
import { mapKeysDeep, toCamel, toSnake } from '../../utils/helpers';
import { DETECTOR_STATE } from '../../utils/constants';
import { DETECTOR_STATE, NO_DATA_ERROR_MSG } from '../../utils/constants';
import { InitProgress } from '../../models/interfaces';

export const convertDetectorKeysToSnakeCase = (payload: any) => {
Expand Down Expand Up @@ -325,15 +325,18 @@ export const appendTaskInfo = (
return detectorMapWithTaskInfo;
};

// Four checks/transformations need to be made here:
// Five checks/transformations need to be made here:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: change Five to Following, so that we don't need to change it every time we have new checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, changed.

// (1) set to DISABLED if there is no existing task for this detector
// (2) set to UNEXPECTED_FAILURE if the task is in a FAILED state to stay consistent
// (3) set to INIT if the task is in a CREATED state
// (4) set to DISABLED if the task is in a STOPPED state
// (2) set to NO_DATA if the task failed with NO_DATA_ERROR_MSG
// (3) set to UNEXPECTED_FAILURE if the task is in a FAILED state to stay consistent
// (4) set to INIT if the task is in a CREATED state
// (5) set to DISABLED if the task is in a STOPPED state
export const getHistoricalDetectorState = (task: any) => {
const state = get(task, 'state', 'DISABLED');
const updatedState =
state === 'FAILED'
state === 'FAILED' && task.error === NO_DATA_ERROR_MSG
? 'NO_DATA'
: state === 'FAILED'
? 'UNEXPECTED_FAILURE'
: state === 'CREATED'
? 'INIT'
Expand Down
3 changes: 3 additions & 0 deletions server/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export enum DETECTOR_STATE {
FEATURE_REQUIRED = 'Feature required',
INIT_FAILURE = 'Initialization failure',
UNEXPECTED_FAILURE = 'Unexpected failure',
NO_DATA = 'No data',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related with this PR. But we may need to test more edge cases like not enough data, exceed our max running AD task limit etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, agreed. I think this "no data" case can be pretty common, so definitely good to have it separated here, but definitely doesn't cover all cases that would currently fall under the 'Unexpected failure' state. I'm assuming those other cases would fall under the FAILED state from the backend, and it's probably a matter of parsing out the error message to categorize into other states similar to what's done here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can also parse the error message for "FAILED" state. But not so easy for exceptions which has no readable error message.

}

export enum SAMPLE_TYPE {
Expand All @@ -83,3 +84,5 @@ export const ENTITY_NAME_PATH_FIELD = 'entity.name';

export const DOC_COUNT_FIELD = 'doc_count';
export const KEY_FIELD = 'key';

export const NO_DATA_ERROR_MSG = 'There is no data in the detection date range';