[Data View Mgmt] Implement state service by mattkime · Pull Request #193660 · elastic/kibana

mattkime · 2024-09-22T18:48:03Z

Summary

Intermediate step in converting Data View Management to DataViewLazy. Moved useCallback functionality into central service so much of the state functionality is in one place and easier to understand and refactor.

Part of #178926

Broken out from #190292 which tried to do everything in one go and became too messy.

…taging'

…nto data_view_mgmt_service

elasticmachine · 2024-09-23T06:06:10Z

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

mattkime · 2024-09-23T06:07:01Z

/ci

jughosta · 2024-09-24T14:13:00Z

+      dataView,
+      fields,
+      indexedFieldTypes,
+      fieldConflictCount: fields.filter((field) => field.type === 'conflict').length,


Can scripted fields cause a conflict?

Suggested change

fieldConflictCount: fields.filter((field) => field.type === 'conflict').length,

fieldConflictCount: dataView.fields.filter((field) => field.type === 'conflict').length,

This is purposeful as the DataViewLazy field list is a separate object and this code aims to minimize changes from DataView to DataViewLazy

fields excludes scripted fields when defined in setDataView. So my question was whether scripted fields still need to be checked for conflicts.

I tested it and it seems like scripted fields "override" normal fields hence we don't report on type conflicts.

That's correct, scripted fields override, no conflict possible.

jughosta

Nice refactoring! Leaving some questions first.

…ew_management_service.ts Co-authored-by: Julia Rechkunova <julia.rechkunova@gmail.com>

…nto data_view_mgmt_service

mattkime · 2024-09-24T15:43:40Z

/ci

mattkime · 2024-09-24T17:25:14Z

/ci

jughosta · 2024-09-25T11:47:22Z

+    this.updateState({
+      dataView,
+      fields,
+      indexedFieldTypes,


Side question: should we run uniq on the resulting indexedFieldTypes or is it fine to have it with duplicates like conflict?

uniq is run in UI code but makes more sense in the service - good call

jughosta

LGTM 👍

mattkime · 2024-09-25T12:59:04Z

/ci

## Summary Intermediate step in converting Data View Management to DataViewLazy. Moved `useCallback` functionality into central service so much of the state functionality is in one place and easier to understand and refactor. Part of elastic#178926 Broken out from elastic#190292 which tried to do everything in one go and became too messy. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Julia Rechkunova <julia.rechkunova@gmail.com> (cherry picked from commit d772a11)

kibanamachine · 2024-09-25T14:42:19Z

💚 All backports created successfully

Status	Branch	Result
✅	8.15
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibana-ci · 2024-09-25T14:56:08Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 9b3a5bd

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dataViewManagement`	236	239	+3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`dataViewManagement`	140.2KB	142.4KB	+2.2KB

Page load bundle

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

id	before	after	diff
`dataViewManagement`	5.0KB	5.2KB	+139.0B
`dataViews`	62.3KB	62.4KB	+108.0B
total			+247.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`dataViewManagement`	3	2	-1

References to deprecated APIs

id	before	after	diff
`dataViewManagement`	45	35	-10

Total ESLint disabled count

id	before	after	diff
`dataViewManagement`	3	2	-1

History

💚 Build #236999 succeeded 9b3a5bd
💚 Build #236751 succeeded e37407e
💔 Build #236722 failed a783436
💚 Build #236539 succeeded 47b470a
💛 Build #236191 was flaky aefcb63
💚 Build #236190 succeeded aefcb63

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

cc @mattkime

kibanamachine · 2024-09-30T18:21:47Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kibanamachine · 2024-09-30T18:21:47Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

# Backport This will backport the following commits from `main` to `8.x`: - [[Data View Mgmt] Implement state service (#193660)](#193660)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Matthew Kime <matt@mattki.me>

mattkime and others added 6 commits September 22, 2024 13:45

initial service implementation

8310954

[CI] Auto-commit changed files from 'make api-docs && make api-docs-s…

b1ec610

…taging'

Merge branch 'main' into data_view_mgmt_service

0b492aa

fix field refresh

fd5d212

Merge branch 'data_view_mgmt_service' of github.com:mattkime/kibana i…

61706fa

…nto data_view_mgmt_service

fix tests

ee6fea3

mattkime changed the title ~~initial service implementation~~ [Data View Mgmt] Implement state service Sep 23, 2024

mattkime self-assigned this Sep 23, 2024

mattkime added 3 commits September 23, 2024 00:26

cleanup

dbb2a11

cleanup

7a848e7

fix field filter tab update

aefcb63

mattkime marked this pull request as ready for review September 23, 2024 06:06

mattkime requested a review from a team as a code owner September 23, 2024 06:06

mattkime added the release_note:skip Skip the PR/issue when compiling release notes label Sep 23, 2024

Merge branch 'main' into data_view_mgmt_service

47b470a

jughosta reviewed Sep 24, 2024

View reviewed changes

mattkime and others added 3 commits September 24, 2024 09:55

Update src/plugins/data_view_management/public/management_app/data_vi…

e8e993c

…ew_management_service.ts Co-authored-by: Julia Rechkunova <julia.rechkunova@gmail.com>

feedback / cleanup

43338d7

Merge branch 'data_view_mgmt_service' of github.com:mattkime/kibana i…

a783436

…nto data_view_mgmt_service

fixes to cleanup

e37407e

jughosta reviewed Sep 25, 2024

View reviewed changes

jughosta approved these changes Sep 25, 2024

View reviewed changes

make sure field types are unique inside service

9b3a5bd

mattkime merged commit d772a11 into elastic:main Sep 25, 2024

kibanamachine added the v9.0.0 label Sep 25, 2024

kibanamachine mentioned this pull request Sep 25, 2024

[8.15] [Data View Mgmt] Implement state service (#193660) #194021

Closed

kibanamachine mentioned this pull request Sep 25, 2024

[8.x] [Data View Mgmt] Implement state service (#193660) #194022

Merged

mattkime added backport:prev-minor and removed backport:prev-major labels Sep 25, 2024

kibanamachine added backport missing Added to PRs automatically when the are determined to be missing a backport. labels Sep 30, 2024

kibanamachine added v8.16.0 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Sep 30, 2024

	fieldConflictCount: fields.filter((field) => field.type === 'conflict').length,
	fieldConflictCount: dataView.fields.filter((field) => field.type === 'conflict').length,

Conversation

mattkime commented Sep 22, 2024 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

elasticmachine commented Sep 23, 2024

Uh oh!

mattkime commented Sep 23, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jughosta Sep 24, 2024

Choose a reason for hiding this comment

Uh oh!

mattkime Sep 24, 2024

Choose a reason for hiding this comment

Uh oh!

jughosta Sep 25, 2024

Choose a reason for hiding this comment

Uh oh!

mattkime Sep 25, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jughosta left a comment

Choose a reason for hiding this comment

Uh oh!

mattkime commented Sep 24, 2024

Uh oh!

mattkime commented Sep 24, 2024

Uh oh!

jughosta Sep 25, 2024

Choose a reason for hiding this comment

Uh oh!

mattkime Sep 25, 2024

Choose a reason for hiding this comment

Uh oh!

jughosta left a comment

Choose a reason for hiding this comment

Uh oh!

mattkime commented Sep 25, 2024

Uh oh!

kibanamachine commented Sep 25, 2024

💚 All backports created successfully

Questions ?

Uh oh!

kibana-ci commented Sep 25, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Async chunks

Page load bundle

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

Uh oh!

kibanamachine commented Sep 30, 2024

Uh oh!

kibanamachine commented Sep 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mattkime commented Sep 22, 2024 •

edited by kibanamachine

Loading