Set group and visibility of reply to that of parent

Set the group and visibility of replies to that of their parents, when
replies are first created.

Also prevent the visibility setting of a reply from overwriting the
cached-in-local-storage visibility level. This prevents the following
problem:

- I'm annotating publicly (my cached-in-local-storage visibility
  setting is public)
- I reply to a private annotation
- The visibility of my reply is set to that of its parent, private
- This visibility is then cached in local storage
- I make a new top-level annotation, and now it's defaulting to private
  instead of public, even though the user never did anything to change
  the visibility from public to private, it was done automatically
  because they replied to a private annotation.

Obviously changing the mode from private to public when replying to a
public annotation was also possible.

This means that if the user _does_ change the visibility of a reply, we
_don't_ cache that either:

- I'm annotating publicly (my cached-in-local-storage visibility
  setting is public)
- I reply to a public annotation
- The visibility of my reply is set to that of its parent, public
- This visibility is not cached in local storage because it's a reply
  (and the value cached in the local storage is already the same anyway)
- I then deliberately change the visibility of my reply from public to
  private
- Again this visibility is not cached in local storage because it's a reply
- The next time I make a new top-level annotation, it will still be
  defaulting to public, will not have changed to private mode

h/static/scripts/directive/annotation.coffee

 ### global -validate ###
 
+STORAGE_KEY = 'hypothesis.privacy'
+
 # Validate an annotation.
 # Annotations must be attributed to a user or marked as deleted.
 # A public annotation is valid only if they have a body.
@@ -44,10 +46,10 @@ errorMessage = (reason) ->
 AnnotationController = [
   '$scope', '$timeout', '$q', '$rootScope', '$document',
   'drafts', 'flash', 'permissions', 'tags', 'time',
-  'annotationUI', 'annotationMapper', 'session', 'groups'
+  'annotationUI', 'annotationMapper', 'session', 'groups', 'localStorage'
   ($scope,   $timeout,   $q,   $rootScope,   $document,
    drafts,   flash,   permissions,   tags,   time,
-   annotationUI,   annotationMapper,   session,   groups) ->
+   annotationUI,   annotationMapper,   session,   groups,   localStorage) ->
 
     @annotation = {}
     @action = 'view'
@@ -60,9 +62,21 @@ AnnotationController = [
     @timestamp = null
 
     model = $scope.annotationGet()
+
+    # Set the group of new annotations.
     if not model.group
       model.group = groups.focused().id
 
+    # Set the permissions of new annotations.
+    if not model.permissions
+      defaultLevel = localStorage.getItem(STORAGE_KEY);
+      if (defaultLevel != 'private') and (defaultLevel != 'shared')
+        defaultLevel = 'shared';
+      if defaultLevel == 'private'
+        model.permissions = permissions.private()
+      else
+        model.permissions = permissions.public(model.group)
+
     highlight = model.$highlight
     original = null
     vm = this
@@ -122,6 +136,14 @@ AnnotationController = [
     # The changes take effect when the annotation is saved
     ###
     this.setPrivacy = (privacy) ->
+
+      # When the user changes the privacy level of an annotation they're
+      # creating or editing, we cache that and use the same privacy level the
+      # next time they create an annotation.
+      # But _don't_ cache it when they change the privacy level of a reply.
+      if not model.references  # If the annotation is not a reply.
+        localStorage.setItem(STORAGE_KEY, privacy);
+
       if privacy == 'private'
         @annotation.permissions = permissions.private()
       else if privacy == 'shared'
@@ -276,9 +298,11 @@ AnnotationController = [
 
       reply = annotationMapper.createAnnotation({references, uri})
 
+      reply.group = model.group
+
       if session.state.userid
-        if permissions.isPublic model.permissions
-          reply.permissions = permissions.public()
+        if permissions.isPublic(model.permissions, model.group)
+          reply.permissions = permissions.public(reply.group)
         else
           reply.permissions = permissions.private()
 

h/static/scripts/directive/publish-annotation-btn.js

 'use strict';
 
-var STORAGE_KEY = 'hypothesis.privacy';
-
 // @ngInject
-function PublishAnnotationBtnController($scope, localStorage) {
+function PublishAnnotationBtnController($scope) {
   var vm = this;
 
   vm.group = {
@@ -24,7 +22,6 @@ function PublishAnnotationBtnController($scope, localStorage) {
   }
 
   this.setPrivacy = function (level) {
-    localStorage.setItem(STORAGE_KEY, level);
     $scope.onSetPrivacy({level: level});
   }
 
@@ -32,18 +29,6 @@ function PublishAnnotationBtnController($scope, localStorage) {
     updatePublishActionLabel();
   });
 
-  if ($scope.isNew) {
-    // set the privacy level for new annotations.
-    // FIXME - This should be done at the time the annotation is created,
-    // not by this control
-    var defaultLevel = localStorage.getItem(STORAGE_KEY);
-    if (defaultLevel !== 'private' &&
-        defaultLevel !== 'shared') {
-      defaultLevel = 'shared';
-    }
-    this.setPrivacy(defaultLevel);
-  }
-
   function updatePublishActionLabel() {
     if ($scope.isShared) {
       vm.publishDestination = vm.group.name;

h/static/scripts/directive/test/annotation-test.coffee

@@ -26,6 +26,7 @@ describe 'annotation', ->
   fakeTags = null
   fakeTime = null
   fakeUrlEncodeFilter = null
+  fakeLocalStorage = null
   sandbox = null
 
   createDirective = ->
@@ -87,6 +88,10 @@ describe 'annotation', ->
       nextFuzzyUpdate: sandbox.stub().returns(30)
     }
     fakeUrlEncodeFilter = (v) -> encodeURIComponent(v)
+    fakeLocalStorage = {
+      setItem: sandbox.stub()
+      getItem: sandbox.stub()
+    }
 
     fakeGroups = {
       focused: -> {}
@@ -108,6 +113,7 @@ describe 'annotation', ->
     $provide.value 'tags', fakeTags
     $provide.value 'time', fakeTime
     $provide.value 'urlencodeFilter', fakeUrlEncodeFilter
+    $provide.value 'localStorage', fakeLocalStorage
     $provide.value 'groups', fakeGroups
     return
 
@@ -177,6 +183,20 @@ describe 'annotation', ->
       controller.reply()
       assert.deepEqual(reply.permissions, {read: ['everybody']})
 
+    it 'makes the annotation shared if the parent is shared', ->
+      $scope.annotation.group = "my group"
+      $scope.annotation.permissions = {read: ["my group"]}
+      reply = {}
+      fakeAnnotationMapper.createAnnotation.returns(reply)
+      fakePermissions.isPublic = (permissions, group) ->
+        return group in permissions.read
+      fakePermissions.public = (group) ->
+        return {read: [group]}
+
+      controller.reply()
+
+      assert "my group" in reply.permissions.read
+
     it 'does not add the world readable principal if the parent is private', ->
       reply = {}
       fakeAnnotationMapper.createAnnotation.returns(reply)
@@ -184,6 +204,13 @@ describe 'annotation', ->
       controller.reply()
       assert.deepEqual(reply.permissions, {read: ['justme']})
 
+    it "sets the reply's group to be the same as its parent's", ->
+      $scope.annotation.group = "my group"
+      reply = {}
+      fakeAnnotationMapper.createAnnotation.returns(reply)
+      controller.reply()
+      assert.equal(reply.group, $scope.annotation.group)
+
   describe '#setPrivacy', ->
     beforeEach ->
       createDirective()
@@ -192,9 +219,6 @@ describe 'annotation', ->
       annotation.$update = sinon.stub().returns(Promise.resolve())
       controller.edit();
       controller.setPrivacy('private')
-      # verify that permissions are not updated until the annotation
-      # is saved
-      assert.deepEqual(annotation.permissions, {})
       controller.save().then(->
         # verify that the permissions are updated once the annotation
         # is saved
@@ -205,11 +229,37 @@ describe 'annotation', ->
       annotation.$update = sinon.stub().returns(Promise.resolve())
       controller.edit();
       controller.setPrivacy('shared')
-      assert.deepEqual(annotation.permissions, {})
       controller.save().then(->
         assert.deepEqual(annotation.permissions, {read: ['everybody']})
       )
 
+    it 'saves the "shared" visibility level to localStorage', ->
+      annotation.$update = sinon.stub().returns(Promise.resolve())
+      controller.edit();
+      controller.setPrivacy('shared')
+      controller.save().then(->
+        assert(fakeLocalStorage.setItem.calledWithExactly(
+          'hypothesis.privacy', 'shared'))
+      )
+
+    it 'saves the "private" visibility level to localStorage', ->
+      annotation.$update = sinon.stub().returns(Promise.resolve())
+      controller.edit();
+      controller.setPrivacy('private')
+      controller.save().then(->
+        assert(fakeLocalStorage.setItem.calledWithExactly(
+          'hypothesis.privacy', 'private'))
+      )
+
+    it "doesn't save the visibility if the annotation is a reply", ->
+      annotation.$update = sinon.stub().returns(Promise.resolve())
+      annotation.references = ["parent id"]
+      controller.edit();
+      controller.setPrivacy('private')
+      controller.save().then(->
+        assert(not fakeLocalStorage.setItem.called)
+      )
+
   describe '#hasContent', ->
     beforeEach ->
       createDirective()
@@ -508,7 +558,7 @@ describe 'annotation', ->
         done()
       $timeout.flush()
 
-  describe "creating a new annotation", ->
+  describe "saving a new annotation", ->
     beforeEach ->
       createDirective()
       fakeFlash.error = sandbox.stub()
@@ -548,7 +598,7 @@ describe 'annotation', ->
       controller.save()
       assert fakeFlash.error.notCalled
 
-  describe "editing an annotation", ->
+  describe "saving an edited an annotation", ->
 
     beforeEach ->
       createDirective()
@@ -617,8 +667,9 @@ describe("AnnotationController", ->
   createAnnotationDirective = ({annotation, personaFilter, momentFilter,
                                 urlencodeFilter, drafts, features, flash,
                                 permissions, session, tags, time, annotationUI,
-                                annotationMapper, groups,
-                                documentTitleFilter, documentDomainFilter}) ->
+                                annotationMapper, groups, documentTitleFilter,
+                                documentDomainFilter, localStorage}) ->
+    session = session or {state: {userid: "acct:fred@hypothes.is"}}
     locals = {
       personaFilter: personaFilter or ->
       momentFilter: momentFilter or {}
@@ -635,11 +686,15 @@ describe("AnnotationController", ->
         error: ->
       }
       permissions: permissions or {
-        isPublic: -> false
-        isPrivate: -> false
+        isPublic: (permissions, group) ->
+          return group in permissions.read
+        isPrivate: (permissions, user) ->
+          return user in permissions.read
         permits: -> true
+        public: (group) -> {"read": [group]}
+        private: -> {"read": [session.state.userid]}
       }
-      session: session or {state: {}}
+      session: session
       tags: tags or {store: ->}
       time: time or {
         toFuzzyString: ->
@@ -653,6 +708,10 @@ describe("AnnotationController", ->
       }
       documentTitleFilter: documentTitleFilter or -> ''
       documentDomainFilter: documentDomainFilter or -> ''
+      localStorage: localStorage or {
+        setItem: ->
+        getItem: ->
+      }
     }
     module(($provide) ->
       $provide.value("personaFilter", locals.personaFilter)
@@ -670,6 +729,7 @@ describe("AnnotationController", ->
       $provide.value("groups", locals.groups)
       $provide.value("documentTitleFilter", locals.documentTitleFilter)
       $provide.value("documentDomainFilter", locals.documentDomainFilter)
+      $provide.value("localStorage", locals.localStorage)
       return
     )
 
@@ -691,6 +751,44 @@ describe("AnnotationController", ->
     it("creates the directive without crashing", ->
       createAnnotationDirective({})
     )
+
+    it("sets the permissions of new annotations from local storage", ->
+      {controller} = createAnnotationDirective({
+        localStorage: {
+          setItem: ->
+          getItem: (key) ->
+            if key == 'hypothesis.privacy'
+              return 'shared'
+            else
+              assert(false, "Wrong key requested from localStorage")
+        }
+      })
+      assert(controller.isShared())
+    )
+
+    it("sets the permissions of new annotations from local storage to private", ->
+      {controller} = createAnnotationDirective({
+        localStorage: {
+          setItem: ->
+          getItem: (key) ->
+            if key == 'hypothesis.privacy'
+              return 'private'
+            else
+              assert(false, "Wrong key requested from localStorage")
+        }
+      })
+      assert(controller.isPrivate())
+    )
+
+    it("defaults to shared if no locally cached visibility", ->
+      {controller} = createAnnotationDirective({
+        localStorage: {
+          setItem: ->
+          getItem: ->
+        }
+      })
+      assert(controller.isShared())
+    )
   )
 
   describe("save", ->

h/static/scripts/directive/test/publish-annotation-btn-test.js

@@ -63,16 +63,6 @@ describe('publishAnnotationBtn', function () {
     assert.equal(buttons[0].innerHTML, 'Post to Research Lab');
   });
 
-  it('should default to "shared" as the privacy level for new annotations', function () {
-    fakeStorage = {};
-    element.link({
-      isNew: true
-    });
-    assert.deepEqual(fakeStorage, {
-      'hypothesis.privacy': 'shared'
-    });
-  });
-
   it('should save when "Post..." is clicked', function () {
     var savedSpy = sinon.spy();
     element.link({