avoid doubling results in rce image content tray

If you type at a specific cadence, you could trigger two api requests
for images and the second one would be for the stale search term, but
the images would still come be inserted into the content tray. This
ignores the stale queries when they resolve.

fixes MAT-400
flag=none

Test Plan
- Upload two files with at least the first six letters of their
  filenames the same.
- Type in the search bar the first three letters, then at a slight
  pause, type the other characters such that they would match both
  files again.
- Do the above over and over again, verifying that the tray never
  has duplicate results.

Change-Id: Ib41c1c16ec1f48aa118ddc4b0f4d658723c08d1e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274943
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
QA-Review: Weston Dransfield <wdransfield@instructure.com>
Product-Review: Jon Scheiding <jon.scheiding@instructure.com>

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

@@ -47,8 +47,8 @@ export function requestImages(contextType) {
 }
 
 export function receiveImages({response, contextType}) {
-  const {files, bookmark} = response
-  return {type: RECEIVE_IMAGES, payload: {files, bookmark, contextType}}
+  const {files, bookmark, searchString} = response
+  return {type: RECEIVE_IMAGES, payload: {files, bookmark, contextType, searchString}}
 }
 
 export function failImagesLoad({error, contextType}) {

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

@@ -68,11 +68,16 @@ export default function imagesReducer(prevState = {}, action) {
       return state
 
     case RECEIVE_IMAGES:
-      state[ctxt] = {
-        files: state[ctxt].files.concat(action.payload.files),
-        isLoading: false,
-        bookmark: action.payload.bookmark,
-        hasMore: !!action.payload.bookmark
+      // If a request resolved with files but the searchString has since
+      // changed, then the files should not be concatenated because this
+      // request will have been redundant at best and wrong at worst.
+      if (action.payload.searchString === state.searchString) {
+        state[ctxt] = {
+          files: state[ctxt].files.concat(action.payload.files),
+          isLoading: false,
+          bookmark: action.payload.bookmark,
+          hasMore: !!action.payload.bookmark
+        }
       }
       return state
 
@@ -91,6 +96,7 @@ export default function imagesReducer(prevState = {}, action) {
     }
 
     case CHANGE_SEARCH_STRING: {
+      state.searchString = action.payload
       return state
     }
 

packages/canvas-rce/src/sidebar/sources/api.js

@@ -130,7 +130,8 @@ class RceApiSource {
         bookmark: null,
         isLoading: false,
         hasMore: true
-      }
+      },
+      searchString: ''
     }
   }
 
@@ -349,7 +350,8 @@ class RceApiSource {
     return this.apiFetch(uri, headers).then(({bookmark, files}) => {
       return {
         bookmark,
-        files: files.map(f => fixupFileUrl(props.contextType, props.contextId, f))
+        files: files.map(f => fixupFileUrl(props.contextType, props.contextId, f)),
+        searchString: props.searchString
       }
     })
   }

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

@@ -23,6 +23,44 @@ import * as actions from '../../../src/sidebar/actions/images'
 const sortBy = {sort: 'alphabetical', order: 'asc'}
 const searchString = 'hello'
 
+describe('Image dispatch shapes', () => {
+  describe('receiveImages', () => {
+    const contextType = 'course'
+    const response = {
+      bookmark: 'p2',
+      files: [],
+      searchString: 'panda'
+    }
+
+    it('returns a type of RECEIVE_IMAGES', () => {
+      const {type} = actions.receiveImages()
+      assert(type === actions.RECEIVE_IMAGES)
+    })
+
+    describe('returning a payload', () => {
+      it('includes contextType', () => {
+        const {payload} = actions.receiveImages({response, contextType})
+        assert(payload.contextType === 'course')
+      })
+
+      it('includes files', () => {
+        const {payload} = actions.receiveImages({response})
+        assert(payload.files === [])
+      })
+
+      it('includes bookmark', () => {
+        const {payload} = actions.receiveImages({response})
+        assert(payload.bookmark === 'p2')
+      })
+
+      it('includes searchString', () => {
+        const {payload} = actions.receiveImages({response})
+        assert(payload.searchString === 'panda')
+      })
+    })
+  })
+})
+
 describe('Image actions', () => {
   describe('createAddImage', () => {
     it('has the right action type', () => {

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

@@ -17,6 +17,7 @@
  */
 
 import assert from 'assert'
+import {CHANGE_SEARCH_STRING} from '../../../src/sidebar/actions/filter'
 import images from '../../../src/sidebar/reducers/images'
 import * as actions from '../../../src/sidebar/actions/images'
 
@@ -31,7 +32,8 @@ describe('Images reducer', () => {
         hasMore: false,
         isLoading: false
       },
-      contextType: 'course'
+      contextType: 'course',
+      searchString: ''
     }
   })
 
@@ -93,17 +95,34 @@ describe('Images reducer', () => {
   })
 
   describe('RECEIVE_IMAGES', () => {
-    it('appends new records to the existing array', () => {
+    it('appends new records to the existing array when the payload searchString matches state', () => {
       const action = {
         type: actions.RECEIVE_IMAGES,
         payload: {
           files: [{id: 1}, {id: 2}],
-          contextType: 'course'
+          contextType: 'course',
+          searchString: 'panda'
         }
       }
+
+      state.searchString = 'panda'
       assert.equal(images(state, action).course.files.length, 2)
     })
 
+    it('does not append new records to the existing array when the payload searchString does not match state', () => {
+      const action = {
+        type: actions.RECEIVE_IMAGES,
+        payload: {
+          files: [{id: 1}, {id: 2}],
+          contextType: 'course',
+          searchString: 'panda'
+        }
+      }
+
+      state.searchString = 'cat'
+      assert.equal(images(state, action).course.files.length, 0)
+    })
+
     it("hasMore if there's a bookmark", () => {
       const action = {
         type: actions.RECEIVE_IMAGES,
@@ -153,4 +172,11 @@ describe('Images reducer', () => {
       assert.equal(images(state, action).course.files.length, 0)
     })
   })
+
+  describe('CHANGE_SEARCH_STRING', () => {
+    it('sets the searchString to the payload', () => {
+      const action = {type: CHANGE_SEARCH_STRING, payload: 'panda'}
+      assert.strictEqual(images(state, action).searchString, 'panda')
+    })
+  })
 })

packages/canvas-rce/test/sidebar/sources/api.test.js

@@ -86,6 +86,10 @@ describe('sources/api', () => {
     it('sets hasMore to true', () => {
       assert.strictEqual(apiSource.initializeImages(props)[props.contextType].hasMore, true)
     })
+
+    it('sets searchString to an empty string', () => {
+      assert.strictEqual(apiSource.initializeImages(props).searchString, '')
+    })
   })
 
   describe('URI construction (baseUri)', () => {
@@ -731,6 +735,7 @@ describe('sources/api', () => {
         files: []
       }
     }
+    props.searchString = 'panda'
 
     it('can fetch folders', () => {
       fetchMock.mock(/\/folders\?/, {body})
@@ -745,7 +750,8 @@ describe('sources/api', () => {
       return apiSource.fetchImages(props).then(page => {
         assert.deepStrictEqual(page, {
           bookmark: 'mo.images',
-          files: [{href: '/some/where?wrap=1', uuid: 'xyzzy'}]
+          files: [{href: '/some/where?wrap=1', uuid: 'xyzzy'}],
+          searchString: 'panda'
         })
         fetchMock.restore()
       })