Skip to content

[IM] Index Templates UI list view#39922

Merged
alisonelizabeth merged 12 commits intomasterfrom
feature/index_templates
Jul 18, 2019
Merged

[IM] Index Templates UI list view#39922
alisonelizabeth merged 12 commits intomasterfrom
feature/index_templates

Conversation

@alisonelizabeth
Copy link
Contributor

@alisonelizabeth alisonelizabeth commented Jun 28, 2019

This PR is the first phase of adding support for Index Templates within the Index Management app.

Changes include:

  • New tabbed content under Index Management - Indices & Index Templates
  • List view of a user's index templates with delete action + filter by ILM policy
  • API integration tests for fetching, deleting and creating index templates
  • Component integration tests for index templates list view
  • Telemetry for index templates list view

Release notes

This feature adds the list of Index Templates as a tab to the Index Management app. Users can also delete templates from this view.

Note: This is the first phase, additional functionality for Index Templates will be added for 7.4 in separate PRs.

Testing

There are several system index templates by default. To create your own index template, go to console and follow the ES API docs.

For example:

PUT _template/template_1
{
  "index_patterns": ["te*", "bar*"],
  "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"
      }
    }
  }
}

Screenshots

Before
Screen Shot 2019-07-08 at 1 48 12 PM

After
Indices (functionality unchanged, but now under Indices tab)

Screen Shot 2019-07-12 at 2 50 15 PM

Index Templates (new)

List view w/o system templates:
Screen Shot 2019-07-10 at 2 57 46 PM

List view with system templates toggled:
Screen Shot 2019-07-12 at 2 47 20 PM

Filtered list view by ILM policy:
Screen Shot 2019-07-10 at 2 58 04 PM

Delete single template:
Screen Shot 2019-07-10 at 2 58 38 PM

Delete multiple templates:
Screen Shot 2019-07-10 at 2 58 28 PM

Delete system template:
Screen Shot 2019-07-12 at 2 48 54 PM

@alisonelizabeth alisonelizabeth added release_note:enhancement 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 t// v7.4.0 labels Jun 28, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui

@elasticmachine
Copy link
Contributor

💔 Build Failed

@alisonelizabeth alisonelizabeth force-pushed the feature/index_templates branch from 8f94c99 to 5d8205f Compare July 1, 2019 19:51
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@alisonelizabeth alisonelizabeth force-pushed the feature/index_templates branch from eb0c9d7 to 5954ef9 Compare July 3, 2019 13:30
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alisonelizabeth alisonelizabeth force-pushed the feature/index_templates branch from 6a10894 to 25f16e4 Compare July 8, 2019 16:25
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@alisonelizabeth
Copy link
Contributor Author

Retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@Conky5
Copy link
Contributor

Conky5 commented Jul 8, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@alisonelizabeth alisonelizabeth force-pushed the feature/index_templates branch from b2c9e89 to 50611d7 Compare July 9, 2019 19:44
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alisonelizabeth alisonelizabeth force-pushed the feature/index_templates branch from 50611d7 to 8365f80 Compare July 10, 2019 16:02
@alisonelizabeth alisonelizabeth marked this pull request as ready for review July 10, 2019 16:08
@alisonelizabeth alisonelizabeth changed the title [IM] Index Templates UI [IM] Index Templates UI list view Jul 10, 2019
@alisonelizabeth alisonelizabeth requested a review from sebelga July 10, 2019 16:09
@alisonelizabeth
Copy link
Contributor Author

FYI @yaronp68 this is ready for review

@alisonelizabeth
Copy link
Contributor Author

@Bamieh do you have capacity to review i18n on this PR?

@alisonelizabeth alisonelizabeth force-pushed the feature/index_templates branch from 8365f80 to 25bf57a Compare July 10, 2019 16:21
@@ -0,0 +1,131 @@
/*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll delete this file and adopt the shared plugin once #40777 is merged.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@yaronp68
Copy link

@cjcenizal @alisonelizabeth I'm fine with a confirmation approach for deleting system templates but worth checking with support later. I'll ask

@alisonelizabeth alisonelizabeth force-pushed the feature/index_templates branch from b0fc167 to 63ab1ae Compare July 12, 2019 18:51
@alisonelizabeth
Copy link
Contributor Author

updated PR to allow deletion of system templates with confirmation (see latest screenshots in first comment)

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

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

LGTM! Awesome work Alison! I tested locally and works great.
I made a few tiny (preference) code comments, nothing important.

There is only 1 UX that I am not 100% convinced (but it seems to be an EUI decision, not yours), is the "ILM Policy" button. For me, it should be a dropdown that indicates that it is a filter, with a label on top Filter templates for example. As a user, I am not used to click a button to filter item in a table.

templatesToDelete.find(templateName => templateName.startsWith('.'))
);

const handleDeleteTemplates = async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the async does not seem to be needed here

);

const handleDeleteTemplates = async () => {
deleteTemplates(templatesToDelete).then(({ data: { templatesDeleted, errors }, error }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't looked at the API yet, but it is a bit confusing to have both "errors" and "error". It does seem though that is is a pattern we have used in another app though (now I am the reader and not the writer of the code 😊 ). If error (singular) is the Http request that failed, could we rename it "apiError"?


const [isDeleteConfirmed, setIsDeleteConfirmed] = useState<boolean>(false);

if (!numTemplatesToDelete) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be better on L23 to exit early.

const Checkmark = ({ tableCellData }: { tableCellData: object }) => {
const isChecked = Object.entries(tableCellData).length > 0;

if (isChecked) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: In these cases, I usually personally prefer the shorter ternary version

return isChecked ? <EuiIcon type="check" /> : null;

} catch (e) {
return response.errors.push({
name,
error: wrapEsError(e),
Copy link
Contributor

Choose a reason for hiding this comment

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

Now I see it 😊

@alisonelizabeth alisonelizabeth requested a review from Bamieh July 15, 2019 15:53
Copy link
Contributor

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

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

LGTM

'xpack.idxMgmt.deleteTemplatesModal.successNotificationMessageText',
{
defaultMessage:
'Deleted {numSuccesses, number} {numSuccesses, plural, one {template} other {templates}}',
Copy link
Contributor

Choose a reason for hiding this comment

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

this is OK, but i'd use this:

Suggested change
'Deleted {numSuccesses, number} {numSuccesses, plural, one {template} other {templates}}',
'Deleted {numSuccesses, plural, one {# template} other {# templates}}',

the # will return the variable value.

@alisonelizabeth alisonelizabeth force-pushed the feature/index_templates branch from 63ab1ae to d48d60b Compare July 18, 2019 00:33
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alisonelizabeth
Copy link
Contributor Author

@sebelga @ahmad thanks for your reviews! I addressed the feedback, so going to go ahead and merge.

@sebelga I agree the ILM button is a little weird, but that is a EUI decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Index Management Index and index templates UI release_note:enhancement Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.4.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants