diff --git a/webapp/src/js/controllers/contacts.js b/webapp/src/js/controllers/contacts.js index b765d979acb..7483e4302e4 100644 --- a/webapp/src/js/controllers/contacts.js +++ b/webapp/src/js/controllers/contacts.js @@ -7,13 +7,11 @@ var _ = require('underscore'), var inboxControllers = angular.module('inboxControllers'); inboxControllers.controller('ContactsCtrl', function( - $element, $log, $q, $scope, $state, $stateParams, - $timeout, $translate, Auth, Changes, @@ -51,7 +49,10 @@ var _ = require('underscore'), var _initScroll = function() { scrollLoader.init(function() { if (!$scope.loading && $scope.moreItems) { - _query({ skip: true }); + _query({ + paginating: true, + reuseExistingDom: true, + }); } }); }; @@ -65,7 +66,7 @@ var _ = require('underscore'), $scope.error = false; } - if (options.skip) { + if (options.paginating) { $scope.appending = true; options.skip = liveList.count(); } else if (!options.silent) { @@ -146,8 +147,11 @@ var _ = require('underscore'), $scope.moreItems = liveList.moreItems = contacts.length >= options.limit; - contacts.forEach(liveList.update); - liveList.refresh(); + const mergedList = options.paginating ? + _.uniq(contacts.concat(liveList.getList()), false, _.property('_id')) + : contacts; + liveList.set(mergedList, !!options.reuseExistingDom); + _initScroll(); $scope.loading = false; $scope.appending = false; @@ -430,16 +434,33 @@ var _ = require('underscore'), var changeListener = Changes({ key: 'contacts-list', callback: function(change) { - var limit = liveList.count(); + const limit = liveList.count(); if (change.deleted && change.doc.type !== 'data_record') { liveList.remove(change.doc); } - var withIds = + if (change.doc) { + liveList.invalidateCache(change.doc._id); + + // Invalidate the contact for changing reports with visited_contact_uuid + if (change.doc.fields) { + liveList.invalidateCache(change.doc.fields.visited_contact_uuid); + } + } + + const withIds = isSortedByLastVisited() && !!isRelevantVisitReport(change.doc) && !change.deleted; - return _query({ limit: limit, silent: true, withIds: withIds }); + return _query({ + limit, + withIds, + silent: true, + + // The logic for updating Primary Contact changes is complex + // So redraw everything when a person changes + reuseExistingDom: change.doc && change.doc.type !== 'person', + }); }, filter: function(change) { return ( diff --git a/webapp/src/js/services/live-list.js b/webapp/src/js/services/live-list.js index da11367bd96..bc576760c5c 100644 --- a/webapp/src/js/services/live-list.js +++ b/webapp/src/js/services/live-list.js @@ -328,9 +328,7 @@ angular.module('inboxServices').factory('LiveList', var activeDom = $(idx.selector); if(activeDom.length) { activeDom.empty(); - _.each(idx.dom, function(li) { - activeDom.append(li); - }); + appendDomWithListOrdering(activeDom, idx); ResourceIcons.replacePlaceholders(activeDom); } } @@ -340,26 +338,29 @@ angular.module('inboxServices').factory('LiveList', return idx.list && idx.list.length; } - function _set(listName, items) { - var i, len, - idx = indexes[listName]; - + /* + reuseExistingDom is a performance optimization wherein live-list can rely on the changes feed to + specifically update dom elements (via update/remove interfaces) making it safe to re-use existing dom + elements for certain scenarios + */ + function _set(listName, items, reuseExistingDom) { + const idx = indexes[listName]; if (!idx) { throw new Error('LiveList not configured for: ' + listName); } idx.lastUpdate = new Date(); - - // TODO we should sort the list in place with a suitable, efficient algorithm - idx.list = []; - idx.dom = []; - for (i=0, len=items.length; i=0; --i) { - if(list[i]._id === item._id) { - return true; - } - } - return false; + return !!indexes[listName].dom[item._id]; } function _insert(listName, newItem, skipDomAppend, removedDomElement) { @@ -392,7 +386,7 @@ angular.module('inboxServices').factory('LiveList', var newItemIndex = findSortedIndex(idx.list, newItem, idx.orderBy); idx.list.splice(newItemIndex, 0, newItem); - idx.dom.splice(newItemIndex, 0, li); + idx.dom[newItem._id] = li; if (skipDomAppend) { return; @@ -410,6 +404,15 @@ angular.module('inboxServices').factory('LiveList', } } + function _invalidateCache(listName, id) { + const idx = indexes[listName]; + if (!idx || !idx.dom || !id || !idx.dom[id]) { + return; + } + + idx.dom[id].invalidateCache = true; + } + function _update(listName, updatedItem) { var removed = _remove(listName, updatedItem); _insert(listName, updatedItem, false, removed); @@ -431,55 +434,44 @@ angular.module('inboxServices').factory('LiveList', } if (removeIndex !== null) { idx.list.splice(removeIndex, 1); - var removed = idx.dom.splice(removeIndex, 1); + const removed = idx.dom[removedItem._id]; + delete idx.dom[removedItem._id]; $(idx.selector).children().eq(removeIndex).remove(); - if (removed.length) { - return removed[0]; - } + return removed; } } function _setSelected(listName, _id) { - var i, len, doc, - idx = indexes[listName], - list = idx.list, - previous = idx.selected; + const idx = indexes[listName], + previous = idx.selected; idx.selected = _id; - if (!list) { + if (!idx.list) { return; } - for (i=0, len=list.length; i=0; --i) { - idx.dom[i] = listItemFor(idx, idx.list[i]); + for (let i = 0; i < idx.list.length; ++i) { + const item = idx.list[i]; + idx.dom[item._id] = listItemFor(idx, item); } api[name].refresh(); - idx.lastUpdate = now; } }); @@ -534,6 +526,11 @@ angular.module('inboxServices').factory('LiveList', return midnight.getTime() - now.getTime(); } + function appendDomWithListOrdering(activeDom, idx) { + const orderedDom = idx.list.map(item => idx.dom[item._id]); + activeDom.append(orderedDom); + } + $timeout(refreshAll, millisTilMidnight(new Date())); api.$listFor = function(name, config) { @@ -543,6 +540,7 @@ angular.module('inboxServices').factory('LiveList', api[name] = { insert: _.partial(_insert, name), + invalidateCache: _.partial(_invalidateCache, name), update: _.partial(_update, name), remove: _.partial(_remove, name), getList: _.partial(_getList, name), diff --git a/webapp/tests/karma/unit/controllers/contacts.js b/webapp/tests/karma/unit/controllers/contacts.js index 93c40aaa4d3..37dad6ce72d 100644 --- a/webapp/tests/karma/unit/controllers/contacts.js +++ b/webapp/tests/karma/unit/controllers/contacts.js @@ -50,6 +50,7 @@ describe('Contacts controller', () => { refresh: sinon.stub(), count: () => elements.length, insert: e => elements.push(e), + invalidateCache: () => {}, set: es => (elements = es), update: e => { if (e !== district || elements[0] !== district) { @@ -565,8 +566,12 @@ describe('Contacts controller', () => { const lhs = contactsLiveList.getList(); assert.equal(lhs.length, 50); scrollLoaderCallback(); - assert.equal(searchService.args[1][2].skip, 50); - assert.equal(searchService.args[1][2].limit, 50); + assert.deepEqual(searchService.args[1][2], { + reuseExistingDom: true, + paginating: true, + limit: 50, + skip: 50, + }); }); }); @@ -580,8 +585,12 @@ describe('Contacts controller', () => { const lhs = contactsLiveList.getList(); assert.equal(lhs.length, 51); scrollLoaderCallback(); - assert.equal(searchService.args[1][2].skip, 50); - assert.equal(searchService.args[1][2].limit, 50); + assert.deepEqual(searchService.args[1][2], { + reuseExistingDom: true, + paginating: true, + limit: 50, + skip: 50, + }); }); }); @@ -599,8 +608,11 @@ describe('Contacts controller', () => { const lhs = contactsLiveList.getList(); changesCallback({}); assert.equal(lhs.length, 10); - assert.equal(searchService.args[1][2].limit, 10); - assert.equal(searchService.args[1][2].skip, undefined); + assert.deepInclude(searchService.args[1][2], { + limit: 10, + silent: true, + withIds: false, + }); }); }); @@ -633,8 +645,7 @@ describe('Contacts controller', () => { searchResults = Array(50).fill(searchResult); searchService.returns(Promise.resolve(searchResults)); scope.search(); - assert.equal(searchService.args[1][2].limit, 50); - assert.equal(searchService.args[1][2].skip, undefined); + assert.deepEqual(searchService.args[1][2], { limit: 50 }); return Promise.resolve(); }) .then(() => { @@ -642,8 +653,12 @@ describe('Contacts controller', () => { assert.equal(lhs.length, 50); //aand paginate the search results, also not skipping the extra place scrollLoaderCallback(); - assert.equal(searchService.args[2][2].skip, 50); - assert.equal(searchService.args[2][2].limit, 50); + assert.deepEqual(searchService.args[2][2], { + limit: 50, + paginating: true, + reuseExistingDom: true, + skip: 50, + }); }); }); @@ -658,36 +673,54 @@ describe('Contacts controller', () => { const lhs = contactsLiveList.getList(); assert.equal(lhs.length, 50); scrollLoaderCallback(); - assert.equal(searchService.args[1][2].skip, 50); - assert.equal(searchService.args[1][2].limit, 50); + assert.deepEqual(searchService.args[1][2], { + limit: 50, + paginating: true, + reuseExistingDom: true, + skip: 50, + }); }); }); it('when paginating, does not modify the skip when it finds homeplace on subsequent pages #4085', () => { - const searchResult = { _id: 'search-result' }; - searchResults = Array(50).fill(searchResult); + const mockResults = (count, startAt = 0) => { + const result = []; + for (let i = startAt; i < startAt + count; i++) { + result.push({ _id: `search-result${i}` }); + } + return result; + }; + searchResults = mockResults(50); return createController() .getSetupPromiseForTesting({ scrollLoaderStub }) .then(() => { const lhs = contactsLiveList.getList(); assert.equal(lhs.length, 51); - searchResults = Array(49).fill(searchResult); + searchResults = mockResults(49, 50); searchResults.push(district); searchService.returns(Promise.resolve(searchResults)); scrollLoaderCallback(); - assert.equal(searchService.args[1][2].skip, 50); - assert.equal(searchService.args[1][2].limit, 50); + assert.deepEqual(searchService.args[1][2], { + limit: 50, + paginating: true, + reuseExistingDom: true, + skip: 50, + }); return Promise.resolve(); }) .then(() => { const lhs = contactsLiveList.getList(); assert.equal(lhs.length, 100); - searchResults = Array(50).fill(searchResult); + searchResults = mockResults(50, 100); searchService.returns(Promise.resolve(searchResults)); scrollLoaderCallback(); - assert.equal(searchService.args[2][2].skip, 100); - assert.equal(searchService.args[2][2].limit, 50); + assert.deepEqual(searchService.args[2][2], { + limit: 50, + paginating: true, + reuseExistingDom: true, + skip: 100, + }); return Promise.resolve(); }) .then(() => { @@ -1114,7 +1147,7 @@ describe('Contacts controller', () => { assert.deepEqual(searchService.args[i], [ 'contacts', { types: { selected: ['childType'] } }, - { limit: 5, withIds: false, silent: true }, + { limit: 5, withIds: false, silent: true, reuseExistingDom: i !== 5 }, { displayLastVisitedDate: true, visitCountSettings: {} }, undefined, ]); @@ -1144,7 +1177,7 @@ describe('Contacts controller', () => { assert.deepEqual(searchService.args[1], [ 'contacts', { types: { selected: ['childType'] } }, - { limit: 5, withIds: true, silent: true }, + { limit: 5, withIds: true, silent: true, reuseExistingDom: true }, { displayLastVisitedDate: true, visitCountSettings: {}, @@ -1157,7 +1190,7 @@ describe('Contacts controller', () => { assert.deepEqual(searchService.args[i], [ 'contacts', { types: { selected: ['childType'] } }, - { limit: 5, withIds: false, silent: true }, + { limit: 5, withIds: false, silent: true, reuseExistingDom: i !== 5 }, { displayLastVisitedDate: true, visitCountSettings: {}, @@ -1200,7 +1233,7 @@ describe('Contacts controller', () => { assert.deepEqual(searchService.args[i], [ 'contacts', { types: { selected: ['childType'] } }, - { limit: 5, withIds: false, silent: true }, + { limit: 5, withIds: false, silent: true, reuseExistingDom: i !== 5 }, { displayLastVisitedDate: true, visitCountSettings: {} }, undefined, ]); @@ -1229,7 +1262,7 @@ describe('Contacts controller', () => { assert.deepEqual(searchService.args[1], [ 'contacts', { types: { selected: ['childType'] } }, - { limit: 5, withIds: true, silent: true }, + { limit: 5, withIds: true, silent: true, reuseExistingDom: true }, { displayLastVisitedDate: true, visitCountSettings: {}, @@ -1242,7 +1275,7 @@ describe('Contacts controller', () => { assert.deepEqual(searchService.args[i], [ 'contacts', { types: { selected: ['childType'] } }, - { limit: 5, withIds: false, silent: true }, + { limit: 5, withIds: false, silent: true, reuseExistingDom: i !== 5 }, { displayLastVisitedDate: true, visitCountSettings: {}, @@ -1285,7 +1318,7 @@ describe('Contacts controller', () => { assert.deepEqual(searchService.args[i], [ 'contacts', { types: { selected: ['childType'] } }, - { limit: 5, withIds: false, silent: true }, + { limit: 5, withIds: false, silent: true, reuseExistingDom: i !== 5 }, {}, undefined, ]); @@ -1324,7 +1357,7 @@ describe('Contacts controller', () => { assert.deepEqual(searchService.args[2], [ 'contacts', { types: { selected: ['childType'] } }, - { limit: 5, withIds: false, silent: true }, + { limit: 5, withIds: false, silent: true, reuseExistingDom: true }, {}, undefined, ]); diff --git a/webapp/tests/karma/unit/services/live-list.js b/webapp/tests/karma/unit/services/live-list.js index fbc220f7bb3..8fb40b52f76 100644 --- a/webapp/tests/karma/unit/services/live-list.js +++ b/webapp/tests/karma/unit/services/live-list.js @@ -123,6 +123,7 @@ describe('LiveListSrv', function() { // then assert.deepEqual(_.keys(service.name), [ 'insert', + 'invalidateCache', 'update', 'remove', 'getList',