From 459863bcfffa4037ff7b3dd14ffae4b1a34fb4cc Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 14 Nov 2014 14:25:53 +0000 Subject: [PATCH] Remove scope.members from RoomController and use modelService instead. This may make things unstable. --- syweb/webclient/app-filter.js | 11 +- .../components/matrix/model-service.js | 14 +- syweb/webclient/room/room-controller.js | 159 ++--------- syweb/webclient/room/room.html | 24 +- syweb/webclient/test/unit/filters.spec.js | 254 +++++++++++++----- 5 files changed, 237 insertions(+), 225 deletions(-) diff --git a/syweb/webclient/app-filter.js b/syweb/webclient/app-filter.js index 65da0d312..916221106 100644 --- a/syweb/webclient/app-filter.js +++ b/syweb/webclient/app-filter.js @@ -49,11 +49,14 @@ angular.module('matrixWebClient') filtered.sort(function (a, b) { // Sort members on their last_active absolute time + a = a.user; + b = b.user; + var aLastActiveTS = 0, bLastActiveTS = 0; - if (undefined !== a.last_active_ago) { + if (a && undefined !== a.last_active_ago) { aLastActiveTS = a.last_updated - a.last_active_ago; } - if (undefined !== b.last_active_ago) { + if (b && undefined !== b.last_active_ago) { bLastActiveTS = b.last_updated - b.last_active_ago; } if (aLastActiveTS || bLastActiveTS) { @@ -68,8 +71,8 @@ angular.module('matrixWebClient') online: 4, free_for_chat: 3 }; - var aPresence = (a.presence in presenceLevels) ? presenceLevels[a.presence] : 0; - var bPresence = (b.presence in presenceLevels) ? presenceLevels[b.presence] : 0; + var aPresence = (a && a.event && a.event.content.presence in presenceLevels) ? presenceLevels[a.event.content.presence] : 0; + var bPresence = (b && b.event && b.event.content.presence in presenceLevels) ? presenceLevels[b.event.content.presence] : 0; return bPresence - aPresence; } }); diff --git a/syweb/webclient/components/matrix/model-service.js b/syweb/webclient/components/matrix/model-service.js index 80b40d878..e1fed6515 100644 --- a/syweb/webclient/components/matrix/model-service.js +++ b/syweb/webclient/components/matrix/model-service.js @@ -181,6 +181,7 @@ angular.module('modelService', []) /***** User Object *****/ var User = function User() { this.event = {}; // the m.presence event representing the User. + this.last_updated = 0; // used with last_active_ago to work out last seen times }; // rooms are stored here when they come in. @@ -241,7 +242,18 @@ angular.module('modelService', []) setUser: function(event) { var usr = new User(); usr.event = event; - users[event.content.user_id] = usr; + + // migrate old data but clobber matching keys + if (users[event.content.user_id] && users[event.content.user_id].event) { + angular.extend(users[event.content.user_id].event, event); + usr = users[event.content.user_id]; + } + else { + users[event.content.user_id] = usr; + } + + usr.last_updated = new Date().getTime(); + // update room members var roomMembers = userIdToRoomMember[event.content.user_id]; if (roomMembers) { diff --git a/syweb/webclient/room/room-controller.js b/syweb/webclient/room/room-controller.js index 83ed59596..f6a1eea70 100644 --- a/syweb/webclient/room/room-controller.js +++ b/syweb/webclient/room/room-controller.js @@ -38,7 +38,6 @@ angular.module('RoomController', ['ngSanitize', 'matrixFilter', 'mFileInput', 'a waiting_for_joined_event: false, // true when the join request is pending. Back to false once the corresponding m.room.member event is received messages_visibility: "hidden", // In order to avoid flickering when scrolling down the message table at the page opening, delay the message table display }; - $scope.members = {}; $scope.imageURLToSend = ""; @@ -156,14 +155,16 @@ angular.module('RoomController', ['ngSanitize', 'matrixFilter', 'mFileInput', 'a }); $scope.$on(eventHandlerService.MEMBER_EVENT, function(ngEvent, event, isLive) { - if (isLive && event.room_id === $scope.room_id) { + // if there is a live event affecting us + if (isLive && event.room_id === $scope.room_id && event.state_key === $scope.state.user_id) { if ($scope.state.waiting_for_joined_event) { // The user has successfully joined the room, we can getting data for this room $scope.state.waiting_for_joined_event = false; onInit3(); } - else if (event.state_key === $scope.state.user_id && "invite" !== event.membership && "join" !== event.membership) { - if ("ban" === event.membership) { + // if someone else changed our state.. + else if (event.user_id !== $scope.state.user_id && "invite" !== event.content.membership && "join" !== event.content.membership) { + if ("ban" === event.content.membership) { $scope.state.permission_denied = "You have been banned by " + mUserDisplayNameFilter(event.user_id); } else { @@ -172,19 +173,12 @@ angular.module('RoomController', ['ngSanitize', 'matrixFilter', 'mFileInput', 'a } else { scrollToBottom(); - updateMemberList(event); } } }); - - $scope.$on(eventHandlerService.PRESENCE_EVENT, function(ngEvent, event, isLive) { - if (isLive) { - updatePresence(event); - } - }); $scope.memberCount = function() { - return Object.keys($scope.members).length; + return Object.keys($scope.room.now.members).length; }; $scope.paginateMore = function() { @@ -257,98 +251,11 @@ angular.module('RoomController', ['ngSanitize', 'matrixFilter', 'mFileInput', 'a } ); }; - - var updateMemberList = function(chunk) { - if (chunk.room_id != $scope.room_id) return; - - - // set target_user_id to keep things clear - var target_user_id = chunk.state_key; - - var isNewMember = !(target_user_id in $scope.members); - if (isNewMember) { - - // Ignore banned and kicked (leave) people - if ("ban" === chunk.membership || "leave" === chunk.membership) { - return; - } - - // FIXME: why are we copying these fields around inside chunk? - if ("presence" in chunk.content) { - chunk.presence = chunk.content.presence; - } - if ("last_active_ago" in chunk.content) { - chunk.last_active_ago = chunk.content.last_active_ago; - $scope.now = new Date().getTime(); - chunk.last_updated = $scope.now; - } - if ("displayname" in chunk.content) { - chunk.displayname = chunk.content.displayname; - } - if ("avatar_url" in chunk.content) { - chunk.avatar_url = chunk.content.avatar_url; - } - $scope.members[target_user_id] = chunk; - - var usr = modelService.getUser(target_user_id); - if (usr) { - updatePresence(usr.event); - } - } - else { - // selectively update membership and presence else it will nuke the picture and displayname too :/ - - // Remove banned and kicked (leave) people - if ("ban" === chunk.membership || "leave" === chunk.membership) { - delete $scope.members[target_user_id]; - return; - } - - var member = $scope.members[target_user_id]; - member.membership = chunk.content.membership; - if ("presence" in chunk.content) { - member.presence = chunk.content.presence; - } - if ("last_active_ago" in chunk.content) { - member.last_active_ago = chunk.content.last_active_ago; - $scope.now = new Date().getTime(); - member.last_updated = $scope.now; - } - } - }; - var updateMemberListPresenceAge = function() { + var updatePresenceTimes = function() { $scope.now = new Date().getTime(); // TODO: don't bother polling every 5s if we know none of our counters are younger than 1 minute - $timeout(updateMemberListPresenceAge, 5 * 1000); - }; - - var updatePresence = function(chunk) { - if (!(chunk.content.user_id in $scope.members)) { - console.log("updatePresence: Unknown member for chunk " + JSON.stringify(chunk)); - return; - } - var member = $scope.members[chunk.content.user_id]; - - // XXX: why not just pass the chunk straight through? - if ("presence" in chunk.content) { - member.presence = chunk.content.presence; - } - - if ("last_active_ago" in chunk.content) { - member.last_active_ago = chunk.content.last_active_ago; - $scope.now = new Date().getTime(); - member.last_updated = $scope.now; - } - - // this may also contain a new display name or avatar url, so check. - if ("displayname" in chunk.content) { - member.displayname = chunk.content.displayname; - } - - if ("avatar_url" in chunk.content) { - member.avatar_url = chunk.content.avatar_url; - } + $timeout(updatePresenceTimes, 5 * 1000); }; $scope.send = function() { @@ -486,9 +393,7 @@ angular.module('RoomController', ['ngSanitize', 'matrixFilter', 'mFileInput', 'a var onInit2 = function() { console.log("onInit2"); - // ============================= $scope.room = modelService.getRoom($scope.room_id); - // ============================= // Scroll down as soon as possible so that we point to the last message // if it already exists in memory @@ -517,14 +422,6 @@ angular.module('RoomController', ['ngSanitize', 'matrixFilter', 'mFileInput', 'a var members = $scope.room.current_room_state.members; - // Update the member list - for (var i in members) { - if (!members.hasOwnProperty(i)) continue; - - var member = members[i].event; - updateMemberList(member); - } - // Check if the user has already join the room if ($scope.state.user_id in members) { if ("join" === members[$scope.state.user_id].event.content.membership) { @@ -572,35 +469,21 @@ angular.module('RoomController', ['ngSanitize', 'matrixFilter', 'mFileInput', 'a // Make recents highlight the current room recentsService.setSelectedRoomId($scope.room_id); + + updatePresenceTimes(); - // Get the up-to-date the current member list - matrixService.getMemberList($scope.room_id).then( - function(response) { - for (var i = 0; i < response.data.chunk.length; i++) { - var chunk = response.data.chunk[i]; - updateMemberList(chunk); - } + // Allow pagination + $scope.state.can_paginate = true; - // Arm list timing update timer - updateMemberListPresenceAge(); - - // Allow pagination - $scope.state.can_paginate = true; - - // Do a first pagination only if it is required - // FIXME: Should be no more require when initialSync/{room_id} will be available - if ($scope.state.first_pagination) { - paginate(MESSAGES_PER_PAGINATION); - } - else { - // There are already messages, go to the last message - scrollToBottom(true); - } - }, - function(error) { - $scope.feedback = "Failed get member list: " + error.data.error; - } - ); + // Do a first pagination only if it is required + // FIXME: Should be no more require when initialSync/{room_id} will be available + if ($scope.state.first_pagination) { + paginate(MESSAGES_PER_PAGINATION); + } + else { + // There are already messages, go to the last message + scrollToBottom(true); + } }; $scope.leaveRoom = function() { diff --git a/syweb/webclient/room/room.html b/syweb/webclient/room/room.html index d282a5dbe..62f6797d0 100644 --- a/syweb/webclient/room/room.html +++ b/syweb/webclient/room/room.html @@ -126,12 +126,12 @@
-
+
{{ member.displayname || member.id.substr(0, member.id.indexOf(':')) }} @@ -139,7 +139,7 @@
{{ member.id | mUserDisplayName:room_id:true }} - ({{ member.last_active_ago + (now - member.last_updated) | duration }}) + ({{ room.now.members[member.id].user.event.content.last_active_ago + (now - room.now.members[member.id].user.last_updated) | duration }})
@@ -161,21 +161,21 @@ -
- {{ msg.content.displayname || members[msg.state_key].displayname || msg.state_key }} joined + {{ msg.content.displayname || room.now.members[msg.state_key].user.event.content.displayname || msg.state_key }} joined - {{ msg.__room_member.cnt.displayname || members[msg.state_key].displayname || msg.state_key }} left + {{ msg.__room_member.cnt.displayname || room.now.members[msg.state_key].user.event.content.displayname || msg.state_key }} left - {{ msg.content.displayname || members[msg.user_id].displayname || msg.user_id }} + {{ msg.content.displayname || room.now.members[msg.user_id].user.event.content.displayname || msg.user_id }} {{ {"invite": "kicked", "join": "kicked", "ban": "unbanned"}[msg.prev_content.membership] }} {{ msg.__target_room_member.content.displayname || msg.state_key }} @@ -198,7 +198,7 @@ - {{ members[msg.user_id].displayname || msg.user_id }} changed the topic to: {{ msg.content.topic }} + {{ msg.__room_member.cnt.displayname || msg.user_id }} changed the topic to: {{ msg.content.topic }} - {{ members[msg.user_id].displayname || msg.user_id }} changed the room name to: {{ msg.content.name }} + {{ msg.__room_member.cnt.displayname || msg.user_id }} changed the room name to: {{ msg.content.name }}
- diff --git a/syweb/webclient/test/unit/filters.spec.js b/syweb/webclient/test/unit/filters.spec.js index c6253aad9..54221e76d 100644 --- a/syweb/webclient/test/unit/filters.spec.js +++ b/syweb/webclient/test/unit/filters.spec.js @@ -299,47 +299,71 @@ describe('orderMembersList filter', function() { it("should sort a single entry", function() { var output = orderMembersList({ "@a:example.com": { - last_active_ago: 50, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 50, + last_updated: 1415266943964 + } } }); expect(output).toEqual([{ id: "@a:example.com", - last_active_ago: 50, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 50, + last_updated: 1415266943964 + } }]); }); it("should sort by taking last_active_ago into account", function() { var output = orderMembersList({ "@a:example.com": { - last_active_ago: 1000, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266943964 + } }, "@b:example.com": { - last_active_ago: 50, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 50, + last_updated: 1415266943964 + } }, "@c:example.com": { - last_active_ago: 99999, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 99999, + last_updated: 1415266943964 + } } }); expect(output).toEqual([ { id: "@b:example.com", - last_active_ago: 50, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 50, + last_updated: 1415266943964 + } }, { id: "@a:example.com", - last_active_ago: 1000, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266943964 + } }, { id: "@c:example.com", - last_active_ago: 99999, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 99999, + last_updated: 1415266943964 + } }, ]); }); @@ -347,33 +371,51 @@ describe('orderMembersList filter', function() { it("should sort by taking last_updated into account", function() { var output = orderMembersList({ "@a:example.com": { - last_active_ago: 1000, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266943964 + } }, "@b:example.com": { - last_active_ago: 1000, - last_updated: 1415266900000 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266900000 + } }, "@c:example.com": { - last_active_ago: 1000, - last_updated: 1415266943000 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266943000 + } } }); expect(output).toEqual([ { id: "@a:example.com", - last_active_ago: 1000, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266943964 + } }, { id: "@c:example.com", - last_active_ago: 1000, - last_updated: 1415266943000 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266943000 + } }, { id: "@b:example.com", - last_active_ago: 1000, - last_updated: 1415266900000 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266900000 + } }, ]); }); @@ -382,33 +424,51 @@ describe('orderMembersList filter', function() { function() { var output = orderMembersList({ "@a:example.com": { - last_active_ago: 1000, - last_updated: 1415266943000 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266943000 + } }, "@b:example.com": { - last_active_ago: 100000, - last_updated: 1415266943900 + user: { + event: {}, + last_active_ago: 100000, + last_updated: 1415266943900 + } }, "@c:example.com": { - last_active_ago: 1000, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266943964 + } } }); expect(output).toEqual([ { id: "@c:example.com", - last_active_ago: 1000, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266943964 + } }, { id: "@a:example.com", - last_active_ago: 1000, - last_updated: 1415266943000 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266943000 + } }, { id: "@b:example.com", - last_active_ago: 100000, - last_updated: 1415266943900 + user: { + event: {}, + last_active_ago: 100000, + last_updated: 1415266943900 + } }, ]); }); @@ -419,33 +479,51 @@ describe('orderMembersList filter', function() { // single undefined entry var output = orderMembersList({ "@a:example.com": { - last_active_ago: 1000, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266943964 + } }, "@b:example.com": { - last_active_ago: 100000, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 100000, + last_updated: 1415266943964 + } }, "@c:example.com": { - last_active_ago: undefined, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: undefined, + last_updated: 1415266943964 + } } }); expect(output).toEqual([ { id: "@a:example.com", - last_active_ago: 1000, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 1000, + last_updated: 1415266943964 + } }, { id: "@b:example.com", - last_active_ago: 100000, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: 100000, + last_updated: 1415266943964 + } }, { id: "@c:example.com", - last_active_ago: undefined, - last_updated: 1415266943964 + user: { + event: {}, + last_active_ago: undefined, + last_updated: 1415266943964 + } }, ]); }); @@ -455,39 +533,75 @@ describe('orderMembersList filter', function() { // single undefined entry var output = orderMembersList({ "@a:example.com": { - last_active_ago: undefined, - last_updated: 1415266943964, - presence: "unavailable" + user: { + event: { + content: { + presence: "unavailable" + } + }, + last_active_ago: undefined, + last_updated: 1415266943964 + } }, "@b:example.com": { - last_active_ago: undefined, - last_updated: 1415266943964, - presence: "online" + user: { + event: { + content: { + presence: "online" + } + }, + last_active_ago: undefined, + last_updated: 1415266943964, + } }, "@c:example.com": { - last_active_ago: undefined, - last_updated: 1415266943964, - presence: "offline" + user: { + event: { + content: { + presence: "offline" + } + }, + last_active_ago: undefined, + last_updated: 1415266943964 + } } }); expect(output).toEqual([ { id: "@b:example.com", - last_active_ago: undefined, - last_updated: 1415266943964, - presence: "online" + user: { + event: { + content: { + presence: "online" + } + }, + last_active_ago: undefined, + last_updated: 1415266943964 + } }, { id: "@a:example.com", - last_active_ago: undefined, - last_updated: 1415266943964, - presence: "unavailable" + user: { + event: { + content: { + presence: "unavailable" + } + }, + last_active_ago: undefined, + last_updated: 1415266943964 + } }, { id: "@c:example.com", - last_active_ago: undefined, - last_updated: 1415266943964, - presence: "offline" + user: { + event: { + content: { + presence: "offline" + } + }, + last_active_ago: undefined, + last_updated: 1415266943964 + } }, ]); });