-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Dashboard] [Controls] Add unmapped runtime field support to options list #144947
[Dashboard] [Controls] Add unmapped runtime field support to options list #144947
Conversation
b9e4113
to
80f31fe
Compare
80f31fe
to
0e08316
Compare
export const getDataControlFieldRegistry = memoize( | ||
async (dataView: DataView) => { | ||
return await loadFieldRegistryFromDataViewId(dataView); | ||
}, | ||
(dataView: DataView) => [dataView.id, JSON.stringify(dataView.fields.getAll())].join('|') | ||
); |
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.
With the old "fake" cache, because it relied solely on data view IDs, adding a runtime field required a hard refresh in order to get this new field to show up in the field list for creating/editing controls. By using Lodash's memoize
instead, I can force the cache to update when the fields change, so a hard refresh is no longer required when runtime fields are added.
}); | ||
}); | ||
|
||
describe('test exists query', async () => { |
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.
Since the runtime field
tests also require adding data (but in a slightly different way), I decided to encapsulate the relevant data addition/removal in to the respective runtime field
and exists query
test suites rather than doing it all as part of the main test suite setup.
Pinging @elastic/kibana-presentation (Team:Presentation) |
0e08316
to
7c5f88c
Compare
@@ -36,20 +44,13 @@ const doubleLinkFields = (dataView: DataView) => { | |||
return fieldRegistry; | |||
}; | |||
|
|||
export const loadFieldRegistryFromDataViewId = async ( | |||
dataViewId: string | |||
const loadFieldRegistryFromDataViewId = async ( |
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.
nit - How about renaming loadFieldRegistryFromDataViewId -> loadFieldRegistryFromDataView?
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.
Oh yup, good catch! I changed it to take the data view instead of the ID so that I could get memoization to work, and I forgot to update the name to reflect this change 👍
Fixed in b1fa3e1
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.
LGTM
code review, tested in chrome
💚 Build Succeeded
Metrics [docs]Async chunks
Unknown metric groupsESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @Heenawter |
* main: (65 commits) Migrate server-side `Root` and `Server` to packages (elastic#144990) [Discover] Handle no data views state for `esQuery` alert (elastic#145052) [ML] Allow updates for number of allocations and priority for trained model deployments (elastic#144704) [api-docs] 2022-11-15 Daily api_docs build (elastic#145203) [Security solution] remove guided onboarding feature flag (elastic#144247) [DOCS] Automate final case APIs (elastic#145007) [Enterprise Search] Name and description flyout for connectors (elastic#143827) [Guided onboarding] Update header button logic (elastic#144634) [Lens] Multi metric partition charts (elastic#143966) [Dashboard] [Controls] Add unmapped runtime field support to options list (elastic#144947) [Security Solution] Add Task Metric Collection to New Tasks (elastic#145181) [TriggersActionsUi] disable jest config in CI (elastic#145186) [TableListView] Enhance tag filtering (elastic#142108) [Cloud Posture] Compliance by CIS section table (elastic#145114) [8.6][Session View] Fix hidden alert flyout in session view (elastic#145141) [customIntegrations] async load all components (elastic#145166) Fix time for logs smoke tests in integration test (elastic#145130) [RAM] Update rule status (elastic#140882) Update babel (main) (elastic#145060) [Actionable Observability] Add context.alertDetailsUrl variable to action connector template for APM rule types (elastic#144791) ...
Closes #144931
Summary
This PR adds options list support for runtime fields that are added to data views - note that runtime fields that are added as part of an index (i.e. they are part of the mapping) already worked as expected.
RuntimeFields.mov
How to Test
exists
options list functionality):Flaky Test Runner
Checklist
For maintainers