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

Update detector list page & add profile API #16

Conversation

ohltyler
Copy link
Contributor

@ohltyler ohltyler commented Apr 3, 2020

Issue #, if available:

Description of changes:

Screen Shot 2020-04-02 at 11 36 40 AM

This PR modifies the previous detector list page:

  • adds the profile api to pull detector state information
  • adds the "Indices", "Detector state", and "Last updated" columns to the table
  • adds detector state and index dropdown filtering capability
  • adds search capability in the index dropdown
  • fixes the bug which was preventing aggregated anomaly results from being pulled from the backend
  • displays the number of selected detectors in the page title
  • adds sorting capabilities by all table fields

Testing:

  • adds unit tests to test indices being displayed and some of the filtering functionality
  • all unit tests pass
  • manual testing to confirm that filtering by detector state and index works properly

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ohltyler ohltyler requested a review from mihirsoni April 3, 2020 00:03
@ohltyler ohltyler force-pushed the updateDetectorListPage branch 5 times, most recently from 41b136e to d93d70c Compare April 3, 2020 18:54
@ohltyler ohltyler force-pushed the updateDetectorListPage branch from fc38470 to 2b939ad Compare April 3, 2020 23:14
Copy link
Contributor

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

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

Great work @ohltyler Thanks for the changes. Few comments.

index.js Outdated Show resolved Hide resolved
public/pages/DetectorsList/List/List.tsx Outdated Show resolved Hide resolved
public/pages/DetectorsList/utils/constants.ts Outdated Show resolved Hide resolved
public/pages/DetectorsList/utils/tableUtils.tsx Outdated Show resolved Hide resolved
public/pages/DetectorsList/utils/tableUtils.tsx Outdated Show resolved Hide resolved
public/pages/utils/helpers.ts Outdated Show resolved Hide resolved
@ohltyler ohltyler force-pushed the updateDetectorListPage branch 2 times, most recently from d482149 to 9b706c8 Compare April 6, 2020 17:09
@ohltyler ohltyler force-pushed the updateDetectorListPage branch from 82bd8f5 to 5bf4dc5 Compare April 6, 2020 22:24
@ohltyler ohltyler removed the request for review from kaituo April 7, 2020 18:51
@ohltyler
Copy link
Contributor Author

ohltyler commented Apr 9, 2020

Will you add a loop here to get profile for each detector?
Yes, that's the plan.

@ohltyler ohltyler closed this Apr 9, 2020
@ohltyler ohltyler reopened this Apr 9, 2020
@ohltyler ohltyler force-pushed the updateDetectorListPage branch 2 times, most recently from 8f999d1 to 54e745e Compare April 10, 2020 00:45
@ohltyler ohltyler changed the title Update detector list page [WIP] Update detector list page & add profile API Apr 10, 2020
@ohltyler ohltyler changed the title [WIP] Update detector list page & add profile API Update detector list page & add profile API Apr 13, 2020
@ohltyler ohltyler force-pushed the updateDetectorListPage branch from 80465d9 to 08cff3d Compare April 13, 2020 18:38
@ohltyler ohltyler force-pushed the updateDetectorListPage branch from fe63261 to 565e981 Compare April 15, 2020 17:10
Copy link
Contributor

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for changes.

@ohltyler ohltyler force-pushed the updateDetectorListPage branch from d090a22 to 5b7782c Compare April 15, 2020 17:43
@ohltyler ohltyler merged commit 93317a0 into opendistro-for-elasticsearch:development Apr 16, 2020
@ohltyler ohltyler deleted the updateDetectorListPage branch April 16, 2020 00:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants