-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[data view editor] Form state refactor - move all state logic into service and out of react state management #143604
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
Merged
mattkime
merged 33 commits into
elastic:main
from
mattkime:reduce_react_state_data_view_create
Nov 29, 2022
Merged
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
93f99cf
partial progress
mattkime 009ca50
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine 9ff8b24
fix title field validation, work in progress
mattkime 0f8884f
Merge branch 'reduce_react_state_data_view_create' of github.com:matt…
mattkime 0723097
Merge branch 'main' into reduce_react_state_data_view_create
mattkime de0361c
remove debounce, partial progress
mattkime 32b9a2c
lots of refactoring, most of the way there, still needs a bit more cl…
mattkime 2c1a7f1
add update method and make most things private
mattkime 19ce8d5
Merge branch 'main' into reduce_react_state_data_view_create
mattkime a0e4fde
partial progress, lots of cleanup, still some todos
mattkime d8ba032
Merge branch 'main' into reduce_react_state_data_view_create
mattkime e5b93ca
remove a few comments
mattkime c2b2a15
export observables rather than subjects
mattkime 27c1fb4
remove service context
mattkime 798edb6
Merge branch 'main' into reduce_react_state_data_view_create
mattkime 535a5ab
display errors on submission of form
mattkime 4f8524d
final cleanup and test fix, all working
mattkime 94ba339
Merge branch 'main' into reduce_react_state_data_view_create
mattkime 1b9ac2f
add destroy method to service
mattkime b0626f4
Merge branch 'reduce_react_state_data_view_create' of github.com:matt…
mattkime 3f4bc34
useState to store service reference instead of useRef
mattkime 33f252b
fix case where index pattern wasn't changed
mattkime 0299122
Merge branch 'main' into reduce_react_state_data_view_create
mattkime fbd0594
Merge branch 'main' into reduce_react_state_data_view_create
mattkime 428a0ad
Merge branch 'main' into reduce_react_state_data_view_create
mattkime 5fb1dde
Merge branch 'reduce_react_state_data_view_create' of github.com:matt…
mattkime 054c1e6
Merge branch 'main' into reduce_react_state_data_view_create
mattkime 2f4e0fd
Merge branch 'main' into reduce_react_state_data_view_create
mattkime b597595
add small delay to functional test
mattkime 3f0e539
Merge branch 'main' into reduce_react_state_data_view_create
mattkime c790676
Merge branch 'main' into reduce_react_state_data_view_create
mattkime 5b6bb93
remove error flash on successful submit
mattkime 659dcd3
Merge branch 'reduce_react_state_data_view_create' of github.com:matt…
mattkime File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ | |
| * Side Public License, v 1. | ||
| */ | ||
|
|
||
| import React, { useEffect, useCallback, useRef, useContext } from 'react'; | ||
| import React, { useEffect, useCallback } from 'react'; | ||
| import { | ||
| EuiTitle, | ||
| EuiFlexGroup, | ||
|
|
@@ -17,10 +17,8 @@ import { | |
| } from '@elastic/eui'; | ||
|
|
||
| import { i18n } from '@kbn/i18n'; | ||
| import memoizeOne from 'memoize-one'; | ||
| import { BehaviorSubject } from 'rxjs'; | ||
| import useObservable from 'react-use/lib/useObservable'; | ||
| import { INDEX_PATTERN_TYPE, MatchedItem } from '@kbn/data-views-plugin/public'; | ||
| import { INDEX_PATTERN_TYPE } from '@kbn/data-views-plugin/public'; | ||
|
|
||
| import { | ||
| DataView, | ||
|
|
@@ -32,7 +30,6 @@ import { | |
| UseField, | ||
| } from '../shared_imports'; | ||
|
|
||
| import { ensureMinimumTime, getMatchedIndices } from '../lib'; | ||
| import { FlyoutPanels } from './flyout_panels'; | ||
|
|
||
| import { removeSpaces } from '../lib'; | ||
|
|
@@ -41,7 +38,6 @@ import { | |
| DataViewEditorContext, | ||
| RollupIndicesCapsResponse, | ||
| IndexPatternConfig, | ||
| MatchedIndicesSet, | ||
| FormInternal, | ||
| } from '../types'; | ||
|
|
||
|
|
@@ -57,7 +53,6 @@ import { | |
| RollupBetaWarning, | ||
| } from '.'; | ||
| import { editDataViewModal } from './confirm_modals/edit_data_view_changed_modal'; | ||
| import { DataViewEditorServiceContext } from './data_view_flyout_content_container'; | ||
| import { DataViewEditorService } from '../data_view_editor_service'; | ||
|
|
||
| export interface Props { | ||
|
|
@@ -70,19 +65,12 @@ export interface Props { | |
| */ | ||
| onCancel: () => void; | ||
| defaultTypeIsRollup?: boolean; | ||
| requireTimestampField?: boolean; | ||
| editData?: DataView; | ||
| showManagementLink?: boolean; | ||
| allowAdHoc: boolean; | ||
| dataViewEditorService: DataViewEditorService; | ||
| } | ||
|
|
||
| export const matchedIndiciesDefault = { | ||
| allIndices: [], | ||
| exactMatchedIndices: [], | ||
| partialMatchedIndices: [], | ||
| visibleIndices: [], | ||
| }; | ||
|
|
||
| const editorTitle = i18n.translate('indexPatternEditor.title', { | ||
| defaultMessage: 'Create data view', | ||
| }); | ||
|
|
@@ -95,17 +83,15 @@ const IndexPatternEditorFlyoutContentComponent = ({ | |
| onSave, | ||
| onCancel, | ||
| defaultTypeIsRollup, | ||
| requireTimestampField = false, | ||
| editData, | ||
| allowAdHoc, | ||
| showManagementLink, | ||
| dataViewEditorService, | ||
| }: Props) => { | ||
| const { | ||
| services: { application, dataViews, uiSettings, overlays }, | ||
| } = useKibana<DataViewEditorContext>(); | ||
|
|
||
| const { dataViewEditorService } = useContext(DataViewEditorServiceContext); | ||
|
|
||
| const canSave = dataViews.getCanSaveSync(); | ||
|
|
||
| const { form } = useForm<IndexPatternConfig, FormInternal>({ | ||
|
|
@@ -132,15 +118,12 @@ const IndexPatternEditorFlyoutContentComponent = ({ | |
| return; | ||
| } | ||
|
|
||
| const rollupIndicesCapabilities = dataViewEditorService.rollupIndicesCapabilities$.getValue(); | ||
|
|
||
| const indexPatternStub: DataViewSpec = { | ||
| title: removeSpaces(formData.title), | ||
| timeFieldName: formData.timestampField?.value, | ||
| id: formData.id, | ||
| name: formData.name, | ||
| }; | ||
| const rollupIndex = rollupIndex$.current.getValue(); | ||
|
|
||
| if (type === INDEX_PATTERN_TYPE.ROLLUP && rollupIndex) { | ||
| indexPatternStub.type = INDEX_PATTERN_TYPE.ROLLUP; | ||
|
|
@@ -175,110 +158,29 @@ const IndexPatternEditorFlyoutContentComponent = ({ | |
| allowHidden = schema.allowHidden.defaultValue, | ||
| type = schema.type.defaultValue, | ||
| }, | ||
| ] = useFormData<FormInternal>({ form }); | ||
|
|
||
| const currentLoadingMatchedIndicesRef = useRef(0); | ||
| ] = useFormData<FormInternal>({ | ||
| form, | ||
| }); | ||
|
|
||
| const isLoadingSources = useObservable(dataViewEditorService.isLoadingSources$, true); | ||
| const existingDataViewNames = useObservable(dataViewEditorService.dataViewNames$); | ||
| const rollupIndex = useObservable(dataViewEditorService.rollupIndex$); | ||
| const rollupIndicesCapabilities = useObservable(dataViewEditorService.rollupIndicesCaps$, {}); | ||
|
|
||
| const loadingMatchedIndices$ = useRef(new BehaviorSubject<boolean>(false)); | ||
|
|
||
| const isLoadingDataViewNames$ = useRef(new BehaviorSubject<boolean>(true)); | ||
| const existingDataViewNames$ = useRef(new BehaviorSubject<string[]>([])); | ||
| const isLoadingDataViewNames = useObservable(isLoadingDataViewNames$.current, true); | ||
|
|
||
| const rollupIndicesCapabilities = useObservable( | ||
| dataViewEditorService.rollupIndicesCapabilities$, | ||
| {} | ||
| ); | ||
|
|
||
| const rollupIndex$ = useRef(new BehaviorSubject<string | undefined>(undefined)); | ||
|
|
||
| // initial loading of indicies and data view names | ||
| useEffect(() => { | ||
| let isCancelled = false; | ||
| const matchedIndicesSub = dataViewEditorService.matchedIndices$.subscribe((matchedIndices) => { | ||
| const timeFieldQuery = editData ? editData.title : title; | ||
| dataViewEditorService.loadTimestampFields( | ||
| removeSpaces(timeFieldQuery), | ||
| type, | ||
| requireTimestampField, | ||
| rollupIndex$.current.getValue() | ||
| ); | ||
| }); | ||
|
|
||
| dataViewEditorService.loadIndices(title, allowHidden).then((matchedIndices) => { | ||
| if (isCancelled) return; | ||
| dataViewEditorService.matchedIndices$.next(matchedIndices); | ||
| }); | ||
| dataViewEditorService.setIndexPattern(title); | ||
| }, [dataViewEditorService, title]); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. setting state |
||
|
|
||
| dataViewEditorService.loadDataViewNames(title).then((names) => { | ||
| if (isCancelled) return; | ||
| const filteredNames = editData ? names.filter((name) => name !== editData?.name) : names; | ||
| existingDataViewNames$.current.next(filteredNames); | ||
| isLoadingDataViewNames$.current.next(false); | ||
| }); | ||
| useEffect(() => { | ||
| dataViewEditorService.setAllowHidden(allowHidden); | ||
| }, [dataViewEditorService, allowHidden]); | ||
|
|
||
| return () => { | ||
| isCancelled = true; | ||
| matchedIndicesSub.unsubscribe(); | ||
| }; | ||
| }, [editData, type, title, allowHidden, requireTimestampField, dataViewEditorService]); | ||
| useEffect(() => { | ||
| dataViewEditorService.setType(type); | ||
| }, [dataViewEditorService, type]); | ||
|
|
||
| const getRollupIndices = (rollupCaps: RollupIndicesCapsResponse) => Object.keys(rollupCaps); | ||
|
|
||
| // used in title field validation | ||
| const reloadMatchedIndices = useCallback( | ||
| async (newTitle: string) => { | ||
| let newRollupIndexName: string | undefined; | ||
|
|
||
| const fetchIndices = async (query: string = '') => { | ||
| const currentLoadingMatchedIndicesIdx = ++currentLoadingMatchedIndicesRef.current; | ||
|
|
||
| loadingMatchedIndices$.current.next(true); | ||
|
|
||
| const allSrcs = await dataViewEditorService.getIndicesCached({ | ||
| pattern: '*', | ||
| showAllIndices: allowHidden, | ||
| }); | ||
|
|
||
| const { matchedIndicesResult, exactMatched } = !isLoadingSources | ||
| ? await loadMatchedIndices(query, allowHidden, allSrcs, dataViewEditorService) | ||
| : { | ||
| matchedIndicesResult: matchedIndiciesDefault, | ||
| exactMatched: [], | ||
| }; | ||
|
|
||
| if (currentLoadingMatchedIndicesIdx === currentLoadingMatchedIndicesRef.current) { | ||
| // we are still interested in this result | ||
| if (type === INDEX_PATTERN_TYPE.ROLLUP) { | ||
| const isRollupIndex = await dataViewEditorService.getIsRollupIndex(); | ||
| const rollupIndices = exactMatched.filter((index) => isRollupIndex(index.name)); | ||
| newRollupIndexName = rollupIndices.length === 1 ? rollupIndices[0].name : undefined; | ||
| rollupIndex$.current.next(newRollupIndexName); | ||
| } else { | ||
| rollupIndex$.current.next(undefined); | ||
| } | ||
|
|
||
| dataViewEditorService.matchedIndices$.next(matchedIndicesResult); | ||
| loadingMatchedIndices$.current.next(false); | ||
| } | ||
|
|
||
| return { matchedIndicesResult, newRollupIndexName }; | ||
| }; | ||
|
|
||
| return fetchIndices(newTitle); | ||
| }, | ||
| [ | ||
| allowHidden, | ||
| type, | ||
| dataViewEditorService, | ||
| rollupIndex$, | ||
| isLoadingSources, | ||
| loadingMatchedIndices$, | ||
| ] | ||
| ); | ||
|
|
||
| const onTypeChange = useCallback( | ||
| (newType) => { | ||
| form.setFieldValue('title', ''); | ||
|
|
@@ -291,7 +193,7 @@ const IndexPatternEditorFlyoutContentComponent = ({ | |
| [form] | ||
| ); | ||
|
|
||
| if (isLoadingSources || isLoadingDataViewNames) { | ||
| if (isLoadingSources || !existingDataViewNames) { | ||
| return <EuiLoadingSpinner size="xl" />; | ||
| } | ||
|
|
||
|
|
@@ -343,24 +245,26 @@ const IndexPatternEditorFlyoutContentComponent = ({ | |
| form={form} | ||
| className="indexPatternEditor__form" | ||
| error={form.getErrors()} | ||
| isInvalid={form.isSubmitted && !form.isValid} | ||
| isInvalid={form.isSubmitted && !form.isValid && form.getErrors().length} | ||
| > | ||
| <UseField path="isAdHoc" /> | ||
| {indexPatternTypeSelect} | ||
| <EuiSpacer size="l" /> | ||
| <EuiFlexGroup> | ||
| <EuiFlexItem> | ||
| <NameField existingDataViewNames$={existingDataViewNames$.current} /> | ||
| <NameField namesNotAllowed={existingDataViewNames || []} /> | ||
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
| <EuiSpacer size="l" /> | ||
| <EuiFlexGroup> | ||
| <EuiFlexItem> | ||
| <TitleField | ||
| isRollup={form.getFields().type?.value === INDEX_PATTERN_TYPE.ROLLUP} | ||
| refreshMatchedIndices={reloadMatchedIndices} | ||
| matchedIndices$={dataViewEditorService.matchedIndices$} | ||
| rollupIndicesCapabilities={rollupIndicesCapabilities} | ||
| indexPatternValidationProvider={ | ||
| dataViewEditorService.indexPatternValidationProvider | ||
| } | ||
| /> | ||
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
|
|
@@ -370,7 +274,6 @@ const IndexPatternEditorFlyoutContentComponent = ({ | |
| <TimestampField | ||
| options$={dataViewEditorService.timestampFieldOptions$} | ||
| isLoadingOptions$={dataViewEditorService.loadingTimestampFields$} | ||
| isLoadingMatchedIndices$={loadingMatchedIndices$.current} | ||
| matchedIndices$={dataViewEditorService.matchedIndices$} | ||
| /> | ||
| </EuiFlexItem> | ||
|
|
@@ -415,59 +318,3 @@ const IndexPatternEditorFlyoutContentComponent = ({ | |
| }; | ||
|
|
||
| export const IndexPatternEditorFlyoutContent = React.memo(IndexPatternEditorFlyoutContentComponent); | ||
|
|
||
| // loadMatchedIndices is called both as an side effect inside of a parent component and the inside forms validation functions | ||
| // that are challenging to synchronize without a larger refactor | ||
| // Use memoizeOne as a caching layer to avoid excessive network requests on each key type | ||
| // TODO: refactor to remove `memoize` when https://github.com/elastic/kibana/pull/109238 is done | ||
| const loadMatchedIndices = memoizeOne( | ||
| async ( | ||
| query: string, | ||
| allowHidden: boolean, | ||
| allSources: MatchedItem[], | ||
| dataViewEditorService: DataViewEditorService | ||
| ): Promise<{ | ||
| matchedIndicesResult: MatchedIndicesSet; | ||
| exactMatched: MatchedItem[]; | ||
| partialMatched: MatchedItem[]; | ||
| }> => { | ||
| const indexRequests = []; | ||
|
|
||
| if (query?.endsWith('*')) { | ||
| const exactMatchedQuery = dataViewEditorService.getIndicesCached({ | ||
| pattern: query, | ||
| showAllIndices: allowHidden, | ||
| }); | ||
| indexRequests.push(exactMatchedQuery); | ||
| // provide default value when not making a request for the partialMatchQuery | ||
| indexRequests.push(Promise.resolve([])); | ||
| } else { | ||
| const exactMatchQuery = dataViewEditorService.getIndicesCached({ | ||
| pattern: query, | ||
| showAllIndices: allowHidden, | ||
| }); | ||
| const partialMatchQuery = dataViewEditorService.getIndicesCached({ | ||
| pattern: `${query}*`, | ||
| showAllIndices: allowHidden, | ||
| }); | ||
|
|
||
| indexRequests.push(exactMatchQuery); | ||
| indexRequests.push(partialMatchQuery); | ||
| } | ||
|
|
||
| const [exactMatched, partialMatched] = (await ensureMinimumTime( | ||
| indexRequests | ||
| )) as MatchedItem[][]; | ||
|
|
||
| const matchedIndicesResult = getMatchedIndices( | ||
| allSources, | ||
| partialMatched, | ||
| exactMatched, | ||
| allowHidden | ||
| ); | ||
|
|
||
| return { matchedIndicesResult, exactMatched, partialMatched }; | ||
| }, | ||
| // compare only query and allowHidden | ||
| (newArgs, oldArgs) => newArgs[0] === oldArgs[0] && newArgs[1] === oldArgs[1] | ||
| ); | ||
Oops, something went wrong.
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.
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.
getting state