From 663eb81bbcc227026bbd6131443bda6b25cdfa4b Mon Sep 17 00:00:00 2001
From: Chris Davies <cgrdavies@gmail.com>
Date: Thu, 7 Nov 2019 11:19:40 -0500
Subject: [PATCH 1/4] fix(content-picker): fix issues; see if tests pass

---
 src/elements/content-picker/Content.js        |  3 +
 src/elements/content-picker/ContentPicker.js  | 75 ++++++-------------
 src/elements/content-picker/ItemList.js       | 23 +++++-
 .../__tests__/cellRendererHelper.test.js      | 66 ++++++++++------
 .../__tests__/selectionCellRenderer.test.js   | 13 +++-
 .../content-picker/cellRendererHelper.js      |  5 +-
 .../content-picker/selectionCellRenderer.js   |  8 +-
 .../content-picker/shareAccessCellRenderer.js |  7 +-
 8 files changed, 111 insertions(+), 89 deletions(-)

diff --git a/src/elements/content-picker/Content.js b/src/elements/content-picker/Content.js
index 61c88198ed..c57c69ec40 100644
--- a/src/elements/content-picker/Content.js
+++ b/src/elements/content-picker/Content.js
@@ -27,6 +27,7 @@ type Props = {
     rootElement?: HTMLElement,
     rootId: string,
     selectableType: string,
+    selected: Array<BoxItem>,
     tableRef: Function,
     view: View,
 };
@@ -60,6 +61,7 @@ const Content = ({
     onShareAccessChange,
     onFocusChange,
     extensionsWhitelist,
+    selected,
 }: Props) => (
     <div className="bcp-content">
         {view === VIEW_ERROR || view === VIEW_SELECTED ? null : (
@@ -85,6 +87,7 @@ const Content = ({
                 onFocusChange={onFocusChange}
                 onShareAccessChange={onShareAccessChange}
                 extensionsWhitelist={extensionsWhitelist}
+                selected={selected}
             />
         )}
     </div>
diff --git a/src/elements/content-picker/ContentPicker.js b/src/elements/content-picker/ContentPicker.js
index 531890f0c1..4db5fce0ce 100644
--- a/src/elements/content-picker/ContentPicker.js
+++ b/src/elements/content-picker/ContentPicker.js
@@ -102,7 +102,7 @@ type State = {
     isUploadModalOpen: boolean,
     rootName: string,
     searchQuery: string,
-    selected: { [string]: BoxItem },
+    selected: Array<BoxItem>,
     sortBy: SortBy,
     sortDirection: SortDirection,
     view: View,
@@ -199,7 +199,7 @@ class ContentPicker extends Component<Props, State> {
             currentCollection: {},
             currentOffset: initialPageSize * (initialPage - 1),
             currentPageSize: initialPageSize,
-            selected: {},
+            selected: [],
             searchQuery: '',
             view: VIEW_FOLDER,
             isCreateFolderModalOpen: false,
@@ -287,11 +287,10 @@ class ContentPicker extends Component<Props, State> {
     choose = (): void => {
         const { selected }: State = this.state;
         const { onChoose }: Props = this.props;
-        const results: BoxItem[] = Object.keys(selected).map(key => {
-            const clone: BoxItem = { ...selected[key] };
-            delete clone.selected;
-            return clone;
+        const results: BoxItem[] = selected.map(selectedItem => {
+            return { ...selectedItem };
         });
+
         onChoose(results);
     };
 
@@ -304,13 +303,9 @@ class ContentPicker extends Component<Props, State> {
      */
     cancel = (): void => {
         const { onCancel }: Props = this.props;
-        const { selected }: State = this.state;
-
-        // Clear out the selected field
-        Object.keys(selected).forEach(key => delete selected[key].selected);
 
         // Reset the selected state
-        this.setState({ selected: {} }, () => onCancel());
+        this.setState({ selected: [] }, () => onCancel());
     };
 
     /**
@@ -563,7 +558,7 @@ class ContentPicker extends Component<Props, State> {
                     sortBy,
                     sortDirection,
                     percentLoaded: 100,
-                    items: Object.keys(selected).map(key => this.api.getCache().get(key)),
+                    items: selected,
                 },
             },
             this.finishNavigation,
@@ -782,46 +777,26 @@ class ContentPicker extends Component<Props, State> {
             return;
         }
 
-        const selectedKeys: Array<string> = Object.keys(selected);
-        const selectedCount: number = selectedKeys.length;
-        const hasHitSelectionLimit: boolean = selectedCount === maxSelectable;
         const isSingleFileSelection: boolean = maxSelectable === 1;
-        const cacheKey: string = this.api.getAPI(type).getCacheKey(id);
-        const existing: BoxItem = selected[cacheKey];
-        const existingFromCache: BoxItem = this.api.getCache().get(cacheKey);
-        const existInSelected = selectedKeys.indexOf(cacheKey) !== -1;
+        const newSelected: Array<BoxItem> = isSingleFileSelection ? [] : [...selected];
+        const selectedCount: number = newSelected.length;
+        const hasHitSelectionLimit: boolean = selectedCount === maxSelectable;
+        const existingIndex: number = newSelected.findIndex(sel => sel.id === id && sel.type === type);
+        const isAlreadySelected =
+            existingIndex !== -1 ||
+            (isSingleFileSelection && selected[0] && selected[0].id === id && selected[0].type === type);
         const itemCanSetShareAccess = getProp(item, 'permissions.can_set_share_access', false);
 
-        // Existing object could have mutated and we just need to update the
-        // reference in the selected map. In that case treat it like a new selection.
-        if (existing && existing === existingFromCache) {
+        if (isAlreadySelected) {
             // We are selecting the same item that was already
             // selected. Unselect it in this case. Toggle case.
-            delete existing.selected;
-            delete selected[cacheKey];
+            newSelected.splice(existingIndex, 1);
         } else {
-            // We are selecting a new item that was never
-            // selected before. However if we are in a single
-            // item selection mode, we should also unselect any
-            // prior item that was item that was selected.
-
-            // Check if we hit the selection limit and if selection
-            // is not already currently in the selected data structure.
-            // Ignore when in single file selection mode.
-            if (hasHitSelectionLimit && !isSingleFileSelection && !existInSelected) {
+            if (hasHitSelectionLimit) {
                 return;
             }
 
-            // Clear out the prior item for single file selection mode
-            if (selectedCount > 0 && isSingleFileSelection) {
-                const prior = selectedKeys[0]; // only one item
-                delete selected[prior].selected;
-                delete selected[prior];
-            }
-
-            // Select the new item
-            item.selected = true;
-            selected[cacheKey] = item;
+            newSelected.push(item);
 
             // If can set share access, fetch the shared link properties of the item
             // In the case where another item is selected, any in flight XHR will get
@@ -832,7 +807,7 @@ class ContentPicker extends Component<Props, State> {
         }
 
         const focusedRow = items.findIndex((i: BoxItem) => i.id === item.id);
-        this.setState({ selected, focusedRow }, () => {
+        this.setState({ selected: newSelected, focusedRow }, () => {
             if (view === VIEW_SELECTED) {
                 // Need to refresh the selected view
                 this.showSelected();
@@ -881,14 +856,8 @@ class ContentPicker extends Component<Props, State> {
         if (!item[FIELD_SHARED_LINK]) {
             this.changeShareAccess(null, item);
         } else {
-            const { selected } = this.state;
-            const { id, type } = item;
-            const cacheKey = this.api.getAPI(type).getCacheKey(id);
             // if shared link already exists, update the collection in state
             this.updateItemInCollection(item);
-            if (item.selected && item !== selected[cacheKey]) {
-                this.select(item, { forceSharedLink: false });
-            }
         }
     };
 
@@ -918,9 +887,6 @@ class ContentPicker extends Component<Props, State> {
 
         this.api.getAPI(type).share(item, access, (updatedItem: BoxItem) => {
             this.updateItemInCollection(updatedItem);
-            if (item.selected) {
-                this.select(updatedItem, { forceSharedLink: false });
-            }
         });
     };
 
@@ -1152,7 +1118,7 @@ class ContentPicker extends Component<Props, State> {
         }: State = this.state;
         const { id, offset, permissions, totalCount }: Collection = currentCollection;
         const { can_upload }: BoxItemPermission = permissions || {};
-        const selectedCount: number = Object.keys(selected).length;
+        const { length: selectedCount } = selected;
         const isSingleSelect = maxSelectable === 1;
         const hasHitSelectionLimit: boolean = selectedCount === maxSelectable && !isSingleSelect;
         const allowUpload: boolean = canUpload && !!can_upload;
@@ -1202,6 +1168,7 @@ class ContentPicker extends Component<Props, State> {
                             onItemClick={this.onItemClick}
                             onFocusChange={this.onFocusChange}
                             onShareAccessChange={this.changeShareAccess}
+                            selected={selected}
                         />
                         <Footer
                             selectedCount={selectedCount}
diff --git a/src/elements/content-picker/ItemList.js b/src/elements/content-picker/ItemList.js
index 17b7bd36b5..c54b337549 100644
--- a/src/elements/content-picker/ItemList.js
+++ b/src/elements/content-picker/ItemList.js
@@ -34,6 +34,7 @@ type Props = {
     rootElement?: HTMLElement,
     rootId: string,
     selectableType: string,
+    selected: Array<BoxItem>,
     tableRef: Function,
     view: View,
 };
@@ -54,6 +55,7 @@ const ItemList = ({
     onShareAccessChange,
     onFocusChange,
     currentCollection,
+    selected,
     tableRef,
 }: Props) => {
     const iconCell = iconCellRenderer();
@@ -64,6 +66,7 @@ const ItemList = ({
         extensionsWhitelist,
         hasHitSelectionLimit,
         isSingleSelect,
+        selected,
     );
     const shareAccessCell = shareAccessCellRenderer(
         onShareAccessChange,
@@ -71,6 +74,7 @@ const ItemList = ({
         selectableType,
         extensionsWhitelist,
         hasHitSelectionLimit,
+        selected,
     );
     const { id, items = [] }: Collection = currentCollection;
     const rowCount: number = items.length;
@@ -80,10 +84,18 @@ const ItemList = ({
             return '';
         }
 
-        const { selected, type } = items[index];
-        const isSelectable = isRowSelectable(selectableType, extensionsWhitelist, hasHitSelectionLimit, items[index]);
+        const { id: itemId, type } = items[index];
+        const isSelected = !!selected.find(sel => sel.id === itemId && sel.type === type);
+        const isSelectable = isRowSelectable(
+            selectableType,
+            extensionsWhitelist,
+            hasHitSelectionLimit,
+            items[index],
+            isSelected,
+        );
+
         return classNames(`bcp-item-row bcp-item-row-${index}`, {
-            'bcp-item-row-selected': selected && view !== VIEW_SELECTED,
+            'bcp-item-row-selected': isSelected && view !== VIEW_SELECTED,
             'bcp-item-row-unselectable': type !== TYPE_FOLDER && !isSelectable, // folder row should never dim
         });
     };
@@ -97,9 +109,12 @@ const ItemList = ({
         index: number,
         rowData: BoxItem,
     }) => {
+        const { id: itemId, type } = rowData;
+        const isSelected = !!selected.find(sel => sel.id === itemId && sel.type === type);
+
         // If the click is happening on a clickable element on the item row, ignore row selection
         if (
-            isRowSelectable(selectableType, extensionsWhitelist, hasHitSelectionLimit, rowData) &&
+            isRowSelectable(selectableType, extensionsWhitelist, hasHitSelectionLimit, rowData, isSelected) &&
             !isFocusableElement(event.target)
         ) {
             onItemSelect(rowData);
diff --git a/src/elements/content-picker/__tests__/cellRendererHelper.test.js b/src/elements/content-picker/__tests__/cellRendererHelper.test.js
index 8b10e6f219..2249df1e9c 100644
--- a/src/elements/content-picker/__tests__/cellRendererHelper.test.js
+++ b/src/elements/content-picker/__tests__/cellRendererHelper.test.js
@@ -2,50 +2,74 @@ import isRowSelectable from '../cellRendererHelper';
 
 describe('picker/components/cellRendererHelper/isRowSelectable()', () => {
     test('should return true when folder picker and type is folder', () => {
-        expect(isRowSelectable('folder', [], false, { type: 'folder' })).toBeTruthy();
+        expect(isRowSelectable('folder', [], false, { type: 'folder' }, false)).toBeTruthy();
     });
     test('should return false when folder picker and type is file', () => {
         expect(
-            isRowSelectable('folder', [], false, {
-                type: 'file',
-                extension: 'doc',
-            }),
+            isRowSelectable(
+                'folder',
+                [],
+                false,
+                {
+                    type: 'file',
+                    extension: 'doc',
+                },
+                false,
+            ),
         ).toBeFalsy();
     });
     test('should return true when file picker and type is file with no whitelist extensions', () => {
         expect(
-            isRowSelectable('file', [], false, {
-                type: 'file',
-                extension: 'doc',
-            }),
+            isRowSelectable(
+                'file',
+                [],
+                false,
+                {
+                    type: 'file',
+                    extension: 'doc',
+                },
+                false,
+            ),
         ).toBeTruthy();
     });
     test('should return false when file picker and type is folder', () => {
-        expect(isRowSelectable('file', [], false, { type: 'folder' })).toBeFalsy();
+        expect(isRowSelectable('file', [], false, { type: 'folder' }, false)).toBeFalsy();
     });
     test('should return true when file picker and type is file and extension is whitelisted', () => {
         expect(
-            isRowSelectable('file', ['doc'], false, {
-                type: 'file',
-                extension: 'doc',
-            }),
+            isRowSelectable(
+                'file',
+                ['doc'],
+                false,
+                {
+                    type: 'file',
+                    extension: 'doc',
+                },
+                false,
+            ),
         ).toBeTruthy();
     });
     test('should return true when file picker and type is file and extension is not whitelisted', () => {
         expect(
-            isRowSelectable('file', ['ppt'], false, {
-                type: 'file',
-                extension: 'doc',
-            }),
+            isRowSelectable(
+                'file',
+                ['ppt'],
+                false,
+                {
+                    type: 'file',
+                    extension: 'doc',
+                },
+                false,
+            ),
         ).toBeFalsy();
     });
     test('should return false when selection limit has reached and item is not selected', () => {
-        expect(isRowSelectable('file', [], true, { type: 'file' })).toBeFalsy();
+        expect(isRowSelectable('file', [], true, { type: 'file' }, false)).toBeFalsy();
     });
     test('should return false when selection limit has not reached', () => {
-        expect(isRowSelectable('file', [], true, { type: 'file' })).toBeFalsy();
+        expect(isRowSelectable('file', [], true, { type: 'file' }, false)).toBeFalsy();
     });
     test('should return true when selection limit has reached and item is selected', () => {
-        expect(isRowSelectable('file', [], true, { type: 'file', selected: true })).toBeTruthy();
+        expect(isRowSelectable('file', [], true, { type: 'file' }, true)).toBeTruthy();
     });
 });
diff --git a/src/elements/content-picker/__tests__/selectionCellRenderer.test.js b/src/elements/content-picker/__tests__/selectionCellRenderer.test.js
index 83b411d83e..8e7c18f3f9 100644
--- a/src/elements/content-picker/__tests__/selectionCellRenderer.test.js
+++ b/src/elements/content-picker/__tests__/selectionCellRenderer.test.js
@@ -3,21 +3,28 @@ import { shallow } from 'enzyme';
 import selectionCellRenderer from '../selectionCellRenderer';
 
 const rowData = {
+    id: '51965506671',
     name: 'test',
-    selected: true,
     type: 'file',
 };
 
+const selected = [
+    {
+        id: '51965506671',
+        type: 'file',
+    },
+];
+
 describe('selectionCellRenderer', () => {
     test.each([['Checkbox', false], ['RadioButton', true]])('should render %s if isRadio is %s', (type, isRadio) => {
-        const Element = selectionCellRenderer(() => {}, 'file, web_link', [], false, isRadio);
+        const Element = selectionCellRenderer(() => {}, 'file, web_link', [], false, isRadio, selected);
 
         const wrapper = shallow(<Element rowData={rowData} />);
         expect(wrapper.exists(type)).toBe(true);
     });
 
     test.each([['isSelected', true], ['isChecked', false]])('should render %s if isRadio is %s', (type, isRadio) => {
-        const Element = selectionCellRenderer(() => {}, 'file, web_link', [], false, isRadio);
+        const Element = selectionCellRenderer(() => {}, 'file, web_link', [], false, isRadio, selected);
 
         const wrapper = shallow(<Element rowData={rowData} />);
         expect(wrapper.prop(type)).toBe(true);
diff --git a/src/elements/content-picker/cellRendererHelper.js b/src/elements/content-picker/cellRendererHelper.js
index 65345cecdb..571e40af0f 100644
--- a/src/elements/content-picker/cellRendererHelper.js
+++ b/src/elements/content-picker/cellRendererHelper.js
@@ -11,9 +11,10 @@ function isRowSelectable(
     extensionsWhitelist: string[],
     hasHitSelectionLimit: boolean,
     rowData: BoxItem,
+    isSelected: boolean,
 ): boolean {
-    const { type, extension = '', selected }: BoxItem = rowData;
-    const shouldAllowSelection: boolean = hasHitSelectionLimit ? !!selected : true;
+    const { type, extension = '' }: BoxItem = rowData;
+    const shouldAllowSelection: boolean = hasHitSelectionLimit ? isSelected : true;
     const isTypeSelectable: boolean = !!type && selectableType.indexOf(type) > -1;
     const isFilePicker: boolean = selectableType.indexOf(TYPE_FILE) > -1;
     const isExtensionWhitelisted: boolean =
diff --git a/src/elements/content-picker/selectionCellRenderer.js b/src/elements/content-picker/selectionCellRenderer.js
index 53d4eca28f..13cf547128 100644
--- a/src/elements/content-picker/selectionCellRenderer.js
+++ b/src/elements/content-picker/selectionCellRenderer.js
@@ -15,11 +15,13 @@ export default (
     extensionsWhitelist: string[],
     hasHitSelectionLimit: boolean,
     isRadio: boolean,
+    selected: Array<BoxItem>,
 ): (({ rowData: BoxItem }) => {}) => ({ rowData }: { rowData: BoxItem }) => {
-    const { name = '', selected = false } = rowData;
+    const { name = '', id, type } = rowData;
     const Component = isRadio ? RadioButton : Checkbox;
+    const isSelected = !!selected.find(sel => sel.id === id && sel.type === type);
 
-    if (!isRowSelectable(selectableType, extensionsWhitelist, hasHitSelectionLimit, rowData)) {
+    if (!isRowSelectable(selectableType, extensionsWhitelist, hasHitSelectionLimit, rowData, isSelected)) {
         return <span />;
     }
 
@@ -30,7 +32,7 @@ export default (
             name={name}
             onChange={() => onItemSelect(rowData)}
             value={name}
-            {...{ [isRadio ? 'isSelected' : 'isChecked']: selected }}
+            {...{ [isRadio ? 'isSelected' : 'isChecked']: isSelected }}
         />
     );
 };
diff --git a/src/elements/content-picker/shareAccessCellRenderer.js b/src/elements/content-picker/shareAccessCellRenderer.js
index 1cdabd615e..f6ccbecadb 100644
--- a/src/elements/content-picker/shareAccessCellRenderer.js
+++ b/src/elements/content-picker/shareAccessCellRenderer.js
@@ -16,14 +16,17 @@ export default (
     selectableType: string,
     extensionsWhitelist: string[],
     hasHitSelectionLimit: boolean,
+    selected: Array<BoxItem>,
 ) => ({ rowData }: { rowData: BoxItem }) => {
+    const { id, type } = rowData;
     const itemCanSetShareAccess = getProp(rowData, 'permissions.can_set_share_access', false);
+    const isSelected = !!selected.find(sel => sel.id === id && sel.type === type);
 
     if (
         !canSetShareAccess ||
         !itemCanSetShareAccess ||
-        !isRowSelectable(selectableType, extensionsWhitelist, hasHitSelectionLimit, rowData) ||
-        !rowData.selected
+        !isRowSelectable(selectableType, extensionsWhitelist, hasHitSelectionLimit, rowData, isSelected) ||
+        !isSelected
     ) {
         return <span />;
     }

From b9139502e971142aeab91b12ec7ac253086b681d Mon Sep 17 00:00:00 2001
From: Chris Davies <cgrdavies@gmail.com>
Date: Thu, 7 Nov 2019 14:39:02 -0500
Subject: [PATCH 2/4] fix(content-picker): factor out commonalities

---
 src/elements/content-picker/ContentPicker.js      |  7 +++----
 src/elements/content-picker/ItemList.js           |  8 ++++----
 .../content-picker/itemSelectionHelper.js         | 15 +++++++++++++++
 .../content-picker/selectionCellRenderer.js       |  5 +++--
 .../content-picker/shareAccessCellRenderer.js     |  4 ++--
 5 files changed, 27 insertions(+), 12 deletions(-)
 create mode 100644 src/elements/content-picker/itemSelectionHelper.js

diff --git a/src/elements/content-picker/ContentPicker.js b/src/elements/content-picker/ContentPicker.js
index 4db5fce0ce..393e1f9249 100644
--- a/src/elements/content-picker/ContentPicker.js
+++ b/src/elements/content-picker/ContentPicker.js
@@ -19,6 +19,7 @@ import Internationalize from '../common/Internationalize';
 import makeResponsive from '../common/makeResponsive';
 import Pagination from '../common/pagination/Pagination';
 import { isFocusableElement, isInputElement, focus } from '../../utils/dom';
+import { isSelected, containsItem } from './itemSelectionHelper';
 import API from '../../api';
 import Content from './Content';
 import Footer from './Footer';
@@ -781,10 +782,8 @@ class ContentPicker extends Component<Props, State> {
         const newSelected: Array<BoxItem> = isSingleFileSelection ? [] : [...selected];
         const selectedCount: number = newSelected.length;
         const hasHitSelectionLimit: boolean = selectedCount === maxSelectable;
-        const existingIndex: number = newSelected.findIndex(sel => sel.id === id && sel.type === type);
-        const isAlreadySelected =
-            existingIndex !== -1 ||
-            (isSingleFileSelection && selected[0] && selected[0].id === id && selected[0].type === type);
+        const existingIndex: number = newSelected.findIndex(containsItem(item));
+        const isAlreadySelected = existingIndex !== -1 || (isSingleFileSelection && isSelected(item, selected));
         const itemCanSetShareAccess = getProp(item, 'permissions.can_set_share_access', false);
 
         if (isAlreadySelected) {
diff --git a/src/elements/content-picker/ItemList.js b/src/elements/content-picker/ItemList.js
index c54b337549..e1f7c88c44 100644
--- a/src/elements/content-picker/ItemList.js
+++ b/src/elements/content-picker/ItemList.js
@@ -15,6 +15,7 @@ import { isFocusableElement, focus } from '../../utils/dom';
 import shareAccessCellRenderer from './shareAccessCellRenderer';
 import selectionCellRenderer from './selectionCellRenderer';
 import isRowSelectable from './cellRendererHelper';
+import { isSelected as isRowSelected } from './itemSelectionHelper';
 import { VIEW_SELECTED, FIELD_NAME, FIELD_ID, FIELD_SHARED_LINK, TYPE_FOLDER } from '../../constants';
 
 import './ItemList.scss';
@@ -84,8 +85,8 @@ const ItemList = ({
             return '';
         }
 
-        const { id: itemId, type } = items[index];
-        const isSelected = !!selected.find(sel => sel.id === itemId && sel.type === type);
+        const { type } = items[index];
+        const isSelected = !!isRowSelected(items[index], selected);
         const isSelectable = isRowSelectable(
             selectableType,
             extensionsWhitelist,
@@ -109,8 +110,7 @@ const ItemList = ({
         index: number,
         rowData: BoxItem,
     }) => {
-        const { id: itemId, type } = rowData;
-        const isSelected = !!selected.find(sel => sel.id === itemId && sel.type === type);
+        const isSelected = !!isRowSelected(rowData, selected);
 
         // If the click is happening on a clickable element on the item row, ignore row selection
         if (
diff --git a/src/elements/content-picker/itemSelectionHelper.js b/src/elements/content-picker/itemSelectionHelper.js
new file mode 100644
index 0000000000..07fd285f29
--- /dev/null
+++ b/src/elements/content-picker/itemSelectionHelper.js
@@ -0,0 +1,15 @@
+/**
+ * @flow
+ * @file Functions to determine whether a given item is selected.
+ * @author Box
+ */
+
+export function containsItem(item: BoxItem) {
+    const { id, type } = item;
+
+    return (selected: BoxItem) => selected.id === id && selected.type === type;
+}
+
+export function isSelected(item: BoxItem, selectedItems: Array<BoxItem>) {
+    return !!selectedItems.find(containsItem(item));
+}
diff --git a/src/elements/content-picker/selectionCellRenderer.js b/src/elements/content-picker/selectionCellRenderer.js
index 13cf547128..43762dda8c 100644
--- a/src/elements/content-picker/selectionCellRenderer.js
+++ b/src/elements/content-picker/selectionCellRenderer.js
@@ -8,6 +8,7 @@ import React from 'react';
 import Checkbox from '../../components/checkbox';
 import RadioButton from '../../components/radio/RadioButton';
 import isRowSelectable from './cellRendererHelper';
+import { isSelected as isRowSelected } from './itemSelectionHelper';
 
 export default (
     onItemSelect: (rowData: BoxItem) => {},
@@ -17,9 +18,9 @@ export default (
     isRadio: boolean,
     selected: Array<BoxItem>,
 ): (({ rowData: BoxItem }) => {}) => ({ rowData }: { rowData: BoxItem }) => {
-    const { name = '', id, type } = rowData;
+    const { name = '' } = rowData;
     const Component = isRadio ? RadioButton : Checkbox;
-    const isSelected = !!selected.find(sel => sel.id === id && sel.type === type);
+    const isSelected = !!isRowSelected(rowData, selected);
 
     if (!isRowSelectable(selectableType, extensionsWhitelist, hasHitSelectionLimit, rowData, isSelected)) {
         return <span />;
diff --git a/src/elements/content-picker/shareAccessCellRenderer.js b/src/elements/content-picker/shareAccessCellRenderer.js
index f6ccbecadb..bdf562a449 100644
--- a/src/elements/content-picker/shareAccessCellRenderer.js
+++ b/src/elements/content-picker/shareAccessCellRenderer.js
@@ -8,6 +8,7 @@ import React from 'react';
 import getProp from 'lodash/get';
 import ShareAccessSelect from '../common/share-access-select';
 import isRowSelectable from './cellRendererHelper';
+import { isSelected as isRowSelected } from './itemSelectionHelper';
 import LoadingIndicator from '../../components/loading-indicator';
 
 export default (
@@ -18,9 +19,8 @@ export default (
     hasHitSelectionLimit: boolean,
     selected: Array<BoxItem>,
 ) => ({ rowData }: { rowData: BoxItem }) => {
-    const { id, type } = rowData;
     const itemCanSetShareAccess = getProp(rowData, 'permissions.can_set_share_access', false);
-    const isSelected = !!selected.find(sel => sel.id === id && sel.type === type);
+    const isSelected = isRowSelected(rowData, selected);
 
     if (
         !canSetShareAccess ||

From 4e67b38c29490b3f1076c30fc508274a0e1d4691 Mon Sep 17 00:00:00 2001
From: Chris Davies <cgrdavies@gmail.com>
Date: Thu, 7 Nov 2019 14:55:52 -0500
Subject: [PATCH 3/4] fix(content-picker): fix linting in checkboxcellrenderer

---
 src/elements/content-picker/checkboxCellRenderer.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/elements/content-picker/checkboxCellRenderer.js b/src/elements/content-picker/checkboxCellRenderer.js
index 2ebf71d350..cd247377bb 100644
--- a/src/elements/content-picker/checkboxCellRenderer.js
+++ b/src/elements/content-picker/checkboxCellRenderer.js
@@ -13,10 +13,11 @@ export default (
     selectableType: string,
     extensionsWhitelist: string[],
     hasHitSelectionLimit: boolean,
+    isSelected: boolean,
 ): Function => ({ rowData }: { rowData: BoxItem }) => {
     const { name, selected = false } = rowData;
 
-    if (!isRowSelectable(selectableType, extensionsWhitelist, hasHitSelectionLimit, rowData)) {
+    if (!isRowSelectable(selectableType, extensionsWhitelist, hasHitSelectionLimit, rowData, isSelected)) {
         return <span />;
     }
 

From 5a28b5258188f3ab0c6e39e9b564e8d366ba3130 Mon Sep 17 00:00:00 2001
From: Chris Davies <cgrdavies@gmail.com>
Date: Wed, 13 Nov 2019 14:18:24 -0500
Subject: [PATCH 4/4] fix(content-picker): Pr Feedback

---
 src/elements/content-picker/ContentPicker.js  | 36 ++++++++++++++++---
 src/elements/content-picker/ItemList.js       |  4 +--
 .../content-picker/itemSelectionHelper.js     |  6 ++--
 .../content-picker/selectionCellRenderer.js   |  2 +-
 4 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/src/elements/content-picker/ContentPicker.js b/src/elements/content-picker/ContentPicker.js
index 393e1f9249..39180be423 100644
--- a/src/elements/content-picker/ContentPicker.js
+++ b/src/elements/content-picker/ContentPicker.js
@@ -855,7 +855,7 @@ class ContentPicker extends Component<Props, State> {
         if (!item[FIELD_SHARED_LINK]) {
             this.changeShareAccess(null, item);
         } else {
-            // if shared link already exists, update the collection in state
+            // if shared link already exists, update the current collection
             this.updateItemInCollection(item);
         }
     };
@@ -896,17 +896,45 @@ class ContentPicker extends Component<Props, State> {
      * @returns {void}
      */
     updateItemInCollection = (item: BoxItem) => {
-        const { currentCollection } = this.state;
-        const { items = [] } = currentCollection;
+        const { currentCollection, selected: selectedItems = [] } = this.state;
+        const { items: collectionItems = [] } = currentCollection;
         const newState = {
             currentCollection: {
                 ...currentCollection,
-                items: items.map(collectionItem => (collectionItem.id === item.id ? item : collectionItem)),
+                items: this.addUpdatedItemToCollection(item, collectionItems),
             },
+            selected: this.addUpdatedItemToCollection(item, selectedItems),
         };
         this.setState(newState);
     };
 
+    /**
+     * Inserts a given item into a collection of box items
+     * if a matching item is found in the collection.
+     *
+     * @private
+     * @param {BoxItem} item - the item that will be inserted into the collection
+     * @param {Array<BoxItem>} collection - the collection of items to be searched for a match
+     * @return {boolean}
+     */
+
+    addUpdatedItemToCollection = (item: BoxItem, collection: Array<BoxItem>): Array<BoxItem> => {
+        return collection.map(collectionItem => (this.itemsMatch(collectionItem, item) ? item : collectionItem));
+    };
+
+    /**
+     * Determines if two items have the same id and type
+     *
+     * @private
+     * @param {BoxItem} item1 - the first item to compare
+     * @param {BoxItem} item2 - the second item to compare
+     * @return {boolean}
+     */
+
+    itemsMatch = (item1: BoxItem, item2: BoxItem): boolean => {
+        return item1.id === item2.id && item1.type === item2.type;
+    };
+
     /**
      * Chages the sort by and sort direction
      *
diff --git a/src/elements/content-picker/ItemList.js b/src/elements/content-picker/ItemList.js
index e1f7c88c44..d715e76207 100644
--- a/src/elements/content-picker/ItemList.js
+++ b/src/elements/content-picker/ItemList.js
@@ -86,7 +86,7 @@ const ItemList = ({
         }
 
         const { type } = items[index];
-        const isSelected = !!isRowSelected(items[index], selected);
+        const isSelected = isRowSelected(items[index], selected);
         const isSelectable = isRowSelectable(
             selectableType,
             extensionsWhitelist,
@@ -110,7 +110,7 @@ const ItemList = ({
         index: number,
         rowData: BoxItem,
     }) => {
-        const isSelected = !!isRowSelected(rowData, selected);
+        const isSelected = isRowSelected(rowData, selected);
 
         // If the click is happening on a clickable element on the item row, ignore row selection
         if (
diff --git a/src/elements/content-picker/itemSelectionHelper.js b/src/elements/content-picker/itemSelectionHelper.js
index 07fd285f29..3fca109eb7 100644
--- a/src/elements/content-picker/itemSelectionHelper.js
+++ b/src/elements/content-picker/itemSelectionHelper.js
@@ -4,12 +4,14 @@
  * @author Box
  */
 
-export function containsItem(item: BoxItem) {
+function containsItem(item: BoxItem): (selected: BoxItem) => boolean {
     const { id, type } = item;
 
     return (selected: BoxItem) => selected.id === id && selected.type === type;
 }
 
-export function isSelected(item: BoxItem, selectedItems: Array<BoxItem>) {
+function isSelected(item: BoxItem, selectedItems: Array<BoxItem>): boolean {
     return !!selectedItems.find(containsItem(item));
 }
+
+export { containsItem, isSelected };
diff --git a/src/elements/content-picker/selectionCellRenderer.js b/src/elements/content-picker/selectionCellRenderer.js
index 43762dda8c..baccde5f05 100644
--- a/src/elements/content-picker/selectionCellRenderer.js
+++ b/src/elements/content-picker/selectionCellRenderer.js
@@ -20,7 +20,7 @@ export default (
 ): (({ rowData: BoxItem }) => {}) => ({ rowData }: { rowData: BoxItem }) => {
     const { name = '' } = rowData;
     const Component = isRadio ? RadioButton : Checkbox;
-    const isSelected = !!isRowSelected(rowData, selected);
+    const isSelected = isRowSelected(rowData, selected);
 
     if (!isRowSelectable(selectableType, extensionsWhitelist, hasHitSelectionLimit, rowData, isSelected)) {
         return <span />;