Skip to content

Commit 9296c73

Browse files
mickrmergify[bot]
andauthored
feat(annotations): add new annotation to sidebar (#2090)
* feat(annotations): add new annotation to sidebar * check for new action in annotatorState, start creation on create_start and update on create_end * Update annotatorState type * Update test * Add new method to Feed.js to handle annotation creation * Moved to using the request id from box-annotations * feat(annotations): add new annotation to sidebar * PR Feedback * feat(annotations): add new annotation to sidebar * fix un-needed move Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
1 parent bcc909e commit 9296c73

File tree

7 files changed

+169
-4
lines changed

7 files changed

+169
-4
lines changed

src/api/Feed.js

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,16 @@ import type {
5454
FileVersions,
5555
User,
5656
} from '../common/types/core';
57-
import type { Comment, Comments, Tasks, Task, FeedItem, FeedItems, AppActivityItems } from '../common/types/feed';
57+
import type {
58+
Annotation,
59+
AppActivityItems,
60+
Comment,
61+
Comments,
62+
FeedItem,
63+
FeedItems,
64+
Task,
65+
Tasks,
66+
} from '../common/types/feed';
5867

5968
const TASK_NEW_INITIAL_STATUS = TASK_NEW_NOT_STARTED;
6069
const TASK = 'task';
@@ -113,6 +122,38 @@ class Feed extends Base {
113122
this.taskLinksAPI = [];
114123
}
115124

125+
/**
126+
* Creates pending card on create_start action, then updates card on next call
127+
* @param {BoxItem} file - The file to which the annotation is assigned
128+
* @param {Object} currentUser - the user who performed the action
129+
* @param {Annotation} annotation - the current annotation to be created
130+
* @param {string} id - unique id for the incoming annotation
131+
* @param {boolean} isPending - indicates the current creation process of the annotation
132+
*/
133+
addAnnotation(file: BoxItem, currentUser: User, annotation: Annotation, id: string, isPending: boolean): void {
134+
if (!file.id) {
135+
throw getBadItemError();
136+
}
137+
138+
this.file = file;
139+
140+
// Add the pending interstitial card
141+
if (isPending) {
142+
const newAnnotation = {
143+
...annotation,
144+
created_by: currentUser,
145+
id,
146+
type: 'annotation',
147+
};
148+
149+
this.addPendingItem(this.file.id, currentUser, newAnnotation);
150+
151+
return;
152+
}
153+
// Create action has completed, so update the existing pending item
154+
this.updateFeedItem({ ...annotation, isPending: false }, id);
155+
}
156+
116157
/**
117158
* Creates a key for the cache
118159
*

src/api/__tests__/Feed.test.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,4 +1362,42 @@ describe('api/Feed', () => {
13621362
expect(feed.feedErrorCallback).toHaveBeenCalledWith(shouldDisplayError, e, code);
13631363
});
13641364
});
1365+
1366+
describe('addAnnotation()', () => {
1367+
beforeEach(() => {
1368+
feed.addPendingItem = jest.fn();
1369+
feed.updateFeedItem = jest.fn();
1370+
});
1371+
1372+
afterEach(() => {
1373+
jest.restoreAllMocks();
1374+
});
1375+
1376+
test('should throw if no file id', () => {
1377+
expect(() => feed.addAnnotation({})).toThrow('Bad box item!');
1378+
});
1379+
1380+
test('should add pending feedItem item if annotation event isPending', () => {
1381+
const expectedAnnotation = {
1382+
created_by: user,
1383+
id: '123',
1384+
type: 'annotation',
1385+
};
1386+
1387+
feed.addAnnotation(file, user, {}, '123', true);
1388+
expect(feed.addPendingItem).toBeCalledWith(file.id, user, expectedAnnotation);
1389+
expect(feed.updateFeedItem).not.toBeCalled();
1390+
});
1391+
1392+
test('should update feedItem if annotation event is not pending', () => {
1393+
const expectedAnnotation = {
1394+
isPending: false,
1395+
};
1396+
1397+
feed.addAnnotation(file, user, {}, '123', false);
1398+
1399+
expect(feed.updateFeedItem).toBeCalledWith(expectedAnnotation, '123');
1400+
expect(feed.addPendingItem).not.toBeCalled();
1401+
});
1402+
});
13651403
});

src/elements/common/annotator-context/__tests__/withAnnotations.test.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ describe('elements/common/annotator-context/withAnnotations', () => {
6565
activeAnnotationId: null,
6666
annotation: null,
6767
error: null,
68+
meta: null,
6869
});
6970
});
7071

@@ -99,6 +100,7 @@ describe('elements/common/annotator-context/withAnnotations', () => {
99100
annotation,
100101
meta: {
101102
status,
103+
requestId: '123',
102104
},
103105
error,
104106
};
@@ -111,6 +113,10 @@ describe('elements/common/annotator-context/withAnnotations', () => {
111113
activeAnnotationId: null,
112114
annotation: expectedAnnotation,
113115
error: expectedError,
116+
meta: {
117+
status,
118+
requestId: '123',
119+
},
114120
});
115121
},
116122
);

src/elements/common/annotator-context/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export interface AnnotatorState {
2020
annotation?: object | null;
2121
action?: Action | null;
2222
error?: Error | null;
23+
meta?: Metadata | null;
2324
}
2425

2526
export interface AnnotatorContext {
@@ -34,6 +35,7 @@ export enum Status {
3435
}
3536

3637
export interface Metadata {
38+
requestId: string;
3739
status: Status;
3840
}
3941

src/elements/common/annotator-context/withAnnotations.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,11 @@ export interface ComponentWithAnnotations {
2121
export type WithAnnotationsComponent<P> = React.ComponentClass<P & WithAnnotationsProps>;
2222

2323
const defaultState: AnnotatorState = {
24+
action: null,
2425
activeAnnotationId: null,
2526
annotation: null,
26-
action: null,
2727
error: null,
28+
meta: null,
2829
};
2930

3031
export default function withAnnotations<P extends object>(
@@ -52,9 +53,11 @@ export default function withAnnotations<P extends object>(
5253
}
5354

5455
handleAnnotationCreate = (eventData: AnnotationActionEvent): void => {
55-
const { annotation = null, error = null } = eventData;
56+
const { annotation = null, error = null, meta = null } = eventData;
57+
5658
const action = this.getAction(eventData);
57-
this.setState({ ...this.state, annotation, action, error });
59+
60+
this.setState({ ...this.state, annotation, action, error, meta });
5861
};
5962

6063
handleActiveChange = (annotationId: string | null): void => {

src/elements/content-sidebar/ActivitySidebar.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ mark(MARK_NAME_JS_READY);
9696

9797
class ActivitySidebar extends React.PureComponent<Props, State> {
9898
static defaultProps = {
99+
annotatorState: {},
99100
emitAnnotatorActiveChangeEvent: noop,
100101
isDisabled: false,
101102
onCommentCreate: noop,
@@ -123,6 +124,36 @@ class ActivitySidebar extends React.PureComponent<Props, State> {
123124
this.fetchCurrentUser(currentUser);
124125
}
125126

127+
componentDidUpdate({ annotatorState: prevAnnotatorState }: Props): void {
128+
const { annotation: prevAnnotation } = prevAnnotatorState;
129+
const {
130+
annotatorState: { annotation },
131+
} = this.props;
132+
133+
if (prevAnnotation !== annotation) {
134+
this.addAnnotation();
135+
}
136+
}
137+
138+
addAnnotation() {
139+
const {
140+
annotatorState: { action, annotation, meta },
141+
api,
142+
file,
143+
} = this.props;
144+
const { requestId } = meta || {};
145+
const { currentUser } = this.state;
146+
const isPending = action === 'create_start';
147+
148+
if (!currentUser) {
149+
throw getBadUserError();
150+
}
151+
152+
api.getFeedAPI(false).addAnnotation(file, currentUser, annotation, requestId, isPending);
153+
154+
this.fetchFeedItems();
155+
}
156+
126157
/**
127158
* Fetches a Users info
128159
*

src/elements/content-sidebar/__tests__/ActivitySidebar.test.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ jest.mock('lodash/debounce', () => jest.fn(i => i));
99

1010
describe('elements/content-sidebar/ActivitySidebar', () => {
1111
const feedAPI = {
12+
addAnnotation: jest.fn(),
1213
feedItems: jest.fn(),
1314
deleteComment: jest.fn(),
1415
deleteTaskNew: jest.fn(),
@@ -96,6 +97,18 @@ describe('elements/content-sidebar/ActivitySidebar', () => {
9697
});
9798
});
9899

100+
describe('componentDidUpdate', () => {
101+
test('should call addAnnotation if annotator action changes', () => {
102+
const wrapper = getWrapper({ annotatorState: { annotation: {} } });
103+
const instance = wrapper.instance();
104+
105+
instance.addAnnotation = jest.fn();
106+
107+
wrapper.setProps({ annotatorState: { annotation: { id: '123' } } });
108+
expect(instance.addAnnotation).toBeCalled();
109+
});
110+
});
111+
99112
describe('render()', () => {
100113
test('should render the activity feed sidebar', () => {
101114
const wrapper = getWrapper();
@@ -670,4 +683,35 @@ describe('elements/content-sidebar/ActivitySidebar', () => {
670683
expect(fetchFeedItems).toHaveBeenCalledWith(true);
671684
});
672685
});
686+
687+
describe('addAnnotation()', () => {
688+
test('should throw if no user', () => {
689+
const wrapper = getWrapper();
690+
const instance = wrapper.instance();
691+
692+
expect(() => instance.addAnnotation()).toThrow('Bad box user!');
693+
});
694+
test.each([true, false])('should call feedApi with pending if action %s', isPending => {
695+
const annotatorStateMock = {
696+
action: isPending ? 'create_start' : null,
697+
annotation: {},
698+
meta: {
699+
requestId: '123',
700+
},
701+
};
702+
703+
const wrapper = getWrapper({ annotatorState: annotatorStateMock, currentUser });
704+
const instance = wrapper.instance();
705+
706+
instance.addAnnotation();
707+
708+
expect(api.getFeedAPI().addAnnotation).toBeCalledWith(
709+
file,
710+
currentUser,
711+
annotatorStateMock.annotation,
712+
annotatorStateMock.meta.requestId,
713+
isPending,
714+
);
715+
});
716+
});
673717
});

0 commit comments

Comments
 (0)