Fix: Make components work with immutable item entires (#196)

box · Mar 1, 2018 · e1005fa · e1005fa
1 parent 25208ab
commit e1005fa
Show file tree

Hide file tree

Showing 11 changed files with 205 additions and 212 deletions.
diff --git a/src/api/Folder.js b/src/api/Folder.js
@@ -213,19 +213,6 @@ class Folder extends Item {
         this.finish();
     };
 
-    /**
-     * Handles the folder fetch error
-     *
-     * @param {Error} error fetch error
-     * @return {void}
-     */
-    folderErrorHandler = (error: any): void => {
-        if (this.isDestroyed()) {
-            return;
-        }
-        this.errorCallback(error);
-    };
-
     /**
      * Does the network request for fetching a folder
      *
@@ -247,7 +234,7 @@ class Folder extends Item {
                 headers: { 'X-Rep-Hints': X_REP_HINTS }
             })
             .then(this.folderSuccessHandler)
-            .catch(this.folderErrorHandler);
+            .catch(this.errorHandler);
     }
 
     /**
@@ -359,7 +346,7 @@ class Folder extends Item {
                 }
             })
             .then(this.createSuccessHandler)
-            .catch(this.folderErrorHandler);
+            .catch(this.errorHandler);
     }
 
     /**

diff --git a/src/api/Item.js b/src/api/Item.js
@@ -53,13 +53,15 @@ class Item extends Base {
         if (this.isDestroyed()) {
             return;
         }
+
         const { response } = error;
+
         if (response) {
-            response.json().then(this.errorCallback);
-        } else if (error instanceof Error) {
-            this.errorCallback();
-            throw error;
+            this.errorCallback(response.data);
+            return;
         }
+
+        this.errorCallback();
     };
 
     /**

diff --git a/src/api/__tests__/Folder-test.js b/src/api/__tests__/Folder-test.js
@@ -143,14 +143,14 @@ describe('api/Folder', () => {
         });
         test('should make xhr to folder and call success callback', () => {
             folder.folderSuccessHandler = jest.fn();
-            folder.folderErrorHandler = jest.fn();
+            folder.errorHandler = jest.fn();
             folder.includePreviewFields = true;
             folder.xhr = {
                 get: jest.fn().mockReturnValueOnce(Promise.resolve('success'))
             };
             return folder.folderRequest().then(() => {
                 expect(folder.folderSuccessHandler).toHaveBeenCalledWith('success');
-                expect(folder.folderErrorHandler).not.toHaveBeenCalled();
+                expect(folder.errorHandler).not.toHaveBeenCalled();
                 expect(folder.xhr.get).toHaveBeenCalledWith({
                     url: 'https://api.box.com/2.0/folders/id',
                     params: {
@@ -167,14 +167,14 @@ describe('api/Folder', () => {
         test('should make xhr to folder and call error callback', () => {
             const error = new Error('error');
             folder.folderSuccessHandler = jest.fn();
-            folder.folderErrorHandler = jest.fn();
+            folder.errorHandler = jest.fn();
             folder.includePreviewFields = true;
             folder.includePreviewSidebarFields = true;
             folder.xhr = {
                 get: jest.fn().mockReturnValueOnce(Promise.reject(error))
             };
             return folder.folderRequest().then(() => {
-                expect(folder.folderErrorHandler).toHaveBeenCalledWith(error);
+                expect(folder.errorHandler).toHaveBeenCalledWith(error);
                 expect(folder.folderSuccessHandler).not.toHaveBeenCalled();
                 expect(folder.xhr.get).toHaveBeenCalledWith({
                     url: 'https://api.box.com/2.0/folders/id',
@@ -191,20 +191,6 @@ describe('api/Folder', () => {
         });
     });
 
-    describe('folderErrorHandler()', () => {
-        test('should not do anything if destroyed', () => {
-            folder.isDestroyed = jest.fn().mockReturnValueOnce(true);
-            folder.errorCallback = jest.fn();
-            folder.folderErrorHandler('foo');
-            expect(folder.errorCallback).not.toHaveBeenCalled();
-        });
-        test('should call error callback', () => {
-            folder.errorCallback = jest.fn();
-            folder.folderErrorHandler('foo');
-            expect(folder.errorCallback).toHaveBeenCalledWith('foo');
-        });
-    });
-
     describe('folderSuccessHandler()', () => {
         beforeEach(() => {
             item1 = {
@@ -628,13 +614,13 @@ describe('api/Folder', () => {
         });
         test('should make xhr to folder create and call success callback', () => {
             folder.createSuccessHandler = jest.fn();
-            folder.folderErrorHandler = jest.fn();
+            folder.errorHandler = jest.fn();
             folder.xhr = {
                 post: jest.fn().mockReturnValueOnce(Promise.resolve('success'))
             };
             return folder.folderCreateRequest('foo').then(() => {
                 expect(folder.createSuccessHandler).toHaveBeenCalledWith('success');
-                expect(folder.folderErrorHandler).not.toHaveBeenCalled();
+                expect(folder.errorHandler).not.toHaveBeenCalled();
                 expect(folder.xhr.post).toHaveBeenCalledWith({
                     url: `https://api.box.com/2.0/folders?fields=${getFieldsAsString()}`,
                     data: {
@@ -649,12 +635,12 @@ describe('api/Folder', () => {
         test('should make xhr to folder and call error callback', () => {
             const error = new Error('error');
             folder.createSuccessHandler = jest.fn();
-            folder.folderErrorHandler = jest.fn();
+            folder.errorHandler = jest.fn();
             folder.xhr = {
                 post: jest.fn().mockReturnValueOnce(Promise.reject(error))
             };
             return folder.folderCreateRequest('foo').then(() => {
-                expect(folder.folderErrorHandler).toHaveBeenCalledWith(error);
+                expect(folder.errorHandler).toHaveBeenCalledWith(error);
                 expect(folder.createSuccessHandler).not.toHaveBeenCalled();
                 expect(folder.xhr.post).toHaveBeenCalledWith({
                     url: `https://api.box.com/2.0/folders?fields=${getFieldsAsString()}`,

diff --git a/src/api/__tests__/Item-test.js b/src/api/__tests__/Item-test.js
@@ -39,24 +39,16 @@ describe('api/Item', () => {
             expect(item.errorCallback).not.toHaveBeenCalled();
         });
 
-        test('should not do anything if no response', () => {
+        test('should call error callback even when no response', () => {
             item.errorCallback = jest.fn();
             item.errorHandler('foo');
-            expect(item.errorCallback).not.toHaveBeenCalled();
-        });
-
-        test('should call error callback', () => {
-            const thenMock = jest.fn();
-            const json = () => ({
-                then: thenMock
-            });
-            item.errorHandler({ response: { json } });
-            expect(thenMock).toHaveBeenCalledWith(item.errorCallback);
+            expect(item.errorCallback).toHaveBeenCalled();
         });
 
-        test('should throw error when getting error event', () => {
+        test('should call error callback with response data', () => {
             item.errorCallback = jest.fn();
-            expect(item.errorHandler.bind(item, new Error())).toThrow(Error);
+            item.errorHandler({ response: { data: 'foo' } });
+            expect(item.errorCallback).toHaveBeenCalledWith('foo');
         });
     });