From a8af79ddff79c369539f982d918cc17d63ea6e0a Mon Sep 17 00:00:00 2001 From: Corey Robertson Date: Fri, 10 Jan 2020 11:41:05 -0500 Subject: [PATCH] [7.5] [Canvas] Fixes bugs with autoplay and refresh (#53149) (#54302) * [Canvas] Fixes bugs with autoplay and refresh (#53149) * Fixes bugs with autoplay and refresh * Fix typecheck Co-authored-by: Elastic Machine * Fix incorrect mocking Co-authored-by: Elastic Machine --- .../plugins/canvas/public/lib/app_state.ts | 2 +- .../__tests__/workpad_autoplay.test.ts | 145 ++++++++++++++++ .../__tests__/workpad_refresh.test.ts | 163 ++++++++++++++++++ ...orkpad_autoplay.js => workpad_autoplay.ts} | 33 ++-- ...{workpad_refresh.js => workpad_refresh.ts} | 36 +++- 5 files changed, 353 insertions(+), 26 deletions(-) create mode 100644 x-pack/legacy/plugins/canvas/public/state/middleware/__tests__/workpad_autoplay.test.ts create mode 100644 x-pack/legacy/plugins/canvas/public/state/middleware/__tests__/workpad_refresh.test.ts rename x-pack/legacy/plugins/canvas/public/state/middleware/{workpad_autoplay.js => workpad_autoplay.ts} (77%) rename x-pack/legacy/plugins/canvas/public/state/middleware/{workpad_refresh.js => workpad_refresh.ts} (65%) diff --git a/x-pack/legacy/plugins/canvas/public/lib/app_state.ts b/x-pack/legacy/plugins/canvas/public/lib/app_state.ts index be0f76b170c70a..955125b7131400 100644 --- a/x-pack/legacy/plugins/canvas/public/lib/app_state.ts +++ b/x-pack/legacy/plugins/canvas/public/lib/app_state.ts @@ -92,7 +92,7 @@ export function setFullscreen(payload: boolean) { } } -export function setAutoplayInterval(payload: string) { +export function setAutoplayInterval(payload: string | null) { const appState = getAppState(); const appValue = appState[AppStateKeys.AUTOPLAY_INTERVAL]; diff --git a/x-pack/legacy/plugins/canvas/public/state/middleware/__tests__/workpad_autoplay.test.ts b/x-pack/legacy/plugins/canvas/public/state/middleware/__tests__/workpad_autoplay.test.ts new file mode 100644 index 00000000000000..11ebdcdc51d4d9 --- /dev/null +++ b/x-pack/legacy/plugins/canvas/public/state/middleware/__tests__/workpad_autoplay.test.ts @@ -0,0 +1,145 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +jest.mock('../../../lib/app_state'); +jest.mock('../../../lib/router_provider'); + +import { workpadAutoplay } from '../workpad_autoplay'; +import { setAutoplayInterval } from '../../../lib/app_state'; +import { createTimeInterval } from '../../../lib/time_interval'; +// @ts-ignore Untyped local +import { routerProvider } from '../../../lib/router_provider'; + +const next = jest.fn(); +const dispatch = jest.fn(); +const getState = jest.fn(); +const routerMock = { navigateTo: jest.fn() }; +routerProvider.mockReturnValue(routerMock); + +const middleware = workpadAutoplay({ dispatch, getState })(next); + +const workpadState = { + persistent: { + workpad: { + id: 'workpad-id', + pages: ['page1', 'page2', 'page3'], + page: 0, + }, + }, +}; + +const autoplayState = { + ...workpadState, + transient: { + autoplay: { + inFlight: false, + enabled: true, + interval: 5000, + }, + fullscreen: true, + }, +}; + +const autoplayDisabledState = { + ...workpadState, + transient: { + autoplay: { + inFlight: false, + enabled: false, + interval: 5000, + }, + }, +}; + +const action = {}; + +describe('workpad autoplay middleware', () => { + beforeEach(() => { + dispatch.mockClear(); + jest.resetAllMocks(); + }); + + describe('app state', () => { + it('sets the app state to the interval from state when enabled', () => { + getState.mockReturnValue(autoplayState); + middleware(action); + + expect(setAutoplayInterval).toBeCalledWith( + createTimeInterval(autoplayState.transient.autoplay.interval) + ); + }); + + it('sets the app state to null when not enabled', () => { + getState.mockReturnValue(autoplayDisabledState); + middleware(action); + + expect(setAutoplayInterval).toBeCalledWith(null); + }); + }); + + describe('autoplay navigation', () => { + it('navigates forward after interval', () => { + jest.useFakeTimers(); + getState.mockReturnValue(autoplayState); + middleware(action); + + jest.advanceTimersByTime(autoplayState.transient.autoplay.interval + 1); + + expect(routerMock.navigateTo).toBeCalledWith('loadWorkpad', { + id: workpadState.persistent.workpad.id, + page: workpadState.persistent.workpad.page + 2, // (index + 1) + 1 more for 1 indexed page number + }); + + jest.useRealTimers(); + }); + + it('navigates from last page back to front', () => { + jest.useFakeTimers(); + const onLastPageState = { ...autoplayState }; + onLastPageState.persistent.workpad.page = onLastPageState.persistent.workpad.pages.length - 1; + + getState.mockReturnValue(autoplayState); + middleware(action); + + jest.advanceTimersByTime(autoplayState.transient.autoplay.interval + 1); + + expect(routerMock.navigateTo).toBeCalledWith('loadWorkpad', { + id: workpadState.persistent.workpad.id, + page: 1, + }); + + jest.useRealTimers(); + }); + + it('continues autoplaying', () => { + jest.useFakeTimers(); + getState.mockReturnValue(autoplayState); + middleware(action); + + jest.advanceTimersByTime(autoplayState.transient.autoplay.interval * 2 + 1); + expect(routerMock.navigateTo).toBeCalledTimes(2); + jest.useRealTimers(); + }); + + it('does not reset timer between middleware calls', () => { + jest.useFakeTimers(); + + getState.mockReturnValue(autoplayState); + middleware(action); + + // Advance until right before timeout + jest.advanceTimersByTime(autoplayState.transient.autoplay.interval - 1); + + // Run middleware again + middleware(action); + + // Advance timer + jest.advanceTimersByTime(1); + + expect(routerMock.navigateTo).toBeCalled(); + }); + }); +}); diff --git a/x-pack/legacy/plugins/canvas/public/state/middleware/__tests__/workpad_refresh.test.ts b/x-pack/legacy/plugins/canvas/public/state/middleware/__tests__/workpad_refresh.test.ts new file mode 100644 index 00000000000000..e4d9cc6294a53f --- /dev/null +++ b/x-pack/legacy/plugins/canvas/public/state/middleware/__tests__/workpad_refresh.test.ts @@ -0,0 +1,163 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +jest.mock( + '../../../../../../../../src/legacy/ui/public/state_management/state_storage/hashed_item_store.js' +); +jest.mock('ui/new_platform'); // actions/elements has some dependencies on ui/new_platform. +jest.mock('../../../lib/app_state'); + +import { workpadRefresh } from '../workpad_refresh'; +import { inFlightComplete } from '../../actions/resolved_args'; +// @ts-ignore untyped local +import { setRefreshInterval } from '../../actions/workpad'; +import { setRefreshInterval as setAppStateRefreshInterval } from '../../../lib/app_state'; + +import { createTimeInterval } from '../../../lib/time_interval'; + +const next = jest.fn(); +const dispatch = jest.fn(); +const getState = jest.fn(); + +const middleware = workpadRefresh({ dispatch, getState })(next); + +const refreshState = { + transient: { + refresh: { + interval: 5000, + }, + }, +}; + +const noRefreshState = { + transient: { + refresh: { + interval: 0, + }, + }, +}; + +const inFlightState = { + transient: { + refresh: { + interval: 5000, + }, + inFlight: true, + }, +}; + +describe('workpad refresh middleware', () => { + beforeEach(() => { + jest.resetAllMocks(); + }); + + describe('onInflightComplete', () => { + it('refreshes if interval gt 0', () => { + jest.useFakeTimers(); + getState.mockReturnValue(refreshState); + + middleware(inFlightComplete()); + + jest.runAllTimers(); + + expect(dispatch).toHaveBeenCalled(); + }); + + it('does not reset interval if another action occurs', () => { + jest.useFakeTimers(); + getState.mockReturnValue(refreshState); + + middleware(inFlightComplete()); + + jest.advanceTimersByTime(refreshState.transient.refresh.interval - 1); + + expect(dispatch).not.toHaveBeenCalled(); + middleware(inFlightComplete()); + + jest.advanceTimersByTime(1); + + expect(dispatch).toHaveBeenCalled(); + }); + + it('does not refresh if interval is 0', () => { + jest.useFakeTimers(); + getState.mockReturnValue(noRefreshState); + + middleware(inFlightComplete()); + + jest.runAllTimers(); + expect(dispatch).not.toHaveBeenCalled(); + }); + }); + + describe('setRefreshInterval', () => { + it('does nothing if refresh interval is unchanged', () => { + getState.mockReturnValue(refreshState); + + jest.useFakeTimers(); + const interval = 1; + middleware(setRefreshInterval(interval)); + jest.runAllTimers(); + + expect(setAppStateRefreshInterval).not.toBeCalled(); + }); + + it('sets the app refresh interval', () => { + getState.mockReturnValue(noRefreshState); + next.mockImplementation(() => { + getState.mockReturnValue(refreshState); + }); + + jest.useFakeTimers(); + const interval = 1; + middleware(setRefreshInterval(interval)); + + expect(setAppStateRefreshInterval).toBeCalledWith(createTimeInterval(interval)); + jest.runAllTimers(); + }); + + it('starts a refresh for the new interval', () => { + getState.mockReturnValue(refreshState); + jest.useFakeTimers(); + + const interval = 1000; + + middleware(inFlightComplete()); + + jest.runTimersToTime(refreshState.transient.refresh.interval - 1); + expect(dispatch).not.toBeCalled(); + + getState.mockReturnValue(noRefreshState); + next.mockImplementation(() => { + getState.mockReturnValue(refreshState); + }); + middleware(setRefreshInterval(interval)); + jest.runTimersToTime(1); + + expect(dispatch).not.toBeCalled(); + + jest.runTimersToTime(interval); + expect(dispatch).toBeCalled(); + }); + }); + + describe('inFlight in progress', () => { + it('requeues the refresh when inflight is active', () => { + jest.useFakeTimers(); + getState.mockReturnValue(inFlightState); + + middleware(inFlightComplete()); + jest.runTimersToTime(refreshState.transient.refresh.interval); + + expect(dispatch).not.toBeCalled(); + + getState.mockReturnValue(refreshState); + jest.runAllTimers(); + + expect(dispatch).toBeCalled(); + }); + }); +}); diff --git a/x-pack/legacy/plugins/canvas/public/state/middleware/workpad_autoplay.js b/x-pack/legacy/plugins/canvas/public/state/middleware/workpad_autoplay.ts similarity index 77% rename from x-pack/legacy/plugins/canvas/public/state/middleware/workpad_autoplay.js rename to x-pack/legacy/plugins/canvas/public/state/middleware/workpad_autoplay.ts index 886620c5404cd3..700905213f54cf 100644 --- a/x-pack/legacy/plugins/canvas/public/state/middleware/workpad_autoplay.js +++ b/x-pack/legacy/plugins/canvas/public/state/middleware/workpad_autoplay.ts @@ -4,16 +4,18 @@ * you may not use this file except in compliance with the Elastic License. */ -import { inFlightComplete } from '../actions/resolved_args'; +import { Middleware } from 'redux'; +import { State } from '../../../types'; import { getFullscreen } from '../selectors/app'; import { getInFlight } from '../selectors/resolved_args'; import { getWorkpad, getPages, getSelectedPageIndex, getAutoplay } from '../selectors/workpad'; +// @ts-ignore untyped local import { routerProvider } from '../../lib/router_provider'; import { setAutoplayInterval } from '../../lib/app_state'; import { createTimeInterval } from '../../lib/time_interval'; -export const workpadAutoplay = ({ getState }) => next => { - let playTimeout; +export const workpadAutoplay: Middleware<{}, State> = ({ getState }) => next => { + let playTimeout: number | undefined; let displayInterval = 0; const router = routerProvider(); @@ -42,18 +44,22 @@ export const workpadAutoplay = ({ getState }) => next => { } } + stopAutoUpdate(); startDelayedUpdate(); } function stopAutoUpdate() { clearTimeout(playTimeout); // cancel any pending update requests + playTimeout = undefined; } function startDelayedUpdate() { - stopAutoUpdate(); - playTimeout = setTimeout(() => { - updateWorkpad(); - }, displayInterval); + if (!playTimeout) { + stopAutoUpdate(); + playTimeout = window.setTimeout(() => { + updateWorkpad(); + }, displayInterval); + } } return action => { @@ -68,21 +74,14 @@ export const workpadAutoplay = ({ getState }) => next => { if (autoplay.enabled) { setAutoplayInterval(createTimeInterval(autoplay.interval)); } else { - setAutoplayInterval(0); + setAutoplayInterval(null); } - // when in-flight requests are finished, update the workpad after a given delay - if (action.type === inFlightComplete.toString() && shouldPlay) { - startDelayedUpdate(); - } // create new update request - - // This middleware creates or destroys an interval that will cause workpad elements to update - // clear any pending timeout - stopAutoUpdate(); - // if interval is larger than 0, start the delayed update if (shouldPlay) { startDelayedUpdate(); + } else { + stopAutoUpdate(); } }; }; diff --git a/x-pack/legacy/plugins/canvas/public/state/middleware/workpad_refresh.js b/x-pack/legacy/plugins/canvas/public/state/middleware/workpad_refresh.ts similarity index 65% rename from x-pack/legacy/plugins/canvas/public/state/middleware/workpad_refresh.js rename to x-pack/legacy/plugins/canvas/public/state/middleware/workpad_refresh.ts index 32822529f320c9..f638c42ec2de08 100644 --- a/x-pack/legacy/plugins/canvas/public/state/middleware/workpad_refresh.js +++ b/x-pack/legacy/plugins/canvas/public/state/middleware/workpad_refresh.ts @@ -4,18 +4,25 @@ * you may not use this file except in compliance with the Elastic License. */ +import { Middleware } from 'redux'; +import { State } from '../../../types'; +// @ts-ignore Untyped Local import { fetchAllRenderables } from '../actions/elements'; +// @ts-ignore Untyped Local import { setRefreshInterval } from '../actions/workpad'; import { inFlightComplete } from '../actions/resolved_args'; import { getInFlight } from '../selectors/resolved_args'; +import { getRefreshInterval } from '../selectors/workpad'; import { setRefreshInterval as setAppStateRefreshInterval } from '../../lib/app_state'; import { createTimeInterval } from '../../lib/time_interval'; -export const workpadRefresh = ({ dispatch, getState }) => next => { - let refreshTimeout; +export const workpadRefresh: Middleware<{}, State> = ({ dispatch, getState }) => next => { + let refreshTimeout: number | undefined; let refreshInterval = 0; function updateWorkpad() { + cancelDelayedUpdate(); + if (refreshInterval === 0) { return; } @@ -31,30 +38,43 @@ export const workpadRefresh = ({ dispatch, getState }) => next => { } } + function cancelDelayedUpdate() { + clearTimeout(refreshTimeout); + refreshTimeout = undefined; + } + function startDelayedUpdate() { - clearTimeout(refreshTimeout); // cancel any pending update requests - refreshTimeout = setTimeout(() => { - updateWorkpad(); - }, refreshInterval); + if (!refreshTimeout) { + clearTimeout(refreshTimeout); // cancel any pending update requests + refreshTimeout = window.setTimeout(() => { + updateWorkpad(); + }, refreshInterval); + } } return action => { + const previousRefreshInterval = getRefreshInterval(getState()); next(action); + refreshInterval = getRefreshInterval(getState()); + // when in-flight requests are finished, update the workpad after a given delay if (action.type === inFlightComplete.toString() && refreshInterval > 0) { startDelayedUpdate(); } // create new update request // This middleware creates or destroys an interval that will cause workpad elements to update - if (action.type === setRefreshInterval.toString()) { + if ( + action.type === setRefreshInterval.toString() && + previousRefreshInterval !== refreshInterval + ) { // update the refresh interval refreshInterval = action.payload; setAppStateRefreshInterval(createTimeInterval(refreshInterval)); // clear any pending timeout - clearTimeout(refreshTimeout); + cancelDelayedUpdate(); // if interval is larger than 0, start the delayed update if (refreshInterval > 0) {