[Canvas] Fixes bugs with autoplay and refresh (#53149)

Corey Robertson · elasticmachine · Corey Robertson · commit d69f78d6687c · 2020-01-08T15:40:24.000-05:00
* Fixes bugs with autoplay and refresh

* Fix typecheck

Co-authored-by: Elastic Machine &lt;elasticmachine@users.noreply.github.com&gt;
diff --git 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
@@ -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
@@ -0,0 +1,161 @@
+/*
+ * 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('../../../legacy');
+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.ts 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.ts b/x-pack/legacy/plugins/canvas/public/state/middleware/workpad_refresh.ts

Original file line number	Diff line number	Diff line change
`@@ -92,7 +92,7 @@ export function setFullscreen(payload: boolean) {`
`92`	`92`	`}`
`93`	`93`	`}`
`94`	`94`
`95`		`-export function setAutoplayInterval(payload: string) {`
	`95`	`+export function setAutoplayInterval(payload: string \| null) {`
`96`	`96`	`const appState = getAppState();`
`97`	`97`	`const appValue = appState[AppStateKeys.AUTOPLAY_INTERVAL];`
`98`	`98`