Skip to content

Commit

Permalink
Fix options action to only open when flyout is closed; fix action tes…
Browse files Browse the repository at this point in the history
…ts and add some

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
  • Loading branch information
ohltyler committed May 24, 2023
1 parent 991ceae commit bb58517
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,28 @@
import { coreMock } from '../../../../../core/public/mocks';
import { CoreStart } from 'opensearch-dashboards/public';
import { OpenEventsFlyoutAction } from './open_events_flyout_action';
import flyoutStateModule from '../flyout_state';

// Mocking the flyout state service. Defaulting to CLOSED. May override
// getFlyoutState() in below individual tests to test out different scenarios.
jest.mock('src/plugins/vis_augmenter/public/view_events_flyout/flyout_state', () => {
return {
VIEW_EVENTS_FLYOUT_STATE: {
OPEN: 'OPEN',
CLOSED: 'CLOSED',
},
getFlyoutState: () => 'CLOSED',
setFlyoutState: () => {},
};
});

let coreStart: CoreStart;
beforeEach(async () => {
coreStart = coreMock.createStart();
});
afterEach(async () => {
jest.clearAllMocks();
});

describe('OpenEventsFlyoutAction', () => {
it('is incompatible with null saved obj id', async () => {
Expand Down Expand Up @@ -43,11 +60,26 @@ describe('OpenEventsFlyoutAction', () => {
await expect(check('')).rejects.toThrow(Error);
});

it('execute calls openFlyout if compatible saved obj id', async () => {
it('execute calls openFlyout if compatible saved obj id and flyout is closed', async () => {
const getFlyoutStateSpy = jest
.spyOn(flyoutStateModule, 'getFlyoutState')
.mockImplementation(() => 'CLOSED');
const savedObjectId = 'test-id';
const action = new OpenEventsFlyoutAction(coreStart);
await action.execute({ savedObjectId });
expect(coreStart.overlays.openFlyout).toHaveBeenCalledTimes(1);
expect(getFlyoutStateSpy).toHaveBeenCalledTimes(1);
});

it('execute does not call openFlyout if compatible saved obj id and flyout is open', async () => {
const getFlyoutStateSpy = jest
.spyOn(flyoutStateModule, 'getFlyoutState')
.mockImplementation(() => 'OPEN');
const savedObjectId = 'test-id';
const action = new OpenEventsFlyoutAction(coreStart);
await action.execute({ savedObjectId });
expect(coreStart.overlays.openFlyout).toHaveBeenCalledTimes(0);
expect(getFlyoutStateSpy).toHaveBeenCalledTimes(1);
});

it('Returns display name', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,29 @@ import { coreMock } from '../../../../../core/public/mocks';
import { CoreStart } from 'opensearch-dashboards/public';
import { ViewEventsOptionAction } from './view_events_option_action';
import { createMockErrorEmbeddable, createMockVisEmbeddable } from '../../mocks';
import flyoutStateModule from '../flyout_state';

// Mocking the flyout state service. Defaulting to CLOSED. May override
// getFlyoutState() in below individual tests to test out different scenarios.
jest.mock('src/plugins/vis_augmenter/public/view_events_flyout/flyout_state', () => {
return {
VIEW_EVENTS_FLYOUT_STATE: {
OPEN: 'OPEN',
CLOSED: 'CLOSED',
},
getFlyoutState: () => 'CLOSED',
setFlyoutState: () => {},
};
});

let coreStart: CoreStart;

beforeEach(async () => {
coreStart = coreMock.createStart();
});
afterEach(async () => {
jest.clearAllMocks();
});

describe('ViewEventsOptionAction', () => {
it('is incompatible with ErrorEmbeddables', async () => {
Expand Down Expand Up @@ -41,11 +59,26 @@ describe('ViewEventsOptionAction', () => {
await expect(check()).rejects.toThrow(Error);
});

it('execute calls openFlyout if compatible embeddable', async () => {
it('execute calls openFlyout if compatible embeddable and flyout is currently closed', async () => {
const getFlyoutStateSpy = jest
.spyOn(flyoutStateModule, 'getFlyoutState')
.mockImplementation(() => 'CLOSED');
const visEmbeddable = createMockVisEmbeddable('test-saved-obj-id', 'test-title');
const action = new ViewEventsOptionAction(coreStart);
await action.execute({ embeddable: visEmbeddable });
expect(coreStart.overlays.openFlyout).toHaveBeenCalledTimes(1);
expect(getFlyoutStateSpy).toHaveBeenCalledTimes(1);
});

it('execute does not call openFlyout if compatible embeddable and flyout is currently open', async () => {
const getFlyoutStateSpy = jest
.spyOn(flyoutStateModule, 'getFlyoutState')
.mockImplementation(() => 'OPEN');
const visEmbeddable = createMockVisEmbeddable('test-saved-obj-id', 'test-title');
const action = new ViewEventsOptionAction(coreStart);
await action.execute({ embeddable: visEmbeddable });
expect(coreStart.overlays.openFlyout).toHaveBeenCalledTimes(0);
expect(getFlyoutStateSpy).toHaveBeenCalledTimes(1);
});

it('Returns display name', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { EmbeddableContext } from '../../../../embeddable/public';
import { Action, IncompatibleActionError } from '../../../../ui_actions/public';
import { openViewEventsFlyout } from './open_events_flyout';
import { isEligibleForVisLayers } from '../../utils';
import { VIEW_EVENTS_FLYOUT_STATE, getFlyoutState } from '../flyout_state';

export const VIEW_EVENTS_OPTION_ACTION = 'VIEW_EVENTS_OPTION_ACTION';

Expand Down Expand Up @@ -45,9 +46,15 @@ export class ViewEventsOptionAction implements Action<EmbeddableContext> {
const visEmbeddable = embeddable as VisualizeEmbeddable;
const savedObjectId = get(visEmbeddable.getInput(), 'savedObjectId', '');

openViewEventsFlyout({
core: this.core,
savedObjectId,
});
// This action may get triggered even when the flyout is already open (e.g.,
// clicking on an annotation point within a chart displayed in the flyout).
// In such case, we want to ignore it such that users can't keep endlessly
// re-opening it.
if (getFlyoutState() === VIEW_EVENTS_FLYOUT_STATE.CLOSED) {
openViewEventsFlyout({
core: this.core,
savedObjectId,
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@ export enum VIEW_EVENTS_FLYOUT_STATE {
export const [getFlyoutState, setFlyoutState] = createGetterSetter<
keyof typeof VIEW_EVENTS_FLYOUT_STATE
>(VIEW_EVENTS_FLYOUT_STATE.CLOSED);

// This is primarily used for mocking this module and each of its fns in tests.
// eslint-disable-next-line import/no-default-export
export default { VIEW_EVENTS_FLYOUT_STATE, getFlyoutState, setFlyoutState };

0 comments on commit bb58517

Please sign in to comment.