Allow typing while searching for course links

For this to work correctly without race conditions, we need to discard
the results of in-flight searches if somebody types more stuff. We
introduce a "cancel" mechanic for this purpose.

closes MAT-701
flag=none
test plan
- Find an RCE
- Open the links sidebar
- Expand one of the collections, e.g. Assignments
  Make sure your course has enough of that kind of thing so you can
  construct useful search things
- Enter something in the Search textbox
  > Confirm that the results match what you entered
- Change your value in the search textbox
- BEFORE THE SEARCH COMPLETES*, change your value again
  > Confirm that you are actually able to type (the text box should not
    prevent you from entering text while the search is in progress)
  > Confirm that the results match what your latest entry

* This can be tough if your searches are coming back quickly. Consider
  installing something like Requestly (https://requestly.io/) which you
  can use to slow them down.

Change-Id: I465db0461aabb3a03ed05e487fe88df29587ff42
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/288708
Reviewed-by: Joe Hernandez <joe.hernandez@instructure.com>
Reviewed-by: Gonzalo Penaranda <gonzalo.penaranda@instructure.com>
QA-Review: Joe Hernandez <joe.hernandez@instructure.com>
Product-Review: David Lyons <lyons@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>

packages/canvas-rce/src/rce/plugins/shared/Filter.js

@@ -201,7 +201,6 @@ export default function Filter(props) {
         <IconButton
           screenReaderLabel={formatMessage('Clear')}
           onClick={handleClear}
-          interaction={isContentLoading ? 'disabled' : 'enabled'}
           withBorder={false}
           withBackground={false}
           size="small"
@@ -306,7 +305,6 @@ export default function Filter(props) {
               doSearch(pendingSearchString)
             }
           }}
-          interaction={isContentLoading ? 'readonly' : 'enabled'}
         />
       </View>
     </View>

packages/canvas-rce/src/rce/plugins/shared/__tests__/Filter.test.js

@@ -409,7 +409,7 @@ describe('RCE Plugins > Filter', () => {
       })
     })
 
-    it('is readonly while content is loading', () => {
+    it('is not readonly while content is loading', () => {
       renderComponent({
         userContextType: 'course',
         contentType: 'course_files',
@@ -418,10 +418,10 @@ describe('RCE Plugins > Filter', () => {
         searchString: 'abc'
       })
       const searchInput = component.getByPlaceholderText('Search')
-      expect(searchInput.hasAttribute('readonly')).toBe(true)
+      expect(searchInput.hasAttribute('readonly')).toBe(false)
 
       const clearBtn = component.getByText('Clear').closest('button')
-      expect(clearBtn.hasAttribute('disabled')).toBe(true)
+      expect(clearBtn.hasAttribute('disabled')).toBe(false)
     })
 
     it('shows the search message when not loading', () => {

packages/canvas-rce/src/sidebar/actions/data.js

@@ -21,12 +21,12 @@ export const REQUEST_PAGE = 'REQUEST_PAGE'
 export const RECEIVE_PAGE = 'RECEIVE_PAGE'
 export const FAIL_PAGE = 'FAIL_PAGE'
 
-export function requestInitialPage(key, searchString) {
-  return {type: REQUEST_INITIAL_PAGE, key, searchString}
+export function requestInitialPage(key, cancel, searchString) {
+  return {type: REQUEST_INITIAL_PAGE, key, cancel, searchString}
 }
 
-export function requestPage(key) {
-  return {type: REQUEST_PAGE, key}
+export function requestPage(key, cancel) {
+  return {type: REQUEST_PAGE, key, cancel}
 }
 
 export function receivePage(key, page) {
@@ -41,14 +41,29 @@ export function failPage(key, error) {
 // dispatches the start of the load, requests a page for the collection from
 // the source, then dispatches the loaded page to the store on success or
 // clears the load on failure
-export function fetchPage(key) {
+export function fetchPage(key, isInitial, searchString) {
   return (dispatch, getState) => {
+    let isCancelled = false
+    const cancel = () => (isCancelled = true)
+
+    if (isInitial) {
+      dispatch(requestInitialPage(key, cancel, searchString))
+    } else {
+      dispatch(requestPage(key, cancel))
+    }
+
     const state = getState()
     const {source} = state
     return source
       .fetchLinks(key, state)
-      .then(page => dispatch(receivePage(key, page)))
-      .catch(error => dispatch(failPage(key, error)))
+      .then(page => {
+        if (isCancelled) return
+        dispatch(receivePage(key, page))
+      })
+      .catch(error => {
+        if (isCancelled) return
+        dispatch(failPage(key, error))
+      })
   }
 }
 
@@ -56,9 +71,9 @@ export function fetchNextPage(key) {
   return (dispatch, getState) => {
     const state = getState()
     const collection = state.collections[key]
-    if (collection && !collection.isLoading) {
-      dispatch(requestPage(key))
-      return dispatch(fetchPage(key))
+    if (collection) {
+      if (collection.cancel) collection.cancel()
+      return dispatch(fetchPage(key, false))
     }
   }
 }
@@ -67,13 +82,13 @@ export function fetchInitialPage(key) {
   return (dispatch, getState) => {
     const state = getState()
     const collection = state.collections[key]
+
     if (
       collection &&
-      !collection.isLoading &&
       (collection.links.length === 0 || collection.searchString !== state.searchString)
     ) {
-      dispatch(requestInitialPage(key, state.searchString))
-      return dispatch(fetchPage(key))
+      if (collection.cancel) collection.cancel()
+      return dispatch(fetchPage(key, true, state.searchString))
     }
   }
 }

packages/canvas-rce/src/sidebar/reducers/collection.js

@@ -20,7 +20,7 @@ import {REQUEST_INITIAL_PAGE, REQUEST_PAGE, RECEIVE_PAGE, FAIL_PAGE} from '../ac
 
 // manages the state for a specific collection. assumes the action is intended
 // for this collection (see collections.js)
-export default function(state = {}, action) {
+export default function (state = {}, action) {
   switch (action.type) {
     case REQUEST_INITIAL_PAGE:
       return {
@@ -28,13 +28,15 @@ export default function(state = {}, action) {
         links: [],
         bookmark: null,
         isLoading: true,
+        cancel: action.cancel,
         hasMore: true,
         searchString: action.searchString
       }
     case REQUEST_PAGE:
       return {
         ...state,
-        isLoading: true
+        isLoading: true,
+        cancel: action.cancel
       }
 
     case RECEIVE_PAGE:
@@ -44,12 +46,14 @@ export default function(state = {}, action) {
         links: state.links.concat(action.links),
         bookmark: action.bookmark,
         isLoading: false,
+        cancel: null,
         hasMore: !!action.bookmark
       }
 
     case FAIL_PAGE: {
       const overrides = {
         isLoading: false,
+        cancel: null,
         error: action.error
       }
       if (state.links.length === 0) {

packages/canvas-rce/test/sidebar/actions/data.test.js

@@ -15,7 +15,6 @@
  * You should have received a copy of the GNU Affero General Public License along
  * with this program. If not, see <http://www.gnu.org/licenses/>.
  */
-
 import assert from 'assert'
 import * as actions from '../../../src/sidebar/actions/data'
 import RceApiSource from '../../../src/rcs/api'
@@ -142,15 +141,21 @@ describe('Sidebar data actions', () => {
       store.dispatch(actions.fetchNextPage(collectionKey))
       sinon.assert.calledWith(store.spy, {
         type: actions.REQUEST_PAGE,
+        cancel: sinon.match.func,
         key: collectionKey
       })
     })
 
-    it('skips fetching next page if collection is already loading', () => {
-      const store = spiedStore(setupState({}, {isLoading: true}))
+    it('cancels previous fetch if collection is already loading', () => {
+      const cancel = sinon.spy()
+      const store = spiedStore(setupState({}, {isLoading: true, cancel}))
       store.dispatch(actions.fetchNextPage(collectionKey))
-      sinon.assert.neverCalledWith(store.spy, {
+
+      sinon.assert.called(cancel)
+
+      sinon.assert.calledWith(store.spy, {
         type: actions.REQUEST_PAGE,
+        cancel: sinon.match.func,
         key: collectionKey
       })
     })
@@ -162,6 +167,7 @@ describe('Sidebar data actions', () => {
       store.dispatch(actions.fetchInitialPage(collectionKey))
       sinon.assert.calledWith(store.spy, {
         type: actions.REQUEST_INITIAL_PAGE,
+        cancel: sinon.match.func,
         key: collectionKey,
         searchString
       })
@@ -172,6 +178,7 @@ describe('Sidebar data actions', () => {
       store.dispatch(actions.fetchInitialPage(collectionKey))
       sinon.assert.neverCalledWith(store.spy, {
         type: actions.REQUEST_INITIAL_PAGE,
+        cancel: sinon.match.func,
         key: collectionKey,
         searchString
       })
@@ -184,11 +191,16 @@ describe('Sidebar data actions', () => {
       sinon.assert.calledWith(source.fetchPage, 'uriFor/bookmark')
     })
 
-    it('fetching initial page if collection is already loading', () => {
-      const store = spiedStore(setupState({}, {isLoading: true}))
+    it('cancels previous fetch if collection is already loading', () => {
+      const cancel = sinon.spy()
+      const store = spiedStore(setupState({}, {isLoading: true, cancel}))
       store.dispatch(actions.fetchInitialPage(collectionKey))
-      sinon.assert.neverCalledWith(store.spy, {
+
+      sinon.assert.called(cancel)
+
+      sinon.assert.calledWith(store.spy, {
         type: actions.REQUEST_INITIAL_PAGE,
+        cancel: sinon.match.func,
         key: collectionKey,
         searchString
       })

packages/canvas-rce/test/sidebar/reducers/collection.test.js

@@ -33,11 +33,13 @@ describe('Collection reducer', () => {
 
   describe('REQUEST_PAGE', () => {
     const action = {
-      type: actions.REQUEST_PAGE
+      type: actions.REQUEST_PAGE,
+      cancel() {}
     }
 
-    it('sets the loading flag', () => {
+    it('sets the loading flag and cancel function', () => {
       assert.strictEqual(collection(state, action).isLoading, true)
+      assert.strictEqual(typeof collection(state, action).cancel, 'function')
     })
 
     it('preserves existing state', () => {