Skip to content

Commit

Permalink
[Lens][Embeddable] Apply the correct references for filters (#204047)
Browse files Browse the repository at this point in the history
## Summary

Fixes #180726

Fix the filter references when inline editing with the correct ones.

I've tried to reduce the fix to a minimal `extract` wrapper, but
unfortunately that is not possible due to some shared logic who rely on
the passed filters references and need to be injected.
So, why not injecting them and instead rely on the search context api?
Right now there's no difference, but formally the `api.filters$` is the
right place to get the latest version, and if in the future the `Edit
filters` flows would change, this api should be the go-to place to have
the right value.

Why not adding a FTR?
There's a bigger problem with the panel filters action who has a dynamic
`data-test-subj` value which is impossible to get right now. I would
rather prefer to fix that first and then add some tests in general for
multiple scenarios in Lens.

## Testing it locally

* Create a viz with a filter in the editor, save and return to dashboard
* Check the filters are shown correctly in the dashboard panel
* Edit inline and change the chart type. Apply changes
* Check the filters are shown correctly
* Now "edit" in the editor without changing anything
* Check the filter can be edited correctly (no empty popover) ✅  or 💥 
* Save and return to dashboard
* Check the filters are shown correctly ✅  or 💥
  • Loading branch information
dej611 authored Dec 19, 2024
1 parent 7678141 commit c521c1c
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { setupPanelManagement } from '../inline_editing/panel_management';
import { mountInlineEditPanel } from '../inline_editing/mount';
import { StateManagementConfig } from './initialize_state_management';
import { apiPublishesInlineEditingCapabilities } from '../type_guards';
import { SearchContextConfig } from './initialize_search_context';

function getSupportedTriggers(
getState: GetStateType,
Expand All @@ -61,6 +62,7 @@ export function initializeEditApi(
internalApi: LensInternalApi,
stateApi: StateManagementConfig['api'],
inspectorApi: LensInspectorAdapters,
searchContextApi: SearchContextConfig['api'],
isTextBasedLanguage: (currentState: LensRuntimeState) => boolean,
startDependencies: LensEmbeddableStartServices,
parentApi?: unknown
Expand Down Expand Up @@ -126,9 +128,34 @@ export function initializeEditApi(
stateApi.updateSavedObjectId(newState.savedObjectId);
};

// Wrap the getState() when inline editing and make sure that the filters in the attributes
// are properly injected with the correct references to avoid issues when saving/navigating to the full editor
const getStateWithInjectedFilters = () => {
const currentState = getState();
// use the search context api here for filters for 2 reasons:
// * the filters here have the correct references already injected
// * the edit filters flow may change in the future and this is the right place to get the filters
const currentFilters = searchContextApi.filters$.getValue() ?? [];
// if there are no filters, avoid to copy the attributes
if (!currentFilters.length) {
return currentState;
}
// otherwise make sure to inject the references into filters
return {
...currentState,
attributes: {
...currentState.attributes,
state: {
...currentState.attributes.state,
filters: currentFilters,
},
},
};
};

const openInlineEditor = prepareInlineEditPanel(
initialState,
getState,
getStateWithInjectedFilters,
updateState,
internalApi,
panelManagementApi,
Expand Down Expand Up @@ -205,6 +232,9 @@ export function initializeEditApi(
const rootEmbeddable = parentApi;
const overlayTracker = tracksOverlays(rootEmbeddable) ? rootEmbeddable : undefined;
const ConfigPanel = await openInlineEditor({
// the getState() here contains the wrong filters references
// but the input attributes are correct as openInlineEditor() handler is using
// the getStateWithInjectedFilters() function
onApply: (attributes: LensRuntimeState['attributes']) =>
updateState({ ...getState(), attributes }),
// restore the first state found when the panel opened
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,26 @@ import {
apiPublishesSearchSession,
} from '@kbn/presentation-publishing/interfaces/fetch/publishes_search_session';
import { buildObservableVariable } from '../helper';
import { LensInternalApi, LensRuntimeState, LensUnifiedSearchContext } from '../types';
import {
LensEmbeddableStartServices,
LensInternalApi,
LensRuntimeState,
LensUnifiedSearchContext,
} from '../types';

export function initializeSearchContext(
initialState: LensRuntimeState,
internalApi: LensInternalApi,
parentApi: unknown
): {
export interface SearchContextConfig {
api: PublishesUnifiedSearch & PublishesSearchSession;
comparators: StateComparators<LensUnifiedSearchContext>;
serialize: () => LensUnifiedSearchContext;
cleanup: () => void;
} {
}

export function initializeSearchContext(
initialState: LensRuntimeState,
internalApi: LensInternalApi,
parentApi: unknown,
{ injectFilterReferences }: LensEmbeddableStartServices
): SearchContextConfig {
const [searchSessionId$] = buildObservableVariable<string | undefined>(
apiPublishesSearchSession(parentApi) ? parentApi.searchSessionId$ : undefined
);
Expand All @@ -38,7 +46,10 @@ export function initializeSearchContext(

const [lastReloadRequestTime] = buildObservableVariable<number | undefined>(undefined);

const [filters$] = buildObservableVariable<Filter[] | undefined>(attributes.state.filters);
// Make sure the panel access the filters with the correct references
const [filters$] = buildObservableVariable<Filter[] | undefined>(
injectFilterReferences(attributes.state.filters, attributes.references)
);

const [query$] = buildObservableVariable<Query | AggregateQuery | undefined>(
attributes.state.query
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,26 @@ export const createLensEmbeddableFactory = (

const inspectorConfig = initializeInspector(services);

const searchContextConfig = initializeSearchContext(
initialState,
internalApi,
parentApi,
services
);

const editConfig = initializeEditApi(
uuid,
initialState,
getState,
internalApi,
stateConfig.api,
inspectorConfig.api,
searchContextConfig.api,
isTextBasedLanguage,
services,
parentApi
);

const searchContextConfig = initializeSearchContext(initialState, internalApi, parentApi);
const integrationsConfig = initializeIntegrations(getState, services);
const actionsConfig = initializeActionApi(
uuid,
Expand Down

0 comments on commit c521c1c

Please sign in to comment.