[Expressions] Refactor expression functions to use observables underneath #100409
Conversation
|
Pinging @elastic/kibana-app-services (Team:AppServices) |
src/plugins/expressions/common/expression_functions/specs/map_column.ts
Outdated
Show resolved
Hide resolved
| columns: [...input.columns], | ||
| rows: [row], | ||
| }) ?? of(null) | ||
| ).pipe(map((value) => ({ ...row, [id]: value }))) |
There was a problem hiding this comment.
i think we don't need the pipe here
There was a problem hiding this comment.
We do need that because the expression returns a value that we need to put into the row. The expression is an observable, so we have to use the pipe here to process the value upon subscription.
| }); | ||
| return defer(() => { | ||
| const rows$ = input.rows.length | ||
| ? zip( |
There was a problem hiding this comment.
zip will wait for all rows to emit the first value .... should we consider emitting rows as they come ?
There was a problem hiding this comment.
I guess that behavior will be backward incompatible and may not work as expected without adapting the code in other plugins.
| const metaSourceFrom = columns.find(({ id }) => id === args.copyMetaFrom); | ||
| newColumn.meta = { ...newColumn.meta, ...(metaSourceFrom?.meta || {}) }; | ||
| } | ||
| return rows$.pipe<Datatable>( |
There was a problem hiding this comment.
That's here because we need to process rows altogether and put them into a datatable.
| return { type: 'case', matches, result }; | ||
| fn(input, { if: condition, then, when }) { | ||
| return defer(() => { | ||
| const matches = condition ?? when?.().pipe(map((value) => value === input)) ?? true; |
There was a problem hiding this comment.
when returns an observable. That's the cleanest way to convert the value.
x-pack/plugins/canvas/canvas_plugin_src/functions/common/case.ts
Outdated
Show resolved
Hide resolved
| return args.else?.().pipe(take(1)).toPromise() ?? input; | ||
| } | ||
| fn(input, args) { | ||
| return defer(() => (args.condition ? args.then?.() : args.else?.()) ?? of(input)); |
There was a problem hiding this comment.
Corrected the argument type, but the code remains the same because then and else are optional.
| byColumns = args.by.map((by) => { | ||
| const column = input.columns.find((col) => col.name === by); | ||
| const byColumns = | ||
| args.by?.map((by) => { |
There was a problem hiding this comment.
by will always exist (will be empty array if not set)
There was a problem hiding this comment.
I've checked that, and optional multi-valued arguments are actually undefined.
dc5e04c to
394fe55
Compare
💚 Build SucceededMetrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
* master: (68 commits) skip flaky suite (elastic#94043) skip flaky suite (elastic#102012) [esArchive] Persists updates for management/saved_objects/* (elastic#101992) skip flaky suite (elastic#101449) remove unnecessary hack (elastic#101909) [Exploratory View] Use human readable formats (elastic#101520) [Expressions] Refactor expression functions to use observables underneath (elastic#100409) [esArchives] Persist migrated Kibana archives (elastic#101950) [kbnArchiver] fix save to non-existent file (elastic#101974) [Enterprise Search] Add owner and description properties to kibana.json (elastic#101957) [DOCS] Fixes terminology in Stack Monitoring:Kibana alerts (elastic#101696) [Observability] [Cases] Cases in the observability app (elastic#101487) [Alerting][Docs] Combine rule creation and management pages (elastic#101498) temporarily disable build-buddy [Fleet] Fix fleet server collector in case settings are not set (elastic#101752) [Event Log] Populated rule.* ECS fields for alert events. (elastic#101132) [APM] Fleet support for merging input.config values with other nested properties in the policy input (elastic#101690) Add comments to some alerting plugin public API items (elastic#101551) [Alerting][Docs] Moving alerting setup to its own page (elastic#101323) remove uptime public API, it's not used. (elastic#101799) ...
Summary
Refactor expression functions to use observables underneath.
Resolves #93016.
Checklist
For maintainers