Merge pull request #2693 from robertknight/avoid-group-reload-on-change

Avoid route reload on change

h/static/scripts/app-controller.coffee

@@ -46,9 +46,8 @@ module.exports = class AppController
       options: ['Newest', 'Oldest', 'Location']
     }
 
-    # Reload the view when the focused group changes or the
-    # list of groups that the user is a member of changes
-    reloadEvents = [events.USER_CHANGED, events.GROUP_FOCUSED];
+    # Reload the view when the user switches accounts
+    reloadEvents = [events.USER_CHANGED];
     reloadEvents.forEach((eventName) ->
       $scope.$on(eventName, (event, data) ->
         if !data || !data.initialLoad

h/static/scripts/app.coffee

@@ -19,15 +19,13 @@ resolve =
     'annotationMapper', 'drafts', 'threading'
     (annotationMapper,   drafts,   threading) ->
       # Unload all the annotations
-      idTable = threading.idTable
-      annotations = (message for id, {message} of idTable when message)
-      annotationMapper.unloadAnnotations(annotations)
+      annotationMapper.unloadAnnotations(threading.annotationList())
 
       # Reset the threading root
       threading.createIdTable([])
       threading.root = mail.messageContainer()
 
-      # Thread all the drafts
+      # Reload all unsaved annotations
       threading.thread(drafts.all())
 
       return threading

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

@@ -145,12 +145,6 @@ describe 'AppController', ->
     createController()
     assert.isFalse($scope.shareDialog.visible)
 
-  it 'reloads the view when the focused group changes', ->
-    createController()
-    fakeRoute.reload = sinon.spy()
-    $scope.$broadcast(events.GROUP_FOCUSED)
-    assert.calledOnce(fakeRoute.reload)
-
   it 'does not reload the view when the logged-in user changes on first load', ->
     createController()
     fakeRoute.reload = sinon.spy()

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

 {module, inject} = angular.mock
 
+events = require('../events')
+
 describe 'WidgetController', ->
   $scope = null
   fakeAnnotationMapper = null
   fakeAnnotationUI = null
   fakeAuth = null
   fakeCrossFrame = null
+  fakeDrafts = null
   fakeStore = null
   fakeStreamer = null
   fakeStreamFilter = null
   fakeThreading = null
   fakeGroups = null
+  lastSearchResult = null
   sandbox = null
   viewer = null
 
@@ -23,13 +27,22 @@ describe 'WidgetController', ->
   beforeEach module ($provide) ->
     sandbox = sinon.sandbox.create()
 
-    fakeAnnotationMapper = {loadAnnotations: sandbox.spy()}
+    fakeAnnotationMapper = {
+      loadAnnotations: sandbox.spy()
+      unloadAnnotations: sandbox.spy()
+    }
+
     fakeAnnotationUI = {
       tool: 'comment'
       clearSelectedAnnotations: sandbox.spy()
     }
     fakeAuth = {user: null}
     fakeCrossFrame = {frames: []}
+    fakeDrafts = {
+      all: sandbox.stub()
+    }
+
+    lastSearchResult = null
 
     fakeStore = {
       SearchResource:
@@ -39,7 +52,7 @@ describe 'WidgetController', ->
           result =
             total: 100
             rows: [offset..offset+limit-1]
-
+          lastSearchResult = result
           callback result
     }
 
@@ -56,7 +69,8 @@ describe 'WidgetController', ->
     }
 
     fakeThreading = {
-      root: {}
+      root: {},
+      thread: sandbox.stub()
     }
 
     fakeGroups = {
@@ -66,6 +80,7 @@ describe 'WidgetController', ->
     $provide.value 'annotationMapper', fakeAnnotationMapper
     $provide.value 'annotationUI', fakeAnnotationUI
     $provide.value 'crossframe', fakeCrossFrame
+    $provide.value 'drafts', fakeDrafts
     $provide.value 'store', fakeStore
     $provide.value 'streamer', fakeStreamer
     $provide.value 'streamFilter', fakeStreamFilter
@@ -92,3 +107,16 @@ describe 'WidgetController', ->
       assert.calledWith(loadSpy, [40..59])
       assert.calledWith(loadSpy, [60..79])
       assert.calledWith(loadSpy, [80..99])
+
+  describe 'when the focused group changes', ->
+    it 'should load annotations for the new group', ->
+      fakeThreading.annotationList = sandbox.stub().returns([{id: '1'}])
+      fakeCrossFrame.frames.push({uri: 'http://example.com'})
+      $scope.$broadcast(events.GROUP_FOCUSED)
+
+      assert.calledWith(fakeAnnotationMapper.unloadAnnotations,
+        [{id: '1'}])
+      $scope.$digest();
+      assert.calledWith(fakeAnnotationMapper.loadAnnotations,
+        lastSearchResult.rows)
+      assert.calledWith(fakeThreading.thread, fakeDrafts.all())

h/static/scripts/threading.coffee

 angular = require('angular')
 mail = require('./vendor/jwz')
 
-
+# The threading service provides the model for the currently loaded
+# set of annotations, structured as a tree of annotations and replies.
+#
+# The service listens for events when annotations are loaded, unloaded,
+# created or deleted and updates the tree model in response.
+#
+# The conversion of a flat list of incoming messages into a tree structure
+# with replies nested under their parents
+# uses an implementation of the `jwz` message threading algorithm
+# (see https://www.jwz.org/doc/threading.html and the JS port
+#  at https://github.com/maxogden/conversationThreading-js).
+#
+# The 'Threading' service "inherits" from 'mail.messageThread'
+#
 module.exports = class Threading
   root: null
 
@@ -61,6 +74,11 @@ module.exports = class Threading
     this.pruneEmpties(@root)
     @root
 
+  # Returns a flat list of every annotation that is currently loaded
+  # in the thread
+  annotationList: ->
+    (message for id, {message} of @idTable when message)
+
   pruneEmpties: (parent) ->
     for container in parent.children
       this.pruneEmpties(container)

h/static/scripts/widget-controller.coffee

 angular = require('angular')
 
+events = require('./events')
 
 module.exports = class WidgetController
   this.$inject = [
-    '$scope', 'annotationUI', 'crossframe', 'annotationMapper', 'groups',
+    '$scope', 'annotationUI', 'crossframe', 'annotationMapper', 'drafts', 'groups',
     'streamer', 'streamFilter', 'store', 'threading'
   ]
   constructor:   (
-     $scope,   annotationUI,   crossframe,   annotationMapper,   groups,
+     $scope,   annotationUI,   crossframe,   annotationMapper,  drafts,    groups,
      streamer,   streamFilter,   store,   threading
   ) ->
     $scope.isStream = true
@@ -17,6 +18,12 @@ module.exports = class WidgetController
     @chunkSize = 200
     loaded = []
 
+    _resetAnnotations = ->
+      # Unload all the annotations
+      annotationMapper.unloadAnnotations(threading.annotationList())
+      # Reload all the drafts
+      threading.thread(drafts.all())
+
     _loadAnnotationsFrom = (query, offset) =>
       queryCore =
         limit: @chunkSize
@@ -45,6 +52,11 @@ module.exports = class WidgetController
         streamFilter.resetFilter().addClause('/uri', 'one_of', loaded)
         streamer.send({filter: streamFilter.getFilter()})
 
+    $scope.$on events.GROUP_FOCUSED, ->
+      _resetAnnotations(annotationMapper, drafts, threading)
+      loaded = []
+      loadAnnotations crossframe.frames
+
     $scope.$watchCollection (-> crossframe.frames), loadAnnotations
 
     $scope.focus = (annotation) ->