Skip to content

Commit db2b0da

Browse files
author
Conrad Chan
authored
feat(annotations): Integrate annotation api with Feed (#2104)
1 parent 35cca4a commit db2b0da

File tree

10 files changed

+222
-25
lines changed

10 files changed

+222
-25
lines changed

src/__mocks__/annotations.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/* eslint-disable @typescript-eslint/camelcase */
2+
export const annotations = [
3+
{ id: 'anno_1', target: { shape: { height: 10, width: 10, x: 10, y: 10, type: 'rect' }, type: 'region' } },
4+
{ id: 'anno_2', target: { shape: { height: 20, width: 20, x: 20, y: 20, type: 'rect' }, type: 'region' } },
5+
{ id: 'anno_3', target: { shape: { height: 30, width: 30, x: 30, y: 30, type: 'rect' }, type: 'region' } },
6+
];
7+
8+
export const scale = 1;
9+
10+
export const rect = {
11+
type: 'rect' as const,
12+
height: 10,
13+
width: 10,
14+
x: 10,
15+
y: 10,
16+
};
17+
18+
export const target = {
19+
id: 'target_1',
20+
location: {
21+
type: 'page' as const,
22+
value: 1,
23+
},
24+
shape: rect,
25+
type: 'region' as const,
26+
};
27+
28+
export const user = {
29+
id: '1',
30+
login: 'johndoe',
31+
name: 'John Doe',
32+
type: 'user' as const,
33+
};
34+
35+
export const annotation = {
36+
created_at: '2020-09-20T00:00:00Z',
37+
created_by: user,
38+
description: {
39+
message: 'mock annotation',
40+
type: 'reply' as const,
41+
},
42+
file_version: {
43+
id: '123',
44+
type: 'file_version' as const,
45+
},
46+
id: '123',
47+
modified_at: '2020-09-20T00:00:00Z',
48+
modified_by: user,
49+
permissions: {
50+
can_delete: true,
51+
can_edit: true,
52+
},
53+
target,
54+
type: 'annotation' as const,
55+
};

src/api/Annotations.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// @flow
22
import merge from 'lodash/merge';
33
import MarkerBasedApi from './MarkerBasedAPI';
4-
import type { Annotation, NewAnnotation } from '../common/types/annotations';
4+
import type { Annotation, Annotations as AnnotationsType, NewAnnotation } from '../common/types/annotations';
55
import type { ElementsXhrError } from '../common/types/api';
66

77
export default class Annotations extends MarkerBasedApi {
@@ -74,7 +74,7 @@ export default class Annotations extends MarkerBasedApi {
7474
getAnnotations(
7575
fileId: string,
7676
fileVersionId?: string,
77-
successCallback: (annotations: Annotation[]) => void,
77+
successCallback: (annotations: AnnotationsType) => void,
7878
errorCallback: (e: ElementsXhrError, code: string) => void,
7979
limit?: number,
8080
shouldFetchAll?: boolean,

src/api/Feed.js

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import commonMessages from '../elements/common/messages';
1212
import messages from './messages';
1313
import { sortFeedItems } from '../utils/sorter';
1414
import Base from './Base';
15+
import AnnotationsAPI from './Annotations';
1516
import CommentsAPI from './Comments';
1617
import VersionsAPI from './Versions';
1718
import TasksNewAPI from './tasks/TasksNew';
@@ -56,6 +57,7 @@ import type {
5657
} from '../common/types/core';
5758
import type {
5859
Annotation,
60+
Annotations,
5961
AppActivityItems,
6062
Comment,
6163
Comments,
@@ -76,6 +78,11 @@ type FeedItemsCache = {
7678
type ErrorCallback = (e: ElementsXhrError, code: string, contextInfo?: Object) => void;
7779

7880
class Feed extends Base {
81+
/**
82+
* @property {AnnotationsAPI}
83+
*/
84+
annotationsAPI: AnnotationsAPI;
85+
7986
/**
8087
* @property {VersionsAPI}
8188
*/
@@ -207,7 +214,10 @@ class Feed extends Base {
207214
successCallback: Function,
208215
errorCallback: (feedItems: FeedItems) => void,
209216
onError: ErrorCallback,
210-
{ shouldShowAppActivity = false }: { shouldShowAppActivity?: boolean } = {},
217+
{
218+
shouldShowAnnotations = false,
219+
shouldShowAppActivity = false,
220+
}: { shouldShowAnnotations?: boolean, shouldShowAppActivity?: boolean } = {},
211221
): void {
212222
const { id, permissions = {} } = file;
213223
const cachedItems = this.getCachedItems(id);
@@ -227,26 +237,44 @@ class Feed extends Base {
227237
this.file = file;
228238
this.hasError = false;
229239
this.errorCallback = onError;
240+
const annotationsPromise = shouldShowAnnotations ? this.fetchAnnotations() : Promise.resolve();
230241
const versionsPromise = this.fetchVersions();
231242
const currentVersionPromise = this.fetchCurrentVersion();
232243
const commentsPromise = this.fetchComments(permissions);
233244
const tasksPromise = this.fetchTasksNew();
234245
const appActivityPromise = shouldShowAppActivity ? this.fetchAppActivity(permissions) : Promise.resolve();
235246

236-
Promise.all([versionsPromise, currentVersionPromise, commentsPromise, tasksPromise, appActivityPromise]).then(
237-
([versions: ?FileVersions, currentVersion: ?BoxItemVersion, ...feedItems]) => {
238-
const versionsWithCurrent = this.versionsAPI.addCurrentVersion(currentVersion, versions, this.file);
239-
const sortedFeedItems = sortFeedItems(versionsWithCurrent, ...feedItems);
240-
if (!this.isDestroyed()) {
241-
this.setCachedItems(id, sortedFeedItems);
242-
if (this.hasError) {
243-
errorCallback(sortedFeedItems);
244-
} else {
245-
successCallback(sortedFeedItems);
246-
}
247+
Promise.all([
248+
versionsPromise,
249+
currentVersionPromise,
250+
commentsPromise,
251+
tasksPromise,
252+
appActivityPromise,
253+
annotationsPromise,
254+
]).then(([versions: ?FileVersions, currentVersion: ?BoxItemVersion, ...feedItems]) => {
255+
const versionsWithCurrent = this.versionsAPI.addCurrentVersion(currentVersion, versions, this.file);
256+
const sortedFeedItems = sortFeedItems(versionsWithCurrent, ...feedItems);
257+
if (!this.isDestroyed()) {
258+
this.setCachedItems(id, sortedFeedItems);
259+
if (this.hasError) {
260+
errorCallback(sortedFeedItems);
261+
} else {
262+
successCallback(sortedFeedItems);
247263
}
248-
},
249-
);
264+
}
265+
});
266+
}
267+
268+
fetchAnnotations(): Promise<?Annotations> {
269+
this.annotationsAPI = new AnnotationsAPI(this.options);
270+
return new Promise(resolve => {
271+
this.annotationsAPI.getAnnotations(
272+
this.file.id,
273+
undefined,
274+
resolve,
275+
this.fetchFeedItemErrorCallback.bind(this, resolve),
276+
);
277+
});
250278
}
251279

252280
/**
@@ -1340,6 +1368,11 @@ class Feed extends Base {
13401368
destroy() {
13411369
super.destroy();
13421370

1371+
if (this.annotationsAPI) {
1372+
this.annotationsAPI.destroy();
1373+
delete this.annotationsAPI;
1374+
}
1375+
13431376
if (this.commentsAPI) {
13441377
this.commentsAPI.destroy();
13451378
delete this.commentsAPI;

src/api/__tests__/Feed.test.js

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import * as sorter from '../../utils/sorter';
44
import * as error from '../../utils/error';
55
import { IS_ERROR_DISPLAYED, TASK_NEW_NOT_STARTED, TASK_MAX_GROUP_ASSIGNEES } from '../../constants';
66
import Feed from '../Feed';
7+
import { annotation as mockAnnotation } from '../../__mocks__/annotations';
78

89
const mockTask = {
910
created_by: {
@@ -98,6 +99,12 @@ const versionsWithCurrent = {
9899
entries: [mockCurrentVersion, mockFirstVersion, deleted_version],
99100
};
100101

102+
const annotations = {
103+
entries: [mockAnnotation],
104+
limit: 1000,
105+
next_marker: null,
106+
};
107+
101108
jest.mock('lodash/uniqueId', () => () => 'uniqueId');
102109

103110
const mockCreateTask = jest.fn().mockImplementation(({ successCallback }) => {
@@ -237,6 +244,12 @@ jest.mock('../Versions', () => {
237244
}));
238245
});
239246

247+
jest.mock('../Annotations', () =>
248+
jest.fn().mockImplementation(() => ({
249+
getAnnotations: jest.fn(),
250+
})),
251+
);
252+
240253
const MOCK_APP_ACTIVITY_ITEM = {
241254
activity_template: {
242255
id: '1',
@@ -380,6 +393,7 @@ describe('api/Feed', () => {
380393
...tasks.entries,
381394
...comments.entries,
382395
...appActivities.entries,
396+
...annotations.entries,
383397
];
384398
let successCb;
385399
let errorCb;
@@ -390,6 +404,7 @@ describe('api/Feed', () => {
390404
feed.fetchTasksNew = jest.fn().mockResolvedValue(tasks);
391405
feed.fetchComments = jest.fn().mockResolvedValue(comments);
392406
feed.fetchAppActivity = jest.fn().mockReturnValue(appActivities);
407+
feed.fetchAnnotations = jest.fn().mockReturnValue(annotations);
393408
feed.setCachedItems = jest.fn();
394409
feed.versionsAPI = {
395410
getVersion: jest.fn().mockReturnValue(versions),
@@ -406,10 +421,19 @@ describe('api/Feed', () => {
406421
});
407422

408423
test('should get feed items, sort, save to cache, and call the success callback', done => {
409-
feed.feedItems(file, false, successCb, errorCb, jest.fn(), { shouldShowAppActivity: true });
424+
feed.feedItems(file, false, successCb, errorCb, jest.fn(), {
425+
shouldShowAnnotations: true,
426+
shouldShowAppActivity: true,
427+
});
410428
setImmediate(() => {
411429
expect(feed.versionsAPI.addCurrentVersion).toHaveBeenCalledWith(mockCurrentVersion, versions, file);
412-
expect(sorter.sortFeedItems).toHaveBeenCalledWith(versionsWithCurrent, comments, tasks, appActivities);
430+
expect(sorter.sortFeedItems).toHaveBeenCalledWith(
431+
versionsWithCurrent,
432+
comments,
433+
tasks,
434+
appActivities,
435+
annotations,
436+
);
413437
expect(feed.setCachedItems).toHaveBeenCalledWith(file.id, sortedItems);
414438
expect(successCb).toHaveBeenCalledWith(sortedItems);
415439
done();
@@ -445,6 +469,22 @@ describe('api/Feed', () => {
445469
});
446470
});
447471

472+
test('should use the annotations api if shouldShowannotations is true', done => {
473+
feed.feedItems(file, false, successCb, errorCb, errorCb, { shouldShowAnnotations: true });
474+
setImmediate(() => {
475+
expect(feed.fetchAnnotations).toHaveBeenCalled();
476+
done();
477+
});
478+
});
479+
480+
test('should not use the annotations api if shouldShowannotations is false', done => {
481+
feed.feedItems(file, false, successCb, errorCb, errorCb, { shouldShowAnnotations: false });
482+
setImmediate(() => {
483+
expect(feed.fetchAnnotations).not.toHaveBeenCalled();
484+
done();
485+
});
486+
});
487+
448488
test('should not call success or error callback if it is destroyed', done => {
449489
feed.isDestroyed = jest.fn().mockReturnValue(true);
450490
feed.feedItems(file, false, successCb, errorCb);
@@ -483,6 +523,19 @@ describe('api/Feed', () => {
483523
});
484524
});
485525

526+
describe('fetchAnnotations()', () => {
527+
beforeEach(() => {
528+
feed.file = file;
529+
feed.fetchFeedItemErrorCallback = jest.fn();
530+
});
531+
532+
test('should return a promise and call the annotations api', () => {
533+
const annotationItems = feed.fetchAnnotations();
534+
expect(annotationItems instanceof Promise).toBeTruthy();
535+
expect(feed.annotationsAPI.getAnnotations).toBeCalled();
536+
});
537+
});
538+
486539
describe('fetchComments()', () => {
487540
beforeEach(() => {
488541
feed.file = file;
@@ -1306,15 +1359,20 @@ describe('api/Feed', () => {
13061359
});
13071360

13081361
describe('destroy()', () => {
1362+
let annotationFn;
13091363
let commentFn;
13101364
let versionFn;
13111365
let taskFn;
13121366

13131367
beforeEach(() => {
1368+
annotationFn = jest.fn();
13141369
commentFn = jest.fn();
13151370
versionFn = jest.fn();
13161371
taskFn = jest.fn();
13171372

1373+
feed.annotationsAPI = {
1374+
destroy: annotationFn,
1375+
};
13181376
feed.tasksNewAPI = {
13191377
destroy: taskFn,
13201378
};
@@ -1331,6 +1389,7 @@ describe('api/Feed', () => {
13311389
expect(versionFn).toBeCalled();
13321390
expect(commentFn).toBeCalled();
13331391
expect(taskFn).toBeCalled();
1392+
expect(annotationFn).toBeCalled();
13341393
});
13351394
});
13361395

src/common/types/annotations.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ export type Annotation = {
4646
type: 'annotation',
4747
};
4848

49+
export type Annotations = {
50+
entries: Array<Annotation>,
51+
limit: number,
52+
next_marker: string | null,
53+
};
54+
4955
export type NewReply = {
5056
message: string,
5157
type: 'reply',

src/common/types/feed.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// @flow strict
22
import type { MessageDescriptor } from 'react-intl';
33
import type { BoxItemPermission, BoxItemVersion, Reply, User } from './core';
4-
import type { Annotation } from './annotations';
4+
import type { Annotation, Annotations } from './annotations';
55

66
// Feed item types that can receive deeplinks inline in the feed
77
type FocusableFeedItemType = 'task' | 'comment' | 'annotation';
@@ -113,6 +113,7 @@ export type {
113113
ActionItemError,
114114
ActivityTemplateItem,
115115
Annotation,
116+
Annotations,
116117
AppActivityAPIItem,
117118
AppActivityAPIItems,
118119
AppActivityItem,

src/elements/content-sidebar/ActivitySidebar.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,14 +408,15 @@ class ActivitySidebar extends React.PureComponent<Props, State> {
408408
fetchFeedItems(shouldRefreshCache: boolean = false, shouldDestroy: boolean = false) {
409409
const { file, api, features } = this.props;
410410
const shouldShowAppActivity = isFeatureEnabled(features, 'activityFeed.appActivity.enabled');
411+
const shouldShowAnnotations = isFeatureEnabled(features, 'activityFeed.annotations.enabled');
411412

412413
api.getFeedAPI(shouldDestroy).feedItems(
413414
file,
414415
shouldRefreshCache,
415416
this.fetchFeedItemsSuccessCallback,
416417
this.fetchFeedItemsErrorCallback,
417418
this.errorCallback,
418-
{ shouldShowAppActivity },
419+
{ shouldShowAnnotations, shouldShowAppActivity },
419420
);
420421
}
421422

0 commit comments

Comments
 (0)