[Composable templates] Table to list templates by sebelga · Pull Request #67282 · elastic/kibana

sebelga · 2020-05-25T14:03:16Z

This PR adds the client and server code change to support listing composable index templates in a table on top of the legacy index template.

They are no actions on the table items (view, delete, edit, clone), we can simply view the list of index templates V2.

How to test

Go to the index templates tab in index management and see the empty table on top of the legacy table
There is a "View" dropdown that allows you to toggle on/off the composable templates table and the system templates.
Open console in a separate tab (so you can easily update the template and reload the templates in the UI to see the changes).
Create a component template

PUT /_component_template/comp-1
{
   "template": {
      "mappings": {
        "properties": {
          "title": {
          "type": "text"
        }
        }
      }
    }
}

Then create an index template that is composed of the component template

PUT _index_template/my-v2-format
{
  "index_patterns": ["pat*"],
  "priority": 2,
  "version": 1,
  "composed_of": ["comp-1"],
  "template": {
    "mappings": {
      "properties": {
        "title": {
          "type": "text"
        }
      }
    },
    "settings": {
      "index": {
        "lifecycle": {
          "name": "my-policy",
          "rollover_alias": "some-alias"
        }
      }
    }
  }
}

Go back to the index template tab and verify that the table display correctly the newly created index template

elasticmachine · 2020-05-26T13:58:10Z

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

sebelga · 2020-06-01T15:04:35Z

@elasticmachine merge upstream

alisonelizabeth

Great work @sebelga!

Tested locally. Everything worked as expected.

I left a few comments in the code. I also had a few other general comments/questions:

With the introduction of component templates, I wonder if we should change the UI and API routes from templates to index_templates?
Since we have two tables now, what do you think about changing the default page size to 10 instead of 20? I didn't test it, but I imagine if you had 20 composable templates, you would have to scroll to see the legacy templates.
It felt a little weird to me having one filter to view composable templates and system templates, since the system filter applies to both tables, whereas selecting composable hides or removes the one table. I'd be interested to see if design has any feedback on this though.

x-pack/plugins/index_management/server/routes/api/templates/register_create_route.ts

alisonelizabeth · 2020-06-01T17:43:03Z

...ent/public/application/sections/home/template_list/components/template_content_indicator.tsx

+
+  return (
+    <>
+      <EuiBadge color={getColor(mappings)}>M</EuiBadge>


What do you think about adding a tooltip to these, so it's a little more clear what each one represents?

Good idea 👍

alisonelizabeth · 2020-06-01T17:49:48Z

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

@@ -5,60 +5,34 @@
 */


What do you think about adding tests for the serialization functions here?

I prefer to have it covered by integration. In this case the API integration tests. They actually caught a few errors in these handlers when I updated them and helped me to fix them.

alisonelizabeth · 2020-06-01T17:51:43Z

x-pack/plugins/index_management/__jest__/client_integration/home.test.ts


 jest.mock('ui/new_platform');

+// jest.mock('@elastic/eui', () => ({


can this be removed?

alisonelizabeth · 2020-06-01T17:58:27Z

..._management/public/application/sections/home/template_list/template_table/template_table.tsx

-        defaultMessage: 'Aliases',
+      field: 'hasMappings',
+      name: i18n.translate('xpack.idxMgmt.templateList.table.overridesColumnTitle', {
+        defaultMessage: 'Overrides',


[nit] Is "Overrides" the right term to use here? As we are just checking whether mappings/settings/aliases exist, it may or may not be an override (if I understand correctly).

Good point, I added it as a feedback to go back to in #68056

alisonelizabeth · 2020-06-01T18:01:31Z

...c/application/sections/home/template_list/legacy_templates/template_table/template_table.tsx

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


I'm assuming this file was just moved into the legacy_templates directory, so only did a light review here.

alisonelizabeth · 2020-06-01T18:04:19Z

...plication/sections/home/template_list/legacy_templates/template_details/template_details.tsx

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


I'm assuming this file was just moved into the legacy_templates directory, so only did a light review here.

alisonelizabeth · 2020-06-01T18:07:34Z

x-pack/plugins/index_management/public/application/services/routing.ts

+export const getTemplateEditLink = (name: string, isLegacy?: boolean) => {
  return encodeURI(
-    `${BASE_PATH}edit_template/${encodeURIComponent(encodeURIComponent(name))}?v=${formatVersion}`
+    `${BASE_PATH}edit_template/${encodeURIComponent(encodeURIComponent(name))}?legacy=${isLegacy}`


Since legacy is marked as optional, should this be something like:

isLegacy ? `?legacy=${isLegacy}` : ''

It would be a cleaner URL yes, but the logic would be the same as we would get

?legacy=undefined

So checking for

const isLegacy = queryParam.legacy === 'true';

would give false.

I made the change to this:

?legacy=${isLegacy === true}`

…gister_create_route.ts Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>

sebelga · 2020-06-03T08:19:55Z

Thanks for the review @alisonelizabeth !

With the introduction of component templates, I wonder if we should change the UI and API routes from templates to index_templates?

Good point, I will update the endpoints.

For the rest of your concerns, I think it would be better to not block this PR on these decisions as they might not be relevant later. I created this issue #68056 to keep track of your feedback and revisit it later. Is that OK for you?

…plate/table-v2

sebelga · 2020-06-03T13:16:24Z

I made the changes you recomended @alisonelizabeth , can you have another look? thanks!

alisonelizabeth

Thanks for making those changes @sebelga!

I noticed a small regression with spacing of the badges. Can you take a look? Otherwise, LGTM. Going to approve to not block you.

…plate/table-v2

sebelga · 2020-06-04T15:43:55Z

Thanks for the review @alisonelizabeth and finding the regression. I made the fix. Will merge as soon as CI gets green 🤞

kibanamachine · 2020-06-04T20:07:11Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5138bab

History

💔 Build #51809 failed fe734bd
💔 Build #51752 failed a6f4726
💔 Build #51726 failed 4e01e1e
💚 Build #51415 succeeded 74a1d70
💔 Build #51378 failed 7fbfb23

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

kibanamachine · 2020-06-08T20:38: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.

Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: spalger <spalger@users.noreply.github.com>

sebelga added Feature:Index Management Index and index templates UI Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v8.0.0 labels May 26, 2020

sebelga added 5 commits May 29, 2020 12:21

Update client code to support both table

f57e9df

Update server code to fetch V2 templates

5aea8c9

Update component integration tests

a687422

Update API integration tests

22f9b26

Misc

c4f7aa6

sebelga force-pushed the feature/index-template/table-v2 branch from 191e9e4 to c4f7aa6 Compare May 29, 2020 10:21

Fix TS issue

11b418b

sebelga added the v7.9.0 label May 29, 2020

sebelga marked this pull request as ready for review May 29, 2020 12:51

sebelga requested a review from a team as a code owner May 29, 2020 12:51

Fix i18n issues

f3dae6d

sebelga requested a review from alisonelizabeth May 29, 2020 13:06

sebelga added the release_note:skip Skip the PR/issue when compiling release notes label May 29, 2020

Merge branch 'master' into feature/index-template/table-v2

35dac8c

alisonelizabeth reviewed Jun 1, 2020

View reviewed changes

Update x-pack/plugins/index_management/server/routes/api/templates/re…

b7ae6ba

…gister_create_route.ts Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>

sebelga mentioned this pull request Jun 3, 2020

Composable Index Templates UI - Feedback #68056

Closed

sebelga added 4 commits June 3, 2020 13:43

Merge remote-tracking branch 'upstream/master' into feature/index-tem…

f1ef4f0

…plate/table-v2

Add tooltips to template content indicator

a1140da

Update API route to use "index-templates" instead of "templates"

d0ef2fb

Add CR changes

7fbfb23

sebelga requested a review from alisonelizabeth June 3, 2020 13:15

sebelga mentioned this pull request Jun 3, 2020

Bug/update follower index validation cr cjcenizal/kibana#24

Open

Fix component integration test

74a1d70

alisonelizabeth approved these changes Jun 3, 2020

View reviewed changes

sebelga added 5 commits June 4, 2020 11:32

Merge remote-tracking branch 'upstream/master' into feature/index-tem…

4e01e1e

…plate/table-v2

Fix component integration tests

a6f4726

Merge remote-tracking branch 'upstream/master' into feature/index-tem…

4e133b4

…plate/table-v2

Add space between template content indicators

e2e8535

Fix eslint issues

fe734bd

Fix component integration test

5138bab

sebelga merged commit c77e9bf into elastic:master Jun 4, 2020

sebelga deleted the feature/index-template/table-v2 branch June 4, 2020 20:34

sebelga mentioned this pull request Jun 4, 2020

[7.x] [Composable templates] Table to list templates (#67282) #68324

Merged

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

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

cjcenizal mentioned this pull request Jul 29, 2020

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

Closed


		jest.mock('ui/new_platform');

		// jest.mock('@elastic/eui', () => ({

Conversation

sebelga commented May 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to test

Uh oh!

elasticmachine commented May 26, 2020

Uh oh!

sebelga commented Jun 1, 2020

Uh oh!

alisonelizabeth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebelga Jun 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebelga commented Jun 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sebelga commented Jun 3, 2020

Uh oh!

alisonelizabeth left a comment

Choose a reason for hiding this comment

Uh oh!

sebelga commented Jun 4, 2020

Uh oh!

kibanamachine commented Jun 4, 2020

💚 Build Succeeded

History

Uh oh!

kibanamachine commented Jun 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sebelga commented May 25, 2020 •

edited

Loading

sebelga Jun 3, 2020 •

edited

Loading

sebelga commented Jun 3, 2020 •

edited

Loading