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

[data views] add getDefaultDataView method #113891

Merged
merged 4 commits into from
Oct 13, 2021

Conversation

mattkime
Copy link
Contributor

@mattkime mattkime commented Oct 5, 2021

Summary

Addresses #112362
Closes: #112846

The DataViews service has this terrible method called ensureDefaultDataView which a number of apps call when loading which a) ensures there's a default data viewand b) redirects to Data View Management if there aren't any.

There's no reason why the code that ensures there's a default data view should be tied to the code that redirects if there aren't any data views at all.

The getDefaultDataView ensures a default data view exists AND returns the default data view. Thats proper coupling. As for the ensureDefaultDataView - consumers should stop using it.

@mattkime mattkime changed the title add new method to data views api [data views] add getDefaultDataView method Oct 8, 2021
@mattkime mattkime added Feature:Data Views Data Views code and UI - index patterns before 8.0 Team:AppServicesSv v7.16.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed labels Oct 8, 2021
@mattkime mattkime marked this pull request as ready for review October 8, 2021 02:11
@mattkime
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
dataViews 38.6KB 38.9KB +316.0B
Unknown metric groups

API count

id before after diff
data 3192 3193 +1
dataViews 681 683 +2
total +3

References to deprecated APIs

id before after diff
dashboard 240 243 +3
dataViews 228 229 +1
discover 1790 1793 +3
lens 346 349 +3
visualize 99 102 +3
total +13

History

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

Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

Code lgtm,

p.s. there was no code owner ping? I think we forgot to add data_views to codeowners? Could you add it?

@mattkime mattkime merged commit 708f06f into elastic:master Oct 13, 2021
@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 113891

mattkime added a commit to mattkime/kibana that referenced this pull request Oct 13, 2021
* add new method to data views api

* add tests

Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	src/plugins/data_views/common/data_views/data_views.ts
mattkime added a commit that referenced this pull request Oct 13, 2021
* add new method to data views api

* add tests

Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	src/plugins/data_views/common/data_views/data_views.ts
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 13, 2021
…ide-users-to-saving-ux

* 'master' of github.com:elastic/kibana: (133 commits)
  [DOCS] Indicate reports are a subscription feature (elastic#114653)
  Update namespace for indices (elastic#114612)
  [DOCS] Adds Logstash pipeline settings (elastic#114648)
  Bump EPR snapshot version used for tests (elastic#114529)
  [Security Solution] [Endpoint] Fleet summary card adjustments (elastic#114291)
  skip flaky suite (elastic#68400)
  [Visualizations] fix usage of optional dependencies (elastic#114286)
  [Security Solution] [Detections] Improves custom query rule upgrade test (elastic#114454)
  [fleet] Add Integration Preference selector (elastic#114432)
  [Reporting] Add new `data-render-error` attribute (elastic#114472)
  Replace EuiCodeEditor with CodeEditor in app-services code (elastic#114316)
  [data views] add getDefaultDataView method  (elastic#113891)
  [Security Solution] [Endpoint] Event filters uses the new card design (elastic#114126)
  [fleet] Tweak Header UI (elastic#114704)
  [APM] Filter on tx metrics for instance stats (elastic#114758)
  [APM] Fix typo in linting docs (elastic#114764)
  [Discover] Removing SavedObject usage for savedSearch (elastic#112983)
  [Fleet] Add Integration Policy Page Improvements (elastic#114556)
  [Lens] Keep the custom label when transitioning to/from Formula (elastic#114270)
  [Security Solution][Endpoint] Host Isolation API changes (elastic#113621)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Data Views Data Views code and UI - index patterns before 8.0 release_note:skip Skip the PR/issue when compiling release notes v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data views] ensureDefaultDataView needs separation between UI and business logic
3 participants