Merge pull request #426 from floatinghotpot/patch-8

bugfix: only care active members, also optimize some code

9 files changed, 62 insertions, 78 deletions

diff --git a/client/components/cards/cardDetails.jade b/client/components/cards/cardDetails.jade
index 9d2cbec7..5746c92c 100644
--- a/client/components/cards/cardDetails.jade
+++ b/client/components/cards/cardDetails.jade
@@ -94,7 +94,7 @@ template(name="moveCardPopup")
 
 template(name="cardMembersPopup")
   ul.pop-over-list.js-card-member-list
-    each board.members
+    each board.activeMembers
       li.item(class="{{#if isCardMember}}active{{/if}}")
         a.name.js-select-member(href="#")
           +userAvatar(userId=user._id)
diff --git a/client/components/lists/listBody.js b/client/components/lists/listBody.js
index 36b60d06..1fc84c82 100644
--- a/client/components/lists/listBody.js
+++ b/client/components/lists/listBody.js
@@ -186,7 +186,7 @@ BlazeComponent.extendComponent({
         match: /\B@(\w*)$/,
         search(term, callback) {
           const currentBoard = Boards.findOne(Session.get('currentBoard'));
-          callback($.map(currentBoard.members, (member) => {
+          callback($.map(currentBoard.activeMembers(), (member) => {
             const user = Users.findOne(member.userId);
             return user.username.indexOf(term) === 0 ? user : null;
           }));
diff --git a/client/components/main/editor.js b/client/components/main/editor.js
index 82fce641..a30a5e1d 100644
--- a/client/components/main/editor.js
+++ b/client/components/main/editor.js
@@ -28,7 +28,7 @@ Template.editor.onRendered(() => {
       match: /\B@(\w*)$/,
       search(term, callback) {
         const currentBoard = Boards.findOne(Session.get('currentBoard'));
-        callback(currentBoard.members.map((member) => {
+        callback(currentBoard.activeMembers().map((member) => {
           const username = Users.findOne(member.userId).username;
           return username.includes(term) ? username : null;
         }));
diff --git a/client/components/sidebar/sidebar.js b/client/components/sidebar/sidebar.js
index 5b58dbd9..ec3ebcf6 100644
--- a/client/components/sidebar/sidebar.js
+++ b/client/components/sidebar/sidebar.js
@@ -279,7 +279,7 @@ BlazeComponent.extendComponent({
       'click .js-select-member'() {
         const userId = this.currentData()._id;
         const currentBoard = Boards.findOne(Session.get('currentBoard'));
-        if (currentBoard.memberIndex(userId)<0) {
+        if (!currentBoard.hasMember(userId)) {
           this.inviteUser(userId);
         }
       },
@@ -305,16 +305,12 @@ Template.changePermissionsPopup.events({
 
 Template.changePermissionsPopup.helpers({
   isAdmin() {
-    const user = Users.findOne(this.userId);
-    return user.isBoardAdmin();
+    const currentBoard = Boards.findOne(Session.get('currentBoard'));
+    return currentBoard.hasAdmin(this.userId);
   },
 
   isLastAdmin() {
-    const user = Users.findOne(this.userId);
-    if (!user.isBoardAdmin())
-      return false;
     const currentBoard = Boards.findOne(Session.get('currentBoard'));
-    const nbAdmins = _.where(currentBoard.members, { isAdmin: true }).length;
-    return nbAdmins === 1;
+    return currentBoard.hasAdmin(this.userId) && (currentBoard.activeAdmins() === 1);
   },
 });
diff --git a/client/components/sidebar/sidebarFilters.jade b/client/components/sidebar/sidebarFilters.jade
index ef26ef76..0e935e3f 100644
--- a/client/components/sidebar/sidebarFilters.jade
+++ b/client/components/sidebar/sidebarFilters.jade
@@ -18,17 +18,16 @@ template(name="filterSidebar")
             i.fa.fa-check
   hr
   ul.sidebar-list
-    each currentBoard.members
-      if isActive
-        with getUser userId
-          li(class="{{#if Filter.members.isSelected _id}}active{{/if}}")
-            a.name.js-toggle-member-filter
-              +userAvatar(userId=this._id)
-              span.sidebar-list-item-description
-                = profile.fullname
-                | (<span class="username">{{ username }}</span>)
-              if Filter.members.isSelected _id
-                i.fa.fa-check
+    each currentBoard.activeMembers
+      with getUser userId
+        li(class="{{#if Filter.members.isSelected _id}}active{{/if}}")
+          a.name.js-toggle-member-filter
+            +userAvatar(userId=this._id)
+            span.sidebar-list-item-description
+              = profile.fullname
+              | (<span class="username">{{ username }}</span>)
+            if Filter.members.isSelected _id
+              i.fa.fa-check
   if Filter.isActive
     hr
     a.sidebar-btn.js-clear-all
@@ -55,19 +54,18 @@ template(name="multiselectionSidebar")
             i.fa.fa-ellipsis-h
   hr
   ul.sidebar-list
-    each currentBoard.members
-      if isActive
-        with getUser userId
-          li(class="{{#if Filter.members.isSelected _id}}active{{/if}}")
-            a.name.js-toggle-member-multiselection
-              +userAvatar(userId=this._id)
-              span.sidebar-list-item-description
-                = profile.fullname
-                | (<span class="username">{{ username }}</span>)
-              if allSelectedElementHave 'member' _id
-                i.fa.fa-check
-              else if someSelectedElementHave 'member' _id
-                i.fa.fa-ellipsis-h
+    each currentBoard.activeMembers
+      with getUser userId
+        li(class="{{#if Filter.members.isSelected _id}}active{{/if}}")
+          a.name.js-toggle-member-multiselection
+            +userAvatar(userId=this._id)
+            span.sidebar-list-item-description
+              = profile.fullname
+              | (<span class="username">{{ username }}</span>)
+            if allSelectedElementHave 'member' _id
+              i.fa.fa-check
+            else if someSelectedElementHave 'member' _id
+              i.fa.fa-ellipsis-h
   hr
   a.sidebar-btn.js-archive-selection
     i.fa.fa-archive
diff --git a/models/boards.js b/models/boards.js
index 6aba0b1e..9c792674 100644
--- a/models/boards.js
+++ b/models/boards.js
@@ -115,6 +115,14 @@ Boards.helpers({
     return _.pluck(this.members, 'userId').indexOf(memberId);
   },
 
+  hasMember(memberId) {
+    return !!_.findWhere(this.members, {userId: memberId, isActive: true});
+  },
+
+  hasAdmin(memberId) {
+    return !!_.findWhere(this.members, {userId: memberId, isActive: true, isAdmin: true});
+  },
+
   absoluteUrl() {
     return FlowRouter.path('board', { id: this._id, slug: this.slug });
   },
@@ -186,34 +194,23 @@ Boards.mutations({
 
   addMember(memberId) {
     const memberIndex = this.memberIndex(memberId);
-    if (memberIndex === -1) {
-      const xIndex = this.memberIndex('x');
-      if (xIndex === -1) {
-        return {
-          $push: {
-            members: {
-              userId: memberId,
-              isAdmin: false,
-              isActive: true,
-            },
-          },
-        };
-      } else {
-        return {
-          $set: {
-            [`members.${xIndex}.userId`]: memberId,
-            [`members.${xIndex}.isActive`]: true,
-            [`members.${xIndex}.isAdmin`]: false,
-          },
-        };
-      }
-    } else {
+    if (memberIndex >= 0) {
       return {
         $set: {
           [`members.${memberIndex}.isActive`]: true,
         },
       };
     }
+
+    return {
+      $push: {
+        members: {
+          userId: memberId,
+          isAdmin: false,
+          isActive: true,
+        },
+      },
+    };
   },
 
   removeMember(memberId) {
@@ -221,22 +218,20 @@ Boards.mutations({
 
     // we do not allow the only one admin to be removed
     const allowRemove = (!this.members[memberIndex].isAdmin) || (this.activeAdmins().length > 1);
-
-    if (allowRemove) {
-      return {
-        $set: {
-          [`members.${memberIndex}.userId`]: 'x',
-          [`members.${memberIndex}.isActive`]: false,
-          [`members.${memberIndex}.isAdmin`]: false,
-        },
-      };
-    } else {
+    if (!allowRemove) {
       return {
         $set: {
           [`members.${memberIndex}.isActive`]: true,
         },
       };
     }
+
+    return {
+      $set: {
+        [`members.${memberIndex}.isActive`]: false,
+        [`members.${memberIndex}.isAdmin`]: false,
+      },
+    };
   },
 
   setMemberPermission(memberId, isAdmin) {
diff --git a/models/users.js b/models/users.js
index 2c9ae380..cf4c4193 100644
--- a/models/users.js
+++ b/models/users.js
@@ -12,16 +12,12 @@ if (Meteor.isClient) {
   Users.helpers({
     isBoardMember() {
       const board = Boards.findOne(Session.get('currentBoard'));
-      return board &&
-        _.contains(_.pluck(board.members, 'userId'), this._id) &&
-        _.where(board.members, {userId: this._id})[0].isActive;
+      return board && board.hasMember(this._id);
     },
 
     isBoardAdmin() {
       const board = Boards.findOne(Session.get('currentBoard'));
-      return board &&
-        this.isBoardMember(board) &&
-        _.where(board.members, {userId: this._id})[0].isAdmin;
+      return board && board.hasAdmin(this._id);
     },
   });
 }
diff --git a/server/lib/utils.js b/server/lib/utils.js
index 3e141f06..b59671fb 100644
--- a/server/lib/utils.js
+++ b/server/lib/utils.js
@@ -1,8 +1,7 @@
 allowIsBoardAdmin = function(userId, board) {
-  const admins = _.pluck(_.where(board.members, {isAdmin: true}), 'userId');
-  return _.contains(admins, userId);
+  return board && board.hasAdmin(userId);
 };
 
 allowIsBoardMember = function(userId, board) {
-  return _.contains(_.pluck(board.members, 'userId'), userId);
+  return board && board.hasMember(userId);
 };
diff --git a/server/publications/boards.js b/server/publications/boards.js
index 814d1df8..ffb077e9 100644
--- a/server/publications/boards.js
+++ b/server/publications/boards.js
@@ -16,7 +16,7 @@ Meteor.publish('boards', function() {
   return Boards.find({
     archived: false,
     $or: [
-      { 'members.userId': this.userId },
+      { members: { $elemMatch: { userId: this.userId, isActive: true }}},
       { _id: { $in: starredBoards } },
     ],
   }, {
@@ -66,7 +66,7 @@ Meteor.publishComposite('board', function(boardId) {
         // it.
         $or: [
           { permission: 'public' },
-          { 'members.userId': this.userId },
+          { members: { $elemMatch: { userId: this.userId, isActive: true }}},
         ],
       }, { limit: 1 });
     },