Skip to content

Commit

Permalink
[embeddable] centralize should fetch embeddable observable logic (ela…
Browse files Browse the repository at this point in the history
…stic#151799)

Fixes elastic#151223 and
elastic#151128

PR does the following
1) creates `shouldFetch$` method that centralizes logic for checking
when an embeddable should fetch.
2) updates Lens and Maps embeddable to use `shouldFetch$`
3) Adds unit tests for Maps embeddable to capture behavior of unique
edge cases

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
2 people authored and bmorelli25 committed Mar 10, 2023
1 parent bb2e202 commit fa7c682
Show file tree
Hide file tree
Showing 9 changed files with 381 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
isFilterPinned,
onlyDisabledFiltersChanged,
} from '@kbn/es-query';
import { shouldRefreshFilterCompareOptions } from '@kbn/embeddable-plugin/public';

import { DashboardContainer } from '../../dashboard_container';
import { DashboardContainerByValueInput } from '../../../../../common';
Expand Down Expand Up @@ -117,12 +118,6 @@ export const unsavedChangesDiffingFunctions: DashboardDiffFunctions = {
viewMode: () => false, // When compared view mode is always considered unequal so that it gets backed up.
};

const shouldRefreshFilterCompareOptions = {
...COMPARE_ALL_OPTIONS,
// do not compare $state to avoid refreshing when filter is pinned/unpinned (which does not impact results)
state: false,
};

export const shouldRefreshDiffingFunctions: DashboardDiffFunctions = {
...unsavedChangesDiffingFunctions,
filters: ({ currentValue, lastValue }) =>
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/embeddable/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ export {
EmbeddableRenderer,
useEmbeddableFactory,
isFilterableEmbeddable,
shouldFetch$,
shouldRefreshFilterCompareOptions,
} from './lib';

export { AttributeService, ATTRIBUTE_SERVICE_KEY } from './lib/attribute_service';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@

export type { FilterableEmbeddable } from './types';
export { isFilterableEmbeddable } from './types';
export { shouldFetch$, shouldRefreshFilterCompareOptions } from './should_fetch';
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import fastIsEqual from 'fast-deep-equal';
import { Observable } from 'rxjs';
import { map, distinctUntilChanged, skip, startWith } from 'rxjs/operators';
import { COMPARE_ALL_OPTIONS, onlyDisabledFiltersChanged } from '@kbn/es-query';
import type { FilterableEmbeddableInput } from './types';

export const shouldRefreshFilterCompareOptions = {
...COMPARE_ALL_OPTIONS,
// do not compare $state to avoid refreshing when filter is pinned/unpinned (which does not impact results)
state: false,
};

export function shouldFetch$<
TFilterableEmbeddableInput extends FilterableEmbeddableInput = FilterableEmbeddableInput
>(
updated$: Observable<unknown>,
getInput: () => TFilterableEmbeddableInput
): Observable<TFilterableEmbeddableInput> {
return updated$.pipe(map(() => getInput())).pipe(
// wrapping distinctUntilChanged with startWith and skip to prime distinctUntilChanged with an initial input value.
startWith(getInput()),
distinctUntilChanged((a: TFilterableEmbeddableInput, b: TFilterableEmbeddableInput) => {
if (
!fastIsEqual(
[a.searchSessionId, a.query, a.timeRange, a.timeslice],
[b.searchSessionId, b.query, b.timeRange, b.timeslice]
)
) {
return false;
}

return onlyDisabledFiltersChanged(a.filters, b.filters, shouldRefreshFilterCompareOptions);
}),
skip(1)
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,15 @@
* Side Public License, v 1.
*/

import { type AggregateQuery, type Filter, type Query } from '@kbn/es-query';
import type { AggregateQuery, Filter, Query, TimeRange } from '@kbn/es-query';
import { EmbeddableInput } from '../embeddables';

export type FilterableEmbeddableInput = EmbeddableInput & {
filters?: Filter[];
query?: Query;
timeRange?: TimeRange;
timeslice?: [number, number];
};

/**
* All embeddables that implement this interface should support being filtered
Expand Down
18 changes: 5 additions & 13 deletions x-pack/plugins/lens/public/embeddable/embeddable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import {
cellValueTrigger,
CELL_VALUE_TRIGGER,
type CellValueContext,
shouldFetch$,
} from '@kbn/embeddable-plugin/public';
import type { Action, UiActionsStart } from '@kbn/ui-actions-plugin/public';
import type { DataViewsContract, DataView } from '@kbn/data-views-plugin/public';
Expand Down Expand Up @@ -501,20 +502,11 @@ export class Embeddable

// Update search context and reload on changes related to search
this.inputReloadSubscriptions.push(
this.getUpdated$()
.pipe(map(() => this.getInput()))
.pipe(
distinctUntilChanged((a, b) =>
fastIsEqual(
[a.filters, a.query, a.timeRange, a.searchSessionId],
[b.filters, b.query, b.timeRange, b.searchSessionId]
)
),
skip(1)
)
.subscribe(async (input) => {
shouldFetch$<LensEmbeddableInput>(this.getUpdated$(), () => this.getInput()).subscribe(
(input) => {
this.onContainerStateChanged(input);
})
}
)
);
}

Expand Down
Loading

0 comments on commit fa7c682

Please sign in to comment.