Skip to content

Commit

Permalink
[Controls] Move diffing from the dashboard to the control group (#175146
Browse files Browse the repository at this point in the history
)

Closes #174703

## Summary

Currently, the burden of determining unsaved changes is handled entirely
by the Dashboard - this means that we need special logic for comparing
the state of every piece of a dashboard, including the controls and the
panels. Once the [embeddable
refactor](#167429) work is
complete, this will no longer be the case; instead, each component will
be responsible for handling its **own** unsaved changes, and the
Dashboard will simply listen for updates.

As an intermediate step, this PR separates out the control group logic
from the Dashboard plugin so that, when it comes time, the transition to
the new embeddable framework will be much smoother. This PR also
unblocks #170396 since, now that
the control group is responsible for handling its own unsaved changes, I
should be able to determine when to enable/disable the "reset changes"
button.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
Heenawter and kibanamachine authored Jan 30, 2024
1 parent 38f0a7a commit 4bb5fcc
Show file tree
Hide file tree
Showing 28 changed files with 418 additions and 222 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ import {
genericControlPanelDiffSystem,
} from './control_group_panel_diff_system';
import { ControlGroupInput } from '..';
import { ControlsPanels, PersistableControlGroupInput, RawControlGroupAttributes } from './types';
import {
ControlsPanels,
PersistableControlGroupInput,
persistableControlGroupInputKeys,
RawControlGroupAttributes,
} from './types';

const safeJSONParse = <OutType>(jsonString?: string): OutType | undefined => {
if (!jsonString && typeof jsonString !== 'string') return;
Expand All @@ -47,6 +52,9 @@ export const getDefaultControlGroupInput = (): Omit<ControlGroupInput, 'id'> =>
},
});

export const getDefaultControlGroupPersistableInput = (): PersistableControlGroupInput =>
pick(getDefaultControlGroupInput(), persistableControlGroupInputKeys);

export const persistableControlGroupInputIsEqual = (
a: PersistableControlGroupInput | undefined,
b: PersistableControlGroupInput | undefined
Expand Down
8 changes: 7 additions & 1 deletion src/plugins/controls/common/control_group/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,15 @@ export interface ControlGroupInput extends EmbeddableInput, ControlInput {
/**
* Only parts of the Control Group Input should be persisted
*/
export const persistableControlGroupInputKeys: Array<
keyof Pick<
ControlGroupInput,
'panels' | 'chainingSystem' | 'controlStyle' | 'ignoreParentSettings'
>
> = ['panels', 'chainingSystem', 'controlStyle', 'ignoreParentSettings'];
export type PersistableControlGroupInput = Pick<
ControlGroupInput,
'panels' | 'chainingSystem' | 'controlStyle' | 'ignoreParentSettings'
typeof persistableControlGroupInputKeys[number]
>;

/**
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/controls/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ export {
type RawControlGroupAttributes,
type PersistableControlGroupInput,
type SerializableControlGroupInput,
persistableControlGroupInputKeys,
} from './control_group/types';
export {
controlGroupInputToRawControlGroupAttributes,
rawControlGroupAttributesToControlGroupInput,
rawControlGroupAttributesToSerializable,
serializableToRawControlGroupAttributes,
getDefaultControlGroupPersistableInput,
persistableControlGroupInputIsEqual,
getDefaultControlGroupInput,
generateNewControlIds,
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/controls/public/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@
*/

export const MIN_POPOVER_WIDTH = 300;

export const CHANGE_CHECK_DEBOUNCE = 100;
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/
import { compareFilters, COMPARE_ALL_OPTIONS, Filter, uniqFilters } from '@kbn/es-query';
import { isEqual } from 'lodash';
import { isEqual, pick } from 'lodash';
import React, { createContext, useContext } from 'react';
import ReactDOM from 'react-dom';
import { Provider, TypedUseSelectorHook, useSelector } from 'react-redux';
Expand All @@ -18,6 +18,11 @@ import { Container, EmbeddableFactory } from '@kbn/embeddable-plugin/public';
import { ReduxEmbeddableTools, ReduxToolsPackage } from '@kbn/presentation-util-plugin/public';
import { KibanaThemeProvider } from '@kbn/react-kibana-context-theme';

import {
PersistableControlGroupInput,
persistableControlGroupInputIsEqual,
persistableControlGroupInputKeys,
} from '../../../common';
import { pluginServices } from '../../services';
import { ControlEmbeddable, ControlInput, ControlOutput } from '../../types';
import { ControlGroup } from '../component/control_group_component';
Expand All @@ -32,12 +37,13 @@ import {
type AddOptionsListControlProps,
type AddRangeSliderControlProps,
} from '../external_api/control_group_input_builder';
import { startDiffingControlGroupState } from '../state/control_group_diffing_integration';
import { controlGroupReducers } from '../state/control_group_reducers';
import {
ControlGroupComponentState,
ControlGroupInput,
ControlGroupOutput,
ControlGroupReduxState,
ControlGroupSettings,
ControlPanelState,
ControlsPanels,
CONTROL_GROUP_TYPE,
Expand Down Expand Up @@ -85,6 +91,7 @@ export class ControlGroupContainer extends Container<
private recalculateFilters$: Subject<null>;
private relevantDataViewId?: string;
private lastUsedDataViewId?: string;
public diffingSubscription: Subscription = new Subscription();

// state management
public select: ControlGroupReduxEmbeddableTools['select'];
Expand All @@ -99,13 +106,16 @@ export class ControlGroupContainer extends Container<
public onFiltersPublished$: Subject<Filter[]>;
public onControlRemoved$: Subject<string>;

/** This currently reports the **entire** persistable control group input on unsaved changes */
public unsavedChanges: BehaviorSubject<PersistableControlGroupInput | undefined>;

public fieldFilterPredicate: FieldFilterPredicate | undefined;

constructor(
reduxToolsPackage: ReduxToolsPackage,
initialInput: ControlGroupInput,
parent?: Container,
settings?: ControlGroupSettings,
initialComponentState?: ControlGroupComponentState,
fieldFilterPredicate?: FieldFilterPredicate
) {
super(
Expand All @@ -120,14 +130,19 @@ export class ControlGroupContainer extends Container<
this.onFiltersPublished$ = new Subject<Filter[]>();
this.onControlRemoved$ = new Subject<string>();

// start diffing control group state
this.unsavedChanges = new BehaviorSubject<PersistableControlGroupInput | undefined>(undefined);
const diffingMiddleware = startDiffingControlGroupState.bind(this)();

// build redux embeddable tools
const reduxEmbeddableTools = reduxToolsPackage.createReduxEmbeddableTools<
ControlGroupReduxState,
typeof controlGroupReducers
>({
embeddable: this,
reducers: controlGroupReducers,
initialComponentState: settings,
additionalMiddleware: [diffingMiddleware],
initialComponentState,
});

this.select = reduxEmbeddableTools.select;
Expand Down Expand Up @@ -190,6 +205,20 @@ export class ControlGroupContainer extends Container<
);
};

public resetToLastSavedState() {
const {
componentState: { lastSavedInput },
} = this.getState();
if (!persistableControlGroupInputIsEqual(this.getPersistableInput(), lastSavedInput)) {
this.updateInput(lastSavedInput);
}
}

public getPersistableInput: () => PersistableControlGroupInput & { id: string } = () => {
const input = this.getInput();
return pick(input, [...persistableControlGroupInputKeys, 'id']);
};

public updateInputAndReinitialize = (newInput: Partial<ControlGroupInput>) => {
this.subscriptions.unsubscribe();
this.subscriptions = new Subscription();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import { lazyLoadReduxToolsPackage } from '@kbn/presentation-util-plugin/public'
import { EmbeddablePersistableStateService } from '@kbn/embeddable-plugin/common';

import {
ControlGroupComponentState,
ControlGroupInput,
ControlGroupSettings,
CONTROL_GROUP_TYPE,
FieldFilterPredicate,
} from '../types';
Expand Down Expand Up @@ -57,7 +57,7 @@ export class ControlGroupContainerFactory implements EmbeddableFactoryDefinition
public create = async (
initialInput: ControlGroupInput,
parent?: Container,
settings?: ControlGroupSettings,
initialComponentState?: ControlGroupComponentState,
fieldFilterPredicate?: FieldFilterPredicate
) => {
const reduxEmbeddablePackage = await lazyLoadReduxToolsPackage();
Expand All @@ -66,7 +66,7 @@ export class ControlGroupContainerFactory implements EmbeddableFactoryDefinition
reduxEmbeddablePackage,
initialInput,
parent,
settings,
initialComponentState,
fieldFilterPredicate
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('control group renderer', () => {
expect(mockControlGroupFactory.create).toHaveBeenCalledWith(
expect.objectContaining({ controlStyle: 'twoLine' }),
undefined,
undefined,
{ lastSavedInput: expect.objectContaining({ controlStyle: 'twoLine' }) },
undefined
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

import { isEqual, pick } from 'lodash';
import React, {
forwardRef,
useEffect,
Expand All @@ -14,28 +15,31 @@ import React, {
useRef,
useState,
} from 'react';
import { isEqual } from 'lodash';
import { v4 as uuidv4 } from 'uuid';

import { compareFilters } from '@kbn/es-query';
import type { Filter, TimeRange, Query } from '@kbn/es-query';
import { EmbeddableFactory } from '@kbn/embeddable-plugin/public';
import type { Filter, Query, TimeRange } from '@kbn/es-query';
import { compareFilters } from '@kbn/es-query';

import {
getDefaultControlGroupInput,
getDefaultControlGroupPersistableInput,
persistableControlGroupInputKeys,
} from '../../../common';
import { ControlGroupContainer } from '../embeddable/control_group_container';
import { ControlGroupContainerFactory } from '../embeddable/control_group_container_factory';
import {
ControlGroupCreationOptions,
ControlGroupInput,
CONTROL_GROUP_TYPE,
ControlGroupOutput,
ControlGroupCreationOptions,
CONTROL_GROUP_TYPE,
} from '../types';
import {
ControlGroupAPI,
AwaitingControlGroupAPI,
buildApiFromControlGroupContainer,
ControlGroupAPI,
} from './control_group_api';
import { getDefaultControlGroupInput } from '../../../common';
import { controlGroupInputBuilder, ControlGroupInputBuilder } from './control_group_input_builder';
import { ControlGroupContainer } from '../embeddable/control_group_container';
import { ControlGroupContainerFactory } from '../embeddable/control_group_container_factory';

export interface ControlGroupRendererProps {
filters?: Filter[];
Expand Down Expand Up @@ -87,7 +91,13 @@ export const ControlGroupRenderer = forwardRef<AwaitingControlGroupAPI, ControlG
...initialInput,
},
undefined,
settings,
{
...settings,
lastSavedInput: {
...getDefaultControlGroupPersistableInput(),
...pick(initialInput, persistableControlGroupInputKeys),
},
},
fieldFilterPredicate
)) as ControlGroupContainer;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* 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 { AnyAction, Middleware } from 'redux';
import { debounceTime, Observable, startWith, Subject, switchMap } from 'rxjs';

import { ControlGroupContainer } from '..';
import { persistableControlGroupInputIsEqual } from '../../../common';
import { CHANGE_CHECK_DEBOUNCE } from '../../constants';
import { controlGroupReducers } from './control_group_reducers';

/**
* An array of reducers which cannot cause unsaved changes. Unsaved changes only compares the explicit input
* and the last saved input, so we can safely ignore any output reducers, and most componentState reducers.
* This is only for performance reasons, because the diffing function itself can be quite heavy.
*/
export const reducersToIgnore: Array<keyof typeof controlGroupReducers> = [
'setDefaultControlWidth',
'setDefaultControlGrow',
];

/**
* Does an initial diff between @param initialInput and @param initialLastSavedInput, and created a middleware
* which listens to the redux store and checks for & publishes the unsaved changes on dispatches.
*/
export function startDiffingControlGroupState(this: ControlGroupContainer) {
const checkForUnsavedChangesSubject$ = new Subject<null>();
this.diffingSubscription.add(
checkForUnsavedChangesSubject$
.pipe(
startWith(null),
debounceTime(CHANGE_CHECK_DEBOUNCE),
switchMap(() => {
return new Observable((observer) => {
if (observer.closed) return;

const {
explicitInput: currentInput,
componentState: { lastSavedInput },
} = this.getState();
const hasUnsavedChanges = !persistableControlGroupInputIsEqual(
currentInput,
lastSavedInput
);
this.unsavedChanges.next(hasUnsavedChanges ? this.getPersistableInput() : undefined);
});
})
)
.subscribe()
);
const diffingMiddleware: Middleware<AnyAction> = (store) => (next) => (action) => {
const dispatchedActionName = action.type.split('/')?.[1];
if (
dispatchedActionName &&
dispatchedActionName !== 'updateEmbeddableReduxOutput' && // ignore any generic output updates.
!reducersToIgnore.includes(dispatchedActionName)
) {
checkForUnsavedChangesSubject$.next(null);
}
next(action);
};
return diffingMiddleware;
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,15 @@ import { PayloadAction } from '@reduxjs/toolkit';
import { WritableDraft } from 'immer/dist/types/types-external';

import { ControlWidth } from '../../types';
import { ControlGroupInput, ControlGroupReduxState } from '../types';
import { ControlGroupComponentState, ControlGroupInput, ControlGroupReduxState } from '../types';

export const controlGroupReducers = {
setLastSavedInput: (
state: WritableDraft<ControlGroupReduxState>,
action: PayloadAction<ControlGroupComponentState['lastSavedInput']>
) => {
state.componentState.lastSavedInput = action.payload;
},
setControlStyle: (
state: WritableDraft<ControlGroupReduxState>,
action: PayloadAction<ControlGroupInput['controlStyle']>
Expand Down
12 changes: 8 additions & 4 deletions src/plugins/controls/public/control_group/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { DataViewField } from '@kbn/data-views-plugin/common';
import { ContainerOutput } from '@kbn/embeddable-plugin/public';
import { ReduxEmbeddableState } from '@kbn/presentation-util-plugin/public';
import { ControlGroupInput } from '../../common/control_group/types';
import { ControlGroupInput, PersistableControlGroupInput } from '../../common/control_group/types';
import { CommonControlOutput } from '../types';

export type ControlGroupOutput = ContainerOutput &
Expand All @@ -19,7 +19,7 @@ export type ControlGroupOutput = ContainerOutput &
export type ControlGroupReduxState = ReduxEmbeddableState<
ControlGroupInput,
ControlGroupOutput,
ControlGroupSettings
ControlGroupComponentState
>;

export type FieldFilterPredicate = (f: DataViewField) => boolean;
Expand All @@ -40,9 +40,13 @@ export interface ControlGroupSettings {
};
}

export type ControlGroupComponentState = ControlGroupSettings & {
lastSavedInput: PersistableControlGroupInput;
};

export {
type ControlsPanels,
CONTROL_GROUP_TYPE,
type ControlGroupInput,
type ControlPanelState,
CONTROL_GROUP_TYPE,
type ControlsPanels,
} from '../../common/control_group/types';
Loading

0 comments on commit 4bb5fcc

Please sign in to comment.