Task/host enhancements #59671

parkiino · 2020-03-09T15:48:47Z

Summary

Unit tests use mock host data function
Renamed 'management' to 'host'
Used EuiHealth for mock policy status
Write functional tests for host details
Format date in host details

List

Details

Checklist

Delete any items that are not applicable to this PR.

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
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-03-09T15:48:50Z

Pinging @elastic/endpoint-management (Team:Endpoint Management)

elasticmachine · 2020-03-09T15:48:52Z

Pinging @elastic/endpoint-app-team (Feature:Endpoint)

oatkiller · 2020-03-09T17:18:25Z

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

    return {
      ...state,
      detailsError: action.payload,
    };
-  } else if (action.type === 'userExitedManagementList') {
+  } else if (action.type === 'userExitedHostList') {


is this action dispatched anywhere? what would it mean?

that action isn't there anymore lol

oatkiller · 2020-03-09T17:21:57Z

@parkiino whats your reasoning for the rename? seems like the 'host lists' is a single component on a single page in the feature of managing hosts (which your team maintains.)

parkiino · 2020-03-09T18:56:58Z

The renaming is mainly to make the code more concise (writing out host vs. management is a lot faster) and also easier to find things (more intuitive to look for host than management). If someone goes from looking at the application to trying to find it in our code base, a key word they might use is 'host' since it's written all over the page, whereas 'managing/management' is something that is more internal to our team.

paul-tavares

Something to consider overall when using Styled components. It is best if we don't try to redefine internal class names from EUI framework. One way to get around this is to define your own class names with specific styles (similar to what we used to do in SMP via sass) and then use the class names along with the Eui component's className prop. To do this, you would defined your styles in a component (ex. div) that would wrap the entire View.

Here is an example:

const HostsViewStyles = styled.div`
  .endpoint-hostsList-pageHeader: {
    //...
    margin-bottom: 0
  }
`;

You would now have your Hosts List view be placed inside of the above component

export const HostList ()=> {
  return <HostsViewSytles>
    { /* Any component rendered here will have ability to use the above class names */}
    <EuiPageHeader className="endpoint-hostsList-pageHeader" >...</EuiPageHeader>
  </HostsViewList>
}

Let me know if you have any questions or need help going through this :)

paul-tavares · 2020-03-16T15:04:50Z

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

+  payload: HostListPagination;
+}
+
+// https://github.com/elastic/endpoint-app-team/issues/273


Maybe just add some words here as to why the action below is there - something like:

"Why is FakeActionWithNoPayload here? see: "

paul-tavares · 2020-03-16T15:06:46Z

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

@@ -25,7 +25,7 @@ export type MiddlewareFactory<S = GlobalState> = (
  api: MiddlewareAPI<Dispatch<AppAction>, S>
 ) => (next: Dispatch<AppAction>) => (action: AppAction) => unknown;

-export interface ManagementListState {
+export interface HostListState {
  endpoints: EndpointMetadata[];


Maybe on next PR, rename endpoints to host as the property in the state?

paul-tavares · 2020-03-16T15:07:50Z

x-pack/plugins/endpoint/public/applications/endpoint/view/formatted_date_time.tsx

+export const FormattedDateAndTime: React.FC<{ date: Date }> = ({ date }) => {
+  // If date is greater than or equal to 24h (ago), then show it as a date
+  // else, show it as relative to "now"
+  return Date.now() - date.getTime() >= 8.64e7 ? (


FYI - one of the outstanding tasks from when I originally implemented this is to change so that relative date will only be displayed if within the last 1h (instead of last 24h)

paul-tavares · 2020-03-16T15:19:58Z

x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details.tsx

+  const HostIds = styled(EuiListGroupItem)`
+    margin-top: 0;
+    .euiListGroupItem__text {
+      padding: 0;
+    }
+  `;


You should not define HostIds component inside of HostDetails - you only need to define the styled component once and then re-use it. Move it to be outside (at module level).

paul-tavares · 2020-03-16T15:22:07Z

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

+    padding: 0;
+  `;
+
+  const HostHeader = styled(EuiPageHeader)`


Same here and below - move the styled component definition to be module level.

paul-tavares · 2020-03-16T15:38:14Z

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

+  `;
+
+  const HostHeader = styled(EuiPageHeader)`
+    background-color: #f5f7fa;


Do not use hardcoded values for colors/padding etc. unless they are not provided by Eui.
Styled components provide a mechanism for access to the Eui Theme which you should use here. To do that, you define a function that accepts props for the CSS property value and then use that to return back the definition of that css property.

Example of using a borders and colors from the Eui Theme:

const Container = styled.div` border-bottom: ${props => props.theme.eui.euiBorderThin}; background-color: ${props => props.theme.eui.euiPageBackgroundColor}; `; const Wrapper = styled.div` margin-right: auto; padding-top: ${props => props.theme.eui.paddingSizes.xl}; padding-left: ${props => props.theme.eui.paddingSizes.m}; padding-right: ${props => props.theme.eui.paddingSizes.m}; `;

kevinlog · 2020-03-17T13:47:12Z

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

@@ -30,7 +30,7 @@ describe('when on the managing page', () => {
          <I18nProvider>
            <Router history={history}>
              <RouteCapture>
-                <ManagementList />
+                <HostList />


My guess is that the jest tests are failing right now because we need to wrap this in <EuiThemeProvider /> like we do here https://github.com/parkiino/kibana/blob/task/host-enhancements/x-pack/plugins/endpoint/public/applications/endpoint/index.tsx#L60

paul-tavares

LGTM
Nice work 😄 👍

paul-tavares · 2020-03-17T16:27:08Z

x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details.tsx

          defaultMessage: 'Last Seen',
        }),
-        description: details['@timestamp'],
+        description: <FormattedDateAndTime date={new Date(details['@timestamp'])} />,


Just FYI (no change needed): I'm pretty sure FormattedDate components recognize a timestamp as well.

marshallmain · 2020-03-17T18:52:02Z

x-pack/plugins/endpoint/public/applications/endpoint/store/hosts/mock_host_result_list.ts

  for (let index = 0; index < actualCountToReturn; index++) {
    const generator = new EndpointDocGenerator('seed');
-    endpoints.push(generator.generateEndpointMetadata());
+    hosts.push(generator.generateHostMetadata());


this will generate documents from the same host repeatedly with only the timestamp on the document changing. Including the index in the random seed string will create different documents on each loop iteration.

x-pack/plugins/endpoint/common/generate_data.ts

marshallmain

one optional change to the mock host list generation, a few nit picky comments where 'endpoint' remains

marshallmain · 2020-03-17T18:57:34Z

x-pack/plugins/endpoint/common/types.ts

@@ -83,9 +83,9 @@ export interface AlertResultList {
  prev: string | null;
 }

-export interface EndpointResultList {
+export interface HostResultList {
  /* the endpoints restricted by the page size */


endpoints -> hosts

marshallmain · 2020-03-17T18:58:05Z

x-pack/plugins/endpoint/common/types.ts

  /* the endpoints restricted by the page size */
-  endpoints: EndpointMetadata[];
+  hosts: HostMetadata[];
  /* the total number of unique endpoints in the index */


endpoints -> hosts

oof these comments

i will probs try to correct those pupsters in the next pr

just kidding. :( build failed so i guess i'll do it now

marshallmain · 2020-03-17T21:13:21Z

x-pack/test/functional/apps/endpoint/host_list.ts

+        await pageObjects.common.navigateToUrlWithBrowserHistory(
+          'endpoint',
+          '/hosts',
+          'selected_host=cbe80003-6964-4e0f-aba1-f94c32b44e95'


ideally values like this wouldn't be hardcoded so the tests aren't so coupled to the particular es_archive

marshallmain · 2020-03-17T21:16:19Z

x-pack/test/functional/apps/endpoint/host_list.ts

+          '0',
+          'C2A9093E-E289-4C0A-AA44-8C32A414FA7A',
+          'active',
+          '10.48.181.22210.116.62.6210.102.83.30',


can we also enhance the way this list of IP addresses is displayed. maybe work for next PR

when you say enhance do you mean in terms of how it's displayed here in the test code or did you mean in the actual ui?

i just mean there's no separator between the IPs here and i think in the UI also

kibanamachine · 2020-03-17T22:24:03Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 377bb03

History

💔 Build #33858 failed 60136b0c9bbe0a16f87cb482aa9fde009581b420
💔 Build #33836 failed 7134bd0e0628d48c61d58430d2db0c93c5cfabfa
💔 Build #33828 failed cbd3ec9ac66e9663b0bfc98883ddd84f2068c593
💚 Build #33805 succeeded ca76eceda35144d2905020a968f52f40addafac4
💔 Build #33626 failed 17d0a295cb11e24ec16e6dfcf1f92f4dbacc7810

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

* master: [ML] Re-enabling file upload telemetry (elastic#60418) [NP] Use local helper shortenDottedString for discover (elastic#60271) [Console] Fix for `_settings` and x-pack autocomplete (elastic#60246) Task/host enhancements (elastic#59671) [Search service] Asynchronous ES search strategy (elastic#53538) Index Action - Moved index params fields to connector config (elastic#60349) Edits UI text for ML nodes and job button (elastic#60184) Publish getIsNavDrawerLocked$ method on core chrome service. (elastic#60191) Disabled edit alert button on management ui for non registered UI alert types (elastic#60439) Revert "[Console] Fix bool filter autocompletions and refactor (elastic#60361)" [Console] Fix bool filter autocompletions and refactor (elastic#60361) Update ingest management team handle (elastic#60457) [IM] Use EuiCodeBlock to render index mapping (elastic#60420) Add additional safeguards for data source wizard step 2 (elastic#60426) [kbn/pm] don't fail when plugins are outside repo (elastic#60164) upgrade react-use (elastic#60427) Remove link to old settings (elastic#60326) Update app arch CODEOWNERS items. (elastic#60396) [ML] Fixing custom urls to dashboards (elastic#60355) Update the ems-client dependency to 7.7.0 (elastic#59936)

kibanamachine · 2020-03-19T03:38:44Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 59671 or prevent reminders by adding the backport:skip label.

…alerting/tls-warning * 'alerting/tls-warning' of github.com:gmmorris/kibana: (33 commits) [ML] Disable functional transform tests Fixes to service map single node banner (elastic#60072) [Uptime] replace fetch with kibana http (elastic#59881) Upgrade @types/node to match Node.js runtime (elastic#60368) [License Management] NP migration (elastic#60250) Fix create alert button from not showing in alerts list (elastic#60444) [SIEM][Case] Update connector through flyout (elastic#60307) add data-test-subj where possible on SO management table (elastic#60226) Enforce `required` presence for value/key validation of `recordOf` and `mapOf`. (elastic#60406) [ML] Re-enabling file upload telemetry (elastic#60418) [NP] Use local helper shortenDottedString for discover (elastic#60271) [Console] Fix for `_settings` and x-pack autocomplete (elastic#60246) Task/host enhancements (elastic#59671) [Search service] Asynchronous ES search strategy (elastic#53538) Index Action - Moved index params fields to connector config (elastic#60349) Edits UI text for ML nodes and job button (elastic#60184) Publish getIsNavDrawerLocked$ method on core chrome service. (elastic#60191) Disabled edit alert button on management ui for non registered UI alert types (elastic#60439) Revert "[Console] Fix bool filter autocompletions and refactor (elastic#60361)" [Console] Fix bool filter autocompletions and refactor (elastic#60361) ...

kibanamachine · 2020-03-20T03:48:44Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kibanamachine · 2020-03-21T03:58:44Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kibanamachine · 2020-03-22T04:08:44Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kibanamachine · 2020-03-23T04:18:43Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

functional tests and ui updates to endpoint host details Co-authored-by: Elastic Machine <[email protected]>

parkiino added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 Team:Endpoint Management Feature:Endpoint Elastic Endpoint feature labels Mar 9, 2020

parkiino requested a review from a team as a code owner March 9, 2020 15:48

parkiino added the WIP Work in progress label Mar 9, 2020

oatkiller reviewed Mar 9, 2020

View reviewed changes

parkiino force-pushed the task/host-enhancements branch 2 times, most recently from bb072ff to 28f9dc4 Compare March 11, 2020 17:49

parkiino removed the WIP Work in progress label Mar 11, 2020

parkiino force-pushed the task/host-enhancements branch 4 times, most recently from be1f3ec to f3d3dd0 Compare March 16, 2020 14:45

paul-tavares reviewed Mar 16, 2020

View reviewed changes

parkiino force-pushed the task/host-enhancements branch from 6c9bc19 to 17d0a29 Compare March 16, 2020 22:09

kevinlog reviewed Mar 17, 2020

View reviewed changes

parkiino force-pushed the task/host-enhancements branch from ca76ece to cbd3ec9 Compare March 17, 2020 15:17

paul-tavares approved these changes Mar 17, 2020

View reviewed changes

parkiino force-pushed the task/host-enhancements branch from 60136b0 to b6f02da Compare March 17, 2020 18:10

peluja1012 approved these changes Mar 17, 2020

View reviewed changes

marshallmain reviewed Mar 17, 2020

View reviewed changes

x-pack/plugins/endpoint/common/generate_data.ts Outdated Show resolved Hide resolved

marshallmain approved these changes Mar 17, 2020

View reviewed changes

changed existing tests to use mock endpoint

5123e19

parkiino added 12 commits March 17, 2020 16:39

why

0f115b3

ui changes updated table

982e339

does this actually change it

347c2e2

address comments

c84f116

fixing rebasing pt 2

ee6f51b

add euiThemeProvider to failed test

dbde665

fix api_integration test

7161db0

yikies

a4b82f0

fix one more

fcffeea

more host changes + don't check date in func test

d255435

one more endpoint change

b83f24f

another one

377bb03

parkiino force-pushed the task/host-enhancements branch from 0643f92 to 377bb03 Compare March 17, 2020 20:39

marshallmain reviewed Mar 17, 2020

View reviewed changes

dplumlee approved these changes Mar 17, 2020

View reviewed changes

parkiino merged commit 65a111f into elastic:master Mar 18, 2020

parkiino deleted the task/host-enhancements branch March 18, 2020 03:31

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 19, 2020

parkiino mentioned this pull request Mar 19, 2020

[7.x] Task/host enhancements (#59671) #60676

Merged

parkiino added a commit that referenced this pull request Mar 23, 2020

Task/host enhancements (#59671) (#60676)

5288e2d

functional tests and ui updates to endpoint host details Co-authored-by: Elastic Machine <[email protected]>

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Task/host enhancements #59671

Task/host enhancements #59671

parkiino commented Mar 9, 2020 •

edited

Loading

elasticmachine commented Mar 9, 2020

elasticmachine commented Mar 9, 2020

oatkiller Mar 9, 2020

parkiino Mar 9, 2020 •

edited

Loading

oatkiller commented Mar 9, 2020

parkiino commented Mar 9, 2020

paul-tavares left a comment

paul-tavares Mar 16, 2020

paul-tavares Mar 16, 2020

paul-tavares Mar 16, 2020

paul-tavares Mar 16, 2020

paul-tavares Mar 16, 2020

paul-tavares Mar 16, 2020

kevinlog Mar 17, 2020

paul-tavares left a comment

paul-tavares Mar 17, 2020

marshallmain Mar 17, 2020

marshallmain left a comment

marshallmain Mar 17, 2020

marshallmain Mar 17, 2020

parkiino Mar 17, 2020

parkiino Mar 17, 2020

parkiino Mar 17, 2020

marshallmain Mar 17, 2020

marshallmain Mar 17, 2020

parkiino Mar 18, 2020

marshallmain Mar 18, 2020

kibanamachine commented Mar 17, 2020

kibanamachine commented Mar 19, 2020

kibanamachine commented Mar 20, 2020

kibanamachine commented Mar 21, 2020

kibanamachine commented Mar 22, 2020

kibanamachine commented Mar 23, 2020

Task/host enhancements #59671

Task/host enhancements #59671

Conversation

parkiino commented Mar 9, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Mar 9, 2020

elasticmachine commented Mar 9, 2020

Choose a reason for hiding this comment

parkiino Mar 9, 2020 • edited Loading

Choose a reason for hiding this comment

oatkiller commented Mar 9, 2020

parkiino commented Mar 9, 2020

paul-tavares left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marshallmain left a 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

kibanamachine commented Mar 17, 2020

💚 Build Succeeded

History

kibanamachine commented Mar 19, 2020

kibanamachine commented Mar 20, 2020

kibanamachine commented Mar 21, 2020

kibanamachine commented Mar 22, 2020

kibanamachine commented Mar 23, 2020

parkiino commented Mar 9, 2020 •

edited

Loading

parkiino Mar 9, 2020 •

edited

Loading