fix-5434

models/activities.js

@@ -1,15 +1,5 @@
 import { ReactiveCache } from '/imports/reactiveCache';
 
-// Activities don't need a schema because they are always set from the a trusted
-// environment - the server - and there is no risk that a user change the logic
-// we use with this collection. Moreover using a schema for this collection
-// would be difficult (different activities have different fields) and wouldn't
-// bring any direct advantage.
-//
-// XXX The activities API is not so nice and need some functionalities. For
-// instance if a user archive a card, and un-archive it a few seconds later we
-// should remove both activities assuming it was an error the user decided to
-// revert.
 Activities = new Mongo.Collection('activities');
 
 Activities.helpers({
@@ -52,15 +42,14 @@ Activities.helpers({
   checklistItem() {
     return ReactiveCache.getChecklistItem(this.checklistItemId);
   },
-  subtasks() {
+  subtask() {
     return ReactiveCache.getCard(this.subtaskId);
   },
   customField() {
     return ReactiveCache.getCustomField(this.customFieldId);
   },
   label() {
-    // Label activity did not work yet, unable to edit labels when tried this.
-    return ReactiveCache.getCard(this.labelId);
+    return ReactiveCache.getLabel(this.labelId); // Fixed: should get a label, not a card
   },
 });
 
@@ -80,9 +69,6 @@ Activities.after.insert((userId, doc) => {
 });
 
 if (Meteor.isServer) {
-  // For efficiency create indexes on the date of creation, and on the date of
-  // creation in conjunction with the card or board id, as corresponding views
-  // are largely used in the App. See #524.
   Meteor.startup(() => {
     Activities._collection.createIndex({ createdAt: -1 });
     Activities._collection.createIndex({ modifiedAt: -1 });
@@ -100,9 +86,6 @@ if (Meteor.isServer) {
       { customFieldId: 1 },
       { partialFilterExpression: { customFieldId: { $exists: true } } },
     );
-    // Label activity did not work yet, unable to edit labels when tried this.
-    //Activities._collection.dropIndex({ labelId: 1 }, { "indexKey": -1 });
-    //Activities._collection.dropIndex({ labelId: 1 }, { partialFilterExpression: { labelId: { $exists: true } } });
   });
 
   Activities.after.insert((userId, doc) => {
@@ -115,36 +98,23 @@ if (Meteor.isServer) {
     const params = {
       activityId: activity._id,
     };
+
     if (activity.userId) {
-      // No need send notification to user of activity
-      // participants = _.union(participants, [activity.userId]);
       const user = activity.user();
       if (user) {
-        if (user.getName()) {
-          params.user = user.getName();
-        }
-        if (user.emails) {
-          params.userEmails = user.emails;
-        }
-        if (activity.userId) {
-          params.userId = activity.userId;
-        }
+        if (user.getName()) params.user = user.getName();
+        if (user.emails) params.userEmails = user.emails;
+        params.userId = activity.userId;
       }
     }
+
     if (activity.boardId) {
-      if (board.title) {
-        if (board.title.length > 0) {
-          params.board = board.title;
-        } else {
-          params.board = '';
-        }
-      } else {
-        params.board = '';
-      }
+      params.board = board?.title || '';
       title = 'act-withBoardTitle';
-      params.url = board.absoluteUrl();
+      params.url = board?.absoluteUrl();
       params.boardId = activity.boardId;
     }
+
     if (activity.oldBoardId) {
       const oldBoard = activity.oldBoard();
       if (oldBoard) {
@@ -153,23 +123,22 @@ if (Meteor.isServer) {
         params.oldBoardId = activity.oldBoardId;
       }
     }
+
     if (activity.memberId) {
       participants = _.union(participants, [activity.memberId]);
       const member = activity.member();
-      if (member) {
-        params.member = member.getName();
-      }
+      if (member) params.member = member.getName();
     }
+
     if (activity.listId) {
       const list = activity.list();
       if (list) {
-        if (list.watchers !== undefined) {
-          watchers = _.union(watchers, list.watchers || []);
-        }
+        watchers = _.union(watchers, list.watchers || []);
         params.list = list.title;
         params.listId = activity.listId;
       }
     }
+
     if (activity.oldListId) {
       const oldList = activity.oldList();
       if (oldList) {
@@ -178,6 +147,7 @@ if (Meteor.isServer) {
         params.oldListId = activity.oldListId;
       }
     }
+
     if (activity.oldSwimlaneId) {
       const oldSwimlane = activity.oldSwimlane();
       if (oldSwimlane) {
@@ -186,6 +156,7 @@ if (Meteor.isServer) {
         params.oldSwimlaneId = activity.oldSwimlaneId;
       }
     }
+
     if (activity.cardId) {
       const card = activity.card();
       participants = _.union(participants, [card.userId], card.members || []);
@@ -195,66 +166,63 @@ if (Meteor.isServer) {
       params.url = card.absoluteUrl();
       params.cardId = activity.cardId;
     }
+
     if (activity.swimlaneId) {
       const swimlane = activity.swimlane();
-      params.swimlane = swimlane.title;
+      params.swimlane = swimlane?.title;
       params.swimlaneId = activity.swimlaneId;
     }
+
     if (activity.commentId) {
       const comment = activity.comment();
-      params.comment = comment.text;
-      if (board) {
-        const comment = params.comment;
-        const knownUsers = board.members.map(member => {
-          const u = ReactiveCache.getUser(member.userId);
-          if (u) {
-            member.username = u.username;
-            member.emails = u.emails;
-          }
-          return member;
-        });
-        const mentionRegex = /\B@(?:(?:"([\w.\s-]*)")|([\w.-]+))/gi; // including space in username
-        let currentMention;
-        while ((currentMention = mentionRegex.exec(comment)) !== null) {
-          /*eslint no-unused-vars: ["error", { "varsIgnorePattern": "[iI]gnored" }]*/
-          const [ignored, quoteduser, simple] = currentMention;
-          const username = quoteduser || simple;
-          if (username === params.user) {
-            // ignore commenter mention himself?
-            continue;
-          }
+      if (comment) {
+        params.comment = comment.text;
+        params.commentId = comment._id;
 
-          if (activity.boardId && username === 'board_members') {
-            // mentions all board members
-            const knownUids = knownUsers.map(u => u.userId);
-            watchers = _.union(watchers, [...knownUids]);
-            title = 'act-atUserComment';
-          } else if (activity.cardId && username === 'card_members') {
-            // mentions all card members if assigned
-            const card = activity.card();
-            watchers = _.union(watchers, [...card.members]);
-            title = 'act-atUserComment';
-          } else {
-            const atUser = _.findWhere(knownUsers, { username });
-            if (!atUser) {
-              continue;
+        if (board) {
+          const knownUsers = board.members.map(member => {
+            const u = ReactiveCache.getUser(member.userId);
+            if (u) {
+              member.username = u.username;
+              member.emails = u.emails;
             }
+            return member;
+          });
 
-            const uid = atUser.userId;
-            params.atUsername = username;
-            params.atEmails = atUser.emails;
-            title = 'act-atUserComment';
-            watchers = _.union(watchers, [uid]);
-          }
+          const mentionRegex = /\B@(?:(?:"([\w.\s-]*)")|([\w.-]+))/gi;
+          let currentMention;
+          while ((currentMention = mentionRegex.exec(comment.text)) !== null) {
+            const [ignored, quoteduser, simple] = currentMention;
+            const username = quoteduser || simple;
+            if (username === params.user) continue;
 
+            if (username === 'board_members') {
+              const knownUids = knownUsers.map(u => u.userId);
+              watchers = _.union(watchers, knownUids);
+              title = 'act-atUserComment';
+            } else if (username === 'card_members' && activity.cardId) {
+              const card = activity.card();
+              watchers = _.union(watchers, card.members);
+              title = 'act-atUserComment';
+            } else {
+              const atUser = _.findWhere(knownUsers, { username });
+              if (atUser) {
+                params.atUsername = username;
+                params.atEmails = atUser.emails;
+                watchers = _.union(watchers, [atUser.userId]);
+                title = 'act-atUserComment';
+              }
+            }
+          }
         }
       }
-      params.commentId = comment._id;
     }
+
     if (activity.attachmentId) {
       params.attachment = activity.attachmentName;
       params.attachmentId = activity.attachmentId;
     }
+
     if (activity.checklistId) {
       const checklist = activity.checklist();
       if (checklist) {
@@ -262,6 +230,7 @@ if (Meteor.isServer) {
         params.checklistId = activity.checklistId;
       }
     }
+
     if (activity.checklistItemId) {
       const checklistItem = activity.checklistItem();
       if (checklistItem) {
@@ -269,24 +238,20 @@ if (Meteor.isServer) {
         params.checklistItemId = activity.checklistItemId;
       }
     }
+
     if (activity.subtaskId) {
-      const subtask = activity.subtasks();
-      params.subtask = subtask.title;
-      params.subtaskId = activity.subtaskId;
+      const subtask = activity.subtask();
+      if (subtask) {
+        params.subtask = subtask.title;
+        params.subtaskId = activity.subtaskId;
+      }
     }
 
-    // Label activity did not work yet, unable to edit labels when tried this.
     if (activity.labelId) {
       const label = activity.label();
-      if (label) {  // Check if label exists
-        if (label.name) {
-          params.label = label.name;
-        } else if (label.color) {
-          params.label = label.color;
-        }
-        if (label._id) {
-          params.labelId = label._id;
-        }
+      if (label) {
+        params.label = label.name || label.color;
+        params.labelId = label._id;
       }
     }