Move new-annotation initialization into the store

Move initialization for new (unsaved) annotations from the (legacy)
annotation component’s controller into the `annotations` store module.

src/sidebar/components/annotation.js

@@ -92,25 +92,13 @@ function AnnotationController(
      */
     newlyCreatedByHighlightButton = self.annotation.$highlight || false;
 
-    // New annotations (just created locally by the client, rather then
-    // received from the server) have some fields missing. Add them.
-    //
-    // FIXME: This logic should go in the `addAnnotations` Redux action once all
-    // required state is in the store.
-    self.annotation.user = self.annotation.user || session.state.userid;
-    self.annotation.user_info =
-      self.annotation.user_info || session.state.user_info;
-    self.annotation.group = self.annotation.group || groups.focused().id;
+    // FIXME: This logic needs to move into the `annotations` store module
     if (!self.annotation.permissions) {
       self.annotation.permissions = permissions.default(
         self.annotation.user,
         self.annotation.group
       );
     }
-    self.annotation.text = self.annotation.text || '';
-    if (!Array.isArray(self.annotation.tags)) {
-      self.annotation.tags = [];
-    }
 
     // Automatically save new highlights to the server when they're created.
     // Note that this line also gets called when the user logs in (since

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

@@ -245,35 +245,14 @@ describe('annotation', function() {
     });
 
     describe('initialization', function() {
-      it("sets the user of annotations that don't have one", function() {
-        // You can create annotations while logged out and then login.
-        // When you login a new AnnotationController instance is created for
-        // each of your annotations, and on initialization it will set the
-        // annotation's user to your username from the session.
-        const annotation = fixtures.newAnnotation();
-        annotation.user = undefined;
-        fakeSession.state.userid = 'acct:bill@localhost';
-        fakeSession.state.user_info = {
-          display_name: 'Bill Jones',
-        };
-
-        createDirective(annotation);
-
-        assert.equal(annotation.user, 'acct:bill@localhost');
-        assert.deepEqual(annotation.user_info, {
-          display_name: 'Bill Jones',
-        });
-      });
-
       it('sets the permissions of new annotations', function() {
         // You can create annotations while logged out and then login.
         // When you login a new AnnotationController instance is created for
         // each of your annotations, and on initialization it will set the
         // annotation's permissions using your username from the session.
         const annotation = fixtures.newAnnotation();
-        annotation.user = annotation.permissions = undefined;
+        annotation.permissions = undefined;
         annotation.group = '__world__';
-        fakeSession.state.userid = 'acct:bill@localhost';
         fakePermissions.default = function(userid, group) {
           return {
             read: [userid, group],
@@ -284,19 +263,10 @@ describe('annotation', function() {
 
         assert.deepEqual(
           annotation.permissions,
-          fakePermissions.default(fakeSession.state.userid, annotation.group)
+          fakePermissions.default(annotation.user, annotation.group)
         );
       });
 
-      it('sets the tags and text fields for new annotations', function() {
-        const annotation = fixtures.newAnnotation();
-        delete annotation.tags;
-        delete annotation.text;
-        createDirective(annotation);
-        assert.equal(annotation.text, '');
-        assert.deepEqual(annotation.tags, []);
-      });
-
       it('preserves the permissions of existing annotations', function() {
         const annotation = fixtures.newAnnotation();
         annotation.permissions = {
@@ -795,14 +765,6 @@ describe('annotation', function() {
           assert.notCalled(fakeStore.removeDraft);
         });
       });
-
-      it("sets the annotation's group to the focused group", function() {
-        fakeGroups.focused = function() {
-          return { id: 'test-id' };
-        };
-        const controller = createDirective(fixtures.newAnnotation()).controller;
-        assert.equal(controller.annotation.group, 'test-id');
-      });
     });
 
     describe('saving an edited an annotation', function() {

src/sidebar/store/modules/annotations.js

@@ -48,17 +48,21 @@ function findByTag(annotations, tag) {
 }
 
 /**
- * Initialize the status flags and properties of a new annotation.
+ * Set custom private fields on an annotation object about to be added to the
+ * store's collection of `annotations`.
+ *
+ * `annotation` may either be new (unsaved) or a persisted annotation retrieved
+ * from the service.
+ *
+ * @param {Object} annotation
+ * @param {Number} tag - The `$tag` value that should be used for this
+ *                       if it doesn't have a `$tag` already
+ * @return {Object} - annotation with local (`$*`) fields set
  */
-function initializeAnnot(annotation, tag) {
+function initializeAnnotation(annotation, tag) {
   let orphan = annotation.$orphan;
 
   if (!annotation.id) {
-    // Currently the user ID, permissions and group of new annotations are
-    // initialized in the <annotation> component controller because the session
-    // state and focused group are not stored in the Redux store. Once they are,
-    // that initialization should be moved here.
-
     // New annotations must be anchored
     orphan = false;
   }
@@ -91,7 +95,7 @@ const update = {
     const updated = [];
     let nextTag = state.nextTag;
 
-    action.annotations.forEach(function(annot) {
+    action.annotations.forEach(annot => {
       let existing;
       if (annot.id) {
         existing = findByID(state.annotations, annot.id);
@@ -111,12 +115,12 @@ const update = {
           updatedTags[existing.$tag] = true;
         }
       } else {
-        added.push(initializeAnnot(annot, 't' + nextTag));
+        added.push(initializeAnnotation(annot, 't' + nextTag));
         ++nextTag;
       }
     });
 
-    state.annotations.forEach(function(annot) {
+    state.annotations.forEach(annot => {
       if (!updatedIDs[annot.id] && !updatedTags[annot.$tag]) {
         unchanged.push(annot);
       }
@@ -218,29 +222,14 @@ function updateFlagStatus(id, isFlagged) {
   };
 }
 
-/** Add annotations to the currently displayed set. */
-function addAnnotations(annotations, now) {
-  now = now || new Date();
-
-  // Add dates to new annotations. These are ignored by the server but used
-  // when sorting unsaved annotation cards.
-  annotations = annotations.map(function(annot) {
-    if (annot.id) {
-      return annot;
-    }
-    return Object.assign(
-      {
-        // Date.prototype.toISOString returns a 0-offset (UTC) ISO8601
-        // datetime.
-        created: now.toISOString(),
-        updated: now.toISOString(),
-      },
-      annot
-    );
-  });
-
+/**
+ * Add these `annotations` to the current collection of annotations in the store.
+ *
+ * @param {Object}[] annotations - Array of annotation objects to add.
+ */
+function addAnnotations(annotations) {
   return function(dispatch, getState) {
-    const added = annotations.filter(function(annot) {
+    const added = annotations.filter(annot => {
       return !findByID(getState().annotations.annotations, annot.id);
     });
 
@@ -250,6 +239,7 @@ function addAnnotations(annotations, now) {
       currentAnnotationCount: getState().annotations.annotations.length,
     });
 
+    // If we're not in the sidebar, we're done here.
     if (!getState().viewer.isSidebar) {
       return;
     }
@@ -330,19 +320,42 @@ function hideAnnotation(id) {
 }
 
 /**
- * Create a new annotation
+ * Create a new annotation (as-yet unpersisted)
+ *
+ * This method has several responsibilities:
+ * 1. Set some default data attributes on the annotation
+ * 2. Remove any existing, empty drafts
+ * 3. Add the annotation to the current collection of annotations
+ * 4. Change focused tab to the applicable one for the new annotation's meta-type
+ * 5. Expand all of the new annotation's parents
+ *
+ */
+function createAnnotation(ann, now = new Date()) {
+  return (dispatch, getState) => {
+    /**
+     * Extend the new, unsaved annotation object with defaults for some
+     * required data fields.
      *
- * The method does 4 tasks:
- * 1. Removes any existing empty drafts.
- * 2. Creates a new annotation.
- * 3. Changes the focused tab to match that of the newly created annotation.
- * 4. Expands the collapsed state of all new annotation's parents.
+     * Note: the `created` and `updated` values will be ignored and superseded
+     * by the service when the annotation is persisted, but they are used
+     * app-side for annotation card sorting until then.
      */
-function createAnnotation(ann) {
-  return dispatch => {
+    ann = Object.assign(
+      {
+        created: now.toISOString(),
+        group: getState().groups.focusedGroupId,
+        tags: [],
+        text: '',
+        updated: now.toISOString(),
+        user: getState().session.userid,
+        user_info: getState().session.user_info,
+      },
+      ann
+    );
     // When a new annotation is created, remove any existing annotations
     // that are empty.
     dispatch(drafts.actions.deleteNewAndEmptyDrafts([ann]));
+
     dispatch(addAnnotations([ann]));
     // If the annotation is of type note or annotation, make sure
     // the appropriate tab is selected. If it is of type reply, user

src/sidebar/store/test/index-test.js

@@ -2,12 +2,10 @@ const immutable = require('seamless-immutable');
 
 const storeFactory = require('../index');
 const annotationFixtures = require('../../test/annotation-fixtures');
-const metadata = require('../../util/annotation-metadata');
 const uiConstants = require('../../ui-constants');
 
 const defaultAnnotation = annotationFixtures.defaultAnnotation;
 const newAnnotation = annotationFixtures.newAnnotation;
-const oldPageNote = annotationFixtures.oldPageNote;
 
 const fixtures = immutable({
   pair: [
@@ -123,190 +121,6 @@ describe('store', function() {
     });
   });
 
-  describe('#addAnnotations()', function() {
-    const ANCHOR_TIME_LIMIT = 1000;
-    let clock;
-
-    beforeEach(function() {
-      clock = sinon.useFakeTimers();
-    });
-
-    afterEach(function() {
-      clock.restore();
-    });
-
-    it('adds annotations not in the store', function() {
-      const annot = defaultAnnotation();
-      store.addAnnotations([annot]);
-      assert.match(store.getState().annotations.annotations, [
-        sinon.match(annot),
-      ]);
-    });
-
-    it('does not change `selectedTab` state if annotations are already loaded', function() {
-      const annot = defaultAnnotation();
-      store.addAnnotations([annot]);
-      const page = oldPageNote();
-      store.addAnnotations([page]);
-      assert.equal(
-        store.getState().selection.selectedTab,
-        uiConstants.TAB_ANNOTATIONS
-      );
-    });
-
-    it('sets `selectedTab` to "note" if only page notes are present', function() {
-      const page = oldPageNote();
-      store.addAnnotations([page]);
-      assert.equal(
-        store.getState().selection.selectedTab,
-        uiConstants.TAB_NOTES
-      );
-    });
-
-    it('leaves `selectedTab` as "annotation" if annotations and/or page notes are present', function() {
-      const page = oldPageNote();
-      const annot = defaultAnnotation();
-      store.addAnnotations([annot, page]);
-      assert.equal(
-        store.getState().selection.selectedTab,
-        uiConstants.TAB_ANNOTATIONS
-      );
-    });
-
-    it('assigns a local tag to annotations', function() {
-      const annotA = Object.assign(defaultAnnotation(), { id: 'a1' });
-      const annotB = Object.assign(defaultAnnotation(), { id: 'a2' });
-
-      store.addAnnotations([annotA, annotB]);
-
-      const tags = store.getState().annotations.annotations.map(function(a) {
-        return a.$tag;
-      });
-
-      assert.deepEqual(tags, ['t1', 't2']);
-    });
-
-    it('updates annotations with matching IDs in the store', function() {
-      const annot = defaultAnnotation();
-      store.addAnnotations([annot]);
-      const update = Object.assign({}, defaultAnnotation(), { text: 'update' });
-      store.addAnnotations([update]);
-
-      const updatedAnnot = store.getState().annotations.annotations[0];
-      assert.equal(updatedAnnot.text, 'update');
-    });
-
-    it('updates annotations with matching tags in the store', function() {
-      const annot = newAnnotation();
-      annot.$tag = 'local-tag';
-      store.addAnnotations([annot]);
-
-      const saved = Object.assign({}, annot, { id: 'server-id' });
-      store.addAnnotations([saved]);
-
-      const annots = store.getState().annotations.annotations;
-      assert.equal(annots.length, 1);
-      assert.equal(annots[0].id, 'server-id');
-    });
-
-    // We add temporary created and updated timestamps to annotations to ensure
-    // that they sort correctly in the sidebar. These fields are ignored by the
-    // server.
-    it('adds created/updated timestamps to new annotations', function() {
-      const now = new Date();
-      const nowStr = now.toISOString();
-
-      store.addAnnotations([newAnnotation()], now);
-      const annot = store.getState().annotations.annotations[0];
-
-      assert.equal(annot.created, nowStr);
-      assert.equal(annot.updated, nowStr);
-    });
-
-    it('does not overwrite existing created/updated timestamps in new annotations', function() {
-      const now = new Date();
-      const annot = newAnnotation();
-      annot.created = '2000-01-01T01:02:03Z';
-      annot.updated = '2000-01-01T04:05:06Z';
-
-      store.addAnnotations([annot], now);
-      const result = store.getState().annotations.annotations[0];
-
-      assert.equal(result.created, annot.created);
-      assert.equal(result.updated, annot.updated);
-    });
-
-    it('preserves anchoring status of updated annotations', function() {
-      const annot = defaultAnnotation();
-      store.addAnnotations([annot]);
-      store.updateAnchorStatus({ [tagForID(annot.id)]: 'anchored' });
-
-      const update = Object.assign({}, defaultAnnotation(), { text: 'update' });
-      store.addAnnotations([update]);
-
-      const updatedAnnot = store.getState().annotations.annotations[0];
-      assert.isFalse(updatedAnnot.$orphan);
-    });
-
-    it('sets the timeout flag on annotations that fail to anchor within a time limit', function() {
-      const annot = defaultAnnotation();
-      store.addAnnotations([annot]);
-
-      clock.tick(ANCHOR_TIME_LIMIT);
-
-      assert.isTrue(store.getState().annotations.annotations[0].$anchorTimeout);
-    });
-
-    it('does not set the timeout flag on annotations that do anchor within a time limit', function() {
-      const annot = defaultAnnotation();
-      store.addAnnotations([annot]);
-      store.updateAnchorStatus({ [tagForID(annot.id)]: 'anchored' });
-
-      clock.tick(ANCHOR_TIME_LIMIT);
-
-      assert.isFalse(
-        store.getState().annotations.annotations[0].$anchorTimeout
-      );
-    });
-
-    it('does not attempt to modify orphan status if annotations are removed before anchoring timeout expires', function() {
-      const annot = defaultAnnotation();
-      store.addAnnotations([annot]);
-      store.updateAnchorStatus({ [tagForID(annot.id)]: 'anchored' });
-      store.removeAnnotations([annot]);
-
-      assert.doesNotThrow(function() {
-        clock.tick(ANCHOR_TIME_LIMIT);
-      });
-    });
-
-    it('does not expect annotations to anchor on the stream', function() {
-      const isOrphan = function() {
-        return !!metadata.isOrphan(store.getState().annotations.annotations[0]);
-      };
-
-      const annot = defaultAnnotation();
-      store.setAppIsSidebar(false);
-      store.addAnnotations([annot]);
-
-      clock.tick(ANCHOR_TIME_LIMIT);
-
-      assert.isFalse(isOrphan());
-    });
-
-    it('initializes the $orphan field for new annotations', function() {
-      store.addAnnotations([newAnnotation()]);
-      assert.isFalse(store.getState().annotations.annotations[0].$orphan);
-    });
-
-    it('adds multiple new annotations', function() {
-      store.addAnnotations([fixtures.newPair[0]]);
-      store.addAnnotations([fixtures.newPair[1]]);
-
-      assert.equal(store.getState().annotations.annotations.length, 2);
-    });
-  });
-
   describe('#removeAnnotations()', function() {
     it('removes annotations from the current state', function() {
       const annot = defaultAnnotation();

src/sidebar/test/annotation-fixtures.js

@@ -45,6 +45,8 @@ function publicAnnotation() {
 
 /** Return an annotation domain model object for a new annotation
  * (newly-created client-side, not yet saved to the server).
+ * Components will never see this data structure, as it will have been
+ * amended by store reducers.
  */
 function newAnnotation() {
   return {
@@ -54,6 +56,7 @@ function newAnnotation() {
     references: [],
     text: 'Annotation text',
     tags: ['tag_1', 'tag_2'],
+    user: 'acct:bill@localhost',
   };
 }
 
@@ -89,6 +92,7 @@ function newHighlight() {
     id: undefined,
     $highlight: true,
     target: [{ source: 'http://example.org' }],
+    user: 'acct:bill@localhost',
   };
 }