Merge pull request #2566 from hypothesis/focus-on-a-group

Implement focus on a group

h/static/scripts/app-controller.coffee

@@ -4,11 +4,13 @@ angular = require('angular')
 module.exports = class AppController
   this.$inject = [
     '$controller', '$document', '$location', '$route', '$scope', '$window',
-    'annotationUI', 'auth', 'drafts', 'features', 'identity', 'session'
+    'annotationUI', 'auth', 'drafts', 'features', 'groups', 'identity',
+    'session'
   ]
   constructor: (
      $controller,   $document,   $location,   $route,   $scope,   $window,
-     annotationUI,   auth,   drafts,   features,   identity,  session
+     annotationUI,   auth,   drafts,   features,   groups,   identity,
+     session
   ) ->
     $controller('AnnotationUIController', {$scope})
 
@@ -30,15 +32,27 @@ module.exports = class AppController
     $scope.accountDialog = visible: false
     $scope.shareDialog = visible: false
 
-    # Check to see if we are on the stream page so we can hide share button.
-    $scope.isEmbedded = $window.top isnt $window
+    # Check to see if we're in the sidebar, or on a standalone page such as
+    # the stream page or an individual annotation page.
+    $scope.isSidebar = $window.top isnt $window
 
     # Default sort
     $scope.sort = name: 'Location'
 
+    $scope.$on('groupFocused', (event) ->
+      $route.reload()
+    )
+
     identity.watch({
       onlogin: (identity) -> $scope.auth.user = auth.userid(identity)
-      onlogout: -> $scope.auth.user = null
+      onlogout: ->
+        $scope.auth.user = null
+
+        # Currently all groups are private so when the user logs out they can
+        # no longer see the annotations from any group they may have had
+        # focused. Focus the public group instead, so that they see any public
+        # annotations in the sidebar.
+        groups.focus('__world__')
       onready: -> $scope.auth.user ?= null
     })
 

h/static/scripts/app.coffee

@@ -12,8 +12,8 @@ socket = null
 resolve =
   store: ['store', (store) -> store.$promise]
   streamer: [
-    '$websocket', 'annotationMapper'
-    ($websocket,   annotationMapper) ->
+    '$websocket', 'annotationMapper', 'groups'
+    ($websocket,   annotationMapper,   groups) ->
       # Get the socket URL
       url = new URL('/ws', baseURI)
       url.protocol = url.protocol.replace('http', 'ws')
@@ -32,6 +32,14 @@ resolve =
         action = message.options.action
         annotations = message.payload
         return unless annotations?.length
+
+        # Discard annotations that aren't from the currently focused group.
+        # FIXME: Have the server only send us annotations from the focused
+        # group in the first place.
+        annotations = annotations.filter((ann) ->
+          return ann.group == groups.focused().id
+        )
+
         switch action
           when 'create', 'update', 'past'
             annotationMapper.loadAnnotations annotations

h/static/scripts/directive/annotation.coffee

@@ -32,7 +32,8 @@ errorMessage = (reason) ->
 # @property {string} action One of 'view', 'edit', 'create' or 'delete'.
 # @property {string} preview If previewing an edit then 'yes', else 'no'.
 # @property {boolean} editing True if editing components are shown.
-# @property {boolean} embedded True if the annotation is an embedded widget.
+# @property {boolean} isSidebar True if we are in the sidebar (not on the
+#                               stream page or an individual annotation page)
 #
 # @description
 #
@@ -53,7 +54,7 @@ AnnotationController = [
     @document = null
     @preview = 'no'
     @editing = false
-    @embedded = false
+    @isSidebar = false
     @hasDiff = false
     @showDiff = undefined
     @timestamp = null
@@ -402,17 +403,14 @@ AnnotationController = [
 # Directive that instantiates
 # {@link annotation.AnnotationController AnnotationController}.
 #
-# If the `annotation-embbedded` attribute is specified, its interpolated
-# value is used to signal whether the annotation is being displayed inside
-# an embedded widget.
 ###
 module.exports = [
   '$document',
   ($document) ->
     linkFn = (scope, elem, attrs, [ctrl, thread, threadFilter, counter]) ->
-      # Observe the embedded attribute
-      attrs.$observe 'annotationEmbedded', (value) ->
-        ctrl.embedded = value? and value != 'false'
+      # Observe the isSidebar attribute
+      attrs.$observe 'isSidebar', (value) ->
+        ctrl.isSidebar = value? and value != 'false'
 
       # Save on Meta + Enter or Ctrl + Enter.
       elem.on 'keydown', (event) ->

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

@@ -5,6 +5,7 @@ describe 'thread', ->
   $element = null
   $scope = null
   controller = null
+  fakeGroups = null
   fakePulse = null
   fakeRender = null
   sandbox = null
@@ -23,8 +24,12 @@ describe 'thread', ->
 
   beforeEach module ($provide) ->
     sandbox = sinon.sandbox.create()
+    fakeGroups = {
+      focused: sandbox.stub().returns({id: '__world__'})
+    }
     fakePulse = sandbox.spy()
     fakeRender = sandbox.spy()
+    $provide.value 'groups', fakeGroups
     $provide.value 'pulse', fakePulse
     $provide.value 'render', fakeRender
     return
@@ -146,6 +151,27 @@ describe 'thread', ->
             message: {}
           assert.isTrue(controller.shouldShow())
 
+      describe 'when the thread root has a group', ->
+        beforeEach ->
+          controller.container =
+            message:
+              id: 123
+              group: 'wibble'
+
+        it 'is false for draft annotations not from the focused group', ->
+          # Set the focused group to one other than the annotation's group.
+          fakeGroups.focused.returns({id: 'foo'})
+
+          # Make the annotation into a "draft" annotation (make isNew() return
+          # true).
+          delete controller.container.message.id
+
+          assert.isFalse(controller.shouldShow())
+
+        it 'is true when the focused group does match', ->
+          fakeGroups.focused.returns({id: 'wibble'})
+          assert.isTrue(controller.shouldShow())
+
     describe '#shouldShowAsReply', ->
       count = null
 

h/static/scripts/directive/thread.coffee

@@ -13,8 +13,8 @@ uuid = require('node-uuid')
 # the collapsing behavior.
 ###
 ThreadController = [
-  '$scope',
-  ($scope) ->
+  '$scope', 'groups',
+  ($scope,   groups) ->
     @container = null
     @collapsed = true
     @parent = null
@@ -44,6 +44,14 @@ ThreadController = [
     # current system state.
     ###
     this.shouldShow = ->
+      # Hide "draft" annotations (new annotations that haven't been saved to
+      # the server yet) that don't belong to the focused group. These draft
+      # annotations persist across route reloads so they have to be hidden
+      # here.
+      group = this.container?.message?.group
+      if this.isNew() and group and group != groups.focused().id
+        return false
+
       if this.container?.message?.$orphan == true
         # Hide unless show_unanchored_annotations is turned on
         if not $scope.feature('show_unanchored_annotations')

h/static/scripts/groups.js

@@ -10,7 +10,7 @@
 var STORAGE_KEY = 'hypothesis.groups.focus';
 
 // @ngInject
-function groups(localStorage, session) {
+function groups(localStorage, session, $rootScope, features) {
   // The currently focused group. This is the group that's shown as selected in
   // the groups dropdown, the annotations displayed are filtered to only ones
   // that belong to this group, and any new annotations that the user creates
@@ -41,12 +41,13 @@ function groups(localStorage, session) {
     focused: function() {
       if (focused) {
         return focused;
-      }
+      } else if (features.flagEnabled('groups')) {
         var fromStorage = get(localStorage.getItem(STORAGE_KEY));
         if (typeof fromStorage !== 'undefined') {
           focused = fromStorage;
           return focused;
         }
+      }
       return all()[0];
     },
 
@@ -56,6 +57,7 @@ function groups(localStorage, session) {
       if (typeof g !== 'undefined') {
         focused = g;
         localStorage.setItem(STORAGE_KEY, g.id);
+        $rootScope.$broadcast('groupFocused', g.id);
       }
     }
   };

h/static/scripts/test/app-controller-test.coffee

@@ -11,6 +11,8 @@ describe 'AppController', ->
   fakeLocation = null
   fakeParams = null
   fakeSession = null
+  fakeGroups = null
+  fakeRoute = null
 
   sandbox = null
 
@@ -19,7 +21,7 @@ describe 'AppController', ->
     $controller('AppController', locals)
 
   before ->
-    angular.module('h', ['ngRoute'])
+    angular.module('h')
     .controller('AppController', require('../app-controller'))
     .controller('AnnotationUIController', angular.noop)
 
@@ -62,12 +64,18 @@ describe 'AppController', ->
 
     fakeSession = {}
 
+    fakeGroups = {focus: sandbox.spy()}
+
+    fakeRoute = {reload: sandbox.spy()}
+
     $provide.value 'annotationUI', fakeAnnotationUI
     $provide.value 'auth', fakeAuth
     $provide.value 'drafts', fakeDrafts
     $provide.value 'features', fakeFeatures
     $provide.value 'identity', fakeIdentity
     $provide.value 'session', fakeSession
+    $provide.value 'groups', fakeGroups
+    $provide.value '$route', fakeRoute
     $provide.value '$location', fakeLocation
     $provide.value '$routeParams', fakeParams
     return
@@ -79,18 +87,18 @@ describe 'AppController', ->
   afterEach ->
     sandbox.restore()
 
-  describe 'isEmbedded property', ->
+  describe 'isSidebar property', ->
 
     it 'is false if the window is the top window', ->
       $window = {}
       $window.top = $window
       createController({$window})
-      assert.isFalse($scope.isEmbedded)
+      assert.isFalse($scope.isSidebar)
 
     it 'is true if the window is not the top window', ->
       $window = {top: {}}
       createController({$window})
-      assert.isTrue($scope.isEmbedded)
+      assert.isTrue($scope.isSidebar)
 
   it 'watches the identity service for identity change events', ->
     createController()
@@ -115,6 +123,12 @@ describe 'AppController', ->
     onlogout()
     assert.strictEqual($scope.auth.user, null)
 
+  it 'focuses the default group in logout', ->
+    createController()
+    {onlogout} = fakeIdentity.watch.args[0][0]
+    onlogout()
+    assert.calledWith(fakeGroups.focus, '__world__')
+
   it 'does not show login form for logged in users', ->
     createController()
     assert.isFalse($scope.accountDialog.visible)
@@ -122,3 +136,8 @@ describe 'AppController', ->
   it 'does not show the share dialog at start', ->
     createController()
     assert.isFalse($scope.shareDialog.visible)
+
+  it 'calls $route.reload() when the focused group changes', ->
+    createController()
+    $scope.$broadcast('groupFocused')
+    assert.calledOnce(fakeRoute.reload)

h/static/scripts/test/groups-test.js

@@ -19,9 +19,11 @@ var sessionWithThreeGroups = function() {
 describe('groups', function() {
   var fakeSession;
   var fakeLocalStorage;
+  var fakeRootScope;
+  var fakeFeatures;
   var sandbox;
 
-  beforeEach(function () {
+  beforeEach(function() {
     sandbox = sinon.sandbox.create();
 
     fakeSession = sessionWithThreeGroups();
@@ -29,6 +31,12 @@ describe('groups', function() {
       getItem: sandbox.stub(),
       setItem: sandbox.stub()
     };
+    fakeRootScope = {
+      $broadcast: sandbox.stub()
+    };
+    fakeFeatures = {
+      flagEnabled: function() {return true;}
+    };
   });
 
   afterEach(function () {
@@ -36,7 +44,7 @@ describe('groups', function() {
   });
 
   function service() {
-    return groups(fakeLocalStorage, fakeSession);
+    return groups(fakeLocalStorage, fakeSession, fakeRootScope, fakeFeatures);
   }
 
   describe('.all()', function() {

h/static/scripts/test/widget-controller-test.coffee

@@ -10,6 +10,7 @@ describe 'WidgetController', ->
   fakeStreamer = null
   fakeStreamFilter = null
   fakeThreading = null
+  fakeGroups = null
   sandbox = null
   viewer = null
 
@@ -58,6 +59,10 @@ describe 'WidgetController', ->
       root: {}
     }
 
+    fakeGroups = {
+      focused: -> {id: 'foo'}
+    }
+
     $provide.value 'annotationMapper', fakeAnnotationMapper
     $provide.value 'annotationUI', fakeAnnotationUI
     $provide.value 'crossframe', fakeCrossFrame
@@ -65,6 +70,7 @@ describe 'WidgetController', ->
     $provide.value 'streamer', fakeStreamer
     $provide.value 'streamFilter', fakeStreamFilter
     $provide.value 'threading', fakeThreading
+    $provide.value 'groups', fakeGroups
     return
 
   beforeEach inject ($controller, $rootScope) ->

h/static/scripts/widget-controller.coffee

@@ -3,15 +3,14 @@ angular = require('angular')
 
 module.exports = class WidgetController
   this.$inject = [
-    '$scope', 'annotationUI', 'crossframe', 'annotationMapper',
+    '$scope', 'annotationUI', 'crossframe', 'annotationMapper', 'groups',
     'streamer', 'streamFilter', 'store', 'threading'
   ]
   constructor:   (
-     $scope,   annotationUI, crossframe, annotationMapper,
+     $scope,   annotationUI,   crossframe,   annotationMapper,   groups,
      streamer,   streamFilter,   store,   threading
   ) ->
     $scope.isStream = true
-    $scope.isSidebar = true
     $scope.threadRoot = threading.root
 
     @chunkSize = 200
@@ -23,6 +22,7 @@ module.exports = class WidgetController
         offset: offset
         sort: 'created'
         order: 'asc'
+        group: groups.focused().id
       q = angular.extend(queryCore, query)
 
       store.SearchResource.get q, (results) ->

h/templates/client/annotation.html

@@ -28,11 +28,11 @@
 
       <span class="annotation-citation"
             ng-bind-html="vm.document | documentTitle"
-            ng-if="!vm.embedded">
+            ng-if="!vm.isSidebar">
       </span>
       <span class="annotation-citation-domain"
             ng-bind-html="vm.document | documentDomain"
-            ng-if="!vm.embedded">
+            ng-if="!vm.isSidebar">
       </span>
     </span>
 

h/templates/client/thread.html

@@ -14,7 +14,7 @@
 <article class="annotation thread-message {{vm.collapsed && 'collapsed'}}"
          name="annotation"
          annotation="vm.container.message"
-         annotation-embedded="{{isEmbedded}}"
+         is-sidebar="{{isSidebar}}"
          annotation-show-reply-count="{{vm.shouldShowNumReplies()}}"
          annotation-reply-count="{{vm.numReplies()}}"
          annotation-reply-count-click="vm.toggleCollapsed()"

h/templates/client/viewer.html

@@ -27,7 +27,7 @@
       </span>
       <ul class="dropdown-menu pull-right" role="menu">
         <li ng-click="sort.name = option"
-            ng-hide="option == 'Location' && !isEmbedded"
+            ng-hide="option == 'Location' && !isSidebar"
             ng-repeat="option in ['Newest', 'Oldest', 'Location']"
             ><a href="">{{option}}</a></li>
       </ul>