Move annotation deletion/flagging to `annotations` service

Replace `annotationMapper.{deleteAnnotation, flagAnnotation}` with
`annotationsService.{delete, flag}`.

This is another step towards consolidating the logic for making
annotation-related API calls and updating the store into the annotations
service and removing the legacy annotationMapper service.

src/sidebar/components/annotation-action-bar.js

@@ -16,7 +16,7 @@ import Button from './button';
  */
 function AnnotationActionBar({
   annotation,
-  annotationMapper,
+  annotationsService,
   onReply,
   settings,
   toastMessenger,
@@ -40,15 +40,10 @@ function AnnotationActionBar({
   const showShareAction = isShareable(annotation, settings);
 
   const createDraft = useStore(store => store.createDraft);
-  const updateFlagFn = useStore(store => store.updateFlagStatus);
-
-  const updateFlag = () => {
-    updateFlagFn(annotation.id, true);
-  };
 
   const onDelete = () => {
     if (window.confirm('Are you sure you want to delete this annotation?')) {
-      annotationMapper.deleteAnnotation(annotation).catch(err => {
+      annotationsService.delete(annotation).catch(err => {
         toastMessenger.error(err.message, 'Deleting annotation failed');
       });
     }
@@ -67,9 +62,8 @@ function AnnotationActionBar({
       toastMessenger.error('You must be logged in to report an annotation');
       return;
     }
-    annotationMapper
-      .flagAnnotation(annotation) // Flag annotation on service
-      .then(updateFlag) // Update app state with flag
+    annotationsService
+      .flag(annotation)
       .catch(() => toastMessenger.error('Flagging annotation failed'));
   };
 
@@ -119,13 +113,13 @@ AnnotationActionBar.propTypes = {
   onReply: propTypes.func.isRequired,
 
   // Injected services
-  annotationMapper: propTypes.object.isRequired,
+  annotationsService: propTypes.object.isRequired,
   settings: propTypes.object.isRequired,
   toastMessenger: propTypes.object.isRequired,
 };
 
 AnnotationActionBar.injectedProps = [
-  'annotationMapper',
+  'annotationsService',
   'settings',
   'toastMessenger',
 ];

src/sidebar/components/test/annotation-action-bar-test.js

@@ -18,7 +18,7 @@ describe('AnnotationActionBar', () => {
   let fakeUserProfile;
 
   // Fake services
-  let fakeAnnotationMapper;
+  let fakeAnnotationsService;
   let fakeToastMessenger;
   let fakePermits;
   let fakeSettings;
@@ -30,7 +30,7 @@ describe('AnnotationActionBar', () => {
     return mount(
       <AnnotationActionBar
         annotation={fakeAnnotation}
-        annotationMapper={fakeAnnotationMapper}
+        annotationsService={fakeAnnotationsService}
         toastMessenger={fakeToastMessenger}
         onReply={fakeOnReply}
         settings={fakeSettings}
@@ -62,9 +62,9 @@ describe('AnnotationActionBar', () => {
       userid: 'account:foo@bar.com',
     };
 
-    fakeAnnotationMapper = {
-      deleteAnnotation: sinon.stub().resolves(),
-      flagAnnotation: sinon.stub().resolves(),
+    fakeAnnotationsService = {
+      delete: sinon.stub().resolves(),
+      flag: sinon.stub().resolves(),
     };
 
     fakeToastMessenger = {
@@ -84,7 +84,6 @@ describe('AnnotationActionBar', () => {
       isLoggedIn: sinon.stub(),
       openSidebarPanel: sinon.stub(),
       profile: sinon.stub().returns(fakeUserProfile),
-      updateFlagStatus: sinon.stub(),
     };
 
     $imports.$mock(mockImportedComponents());
@@ -154,7 +153,7 @@ describe('AnnotationActionBar', () => {
       });
 
       assert.calledOnce(confirm);
-      assert.notCalled(fakeAnnotationMapper.deleteAnnotation);
+      assert.notCalled(fakeAnnotationsService.delete);
     });
 
     it('invokes delete on service when confirmed', () => {
@@ -166,13 +165,13 @@ describe('AnnotationActionBar', () => {
         button.props().onClick();
       });
 
-      assert.calledWith(fakeAnnotationMapper.deleteAnnotation, fakeAnnotation);
+      assert.calledWith(fakeAnnotationsService.delete, fakeAnnotation);
     });
 
     it('sets a flash message if there is an error with deletion', async () => {
       allowOnly('delete');
       window.confirm.returns(true);
-      fakeAnnotationMapper.deleteAnnotation.rejects();
+      fakeAnnotationsService.delete.rejects();
 
       const button = getButton(createComponent(), 'trash');
       act(() => {
@@ -264,7 +263,7 @@ describe('AnnotationActionBar', () => {
       });
 
       assert.calledOnce(fakeToastMessenger.error);
-      assert.notCalled(fakeAnnotationMapper.flagAnnotation);
+      assert.notCalled(fakeAnnotationsService.flag);
     });
 
     it('invokes flag on service when clicked', () => {
@@ -276,26 +275,12 @@ describe('AnnotationActionBar', () => {
         button.props().onClick();
       });
 
-      assert.calledWith(fakeAnnotationMapper.flagAnnotation, fakeAnnotation);
-    });
-
-    it('updates flag state in store after flagging on service is successful', async () => {
-      window.confirm.returns(true);
-      fakeAnnotationMapper.flagAnnotation.resolves(fakeAnnotation);
-
-      const button = getButton(createComponent(), 'flag');
-
-      act(() => {
-        button.props().onClick();
-      });
-
-      await fakeAnnotation;
-      assert.calledWith(fakeStore.updateFlagStatus, fakeAnnotation.id, true);
+      assert.calledWith(fakeAnnotationsService.flag, fakeAnnotation);
     });
 
     it('sets flash error message if flagging fails on service', async () => {
       window.confirm.returns(true);
-      fakeAnnotationMapper.flagAnnotation.rejects();
+      fakeAnnotationsService.flag.rejects();
 
       const button = getButton(createComponent(), 'flag');
 
@@ -304,7 +289,6 @@ describe('AnnotationActionBar', () => {
       });
 
       await waitFor(() => fakeToastMessenger.error.called);
-      assert.notCalled(fakeStore.updateFlagStatus);
     });
 
     it('does not show flag action button if user is author', () => {

src/sidebar/services/annotation-mapper.js

@@ -4,7 +4,7 @@ import events from '../events';
 // updating the store afterwards. This is being removed.
 //
 // @ngInject
-export default function annotationMapper($rootScope, store, api) {
+export default function annotationMapper($rootScope, store) {
   function loadAnnotations(annotations, replies = []) {
     store.addAnnotations([...annotations, ...replies]);
   }
@@ -14,31 +14,8 @@ export default function annotationMapper($rootScope, store, api) {
     return annotation;
   }
 
-  function deleteAnnotation(annotation) {
-    return api.annotation
-      .delete({
-        id: annotation.id,
-      })
-      .then(function () {
-        store.removeAnnotations([annotation]);
-        return annotation;
-      });
-  }
-
-  function flagAnnotation(annot) {
-    return api.annotation
-      .flag({
-        id: annot.id,
-      })
-      .then(function () {
-        return annot;
-      });
-  }
-
   return {
     loadAnnotations: loadAnnotations,
     createAnnotation: createAnnotation,
-    deleteAnnotation: deleteAnnotation,
-    flagAnnotation: flagAnnotation,
   };
 }

src/sidebar/services/annotations.js

@@ -116,6 +116,16 @@ export default function annotationsService(api, store) {
     });
   }
 
+  async function delete_(annotation) {
+    await api.annotation.delete({ id: annotation.id });
+    store.removeAnnotations([annotation]);
+  }
+
+  async function flag(annotation) {
+    await api.annotation.flag({ id: annotation.id });
+    store.updateFlagStatus(annotation.id, true);
+  }
+
   /**
    * Create a reply to `annotation` by the user `userid` and add to the store.
    *
@@ -172,6 +182,8 @@ export default function annotationsService(api, store) {
 
   return {
     create,
+    delete: delete_,
+    flag,
     reply,
     save,
   };

src/sidebar/services/test/annotation-mapper-test.js

@@ -4,18 +4,10 @@ import annotationMapperFactory from '../annotation-mapper';
 
 describe('annotationMapper', function () {
   let $rootScope;
-  let fakeApi;
   let fakeStore;
   let annotationMapper;
 
   beforeEach(function () {
-    fakeApi = {
-      annotation: {
-        delete: sinon.stub().returns(Promise.resolve({})),
-        flag: sinon.stub().returns(Promise.resolve({})),
-      },
-    };
-
     $rootScope = {
       $broadcast: sinon.stub(),
     };
@@ -27,8 +19,6 @@ describe('annotationMapper', function () {
 
     const injector = new Injector()
       .register('$rootScope', { value: $rootScope })
-      .register('api', { value: fakeApi })
-      .register('settings', { value: {} })
       .register('store', { value: fakeStore })
       .register('annotationMapper', annotationMapperFactory);
     annotationMapper = injector.get('annotationMapper');
@@ -45,15 +35,6 @@ describe('annotationMapper', function () {
     });
   });
 
-  describe('#flagAnnotation()', function () {
-    it('flags an annotation', function () {
-      const ann = { id: 'test-id' };
-      annotationMapper.flagAnnotation(ann);
-      assert.calledOnce(fakeApi.annotation.flag);
-      assert.calledWith(fakeApi.annotation.flag, { id: ann.id });
-    });
-  });
-
   describe('#createAnnotation()', function () {
     it('creates a new annotation resource', function () {
       const ann = {};
@@ -71,26 +52,4 @@ describe('annotationMapper', function () {
       );
     });
   });
-
-  describe('#deleteAnnotation', function () {
-    it('deletes the annotation on the server', function () {
-      const ann = { id: 'test-id' };
-      annotationMapper.deleteAnnotation(ann);
-      assert.calledWith(fakeApi.annotation.delete, { id: 'test-id' });
-    });
-
-    it('removes the annotation from the store on success', async () => {
-      const ann = {};
-      await annotationMapper.deleteAnnotation(ann);
-      assert.calledWith(fakeStore.removeAnnotations, [ann]);
-    });
-
-    it('does not remove the annotation from the store on error', () => {
-      fakeApi.annotation.delete.returns(Promise.reject());
-      const ann = { id: 'test-id' };
-      return annotationMapper.deleteAnnotation(ann).catch(function () {
-        assert.notCalled(fakeStore.removeAnnotations);
-      });
-    });
-  });
 });

src/sidebar/services/test/annotations-test.js

@@ -18,6 +18,8 @@ describe('annotationsService', () => {
     fakeApi = {
       annotation: {
         create: sinon.stub().resolves(fixtures.defaultAnnotation()),
+        delete: sinon.stub().resolves(),
+        flag: sinon.stub().resolves(),
         update: sinon.stub().resolves(fixtures.defaultAnnotation()),
       },
     };
@@ -42,9 +44,11 @@ describe('annotationsService', () => {
       getDefault: sinon.stub(),
       getDraft: sinon.stub().returns(null),
       profile: sinon.stub().returns({}),
+      removeAnnotations: sinon.stub(),
       removeDraft: sinon.stub(),
       selectTab: sinon.stub(),
       setCollapsed: sinon.stub(),
+      updateFlagStatus: sinon.stub(),
     };
 
     $imports.$mock({
@@ -221,6 +225,60 @@ describe('annotationsService', () => {
     });
   });
 
+  describe('delete', () => {
+    it('removes the annotation via the API', async () => {
+      const annot = fixtures.defaultAnnotation();
+      await svc.delete(annot);
+      assert.calledWith(fakeApi.annotation.delete, { id: annot.id });
+    });
+
+    it('removes the annotation from the store', async () => {
+      const annot = fixtures.defaultAnnotation();
+      await svc.delete(annot);
+      assert.calledWith(fakeStore.removeAnnotations, [annot]);
+    });
+
+    it('does not remove the annotation from the store if the API call fails', async () => {
+      fakeApi.annotation.delete.rejects(new Error('Annotation does not exist'));
+      const annot = fixtures.defaultAnnotation();
+
+      try {
+        await svc.delete(annot);
+      } catch (e) {
+        /* Ignored */
+      }
+
+      assert.notCalled(fakeStore.removeAnnotations);
+    });
+  });
+
+  describe('flag', () => {
+    it('flags the annotation via the API', async () => {
+      const annot = fixtures.defaultAnnotation();
+      await svc.flag(annot);
+      assert.calledWith(fakeApi.annotation.flag, { id: annot.id });
+    });
+
+    it('updates the flag status in the store', async () => {
+      const annot = fixtures.defaultAnnotation();
+      await svc.flag(annot);
+      assert.calledWith(fakeStore.updateFlagStatus, annot.id, true);
+    });
+
+    it('does not update the flag status if the API call fails', async () => {
+      fakeApi.annotation.flag.rejects(new Error('Annotation does not exist'));
+      const annot = fixtures.defaultAnnotation();
+
+      try {
+        await svc.flag(annot);
+      } catch (e) {
+        /* Ignored */
+      }
+
+      assert.notCalled(fakeStore.updateFlagStatus);
+    });
+  });
+
   describe('reply', () => {
     const filledAnnotation = () => {
       const annot = fixtures.defaultAnnotation();