[Discover] Make use of stateContainer action onDataViewEdited #199588
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/ci |
kertal
commented
Nov 11, 2024
| @@ -93,7 +85,7 @@ export const DiscoverTopNav = ({ | |||
| const editField = useMemo( | |||
| () => | |||
| canEditDataView | |||
| ? async (fieldName?: string, uiAction: 'edit' | 'add' = 'edit') => { | |||
There was a problem hiding this comment.
just a little cleanup since the second param is not in use
kertal
commented
Nov 11, 2024
Comment on lines
-126
to
-141
| const onEditDataView = useCallback( | ||
| async (editedDataView: DataView) => { | ||
| if (editedDataView.isPersisted()) { | ||
| // Clear the current data view from the cache and create a new instance | ||
| // of it, ensuring we have a new object reference to trigger a re-render | ||
| dataViews.clearInstanceCache(editedDataView.id); | ||
| stateContainer.actions.setDataView(await dataViews.create(editedDataView.toSpec(), true)); | ||
| } else { | ||
| await stateContainer.actions.updateAdHocDataViewId(); | ||
| } | ||
| stateContainer.actions.loadDataViewList(); | ||
| addLog('[DiscoverTopNav] onEditDataView triggers data fetching'); | ||
| stateContainer.dataState.fetch(); | ||
| }, | ||
| [dataViews, stateContainer.actions, stateContainer.dataState] | ||
| ); |
There was a problem hiding this comment.
Removed for code-deduplication, because this function is provided by our state container, but it seems it never has been used. so we're switching to make use of this part of the code:
This has another advantage, there's unit test code coverage for the state container function
kertal
added
Feature:Discover
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
akowalska622
approved these changes
Nov 11, 2024
|
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/11779046085 |
💚 Build Succeeded
Metrics [docs]Async chunks
History
cc @kertal |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Nov 11, 2024
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
3 tasks
kibanamachine
added a commit
that referenced
this pull request
Nov 11, 2024
…199588) (#199641) # Backport This will backport the following commits from `main` to `8.x`: - [[Discover] Make use of stateContainer action onDataViewEdited (#199588)](#199588) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Matthias Wilhelm","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-11T13:00:08Z","message":"[Discover] Make use of stateContainer action onDataViewEdited (#199588)\n\nDuplicating code that's triggered when a dataview is being edited","sha":"8136bcc0e43a682dfa4484f112a287210936f817","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Discover","release_note:skip","v9.0.0","Team:DataDiscovery","backport:prev-minor"],"title":"[Discover] Make use of stateContainer action onDataViewEdited","number":199588,"url":"https://github.com/elastic/kibana/pull/199588","mergeCommit":{"message":"[Discover] Make use of stateContainer action onDataViewEdited (#199588)\n\nDuplicating code that's triggered when a dataview is being edited","sha":"8136bcc0e43a682dfa4484f112a287210936f817"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199588","number":199588,"mergeCommit":{"message":"[Discover] Make use of stateContainer action onDataViewEdited (#199588)\n\nDuplicating code that's triggered when a dataview is being edited","sha":"8136bcc0e43a682dfa4484f112a287210936f817"}}]}] BACKPORT--> Co-authored-by: Matthias Wilhelm <[email protected]>
tkajtoch
pushed a commit
to tkajtoch/kibana
that referenced
this pull request
Nov 12, 2024
…c#199588) Duplicating code that's triggered when a dataview is being edited
CAWilson94
pushed a commit
to CAWilson94/kibana
that referenced
this pull request
Nov 18, 2024
…c#199588) Duplicating code that's triggered when a dataview is being edited
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
When working on #199419, I noticed that we don't make use of the stateContainer
onDataViewEditedfunction, but use duplicate code indiscover_topnav.tsxThis PR applies this change, deduplicating our code in this area, containing more smallish cleanups in the edited file.
Testing
This is just a code cleanup and it should be covered pretty well by functional testing. Now even the unit test coverage increased. win-win. But if there's interest which part of Discover this code is being used: When users edit a DataView in a flyout, it's triggered when they finish and save: