Merge pull request #1993 from hypothesis/remove-annotation-mapper

Remove annotation-mapper service and associated Angular events

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/annotation-viewer-content.js

@@ -30,8 +30,7 @@ function AnnotationViewerContentController(
   api,
   rootThread,
   streamer,
-  streamFilter,
-  annotationMapper
+  streamFilter
 ) {
   store.clearAnnotations();
 
@@ -44,7 +43,7 @@ function AnnotationViewerContentController(
   };
 
   this.ready = fetchThread(api, annotationId).then(function (annots) {
-    annotationMapper.loadAnnotations(annots);
+    store.addAnnotations(annots);
 
     const topLevelAnnot = annots.filter(function (annot) {
       return (annot.references || []).length === 0;

src/sidebar/components/hypothesis-app.js

@@ -154,9 +154,7 @@ function HypothesisAppController(
     }
 
     store.clearGroups();
-    store.unsavedAnnotations().forEach(function (annotation) {
-      $rootScope.$emit(events.ANNOTATION_DELETED, annotation);
-    });
+    store.removeAnnotations(store.unsavedAnnotations());
     store.discardAllDrafts();
 
     if (serviceConfig(settings)) {

src/sidebar/components/stream-content.js

 // @ngInject
-function StreamContentController(
-  $scope,
-  annotationMapper,
-  store,
-  api,
-  rootThread,
-  searchFilter
-) {
+function StreamContentController($scope, store, api, rootThread, searchFilter) {
   /** `offset` parameter for the next search API call. */
   let offset = 0;
 
   /** Load annotations fetched from the API into the app. */
   const load = function (result) {
     offset += result.rows.length;
-    annotationMapper.loadAnnotations(result.rows, result.replies);
+    const annots = [...result.rows, ...result.replies];
+    store.addAnnotations(annots);
   };
 
   const currentQuery = () => store.routeParams().q;

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/components/test/annotation-viewer-content-test.js

@@ -42,6 +42,7 @@ describe('annotationViewerContent', function () {
   function createController(opts) {
     const locals = {
       store: {
+        addAnnotations: sinon.stub(),
         clearAnnotations: sinon.stub(),
         setCollapsed: sinon.stub(),
         highlightAnnotations: sinon.stub(),
@@ -62,9 +63,6 @@ describe('annotationViewerContent', function () {
         },
         getFilter: function () {},
       },
-      annotationMapper: {
-        loadAnnotations: sinon.spy(),
-      },
     };
 
     let $componentController;
@@ -85,9 +83,9 @@ describe('annotationViewerContent', function () {
       ]);
       const controller = createController({ api: fakeApi });
       return controller.ctrl.ready.then(function () {
-        assert.calledOnce(controller.annotationMapper.loadAnnotations);
+        assert.calledOnce(controller.store.addAnnotations);
         assert.calledWith(
-          controller.annotationMapper.loadAnnotations,
+          controller.store.addAnnotations,
           sinon.match(fakeApi.annots)
         );
       });
@@ -114,7 +112,7 @@ describe('annotationViewerContent', function () {
       const controller = createController({ api: fakeApi });
       return controller.ctrl.ready.then(function () {
         assert.calledWith(
-          controller.annotationMapper.loadAnnotations,
+          controller.store.addAnnotations,
           sinon.match(fakeApi.annots)
         );
       });

src/sidebar/components/test/hypothesis-app-test.js

@@ -79,6 +79,7 @@ describe('sidebar.components.hypothesis-app', function () {
         countDrafts: sandbox.stub().returns(0),
         discardAllDrafts: sandbox.stub(),
         unsavedAnnotations: sandbox.stub().returns([]),
+        removeAnnotations: sandbox.stub(),
       };
 
       fakeAnalytics = {
@@ -382,31 +383,22 @@ describe('sidebar.components.hypothesis-app', function () {
         assert.called(fakeStore.clearGroups);
       });
 
-      it('emits "annotationDeleted" for each unsaved draft annotation', function () {
+      it('removes unsaved annotations', function () {
         fakeStore.unsavedAnnotations = sandbox
           .stub()
           .returns(['draftOne', 'draftTwo', 'draftThree']);
         const ctrl = createController();
-        $rootScope.$emit = sandbox.stub();
 
         ctrl.logout();
 
-        assert($rootScope.$emit.calledThrice);
-        assert.deepEqual($rootScope.$emit.firstCall.args, [
-          'annotationDeleted',
+        assert.calledWith(fakeStore.removeAnnotations, [
           'draftOne',
-        ]);
-        assert.deepEqual($rootScope.$emit.secondCall.args, [
-          'annotationDeleted',
           'draftTwo',
-        ]);
-        assert.deepEqual($rootScope.$emit.thirdCall.args, [
-          'annotationDeleted',
           'draftThree',
         ]);
       });
 
-      it('discards draft annotations', function () {
+      it('discards drafts', function () {
         const ctrl = createController();
 
         ctrl.logout();
@@ -414,7 +406,7 @@ describe('sidebar.components.hypothesis-app', function () {
         assert(fakeStore.discardAllDrafts.calledOnce);
       });
 
-      it('does not emit "annotationDeleted" if the user cancels the prompt', function () {
+      it('does not remove unsaved annotations if the user cancels the prompt', function () {
         const ctrl = createController();
         fakeStore.countDrafts.returns(1);
         $rootScope.$emit = sandbox.stub();
@@ -422,7 +414,7 @@ describe('sidebar.components.hypothesis-app', function () {
 
         ctrl.logout();
 
-        assert($rootScope.$emit.notCalled);
+        assert.notCalled(fakeStore.removeAnnotations);
       });
 
       it('does not discard drafts if the user cancels the prompt', function () {

src/sidebar/components/test/stream-content-test.js

@@ -12,7 +12,6 @@ class FakeRootThread extends EventEmitter {
 
 describe('StreamContentController', function () {
   let $componentController;
-  let fakeAnnotationMapper;
   let fakeStore;
   let fakeRootThread;
   let fakeSearchFilter;
@@ -25,11 +24,8 @@ describe('StreamContentController', function () {
   });
 
   beforeEach(function () {
-    fakeAnnotationMapper = {
-      loadAnnotations: sinon.spy(),
-    };
-
     fakeStore = {
+      addAnnotations: sinon.stub(),
       clearAnnotations: sinon.spy(),
       routeParams: sinon.stub().returns({ id: 'test' }),
       setCollapsed: sinon.spy(),
@@ -44,9 +40,7 @@ describe('StreamContentController', function () {
     };
 
     fakeApi = {
-      search: sinon.spy(function () {
-        return Promise.resolve({ rows: [], total: 0 });
-      }),
+      search: sinon.stub().resolves({ rows: [], replies: [], total: 0 }),
     };
 
     fakeStreamer = {
@@ -65,7 +59,6 @@ describe('StreamContentController', function () {
     fakeRootThread = new FakeRootThread();
 
     angular.mock.module('h', {
-      annotationMapper: fakeAnnotationMapper,
       store: fakeStore,
       api: fakeApi,
       rootThread: fakeRootThread,
@@ -104,12 +97,14 @@ describe('StreamContentController', function () {
     createController();
 
     return Promise.resolve().then(function () {
-      assert.calledOnce(fakeAnnotationMapper.loadAnnotations);
-      assert.calledWith(
-        fakeAnnotationMapper.loadAnnotations,
-        ['annotation_1', 'annotation_2'],
-        ['reply_1', 'reply_2', 'reply_3']
-      );
+      assert.calledOnce(fakeStore.addAnnotations);
+      assert.calledWith(fakeStore.addAnnotations, [
+        'annotation_1',
+        'annotation_2',
+        'reply_1',
+        'reply_2',
+        'reply_3',
+      ]);
     });
   });
 

src/sidebar/events.js

@@ -25,19 +25,4 @@ export default {
 
   /** Annotations were anchored in a connected document. */
   ANNOTATIONS_SYNCED: 'sync',
-
-  /** An annotation was created on the server and assigned an ID. */
-  ANNOTATION_CREATED: 'annotationCreated',
-
-  /** An annotation was either deleted or unloaded. */
-  ANNOTATION_DELETED: 'annotationDeleted',
-
-  /** An annotation was flagged. */
-  ANNOTATION_FLAGGED: 'annotationFlagged',
-
-  /** An annotation has been updated. */
-  ANNOTATION_UPDATED: 'annotationUpdated',
-
-  /** A set of annotations were loaded from the server. */
-  ANNOTATIONS_LOADED: 'annotationsLoaded',
 };

src/sidebar/index.js

@@ -142,7 +142,6 @@ import threadList from './components/thread-list';
 import bridgeService from '../shared/bridge';
 
 import analyticsService from './services/analytics';
-import annotationMapperService from './services/annotation-mapper';
 import annotationsService from './services/annotations';
 import apiService from './services/api';
 import apiRoutesService from './services/api-routes';
@@ -186,7 +185,6 @@ function startAngularApp(config) {
   // Register services.
   container
     .register('analytics', analyticsService)
-    .register('annotationMapper', annotationMapperService)
     .register('annotationsService', annotationsService)
     .register('api', apiService)
     .register('apiRoutes', apiRoutesService)
@@ -264,7 +262,6 @@ function startAngularApp(config) {
     // Register services, the store and utilities with Angular, so that
     // Angular components can use them.
     .service('analytics', () => container.get('analytics'))
-    .service('annotationMapper', () => container.get('annotationMapper'))
     .service('annotationsService', () => container.get('annotationsService'))
     .service('api', () => container.get('api'))
     .service('auth', () => container.get('auth'))

src/sidebar/services/annotation-mapper.js

-import events from '../events';
-
-function getExistingAnnotation(store, id) {
-  return store.getState().annotations.annotations.find(function (annot) {
-    return annot.id === id;
-  });
-}
-
-// Wraps the annotation store to trigger events for the CRUD actions
-// @ngInject
-export default function annotationMapper($rootScope, store, api) {
-  function loadAnnotations(annotations, replies) {
-    annotations = annotations.concat(replies || []);
-
-    const loaded = [];
-    annotations.forEach(function (annotation) {
-      const existing = getExistingAnnotation(store, annotation.id);
-      if (existing) {
-        $rootScope.$broadcast(events.ANNOTATION_UPDATED, annotation);
-        return;
-      }
-      loaded.push(annotation);
-    });
-
-    $rootScope.$broadcast(events.ANNOTATIONS_LOADED, loaded);
-  }
-
-  function createAnnotation(annotation) {
-    $rootScope.$broadcast(events.BEFORE_ANNOTATION_CREATED, annotation);
-    return annotation;
-  }
-
-  function deleteAnnotation(annotation) {
-    return api.annotation
-      .delete({
-        id: annotation.id,
-      })
-      .then(function () {
-        $rootScope.$broadcast(events.ANNOTATION_DELETED, annotation);
-        return annotation;
-      });
-  }
-
-  function flagAnnotation(annot) {
-    return api.annotation
-      .flag({
-        id: annot.id,
-      })
-      .then(function () {
-        $rootScope.$broadcast(events.ANNOTATION_FLAGGED, annot);
-        return annot;
-      });
-  }
-
-  return {
-    loadAnnotations: loadAnnotations,
-    createAnnotation: createAnnotation,
-    deleteAnnotation: deleteAnnotation,
-    flagAnnotation: flagAnnotation,
-  };
-}

src/sidebar/services/annotations.js

@@ -116,6 +116,22 @@ export default function annotationsService(api, store) {
     });
   }
 
+  /**
+   * Delete an annotation via the API and update the store.
+   */
+  async function delete_(annotation) {
+    await api.annotation.delete({ id: annotation.id });
+    store.removeAnnotations([annotation]);
+  }
+
+  /**
+   * Flag an annotation for review by a moderator.
+   */
+  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.
    *
@@ -178,6 +194,8 @@ export default function annotationsService(api, store) {
 
   return {
     create,
+    delete: delete_,
+    flag,
     reply,
     save,
   };

src/sidebar/services/load-annotations.js

@@ -5,7 +5,6 @@ import SearchClient from '../search-client';
 
 // @ngInject
 export default function loadAnnotationsService(
-  annotationMapper,
   api,
   store,
   streamer,
@@ -41,7 +40,7 @@ export default function loadAnnotationsService(
     });
     searchClient.on('results', results => {
       if (results.length) {
-        annotationMapper.loadAnnotations(results);
+        store.addAnnotations(results);
       }
     });
     searchClient.on('error', error => {

src/sidebar/services/root-thread.js

@@ -98,31 +98,10 @@ export default function RootThread(
     });
   }
 
-  // Listen for annotations being created or loaded
-  // and show them in the UI.
-  //
-  // Note: These events could all be converted into actions that are handled by
-  // the Redux store in store.
-  const loadEvents = [
-    events.ANNOTATION_CREATED,
-    events.ANNOTATION_UPDATED,
-    events.ANNOTATIONS_LOADED,
-  ];
-  loadEvents.forEach(function (event) {
-    $rootScope.$on(event, function (event, annotation) {
-      store.addAnnotations([].concat(annotation));
-    });
-  });
-
   $rootScope.$on(events.BEFORE_ANNOTATION_CREATED, function (event, ann) {
     annotationsService.create(ann);
   });
 
-  // Remove any annotations that are deleted or unloaded
-  $rootScope.$on(events.ANNOTATION_DELETED, function (event, annotation) {
-    store.removeAnnotations([annotation]);
-  });
-
   /**
    * Build the root conversation thread from the given UI state.
    * @return {Thread}

src/sidebar/services/streamer.js

@@ -8,25 +8,9 @@ import Socket from '../websocket';
  * Open a new WebSocket connection to the Hypothesis push notification service.
  * Only one websocket connection may exist at a time, any existing socket is
  * closed.
- *
- * @param $rootScope - Scope used to $apply() app state changes
- *                     resulting from WebSocket messages, in order to update
- *                     appropriate watchers.
- * @param annotationMapper - The local annotation store
- * @param groups - The local groups store
- * @param session - Provides access to read and update the session state
- * @param settings - Application settings
  */
 // @ngInject
-export default function Streamer(
-  $rootScope,
-  annotationMapper,
-  store,
-  auth,
-  groups,
-  session,
-  settings
-) {
+export default function Streamer(store, auth, groups, session, settings) {
   // The randomly generated session ID
   const clientId = generateHexString(32);
 
@@ -81,10 +65,6 @@ export default function Streamer(
   }
 
   function handleSocketOnMessage(event) {
-    // Wrap message dispatches in $rootScope.$apply() so that
-    // scope watches on app state affected by the received message
-    // are updated
-    $rootScope.$apply(function () {
     const message = JSON.parse(event.data);
     if (!message) {
       return;
@@ -106,7 +86,6 @@ export default function Streamer(
     } else {
       warnOnce('received unsupported notification', message.type);
     }
-    });
   }
 
   function sendClientConfig() {
@@ -217,7 +196,7 @@ export default function Streamer(
   function applyPendingUpdates() {
     const updates = Object.values(store.pendingUpdates());
     if (updates.length) {
-      annotationMapper.loadAnnotations(updates);
+      store.addAnnotations(updates);
     }
 
     const deletions = Object.keys(store.pendingDeletions()).map(id => ({ id }));

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

-import { Injector } from '../../../shared/injector';
-import events from '../../events';
-import storeFactory from '../../store';
-import annotationMapperFactory from '../annotation-mapper';
-import immutable from '../../util/immutable';
-
-describe('annotationMapper', function () {
-  let $rootScope;
-  let store;
-  let fakeApi;
-  let annotationMapper;
-
-  beforeEach(function () {
-    fakeApi = {
-      annotation: {
-        delete: sinon.stub().returns(Promise.resolve({})),
-        flag: sinon.stub().returns(Promise.resolve({})),
-      },
-    };
-
-    $rootScope = {
-      // nb. `$applyAsync` is needed because this test uses the real `store`
-      // service.
-      $applyAsync: sinon.stub().yields(),
-      $broadcast: sinon.stub(),
-    };
-
-    const injector = new Injector()
-      .register('$rootScope', { value: $rootScope })
-      .register('api', { value: fakeApi })
-      .register('settings', { value: {} })
-      .register('store', storeFactory)
-      .register('annotationMapper', annotationMapperFactory);
-    store = injector.get('store');
-    annotationMapper = injector.get('annotationMapper');
-  });
-
-  describe('#loadAnnotations()', function () {
-    it('triggers the annotationLoaded event', function () {
-      const annotations = [{ id: 1 }, { id: 2 }, { id: 3 }];
-      annotationMapper.loadAnnotations(annotations);
-      assert.called($rootScope.$broadcast);
-      assert.calledWith($rootScope.$broadcast, events.ANNOTATIONS_LOADED, [
-        { id: 1 },
-        { id: 2 },
-        { id: 3 },
-      ]);
-    });
-
-    it('also includes replies in the annotationLoaded event', function () {
-      const annotations = [{ id: 1 }];
-      const replies = [{ id: 2 }, { id: 3 }];
-      annotationMapper.loadAnnotations(annotations, replies);
-      assert.called($rootScope.$broadcast);
-      assert.calledWith($rootScope.$broadcast, events.ANNOTATIONS_LOADED, [
-        { id: 1 },
-        { id: 2 },
-        { id: 3 },
-      ]);
-    });
-
-    it('triggers the annotationUpdated event for each loaded annotation', function () {
-      const annotations = immutable([{ id: 1 }, { id: 2 }, { id: 3 }]);
-      store.addAnnotations(annotations);
-
-      annotationMapper.loadAnnotations(annotations);
-      assert.called($rootScope.$broadcast);
-      assert.calledWith(
-        $rootScope.$broadcast,
-        events.ANNOTATION_UPDATED,
-        annotations[0]
-      );
-    });
-
-    it('also triggers annotationUpdated for cached replies', function () {
-      const annotations = [{ id: 1 }];
-      const replies = [{ id: 2 }, { id: 3 }, { id: 4 }];
-      store.addAnnotations([{ id: 3 }]);
-
-      annotationMapper.loadAnnotations(annotations, replies);
-      assert(
-        $rootScope.$broadcast.calledWith(events.ANNOTATION_UPDATED, { id: 3 })
-      );
-    });
-
-    it('replaces the properties on the cached annotation with those from the loaded one', function () {
-      const annotations = [{ id: 1, url: 'http://example.com' }];
-      store.addAnnotations([{ id: 1, $tag: 'tag1' }]);
-
-      annotationMapper.loadAnnotations(annotations);
-      assert.called($rootScope.$broadcast);
-      assert.calledWith($rootScope.$broadcast, events.ANNOTATION_UPDATED, {
-        id: 1,
-        url: 'http://example.com',
-      });
-    });
-
-    it('excludes cached annotations from the annotationLoaded event', function () {
-      const annotations = [{ id: 1, url: 'http://example.com' }];
-      store.addAnnotations([{ id: 1, $tag: 'tag1' }]);
-
-      annotationMapper.loadAnnotations(annotations);
-      assert.called($rootScope.$broadcast);
-      assert.calledWith($rootScope.$broadcast, events.ANNOTATIONS_LOADED, []);
-    });
-  });
-
-  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 });
-    });
-
-    it('emits the "annotationFlagged" event', function (done) {
-      const ann = { id: 'test-id' };
-      annotationMapper
-        .flagAnnotation(ann)
-        .then(function () {
-          assert.calledWith(
-            $rootScope.$broadcast,
-            events.ANNOTATION_FLAGGED,
-            ann
-          );
-        })
-        .then(done, done);
-    });
-  });
-
-  describe('#createAnnotation()', function () {
-    it('creates a new annotation resource', function () {
-      const ann = {};
-      const ret = annotationMapper.createAnnotation(ann);
-      assert.equal(ret, ann);
-    });
-
-    it('emits the "beforeAnnotationCreated" event', function () {
-      const ann = {};
-      annotationMapper.createAnnotation(ann);
-      assert.calledWith(
-        $rootScope.$broadcast,
-        events.BEFORE_ANNOTATION_CREATED,
-        ann
-      );
-    });
-  });
-
-  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('triggers the "annotationDeleted" event on success', function (done) {
-      const ann = {};
-      annotationMapper
-        .deleteAnnotation(ann)
-        .then(function () {
-          assert.calledWith(
-            $rootScope.$broadcast,
-            events.ANNOTATION_DELETED,
-            ann
-          );
-        })
-        .then(done, done);
-    });
-
-    it('does not emit an event on error', function (done) {
-      fakeApi.annotation.delete.returns(Promise.reject());
-      const ann = { id: 'test-id' };
-      annotationMapper
-        .deleteAnnotation(ann)
-        .catch(function () {
-          assert.notCalled($rootScope.$broadcast);
-        })
-        .then(done, done);
-    });
-  });
-});

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()),
       },
     };
@@ -44,9 +46,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({
@@ -223,6 +227,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();

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

@@ -29,7 +29,6 @@ class FakeSearchClient extends EventEmitter {
 }
 
 describe('loadAnnotationsService', () => {
-  let fakeAnnotationMapper;
   let fakeApi;
   let fakeStore;
   let fakeStreamer;
@@ -43,15 +42,12 @@ describe('loadAnnotationsService', () => {
     searchClients = [];
     longRunningSearchClient = false;
 
-    fakeAnnotationMapper = {
-      loadAnnotations: sinon.stub(),
-    };
-
     fakeApi = {
       search: sinon.stub(),
     };
 
     fakeStore = {
+      addAnnotations: sinon.stub(),
       annotationFetchFinished: sinon.stub(),
       annotationFetchStarted: sinon.stub(),
       frames: sinon.stub(),
@@ -90,7 +86,6 @@ describe('loadAnnotationsService', () => {
       })
     );
     return loadAnnotationsService(
-      fakeAnnotationMapper,
       fakeApi,
       fakeStore,
       fakeStreamer,
@@ -119,10 +114,10 @@ describe('loadAnnotationsService', () => {
       const svc = createService();
 
       svc.load(fakeUris, fakeGroupId);
-      assert.calledWith(fakeAnnotationMapper.loadAnnotations, [
+      assert.calledWith(fakeStore.addAnnotations, [
         sinon.match({ id: fakeUris[0] + '123' }),
       ]);
-      assert.calledWith(fakeAnnotationMapper.loadAnnotations, [
+      assert.calledWith(fakeStore.addAnnotations, [
         sinon.match({ id: fakeUris[0] + '456' }),
       ]);
     });
@@ -151,16 +146,16 @@ describe('loadAnnotationsService', () => {
       ]);
 
       svc.load(fakeUris, fakeGroupId);
-      assert.calledWith(fakeAnnotationMapper.loadAnnotations, [
+      assert.calledWith(fakeStore.addAnnotations, [
         sinon.match({ id: uri + '123' }),
       ]);
-      assert.calledWith(fakeAnnotationMapper.loadAnnotations, [
+      assert.calledWith(fakeStore.addAnnotations, [
         sinon.match({ id: fingerprint + '123' }),
       ]);
-      assert.calledWith(fakeAnnotationMapper.loadAnnotations, [
+      assert.calledWith(fakeStore.addAnnotations, [
         sinon.match({ id: uri + '456' }),
       ]);
-      assert.calledWith(fakeAnnotationMapper.loadAnnotations, [
+      assert.calledWith(fakeStore.addAnnotations, [
         sinon.match({ id: fingerprint + '456' }),
       ]);
     });
@@ -177,9 +172,7 @@ describe('loadAnnotationsService', () => {
         fakeUris[1] + '123',
         fakeUris[1] + '456',
       ].forEach(uri => {
-        assert.calledWith(fakeAnnotationMapper.loadAnnotations, [
-          sinon.match({ id: uri }),
-        ]);
+        assert.calledWith(fakeStore.addAnnotations, [sinon.match({ id: uri })]);
       });
     });
 
@@ -235,7 +228,7 @@ describe('loadAnnotationsService', () => {
       const svc = createService();
 
       svc.load(fakeUris, fakeGroupId);
-      assert.notCalled(fakeAnnotationMapper.loadAnnotations);
+      assert.notCalled(fakeStore.addAnnotations);
     });
 
     it('calls annotationFetchStarted when it starts searching for annotations', () => {

src/sidebar/services/test/root-thread-test.js

@@ -366,31 +366,9 @@ describe('rootThread', function () {
       assert.calledWith(fakeAnnotationsService.create, sinon.match(annot));
     });
 
-    [
-      { event: events.ANNOTATION_CREATED, annotations: annot },
-      { event: events.ANNOTATION_UPDATED, annotations: annot },
-      { event: events.ANNOTATIONS_LOADED, annotations: [annot] },
-    ].forEach(testCase => {
-      it(`adds or updates annotations when ${testCase.event} event occurs`, () => {
-        $rootScope.$broadcast(testCase.event, testCase.annotations);
-        const annotations = [].concat(testCase.annotations);
-        assert.notCalled(fakeStore.removeAnnotations);
-        assert.calledWith(fakeStore.addAnnotations, sinon.match(annotations));
-      });
-    });
-
-    it('removes annotations when ANNOTATION_DELETED event occurs', function () {
-      $rootScope.$broadcast(events.ANNOTATION_DELETED, annot);
-      assert.calledWith(fakeStore.removeAnnotations, sinon.match([annot]));
-    });
-
     describe('when a new annotation is created', function () {
       let existingNewAnnot;
-      let onDelete;
       beforeEach(function () {
-        onDelete = sinon.stub();
-        $rootScope.$on(events.ANNOTATION_DELETED, onDelete);
-
         existingNewAnnot = { $tag: 'a-new-tag' };
         fakeStore.state.annotations.annotations.push(existingNewAnnot);
       });
@@ -403,7 +381,6 @@ describe('rootThread', function () {
         );
 
         assert.notCalled(fakeStore.removeDraft);
-        assert.notCalled(onDelete);
       });
 
       it('does not remove saved annotations', function () {
@@ -416,7 +393,6 @@ describe('rootThread', function () {
         );
 
         assert.notCalled(fakeStore.removeDraft);
-        assert.notCalled(onDelete);
       });
     });
   });

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

@@ -71,19 +71,15 @@ class FakeSocket extends EventEmitter {
 }
 
 describe('Streamer', function () {
-  let fakeAnnotationMapper;
   let fakeStore;
   let fakeAuth;
   let fakeGroups;
-  let fakeRootScope;
   let fakeSession;
   let fakeSettings;
   let activeStreamer;
 
   function createDefaultStreamer() {
     activeStreamer = new Streamer(
-      fakeRootScope,
-      fakeAnnotationMapper,
       fakeStore,
       fakeAuth,
       fakeGroups,
@@ -93,29 +89,14 @@ describe('Streamer', function () {
   }
 
   beforeEach(function () {
-    const emitter = new EventEmitter();
-
     fakeAuth = {
       tokenGetter: function () {
         return Promise.resolve('dummy-access-token');
       },
     };
 
-    fakeRootScope = {
-      $apply: function (callback) {
-        callback();
-      },
-      $on: emitter.on.bind(emitter),
-      $broadcast: function (event, data) {
-        emitter.emit(event, { event: event }, data);
-      },
-    };
-
-    fakeAnnotationMapper = {
-      loadAnnotations: sinon.stub(),
-    };
-
     fakeStore = {
+      addAnnotations: sinon.stub(),
       annotationExists: sinon.stub().returns(false),
       clearPendingUpdates: sinon.stub(),
       getState: sinon.stub().returns({
@@ -290,7 +271,7 @@ describe('Streamer', function () {
           updatedAnnotations: [ann],
         });
         assert.calledWith(
-          fakeAnnotationMapper.loadAnnotations,
+          fakeStore.addAnnotations,
           fixtures.createNotification.payload
         );
       });
@@ -319,7 +300,7 @@ describe('Streamer', function () {
 
         fakeWebSocket.notify(fixtures.createNotification);
 
-        assert.notCalled(fakeAnnotationMapper.loadAnnotations);
+        assert.notCalled(fakeStore.addAnnotations);
       });
 
       it('does not apply deletions immediately', function () {
@@ -344,9 +325,7 @@ describe('Streamer', function () {
     it('applies pending updates', function () {
       fakeStore.pendingUpdates.returns({ 'an-id': { id: 'an-id' } });
       activeStreamer.applyPendingUpdates();
-      assert.calledWith(fakeAnnotationMapper.loadAnnotations, [
-        { id: 'an-id' },
-      ]);
+      assert.calledWith(fakeStore.addAnnotations, [{ id: 'an-id' }]);
     });
 
     it('applies pending deletions', function () {