Fix: Don't render until AFTER all are fetched (#311)

box · Dec 10, 2018 · 0e3fbff · 0e3fbff
1 parent b50d72b
commit 0e3fbff
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 55 deletions.
diff --git a/src/api/API.js b/src/api/API.js
@@ -3,7 +3,7 @@ import EventEmitter from 'events';
 import axios from 'axios';
 
 import { getHeaders } from '../util';
-import { PLACEHOLDER_USER, ANNOTATOR_EVENT, ERROR_TYPE } from '../constants';
+import { PLACEHOLDER_USER, ANNOTATOR_EVENT } from '../constants';
 
 class API extends EventEmitter {
     /** @property {string} */
@@ -95,7 +95,7 @@ class API extends EventEmitter {
      */
     errorHandler = (error: $AxiosError): void => {
         this.emit(ANNOTATOR_EVENT.error, {
-            reason: ERROR_TYPE.auth,
+            reason: error.name,
             error: error.toString()
         });
     };

diff --git a/src/api/AnnotationAPI.js b/src/api/AnnotationAPI.js
@@ -1,6 +1,6 @@
 // @flow
 import API from './API';
-import { ANNOTATOR_EVENT, ERROR_TYPE } from '../constants';
+import { ERROR_TYPE } from '../constants';
 
 const HTTP_POST = 'POST';
 const HTTP_DELETE = 'DELETE';
@@ -57,10 +57,8 @@ class AnnotationAPI extends API {
     createSuccessHandler = (data: Object): CommentProps => {
         if (data.type === 'error') {
             const error = new Error('Could not create annotation');
-            this.emit(ANNOTATOR_EVENT.error, {
-                reason: ERROR_TYPE.create,
-                error: error.toString()
-            });
+            error.name = ERROR_TYPE.create;
+            this.errorHandler(error);
             return data;
         }
 
@@ -94,10 +92,8 @@ class AnnotationAPI extends API {
     deleteSuccessHandler = (data: Object, id: string) => {
         if (data.type === 'error' || !data.id) {
             const error = new Error(`Could not delete annotation with ID ${id}`);
-            this.emit(ANNOTATOR_EVENT.error, {
-                reason: ERROR_TYPE.delete,
-                error: error.toString()
-            });
+            error.name = ERROR_TYPE.delete;
+            this.errorHandler(error);
         }
 
         return data;

diff --git a/src/api/FileVersionAPI.js b/src/api/FileVersionAPI.js
@@ -1,6 +1,6 @@
 // @flow
 import API from './API';
-import { ANNOTATOR_EVENT, ERROR_TYPE, PLACEHOLDER_USER, TYPES } from '../constants';
+import { ERROR_TYPE, PLACEHOLDER_USER, TYPES } from '../constants';
 
 const FIELDS = 'item,thread,details,message,created_by,created_at,modified_at,permissions';
 
@@ -46,7 +46,7 @@ class FileVersionAPI extends API {
      */
     fetchVersionAnnotations(version: string): Promise<AnnotationMap> {
         this.fileVersionId = version;
-        
+
         // $FlowFixMe
         return this.fetchFromMarker({ version, fields: FIELDS }).then(this.createAnnotationMap);
     }
@@ -77,11 +77,17 @@ class FileVersionAPI extends API {
      * @param {Params} queryParams - Key-value map of querystring params
      * @return {void}
      */
-    successHandler = (data: Data, queryParams: Params): void => {
+    successHandler = (data: Data, queryParams: Params): Promise<any> => {
+        if (data.type === 'error' || !Array.isArray(data.entries)) {
+            const error = new Error(`Could not read annotations from file version with ID ${this.fileVersionId}`);
+            error.name = ERROR_TYPE.read;
+            return Promise.reject(error);
+        }
+
         const entries = this.data ? this.data.entries : [];
         this.data = {
             ...data,
-            entries: entries.concat(data.entries)
+            entries: [...entries, ...data.entries]
         };
 
         const { next_marker } = data;
@@ -91,16 +97,10 @@ class FileVersionAPI extends API {
                 marker: next_marker
             };
 
-            this.fetchFromMarker(params);
+            return this.fetchFromMarker(params);
         }
 
-        if (data.type === 'error' || !Array.isArray(data.entries)) {
-            const error = new Error(`Could not read annotations from file version with ID ${this.fileVersionId}`);
-            this.emit(ANNOTATOR_EVENT.error, {
-                reason: ERROR_TYPE.read,
-                error: error.toString()
-            });
-        }
+        return Promise.resolve(this.data);
     };
 
     /**

diff --git a/src/api/__tests__/API-test.js b/src/api/__tests__/API-test.js
@@ -1,6 +1,6 @@
 /* eslint-disable no-unused-expressions */
 import API from '../API';
-import { ANNOTATOR_EVENT, ERROR_TYPE } from '../../constants';
+import { ANNOTATOR_EVENT } from '../../constants';
 
 const API_HOST = 'https://app.box.com/api';
 let api;
@@ -49,7 +49,7 @@ describe('api/API', () => {
 
             api.errorHandler(error);
             expect(api.emit).toBeCalledWith(ANNOTATOR_EVENT.error, {
-                reason: ERROR_TYPE.auth,
+                reason: error.name,
                 error: error.toString()
             });
         });

diff --git a/src/api/__tests__/AnnotationAPI-test.js b/src/api/__tests__/AnnotationAPI-test.js
@@ -1,6 +1,6 @@
 /* eslint-disable no-unused-expressions */
 import AnnotationAPI from '../AnnotationAPI';
-import { ANNOTATOR_EVENT, ERROR_TYPE } from '../../constants';
+import { ERROR_TYPE } from '../../constants';
 
 const API_HOST = 'https://app.box.com/api';
 const HTTP_POST = 'POST';
@@ -24,6 +24,7 @@ describe('api/AnnotationAPI', () => {
             token: 'someToken'
         });
         api.axios = jest.fn().mockReturnValue(promise);
+        api.errorHandler = jest.fn();
         api.axiosSource = { token: '123abc' };
         api.headers = {};
     });
@@ -67,14 +68,11 @@ describe('api/AnnotationAPI', () => {
 
     describe('createSuccessHandler()', () => {
         it('should emit an error event', () => {
-            api.emit = jest.fn();
             const error = new Error('Could not create annotation');
+            error.name = ERROR_TYPE.create;
 
             api.createSuccessHandler({ type: 'error' });
-            expect(api.emit).toBeCalledWith(ANNOTATOR_EVENT.error, {
-                reason: ERROR_TYPE.create,
-                error: error.toString()
-            });
+            expect(api.errorHandler).toBeCalledWith(error);
         });
 
         it('should return the created annotation', () => {
@@ -87,14 +85,11 @@ describe('api/AnnotationAPI', () => {
 
     describe('deleteSuccessHandler()', () => {
         it('should emit an error event', () => {
-            api.emit = jest.fn();
             const error = new Error('Could not delete annotation with ID 123');
+            error.name === ERROR_TYPE.delete;
 
             api.deleteSuccessHandler({ type: 'error' }, '123');
-            expect(api.emit).toBeCalledWith(ANNOTATOR_EVENT.error, {
-                reason: ERROR_TYPE.delete,
-                error: error.toString()
-            });
+            expect(api.errorHandler).toBeCalledWith(error);
         });
 
         it('should return the deleted annotation', () => {

diff --git a/src/api/__tests__/FileVersionAPI-test.js b/src/api/__tests__/FileVersionAPI-test.js
@@ -1,6 +1,6 @@
 /* eslint-disable no-unused-expressions */
 import FileVersionAPI from '../FileVersionAPI';
-import { ANNOTATOR_EVENT, ERROR_TYPE } from '../../constants';
+import { ERROR_TYPE } from '../../constants';
 
 const API_HOST = 'https://app.box.com/api';
 
@@ -55,34 +55,34 @@ describe('api/FileVersionAPI', () => {
 
     describe('successHandler()', () => {
         beforeEach(() => {
-            api.fetchFromMarker = jest.fn();
+            api.fetchFromMarker = jest.fn().mockReturnValue(Promise.resolve());
             api.emit = jest.fn();
         });
 
         it('should call fetch the remaining annotations if the version has more to fetch', () => {
             api.data = { entries: [{}] };
-            api.successHandler({ entries: [{}, {}], next_marker: 'marker', limit: 1 });
-            expect(api.data.entries.length).toEqual(3);
-            expect(api.fetchFromMarker).toBeCalled();
+            api.successHandler({ entries: [{}, {}], next_marker: 'marker', limit: 1 }).then(() => {
+                expect(api.data.entries.length).toEqual(3);
+                expect(api.fetchFromMarker).toBeCalled();
+            });
         });
 
         it('should not call fetch if no more annotations need to be fetched', () => {
             api.data = { entries: [{}] };
-            api.successHandler({ entries: [{}, {}], limit: 1 });
-            expect(api.data.entries.length).toEqual(3);
-            expect(api.fetchFromMarker).not.toBeCalled();
+            api.successHandler({ entries: [{}, {}], limit: 1 }).then(() => {
+                expect(api.data.entries.length).toEqual(3);
+                expect(api.fetchFromMarker).not.toBeCalled();
+            });
         });
 
         it('should emit an error if the success response is of type error', () => {
             api.fileVersionId = 123;
-            api.successHandler({ type: 'error' });
-            const error = new Error(`Could not read annotations from file version with ID ${api.fileVersionId}`);
+            api.successHandler({ type: 'error' }).catch(() => {
+                const error = new Error(`Could not read annotations from file version with ID ${api.fileVersionId}`);
+                error.name = ERROR_TYPE.read;
 
-            expect(api.data.entries.length).toEqual(1);
-            expect(api.fetchFromMarker).not.toBeCalled();
-            expect(api.emit).toBeCalledWith(ANNOTATOR_EVENT.error, {
-                reason: ERROR_TYPE.read,
-                error: error.toString()
+                expect(api.data.entries.length).toEqual(1);
+                expect(api.fetchFromMarker).not.toBeCalled();
             });
         });
     });

diff --git a/src/constants.js b/src/constants.js
@@ -178,10 +178,10 @@ export const CREATE_EVENT = {
 };
 
 export const ERROR_TYPE = {
-    auth: 'authorization',
-    read: 'read',
-    create: 'create',
-    delete: 'delete'
+    auth: 'AuthorizationError',
+    read: 'ReadError',
+    create: 'CreateError',
+    delete: 'DeleteError'
 };
 
 export const POINT_ANNOTATION_ICON_HEIGHT = 31;