Skip to content
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

Address feedback on new IndexPatterns implementation. #42646

Merged
merged 4 commits into from
Aug 6, 2019

Conversation

lukeelmers
Copy link
Member

@lukeelmers lukeelmers commented Aug 5, 2019

Closes #42511

Dev Docs

In #39247, changes were introduced that added a required refresh parameter on the following IndexPatterns methods:

  • getIds
  • getTitles
  • getFields

While the code technically still worked without a param because undefined is falsey, it makes it less convenient to use from TS files.

Based on the feedback in #42511, this PR updates the service so that each of these params is optional, with a default of false.

This also replaces usages of lodash get in the IndexPatterns class with the more TS-friendly idx.

@lukeelmers lukeelmers added Feature:Data Views Data Views code and UI - index patterns before 8.0 v8.0.0 Team:AppArch release_note:skip Skip the PR/issue when compiling release notes v7.4.0 Feature:NP Migration labels Aug 5, 2019
@lukeelmers lukeelmers requested review from walterra and ppisljar August 5, 2019 22:48
@lukeelmers lukeelmers self-assigned this Aug 5, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch

@lukeelmers lukeelmers changed the title Make the refresh param in IndexPatterns get methods optional. Address feedback on new IndexPatterns implementation. Aug 5, 2019
@lukeelmers lukeelmers marked this pull request as ready for review August 5, 2019 23:29
@lukeelmers lukeelmers requested a review from a team as a code owner August 5, 2019 23:29
@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for this update!

…tterns-getTitle

# Conflicts:
#	src/legacy/ui/public/index_patterns/index_patterns.ts
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@lukeelmers lukeelmers merged commit 479e971 into elastic:master Aug 6, 2019
@lukeelmers lukeelmers deleted the feat/index-patterns-getTitle branch August 6, 2019 20:41
@lukeelmers lukeelmers removed the review label Aug 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Data Views Data Views code and UI - index patterns before 8.0 Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adjustments to IndexPatterns.getTitles
4 participants