[Endpoint] Task/basic endpoint list #55623

parkiino · 2020-01-22T22:22:58Z

Summary

https://github.com/elastic/endpoint-app-team/issues/63
This PR adds a basic UI for the endpoint list page. Although it connects to the endpoints api, data is still being mocked.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, including a check against IE11~~

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
~~- [ ] Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
~~- [ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

Test cases

Endpoint list component renders correctly on the page when Endpoint app is enabled
columns are rendered in proper order

kevinlog · 2020-01-28T23:21:38Z

@parkiino

Add these labels:

Add [Endpoint] to the PR title - ex: [Endpoint] Task/basic endpoint list

peluja1012 · 2020-01-29T06:47:15Z

x-pack/plugins/endpoint/public/applications/endpoint/index.tsx

@@ -53,15 +54,7 @@ const AppRoot: React.FunctionComponent<RouterProps> = React.memo(({ basename, st
            render={() => {
              // FIXME: This is temporary. Will be removed in next PR for endpoint list
              store.dispatch({ type: 'userEnteredEndpointListPage' });


We should use the usePageId hook instead.

peluja1012 · 2020-01-29T06:53:49Z

x-pack/plugins/endpoint/public/applications/endpoint/store/endpoint_list/types.ts

+export interface EndpointListPagination {
+  pageIndex: number;
+  pageSize: number;
+}


Could we move these types to the applications/endpoint/types.ts file? @paul-tavares suggested this approach and the file was added when we merged the basic alert list PR.

@peluja1012 - Yes, we'll move it (actually mostly delete this, since it was a copy of the server types as a temporary work-around).
I actually opened a separate issue a few days ago (see elastic/endpoint-app-team/issues/131 ) to handle the refactoring items so that @parkiino could continue to focus on getting the list functionality working.

peluja1012 · 2020-01-29T06:58:32Z

x-pack/plugins/endpoint/public/applications/endpoint/store/endpoint_list/saga.ts

-  for await (const { action } of actionsAndState()) {
-    if (action.type === 'userEnteredEndpointListPage') {
-      const response = await httpPost('/api/endpoint/endpoints');
+  for await (const { action, state } of actionsAndState()) {


@paul-tavares @kevinlog @parkiino Are we going to go with the oatSaga approach or with the simple middleware approach? We discussed using the simpler approach until we really needed to add complexity. While the alert and endpoint pages can go in different directions, I personally would favor consistency.

no, we'll refactor to use Redux middleware. this is also going to be done under elastic/endpoint-app-team/issues/131

peluja1012 · 2020-01-29T07:06:21Z

x-pack/plugins/endpoint/public/applications/endpoint/components/management.tsx

@@ -0,0 +1,133 @@
+/*


This file should be moved to the applications/endpoint/view/ directory

peluja1012 · 2020-01-29T07:11:26Z

x-pack/plugins/endpoint/public/applications/endpoint/components/management.tsx

+    hidePerPageOptions: false,
+  };
+
+  const onTableChange = ({ page }: { page: { index: number; size: number } }) => {


Consider using useCallback here

paul-tavares · 2020-01-29T20:20:51Z

x-pack/plugins/endpoint/public/applications/endpoint/components/management.tsx

+  const columns = [
+    {
+      field: 'host.hostname',
+      name: 'Host',


we should i18n visible names using the i18n package from kibana. Import i18n from @kbn/i18n, then use it as:

i18n.translate('xpack.endpoint.management.list', { defaultMessage: 'Host', })

paul-tavares · 2020-01-29T20:22:27Z

x-pack/plugins/endpoint/public/applications/endpoint/components/management.tsx

+          <EuiPageContentHeader>
+            <EuiPageContentHeaderSection>
+              <EuiTitle>
+                <h2>Hosts</h2>


to localize Hosts here there is also a React component you can use -

<FormattedMessage id="xpack.endpoint.managment.list" defaultMessage="Hello World" />

The import path is a little different - import @kbn/i18n/react

paul-tavares · 2020-01-29T20:25:24Z

x-pack/plugins/endpoint/public/applications/endpoint/index.tsx

@@ -53,15 +54,7 @@ const AppRoot: React.FunctionComponent<RouterProps> = React.memo(({ basename, st
            render={() => {
              // FIXME: This is temporary. Will be removed in next PR for endpoint list
              store.dispatch({ type: 'userEnteredEndpointListPage' });


We probably should follow the same pattern as the Alerts PR use the component={} attribute fo the Route and move the usePageId() call to the Management Component (we can then delete the userEnteredEndpointListPage action)

oatkiller · 2020-01-29T20:35:32Z

x-pack/plugins/endpoint/public/applications/endpoint/components/management.tsx

+  const pageSize = useEndpointListSelector(endpointListPageSize);
+  const totalItemCount = useEndpointListSelector(endpointTotalHits);
+
+  const paginationSetup = {


x-pack/plugins/endpoint/public/applications/endpoint/view/managing/index.tsx

paul-tavares · 2020-01-30T21:59:14Z

x-pack/plugins/endpoint/public/applications/endpoint/store/endpoint_list/saga.ts

+    ) {
+      const pageIndex = endpointListPageIndex(state.endpointList);
+      const pageSize = endpointListPageSize(state.endpointList);
+      const response = await httpPost('/api/endpoint/endpoints', {


probably ok for now, but when we move this to Middleware, we should also handle errors (or throw's) here. Question will be: what should we do with the errors from the UI standpoint.

paul-tavares · 2020-01-30T22:01:58Z

x-pack/plugins/endpoint/public/applications/endpoint/store/endpoint_list/saga.ts

+        }),
+      });
+      // temp: request_page_index to reflect user request page index, not es page index
+      response.request_page_index = pageIndex;
      dispatch({
        type: 'serverReturnedEndpointList',
        payload: response,


@oatkiller HALP

I heard you were a TS Master Wizard - why are we NOT getting an TS error here? response is any (because we did not tell post what the response from the API looks like). It should have thrown a TS error when trying to assign any to payload which has an explicit type on it.

WHYYYYYYY 😄

you can assign any to anything.

aisantos · 2020-02-04T17:08:02Z

See test cases we need to add for this PR here: https://github.com/elastic/endpoint-app-team/issues/63

peluja1012 · 2020-02-12T20:09:59Z

x-pack/plugins/endpoint/public/applications/endpoint/store/endpoint_list/action.ts

 }

 interface UserExitedEndpointListPage {
  type: 'userExitedEndpointListPage';
 }

+interface UserPaginatedEndpointListTable {


we should rename this for consistency

peluja1012 · 2020-02-12T20:10:14Z

x-pack/plugins/endpoint/public/applications/endpoint/store/endpoint_list/action.ts

@@ -4,22 +4,24 @@
 * you may not use this file except in compliance with the Elastic License.
 */

-import { EndpointListData } from './types';
+import { ManagementPagination } from '../../types';
+import { EndpointResultList } from '../../../../../common/types';

 interface ServerReturnedEndpointList {
  type: 'serverReturnedEndpointList';


we should rename this for consistency

peluja1012 · 2020-02-12T20:11:52Z

x-pack/plugins/endpoint/public/applications/endpoint/store/endpoint_list/middleware.ts

+        type: 'serverReturnedEndpointList',
+        payload: response,
+      });
+      dispatch({ type: 'serverReturnedAlertsData', payload: response });


What does this do?

this looks like a bad "copy-and-paste"

it should not be there

peluja1012 · 2020-02-12T20:33:33Z

x-pack/plugins/endpoint/public/applications/endpoint/store/endpoint_list/reducer.ts

  };
 };

-export const endpointListReducer: Reducer<EndpointListState, AppAction> = (
+export const endpointListReducer: Reducer<ManagementState, AppAction> = (


we should rename this for consistency

@parkiino we're going with Management naming, aren't we? @peluja1012 what would we change here?

@kevinlog I think the exported const is what needs to be renamed.

you are correct - ignore me

peluja1012 · 2020-02-12T20:36:52Z

x-pack/plugins/endpoint/public/applications/endpoint/view/managing/hooks.ts

+import { useSelector } from 'react-redux';
+import { GlobalState, ManagementState } from '../../types';
+
+export function useEndpointListSelector<TSelected>(


we should rename this for consistency

peluja1012 · 2020-02-12T20:37:09Z

x-pack/plugins/endpoint/public/applications/endpoint/view/managing/index.tsx

+import { CreateStructuredSelector } from '../../types';
+
+const selector = (createStructuredSelector as CreateStructuredSelector)(selectors);
+export const EndpointList = () => {


we should rename this for consistency

peluja1012 · 2020-02-12T20:37:19Z

x-pack/plugins/endpoint/public/applications/endpoint/view/managing/index.tsx

+const selector = (createStructuredSelector as CreateStructuredSelector)(selectors);
+export const EndpointList = () => {
+  usePageId('managementPage');
+  const dispatch = useDispatch<(a: EndpointListAction) => void>();


we should rename this for consistency

paul-tavares · 2020-02-12T21:39:26Z

x-pack/plugins/endpoint/public/applications/endpoint/view/managing/index.tsx

+    [dispatch]
+  );
+
+  const columns = [


memoize this with useMemo()?

paul-tavares

I'm ok if my comments are addressed in a subsequent PR.

kevinlog · 2020-02-12T21:45:16Z

Looks like test is failing due to this https://github.com/elastic/kibana/pull/57506/files

parkiino · 2020-02-13T15:35:55Z

@elasticmachine merge upstream

LeeDr · 2020-02-13T20:24:43Z

The visualize test failure looks like this (with some unimportant lines removed). From this log we can see it only waited 10 seconds before it failed;

[00:47:04]                 │ debg navigateToActualUrl http://localhost:6151/s/custom_space/app/kibana#/visualize/edit/i-exist

[00:47:04]                 │ debg TestSubjects.exists(visualizationLoader)
[00:47:04]                 │ debg Find.existsByDisplayedByCssSelector('[data-test-subj="visualizationLoader"]') with timeout=10000
[00:47:08]                 │ debg --- retry.tryForTime error: [data-test-subj="visualizationLoader"] is not displayed

[00:47:14]                 │ debg --- retry.tryForTime failed again with the same message...
[00:47:14]                 │ info [o.e.c.m.MetaDataMappingService] [kibana-ci-immutable-centos-tests-xl-1581608204125234605] [.kibana_2/GXJ1IFuzRfGoxPjdwZikTg] update_mapping [_doc]

[00:47:14]                 │ info Taking screenshot "/dev/shm/workspace/kibana/x-pack/test/functional/screenshots/failure/Visualize visualize space with no features disabled can view existing Visualization.png"

And the failure screenshot shows a blank page with the pink progress bar;

I haven't tested this yet, but I think we need to wait for kibanaChrome after the get here;

kibana/test/functional/page_objects/common_page.ts

Line 139 in 5fe53d1

await browser.get(appUrl);

and/or here;

kibana/test/functional/page_objects/common_page.ts

Line 59 in 5fe53d1

await browser.get(url);

Something like we do here;

kibana/test/functional/page_objects/common_page.ts

Line 103 in 5fe53d1

    
           await find.byCssSelector('[data-test-subj="kibanaChrome"]', 6 * defaultFindTimeout); // 60 sec waiting

The only case where it might be a problem to wait for kibanaChrome is if we're navigating to the status page. But I'm not sure if tests are doing that.

parkiino · 2020-02-13T20:48:42Z

@elasticmachine merge upstream

parkiino · 2020-02-13T22:10:11Z

@elasticmachine merge upstream

kevinlog · 2020-02-14T13:20:09Z

@elasticmachine merge upstream

kibanamachine · 2020-02-14T14:57:48Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a9c6b2c

History

💔 Build #26689 failed bf678f3
💔 Build #26540 failed a6782a4
💔 Build #26296 failed 9057c06
💔 Build #26285 failed c2a6d5c
💚 Build #25741 succeeded b561cbb

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* Add Endpoint plugin and Resolver embeddable (#51994) * Add functional tests for plugins to x-pack (so we can do a functional test of the Resolver embeddable) * Add Endpoint plugin * Add Resolver embeddable * Test that Resolver embeddable can be rendered Conflicts: x-pack/.i18nrc.json x-pack/test/api_integration/apis/index.js * [Endpoint] Register endpoint app (#53527) * register app, create functional test * formatting * update tests * adjust test data for endpoint * add endpoint tests for testing spaces, app enabled, disabled, etc * linting * add read privileges to endpoint * rename variable since its used now * remove deprecated context * remove unused variable * fix type check * correct test suite message Co-Authored-By: Larry Gregory <lgregorydev@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Larry Gregory <lgregorydev@gmail.com> * [Endpoint] add react router to endpoint app (#53808) * add react router to endpoint app * linting * linting * linting * correct tests * change history from hash to browser, add new test util * remove default values in helper functions * fix type check, use FunctionComponent as oppsed to FC * use BrowserRouter component * use BrowserRouter component lin * add comments to test framework, change function name to include browserHistory Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * EMT-issue-65: add endpoint list api (#53861) add endpoint list api * EMT-65:always return accurate endpoint count (#54423) EMT-65:always return accurate endpoint count, independent of paging properties * Resolver component w/ sample data (#53619) Resolver is a map. It shows processes that ran on a computer. The processes are drawn as nodes and lines connect processes with their parents. Resolver is not yet implemented in Kibana. This PR adds a 'map' type UX. The user can click and drag to pan the map and zoom using trackpad pinching (or ctrl and mousewheel.) There is no code providing actual data. Sample data is included. The sample data is used to draw a map. The fundamental info needed is: process names the parent of a process With this info we can topologically lay out the processes. The sample data isn't yet in a realistic format. We'll be fixing that soon. Related issue: elastic/endpoint-app-team#30 * Resolver test plugin not using mount context. (#54933) Mount context was deprecated. Use core.getStartServices() instead. * Resolver nonlinear zoom (#54936) * [Endpoint] add Redux saga Middleware and app Store (#53906) * Added saga library * Initialize endpoint app redux store * Resolver is overflow: hidden to prevent obscured elements from showing up (#55076) * [Endpoint] Fix saga to start only after store is created and stopped on app unmount (#55245) - added `stop()`/`start()` methods to the Saga Middleware creator factory - adjust tests based on changes - changed application `renderApp` to stop sagas when react app is unmounted * Resolver zoom, pan, and center controls (#55221) * Resolver zoom, pan, and center controls * add tests, fix north panning * fix type issue * update west and east panning to behave like google maps * [Endpoint] FIX: Increase tests `sleep` default duration back to 100ms (#55492) Revert `sleep()` default duration, in the saga tests, back to 100ms in order to prevent intermittent failures during CI runs. Fixes #55464 Fixes #55465 * [Endpoint] EMT-65: make endpoint data types common, restructure (#54772) [Endpoint] EMT-65: make endpoint data types common, use schema changes * Basic Functionality Alert List (#55800) * sets up initial grid and data type * data feeds in from backend but doesnt update * sample data feeding in correctly * Fix combineReducers issue by importing Redux type from 'redux' package * Add usePageId hook that fires action when user navigates to page * Strict typing for middleware * addresses comments and uses better types * move types to common/types.ts * Move types to endpoint/types.ts, address PR comments blah 2 Co-authored-by: Pedro Jaramillo <peluja1012@gmail.com> * [Endpoint] Add Endpoint Details route (#55746) * Add Endpoint Details route * add Endpoint Details tests * sacrifices to the Type gods * update to latest endpoint schema Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * [Endpoint] EMT-67: add kql support for endpoint list (#56328) [Endpoint] EMT-67: add kql support for endpoint list * [Endpoint] ERT-82 ERT-83 ERT-84: Alert list API with pagination (#56538) * ERT-82 ERT-83 ERT-84 (partial): Add Alert List API with pagination * Better type safety for alert list API * Add Test to Verify Endpoint App Landing Page (#57129) Conflicts: x-pack/test/functional/page_objects/index.ts * fixes render bug in alert list (#57152) Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * Resolver: Animate camera, add sidebar (#55590) This PR adds a sidebar navigation. clicking the icons in the nav will focus the camera on the different nodes. There is an animation effect when the camera moves. Conflicts: yarn.lock * [Endpoint] Task/basic endpoint list (#55623) * Adds host management list to endpoint security plugin Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * [Endpoint] Policy List UI route and initial view (#56918) * Initial Policy List view * Add `endpoint/policy` route and displays Policy List * test cases (both unit and functional) Does not yet interact with API (Ingest). * Add ApplicationService app status management (#50223) This was already backported, but changes to endpoint app could not be backported, since endpoint app itself hadn't been backported. Now that the endpoint app is backported, reapply the endpoint specific changes from the original commit. * Implements `getStartServices` on server-side (#55156) This was already backported, but changes to endpoint app could not be backported, since endpoint app itself hadn't been backported. Now that the endpoint app is backported, reapply the endpoint specific changes from the original commit. * [ui/utils/query_string]: Remove unused methods & migrate apps to querystring lib (#56957) This was already backported, but changes to endpoint app could not be backported, since endpoint app itself hadn't been backported. Now that the endpoint app is backported, reapply the endpoint specific changes from the original commit. Co-authored-by: Kevin Logan <56395104+kevinlog@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Larry Gregory <lgregorydev@gmail.com> Co-authored-by: nnamdifrankie <56440728+nnamdifrankie@users.noreply.github.com> Co-authored-by: Davis Plumlee <56367316+dplumlee@users.noreply.github.com> Co-authored-by: Paul Tavares <56442535+paul-tavares@users.noreply.github.com> Co-authored-by: Pedro Jaramillo <peluja1012@gmail.com> Co-authored-by: Dan Panzarella <pzl@users.noreply.github.com> Co-authored-by: Madison Caldwell <madison.rey.caldwell@gmail.com> Co-authored-by: Charlie Pichette <56399229+charlie-pichette@users.noreply.github.com> Co-authored-by: Candace Park <56409205+parkiino@users.noreply.github.com> Co-authored-by: Pierre Gayvallet <pierre.gayvallet@gmail.com> Co-authored-by: Alexey Antonov <alexwizp@gmail.com>

parkiino added WIP Work in progress Team:Endpoint Management labels Jan 22, 2020

parkiino force-pushed the task/basic-endpoint-list branch 3 times, most recently from 9d6f576 to a9b280c Compare January 28, 2020 17:57

parkiino removed the WIP Work in progress label Jan 28, 2020

parkiino requested review from paul-tavares, kevinlog, peluja1012 and oatkiller January 28, 2020 23:17

parkiino added release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0 labels Jan 28, 2020

peluja1012 reviewed Jan 29, 2020

View reviewed changes

parkiino added Team:Endpoint Data Visibility Team managing the endpoint resolver Team:Endpoint Response Endpoint Response Team Feature:Endpoint Elastic Endpoint feature labels Jan 29, 2020

paul-tavares reviewed Jan 29, 2020

View reviewed changes

oatkiller reviewed Jan 29, 2020

View reviewed changes

parkiino force-pushed the task/basic-endpoint-list branch from 017b998 to c0fccd6 Compare January 30, 2020 15:42

paul-tavares reviewed Jan 30, 2020

View reviewed changes

x-pack/plugins/endpoint/public/applications/endpoint/view/managing/index.tsx Outdated Show resolved Hide resolved

paul-tavares reviewed Jan 30, 2020

View reviewed changes

parkiino force-pushed the task/basic-endpoint-list branch from 4d6294a to 5335cc6 Compare February 3, 2020 16:54

parkiino added 3 commits February 12, 2020 14:27

fixing tests and adding total count

9d374ea

middleware stuff

b5adfe1

fixes typies? beat davis! go

9057c06

parkiino force-pushed the task/basic-endpoint-list branch from 25e3d35 to 9057c06 Compare February 12, 2020 19:27

peluja1012 reviewed Feb 12, 2020

View reviewed changes

paul-tavares reviewed Feb 12, 2020

View reviewed changes

paul-tavares approved these changes Feb 12, 2020

View reviewed changes

renaming peluja whale

9721e6a

peluja1012 approved these changes Feb 13, 2020

View reviewed changes

Merge branch 'master' into task/basic-endpoint-list

a6782a4

Merge branch 'master' into task/basic-endpoint-list

594d036

Merge branch 'master' into task/basic-endpoint-list

bf678f3

Merge branch 'master' into task/basic-endpoint-list

a9c6b2c

kevinlog changed the title ~~Task/basic endpoint list~~ [Endpoint] Task/basic endpoint list Feb 14, 2020

parkiino merged commit 1492d5a into elastic:master Feb 14, 2020

parkiino deleted the task/basic-endpoint-list branch February 14, 2020 15:12

[Endpoint] Task/basic endpoint list #55623

[Endpoint] Task/basic endpoint list #55623

Conversation

parkiino commented Jan 22, 2020 • edited by aisantos Loading

Summary

Checklist

For maintainers

Test cases

kevinlog commented Jan 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paul-tavares Jan 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aisantos commented Feb 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paul-tavares left a comment

Choose a reason for hiding this comment

kevinlog commented Feb 12, 2020

parkiino commented Feb 13, 2020

LeeDr commented Feb 13, 2020

parkiino commented Feb 13, 2020

parkiino commented Feb 13, 2020

kevinlog commented Feb 14, 2020

kibanamachine commented Feb 14, 2020

💚 Build Succeeded

History

parkiino commented Jan 22, 2020 •

edited by aisantos

Loading

paul-tavares Jan 30, 2020 •

edited

Loading