Use SavedObjectsClient for Courier Index Pattern by tylersmalley · Pull Request #12719 · elastic/kibana

tylersmalley · 2017-07-08T19:15:17Z

Index pattern is created, by default, with a random ID by Elasticsearch
Updated all references requiring the pattern itself to use indexPattern.title
Advanced options toggle added to index pattern creation page to provide a specified ID
If an index pattern tied to a visualization does not exist, the user is given a link to create a pattern with the referenced ID.

* Index pattern is created, by default, with a random ID by Elasticsearch * Updated all references requiring the pattern itself to use indexPattern.title * Advanced options toggle added to index pattern creation page to provide a specified ID * If an index pattern does not exist, the user is given a link to create a pattern with the referenced ID. Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley · 2017-07-09T15:15:21Z

This PR is similar to #12407

thomasneirynck

Lgtm from my end.

I did notice the following UX difference:

When a user creates a new index pattern, and uses a matching-pattern that already exists, the user is no longer presented with a dialog that ask whether they want to override the pattern. The practical result it that it is possible now to create multiple index-pattern that have identical titles:

tylersmalley · 2017-07-10T04:36:12Z

@tbragin - what are your thoughts on @thomasneirynck's comment? Technically users can add index patterns with the same pattern but I could see how this could lead to customer confusion.

If they attempt to create a pattern with the same name, what are your thoughts on presenting them with a popup linking them to the index pattern to manage? In my opinion, the "force overwrite" is confusing. Is it merging the attributes with the existing pattern, or completely swapping the pattern out and removing any scripts, etc?

alexfrancoeur · 2017-07-10T10:29:39Z

@tylersmalley @thomasneirynck I agree, duplicate index patterns could be pretty confusing to a user. Is there any reason why a user would want duplicate index pattern names? It seems as if it'd be best to avoid this situation as much as possible. We handled duplicate dashboard names (slightly different use case) with a simple warning. If we agree that there is no need to have duplicate index pattern names, I think the proposed solution with a warning and link to the index pattern management page makes sense. Though we may want to get UX's input here.

I do have one question. Do we consider it a regression to remove the ability to override an index pattern?

kobelb · 2017-07-10T14:36:12Z

When creating a new visualization, the index pattern ID is displayed as opposed to the title:

However, it looks right after Saving.

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

kobelb · 2017-07-10T15:28:32Z

When clicking on the "Discover" app in the left navigation, I'm getting the following warning:

kobelb · 2017-07-10T15:44:32Z

When saving a new Saved Search, I'm immediately redirected to the Saved Objects Management screen and displayed the following error:

kobelb

There appear to be a number of bugs associated with switching from the index id to the title. Is there any possibility of avoiding this switch while still using the SavedObjectsClient?

kobelb · 2017-07-10T14:28:30Z

...ugins/kibana/public/management/sections/indices/create_index_pattern/create_index_pattern.js

-        if (!config.get('defaultIndex')) {
-          config.set('defaultIndex', id);
-        }
+      if (!config.get('defaultIndex')) {


We're no longer refreshing the kibana index, do you mind explaining why this changed?

The saved objects API uses refresh: wait_for when modifying objects instead of calling the refresh API directly to ensure the consistency.

kobelb · 2017-07-10T14:49:26Z

src/ui/public/courier/__tests__/saved_object.js

+          attributes: {
+            title: 'testIndexPattern'
+          },
+          _version: 2


It doesn't impact the test, but _version is actually version.

It's actually _version since it comes from SavedObject - we can revisit later if that's ok.

kobelb · 2017-07-10T14:53:26Z

src/ui/public/directives/__tests__/validate_index_name.js

 // Load the kibana app dependencies.

 describe('Validate index name directive', function () {
+  // Fox


Not sure what this comment means...

kobelb · 2017-07-10T15:34:21Z

src/ui/public/errors.js

-    super(
-      `Could not locate that ${type}${idMsg}`,
-      SavedObjectNotFound);
+    let message = `Could not locate that ${type}${idMsg}`;


Using html directly here doesn't appear to give us the desired result:

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley · 2017-07-11T17:52:27Z

src/ui/public/index_patterns/_index_pattern.js

@spalger, there is a bug in marked that when a URL contains an asterisk it won't render the URL.

That project is really in a bad place - in the future we might want to find something else: https://github.com/chjj/marked/issues

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

kobelb

LGTM

* Index pattern is created, by default, with a random ID by Elasticsearch * Updated all references requiring the pattern itself to use indexPattern.title * Advanced options toggle added to index pattern creation page to provide a specified ID * If an index pattern does not exist, the user is given a link to create a pattern with the referenced ID.

tylersmalley · 2017-07-12T04:43:48Z

5.x/5.6: 3c64283

sophiec20 · 2017-07-18T10:56:50Z

For info, just to mention, this change had a downstream effect on ML as we use the index pattern / saved object selector.

tylersmalley · 2017-07-18T14:38:47Z

Thanks @sophiec20, looking into that now.

jgowdyelastic · 2017-07-18T14:45:32Z

@tylersmalley This has been resolved now for ML under https://github.com/elastic/x-pack-kibana/pull/1954

tylersmalley · 2017-07-18T15:06:16Z

Thanks @jgowdyelastic, apologies for the breaking change.

ui-select-match's expression was expecting an IndexPattern object, but the indexPatternList passed to ui-select-choices contained SavedObject instances due to the [SavedObjectsClient refactor][1]. This wasn't a problem most of the time because switching index patterns caused the entire directive to get destroyed and re-created. However, when the directive didn't get re-created (for example, when clicking the already selected pattern) it would result in a blank select box. Fixes elastic#13080 [1]: elastic#12719

ui-select-match's expression was expecting an IndexPattern object, but the indexPatternList passed to ui-select-choices contained SavedObject instances due to the [SavedObjectsClient refactor][1]. This wasn't a problem most of the time because switching index patterns caused the entire directive to get destroyed and re-created. However, when the directive didn't get re-created (for example, when clicking the already selected pattern) it would result in a blank select box. Fixes #13080 [1]: #12719

tylersmalley force-pushed the 12670-index-pattern branch 6 times, most recently from eec2e06 to 8991e22 Compare July 8, 2017 23:23

tylersmalley force-pushed the 12670-index-pattern branch from 8991e22 to 3025f46 Compare July 9, 2017 01:10

tylersmalley mentioned this pull request Jul 9, 2017

Use SavedObjectsClient in Courier Index Pattern #12670

Closed

tylersmalley changed the title ~~[WIP] Use SavedObjectsClient for Courier Index Pattern~~ Use SavedObjectsClient for Courier Index Pattern Jul 9, 2017

tylersmalley added blocker review v5.6.0 v6.0.0 labels Jul 9, 2017

tylersmalley mentioned this pull request Jul 9, 2017

Single Type Compatibility #12723

Closed

7 tasks

thomasneirynck approved these changes Jul 10, 2017

View reviewed changes

kobelb self-requested a review July 10, 2017 13:37

Uses index pattern title for viz sidebar

7416173

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

kobelb requested changes Jul 10, 2017

View reviewed changes

tylersmalley added 5 commits July 10, 2017 10:14

Prevent duplicate index patterns

9ab4ae1

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Add files

485aa9a

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Fixes dashboard nav link and saving searches

c5cf5ab

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Use custom notify for index pattern create link

1a34a53

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Handle setting duplicate ID

439977a

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley commented Jul 11, 2017

View reviewed changes

tylersmalley force-pushed the 12670-index-pattern branch from ff588bd to 36e24dc Compare July 11, 2017 19:24

Adds markdown support to kbnError message

143921f

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley force-pushed the 12670-index-pattern branch from 36e24dc to 143921f Compare July 11, 2017 19:28

kobelb approved these changes Jul 11, 2017

View reviewed changes

tylersmalley merged commit 0d502ae into elastic:master Jul 11, 2017

tylersmalley mentioned this pull request Jul 11, 2017

[5.x] Use SavedObjectsClient for Courier Index Pattern #12779

Merged

spalger mentioned this pull request Jul 14, 2017

Migrate apps to use the SavedObject Client/API #12229

Closed

4 tasks

lukasolson mentioned this pull request Jul 19, 2017

Import is broken #12992

Closed

jimgoodwin added release_note:roadmap release_note:enhancement Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// labels Jul 25, 2017

chrisronline mentioned this pull request Jul 31, 2017

[Management] Update saved objects client usage to attributes instead of _source #13223

Merged

tylersmalley mentioned this pull request Aug 7, 2017

Improve UX of imported objects with missing Index Pattern #13359

Closed

Bargs mentioned this pull request Aug 25, 2017

Fix blank index pattern selector in Discover #13715

Merged

andyh69uk mentioned this pull request Dec 14, 2017

Kibana Index ID istresearch/kibana-object-format#14

Closed

Conversation

tylersmalley commented Jul 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tylersmalley commented Jul 9, 2017

Uh oh!

thomasneirynck left a comment

Choose a reason for hiding this comment

Uh oh!

tylersmalley commented Jul 10, 2017

Uh oh!

alexfrancoeur commented Jul 10, 2017

Uh oh!

kobelb commented Jul 10, 2017

Uh oh!

kobelb commented Jul 10, 2017

Uh oh!

kobelb commented Jul 10, 2017

Uh oh!

kobelb left a comment

Choose a reason for hiding this comment

Uh oh!

kobelb Jul 10, 2017

Choose a reason for hiding this comment

Uh oh!

tylersmalley Jul 10, 2017

Choose a reason for hiding this comment

Uh oh!

kobelb Jul 10, 2017

Choose a reason for hiding this comment

Uh oh!

tylersmalley Jul 10, 2017

Choose a reason for hiding this comment

Uh oh!

kobelb Jul 10, 2017

Choose a reason for hiding this comment

Uh oh!

kobelb Jul 10, 2017

Choose a reason for hiding this comment

Uh oh!

tylersmalley Jul 11, 2017

Choose a reason for hiding this comment

Uh oh!

kobelb left a comment

Choose a reason for hiding this comment

Uh oh!

tylersmalley commented Jul 12, 2017

Uh oh!

sophiec20 commented Jul 18, 2017

Uh oh!

tylersmalley commented Jul 18, 2017

Uh oh!

jgowdyelastic commented Jul 18, 2017

Uh oh!

tylersmalley commented Jul 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tylersmalley commented Jul 8, 2017 •

edited

Loading

tylersmalley commented Jul 18, 2017 •

edited

Loading