[Component templates] Table view #68031

alisonelizabeth · 2020-06-02T22:41:13Z

This PR adds the "Component templates" tab with table view. It also includes the ability to delete a component template.

Create/edit/clone actions and the details flyout will come in subsequent PRs.

Note to reviewer: All client code is contained within a new component_templates directory under components, including associated jest tests.

How to test

Create >1 component templates and associate one or more with an index template.

For example:

PUT _component_template/comp_1
{
  "template": {
    "mappings": {
      "properties": {
        "@timestamp": {
          "type": "date"
        }
      }
    }
  }
}

PUT _component_template/comp_2
{
  "template": {
    "mappings": {
      "properties": {
        "ip_address": {
          "type": "ip"
        }
      }
    }
  }
}

PUT _index_template/template_1
{
  "index_patterns": ["te*", "bar*"],
  "template": {
    "settings": {
      "number_of_shards": 1
    },
    "mappings": {
      "_source": {
        "enabled": false
      },
      "properties": {
        "host_name": {
          "type": "keyword"
        },
        "created_at": {
          "type": "date",
          "format": "EEE MMM dd HH:mm:ss Z yyyy"
        }
      }
    },
    "aliases": {
      "mydata": { }
    }
  },
  "composed_of": ["comp_1", "comp_2"],
}

Test cases:

Empty prompt displays with no component templates
Delete functionality is disabled if the component template is being used by an index template.
Delete a single component template.
Delete more than one component template.
Filter by templates in use or not in use.
Reload button refetches component templates data.

Screenshots

elasticmachine · 2020-06-03T17:25:41Z

Pinging @elastic/es-ui (Team:Elasticsearch UI)

...n/components/component_templates/__jest__/client_integration/component_template_list.test.ts

…nent_templates_list_view

alisonelizabeth · 2020-06-08T18:11:45Z

@elasticmachine merge upstream

sebelga · 2020-06-09T09:32:04Z

Great job @alisonelizabeth ! I will review the code now but leaving here a comment about UX.

I am not sure about repeating in each cell "index template" under the "Used by" column. I wonder if by just having the number (e.g. 3) it would be enough. In a following iteration, we could add a link on that number that would point to the index template table filtered on that component. This is similar behaviour that @cjcenizal demoed yesterday with data streams. WDYT?

Also, the cross icon looks like there is an error. What do you think about not having the icon, and setting the text in italic with lighter text color like this?

In a following iteration, as an enhancement, I can see that the current "in use/not in use" filter is replaced by a dropdown select with those 2 options + 3 others

With mapping
With settings
With aliases

sebelga

Code LGTM overall. I left a few comments, none are really blockers so I approve but would like the performance once to be looked at 👍

sebelga · 2020-06-09T09:54:22Z

x-pack/plugins/index_management/common/lib/component_template_serialization.ts

+
+const hasEntries = (data: object = {}) => Object.entries(data).length > 0;
+
+const getAssociatedIndexTemplates = (


As the deserializeComponenTemplateList is called inside a map, and here we have 2 loops, that's a total of 3 nested loops. Depending on the size of the arrays this could be a performance issue.

It would be better to first (once) convert the array of index template to a flat map with this function

/** * Normalize a list of component templates to a map where each key * is a component template name, and the value is an array of index templates name using it * * @example * { "comp-1": [ "template-1", "template-2" ], "comp2": [ "template-1", "template-2" ] } * * @param indexTemplatesEs List of component templates */ export const indexTemplatesToUsedBy = (indexTemplatesEs: TemplateV2Es[]) => { return indexTemplatesEs.reduce((acc, item) => { if (item.index_template.composed_of) { item.index_template.composed_of.forEach((component) => { acc[component] = acc[component] ? [...acc[component], item.name] : [item.name]; }); } return acc; }, {} as { [key: string]: string[] }); };

then it is direct access to the index templates used by a component template

Great point. I went ahead and addressed this.

sebelga · 2020-06-09T09:57:39Z

x-pack/plugins/index_management/common/types/templates.ts

@@ -49,6 +49,11 @@ export interface TemplateDeserialized {
  };
 }

+export interface TemplateV2Es {


I have removed all references to "V1" and "V2", can we rename this interface TemplateListItemES?

Also, there is no more "name" prop so it can be

index_template: TemplateSerialized

Good catch. I forgot to update this when pulling your changes.

sebelga · 2020-06-09T10:27:37Z

...t/public/application/components/component_templates/component_template_list/delete_modal.tsx

+      .then(({ data: { itemsDeleted, errors }, error }) => {
+        const hasDeletedComponentTemplates = itemsDeleted && itemsDeleted.length;
+
+        if (hasDeletedComponentTemplates) {


nit: this is the typical example where I prefer to have an i18nTexts object declared on top of the component and use it in the JSX. It makes things much clearer to understand IMO

👍 I'll keep this in mind. I'm going to leave as is for now, since I am not following this pattern elsewhere.

sebelga · 2020-06-09T10:32:42Z

...nagement/public/application/components/component_templates/component_template_list/table.tsx

+      onSelectionChange: setSelection,
+      selectable: ({ usedBy }) => usedBy.length === 0,
+      selectableMessage: (selectable) =>
+        !selectable


nit: I prefer to use the affirmative in ternary

sebelga · 2020-06-09T10:33:49Z

...nagement/public/application/components/component_templates/component_template_list/table.tsx

+      const { field } = column as EuiTableFieldDataColumnType<ComponentTemplateListItem>;
+
+      return {
+        'data-test-subj': `componentTemplateTableRow-${field}`,


Is all the logic in cellProps necessary? Do we need those specific test subject or can we test the table without them?

This allows the name of the cell to be part of the data-test-subj. I see I'm not currently using this in any of the tests though, so I removed for now.

sebelga · 2020-06-09T10:38:41Z

...gins/index_management/public/application/components/component_templates/lib/documentation.ts

+  const docsBase = `${ELASTIC_WEBSITE_URL}guide/en`;
+  const esDocsBase = `${docsBase}/elasticsearch/reference/${DOC_LINK_VERSION}`;
+
+  function getComponentTemplatesLink() {


Why not use an object instead of declaring methods? Do we expect parameters to be passed?

export const getDocumentation = ({ ELASTIC_WEBSITE_URL, DOC_LINK_VERSION }: DocLinksSetup) => { const docsBase = `${ELASTIC_WEBSITE_URL}guide/en`; const esDocsBase = `${docsBase}/elasticsearch/reference/${DOC_LINK_VERSION}`; return { componentTemplates: `${esDocsBase}/indices-component-template.html`, }; };

Great point. Fixed.

sebelga · 2020-06-09T10:40:01Z

x-pack/plugins/index_management/public/application/index.tsx

+    httpClient: services.httpService.httpClient,
+    apiBasePath: API_BASE_PATH,
+    appBasePath: BASE_PATH,
+    trackMetric: services.uiMetricService.trackMetric.bind(services.uiMetricService),


sebelga · 2020-06-09T10:42:07Z

x-pack/plugins/index_management/server/client/elasticsearch.ts

@@ -60,4 +61,14 @@ export const elasticsearchJsPlugin = (Client: any, config: any, components: any)
    ],
    method: 'DELETE',
  });
+
+  // Index templates v2


Nit: Can we update the comment // Composable templates?

…nent_templates_list_view

alisonelizabeth · 2020-06-09T14:56:01Z

Thanks for the review @sebelga! I believe I addressed all of your feedback.

I am not sure about repeating in each cell "index template" under the "Used by" column. I wonder if by just having the number (e.g. 3) it would be enough. In a following iteration, we could add a link on that number that would point to the index template table filtered on that component. This is similar behaviour that @cjcenizal demoed yesterday with data streams. WDYT?

Also, the cross icon looks like there is an error. What do you think about not having the icon, and setting the text in italic with lighter text color like this?

Thanks for bringing this up. I was thinking the same thing after seeing CJ's changes. I went ahead and followed your suggestions. I changed the column title from "Used by" to "Index templates" to make it a little more clear what the number represents. There might still be some better copy we can use here; I'll follow up on this during copy review.

kibanamachine · 2020-06-09T18:10:58Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 8d2222e

History

💔 Build #52729 failed 5ddb122
💚 Build #52494 succeeded 87ca2e9
💚 Build #52213 succeeded d0fc261
💔 Build #52156 failed 07aeb9b
💚 Build #51635 succeeded 6378956

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

Co-authored-by: Elastic Machine <[email protected]>

* master: (22 commits) Partial revert of "Sync Kerberos + Anonymous access tests with the latest `security/_authenticate` API (user roles now include roles of anonymous user)." (elastic#68624) adapt some snapshot test (elastic#68489) [APM] Service maps - Fix missing ML status for services with jobs but no anomalies (elastic#68486) [skip test] apis Kerberos security Kerberos authentication finishing SPNEGO should properly set cookie and authenticate user [SIEM][Exceptions] - ExceptionsViewer UI component part 2 (elastic#68294) Surface data streams in Index Management. (elastic#67806) Fix edit datasource not working following changes in elastic#67234 (elastic#68583) [Logs + Metrics UI] Clean up async plugin initialization (elastic#67654) APM Storybook fixes (elastic#68671) Upgrade EUI to v24.1.0 (elastic#68141) [ML] DF Analytics: Creation wizard part 2 (elastic#68462) [Uptime] Fix race on overview page query (elastic#67843) Prefer using npm_execpath when spawning Yarn (elastic#68673) [Security] [Cases] Attach timeline to existing case (elastic#68580) Use Search API in Vega (elastic#68257) [Component templates] Table view (elastic#68031) [Uptime] Added relative date info in cert status column (elastic#67612) [Endpoint] Re-enable Functional test case for Endpoint related pages (elastic#68445) run page_load_metrics tests in visual regresssion jobs (elastic#68570) Enable exhaustive-deps; correct any lint warnings (elastic#68453) ...

alisonelizabeth added 8 commits June 2, 2020 18:11

add component templates tab

2335451

add table

f58b39e

add delete functionality and error handling

808435a

disable delete for active templates

78f939b

refactor

bcfef27

modify get requests + various cleanup

7010b2f

cleanup

098fd8d

add tests

9bd51b8

alisonelizabeth added Feature:Index Management Index and index templates UI v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes v7.9.0 labels Jun 2, 2020

alisonelizabeth added 3 commits June 2, 2020 21:27

fix functional test

d078443

address initial feedback

d5a293e

update component integration tests

ee7293d

alisonelizabeth marked this pull request as ready for review June 3, 2020 17:25

alisonelizabeth requested a review from a team as a code owner June 3, 2020 17:25

alisonelizabeth requested a review from sebelga June 3, 2020 17:25

alisonelizabeth commented Jun 3, 2020

View reviewed changes

...n/components/component_templates/__jest__/client_integration/component_template_list.test.ts Outdated Show resolved Hide resolved

alisonelizabeth added 7 commits June 3, 2020 15:53

update api test

f8dd123

i18n fix

7d59c8c

Merge branch 'master' of github.com:elastic/kibana into feature/compo…

fcf38d4

…nent_templates_list_view

fix merge conflict

6378956

Merge branch 'master' of github.com:elastic/kibana into feature/compo…

07aeb9b

…nent_templates_list_view

fix merge

32616c4

remove nextTick() from tests

d0fc261

Merge branch 'master' into feature/component_templates_list_view

87ca2e9

sebelga approved these changes Jun 9, 2020

View reviewed changes

alisonelizabeth added 2 commits June 9, 2020 10:07

Merge branch 'master' of github.com:elastic/kibana into feature/compo…

906789e

…nent_templates_list_view

address review feedback

5ddb122

update tests

8d2222e

alisonelizabeth merged commit ee5284e into elastic:master Jun 9, 2020

alisonelizabeth deleted the feature/component_templates_list_view branch June 9, 2020 18:24

alisonelizabeth mentioned this pull request Jun 9, 2020

[7.x] [Component templates] Table view (#68031) #68687

Merged

alisonelizabeth added a commit that referenced this pull request Jun 10, 2020

[7.x] [Component templates] Table view (#68031) (#68687)

97f0353

Co-authored-by: Elastic Machine <[email protected]>

alisonelizabeth mentioned this pull request Jul 1, 2020

[Component templates] Form wizard #69732

Merged

cjcenizal mentioned this pull request Jul 29, 2020

Add index template v2 and component template support to Index Management app #64771

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Component templates] Table view #68031

[Component templates] Table view #68031

alisonelizabeth commented Jun 2, 2020 •

edited

Loading

elasticmachine commented Jun 3, 2020

alisonelizabeth commented Jun 8, 2020

sebelga commented Jun 9, 2020

sebelga left a comment

sebelga Jun 9, 2020

alisonelizabeth Jun 9, 2020

sebelga Jun 9, 2020

alisonelizabeth Jun 9, 2020

sebelga Jun 9, 2020

alisonelizabeth Jun 9, 2020

sebelga Jun 9, 2020

alisonelizabeth Jun 9, 2020

sebelga Jun 9, 2020 •

edited

Loading

alisonelizabeth Jun 9, 2020

sebelga Jun 9, 2020

alisonelizabeth Jun 9, 2020

sebelga Jun 9, 2020

sebelga Jun 9, 2020

alisonelizabeth commented Jun 9, 2020

kibanamachine commented Jun 9, 2020


		const hasEntries = (data: object = {}) => Object.entries(data).length > 0;

		const getAssociatedIndexTemplates = (

[Component templates] Table view #68031

[Component templates] Table view #68031

Conversation

alisonelizabeth commented Jun 2, 2020 • edited Loading

How to test

Screenshots

elasticmachine commented Jun 3, 2020

alisonelizabeth commented Jun 8, 2020

sebelga commented Jun 9, 2020

sebelga 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

sebelga Jun 9, 2020 • edited Loading

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

alisonelizabeth commented Jun 9, 2020

kibanamachine commented Jun 9, 2020

💚 Build Succeeded

History

alisonelizabeth commented Jun 2, 2020 •

edited

Loading

sebelga Jun 9, 2020 •

edited

Loading