-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Index Patterns] Fix return saved index pattern object #101051
Conversation
if (this.savedObjectsCache) { | ||
this.savedObjectsCache.push(response as SavedObject<IndexPatternSavedObjectAttrs>); | ||
} | ||
return indexPattern; | ||
return createdIndexPattern; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the main change. I want returned index pattern to go through the all transformations that are happening when saved objects is saved
}); | ||
|
||
expect(response.status).to.be(200); | ||
expect(response.body.index_pattern.title).to.be(title); | ||
expect(response.body.index_pattern.fields.foo.name).to.be('foo'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the functional change this test stopped working because foo
field was actually never saved into saved object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good although I think we could preserve version
as part of the index pattern spec.
@mattkime, I reverted functional |
Pinging @elastic/kibana-app-services (Team:AppServices) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes look good and work well. nice cleanup!
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Page load bundle
Unknown metric groupsReferences to deprecated APIs
History
To update your PR or re-run it, just comment with: |
…sens/kibana into reporting/new-png-pdf-report-type * 'reporting/new-png-pdf-report-type' of github.com:jloleysens/kibana: (46 commits) [Security Solution] Add Ransomware canary advanced policy option (elastic#101068) [Exploratory view] Core web vitals (elastic#100320) [Security solution][Endpoint] Add unit tests for fleet event filters/trusted apps cards (elastic#101034) [Lens] Use a setter function for the dimension panel (elastic#101123) [Index Patterns] Fix return saved index pattern object (elastic#101051) [CI] For PRs, build TS refs before public api docs check (elastic#100791) [Maps] fix line and polygon label regression (elastic#101085) Migrate CCR to new ES JS client. (elastic#100131) [Canvas] Switch Canvas to use React Router (elastic#100579) [Expressions] Use table column ID instead of name when set (elastic#99724) [DOCS] Updates docs landing page (elastic#100749) [DOCS] Corrects typo in step 3 (elastic#101079) [DOCS] Updates runtime example in Discover (elastic#100926) Migrate kibana.autocomplete config to data plugin (elastic#100586) [Uptime] New width/delay definition for waterfall sidebar item tooltip (elastic#100147) [FTR] Use importExport for saved_object/basic archive (elastic#100244) [Fleet] Better input for multi text input in agent policy builder (elastic#101020) [CI] Buildkite support with Baseline pipeline (elastic#100492) [Reporting/Telemetry] Do not send telemetry if we are in screenshot mode (elastic#100388) Create API keys with metadata (elastic#100682) ...
* master: (68 commits) Unskip advanced settings a11y test (elastic#100558) [App Search] Crawler Landing Page (elastic#100822) [DOCS] Clarify when to use kbn clean (elastic#101155) change label behavior (elastic#100991) skip flaky suite (elastic#101126) Fix cases plugin ownership (elastic#101073) [Home] Adding file upload to add data page (elastic#100863) [ML] Functional tests - reenable categorization tests (elastic#101137) [DOCS] Adds server.uuid to settings docs (elastic#101121) Fix newsfeed unread notifications always on when reloading Kibana (elastic#100357) [Lens] Time shift metrics (elastic#98781) [Deprecations service] make `correctiveActions.manualSteps` required (elastic#100997) Add "Risk Matrix" section to the PR template (elastic#100649) [Maps] spatially filter by all geo fields (elastic#100735) [Security Solution] Add Ransomware canary advanced policy option (elastic#101068) [Exploratory view] Core web vitals (elastic#100320) [Security solution][Endpoint] Add unit tests for fleet event filters/trusted apps cards (elastic#101034) [Lens] Use a setter function for the dimension panel (elastic#101123) [Index Patterns] Fix return saved index pattern object (elastic#101051) [CI] For PRs, build TS refs before public api docs check (elastic#100791) ...
Summary
While working on runtime fields REST API I noticed the following problem:
This pr makes so returned
indexPattern
object fromcreateAndSave
is the same that would be later returned fromget
. Also fixes inconsistencies in tests.Further open question I'd like to discuss later. Separate from this pr:
Does it make sense to allow to create any fields except scripted fields? Looks likes any other fields would be overridden by the es index. Such fields are also not saved to SO:
kibana/src/plugins/data/common/index_patterns/index_patterns/index_pattern.ts
Lines 324 to 326 in f7698bd
Considering that only scripted fields are saved, how
allowNoIndex
option supposed to work then? Looks like currently, it is not possible to create a non-scripted/non-runtime field if a field is not in es index?Checklist
Delete any items that are not applicable to this PR.
For maintainers