Merge branch 'zarnifoulette-devel' into devel

Fix: Activity user messed up when creating a card using the REST-API.
Thanks to zarnifoulette ! Closes #1045

3 files changed, 206 insertions, 150 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4d9acf35..7e38c994 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,7 +4,11 @@ This release adds the following new features:
 
 * [Export and import attachments as base64 encoded files](https://github.com/wekan/wekan/pull/1134);
 
-Thanks to GitHub user GhassenRjab for contributions.
+and fixes the following bugs:
+
+* [Activity user messed up when creating a card using the REST-API](https://github.com/wekan/wekan/pull/1116);
+
+Thanks to GitHub users GhassenRjab and zarnifoulette for their contributions.
 
 # v0.28 2017-07-15 Wekan release
 
diff --git a/models/cardComments.js b/models/cardComments.js
index e51275a4..352030f1 100644
--- a/models/cardComments.js
+++ b/models/cardComments.js
@@ -56,6 +56,16 @@ CardComments.helpers({
 
 CardComments.hookOptions.after.update = { fetchPrevious: false };
 
+function commentCreation(userId, doc){
+  Activities.insert({
+    userId,
+    activityType: 'addComment',
+    boardId: doc.boardId,
+    cardId: doc.cardId,
+    commentId: doc._id,
+  });
+}
+
 if (Meteor.isServer) {
   // Comments are often fetched within a card, so we create an index to make these
   // queries more efficient.
@@ -64,13 +74,7 @@ if (Meteor.isServer) {
   });
 
   CardComments.after.insert((userId, doc) => {
-    Activities.insert({
-      userId,
-      activityType: 'addComment',
-      boardId: doc.boardId,
-      cardId: doc.cardId,
-      commentId: doc._id,
-    });
+    commentCreation(userId, doc);
   });
 
   CardComments.after.remove((userId, doc) => {
@@ -114,12 +118,16 @@ if (Meteor.isServer) {
     Authentication.checkUserId( req.userId);
     const paramBoardId = req.params.boardId;
     const paramCardId = req.params.cardId;
-    const id = CardComments.insert({
+    const id = CardComments.direct.insert({
       userId: req.body.authorId,
       text: req.body.comment,
       cardId: paramCardId,
       boardId: paramBoardId,
     });
+
+    const cardComment = CardComments.findOne({_id: id, cardId:paramCardId, boardId: paramBoardId });
+    commentCreation(req.body.authorId, cardComment);
+
     JsonRoutes.sendResult(res, {
       code: 200,
       data: {
diff --git a/models/cards.js b/models/cards.js
index 415c3bde..0a440697 100644
--- a/models/cards.js
+++ b/models/cards.js
@@ -18,9 +18,9 @@ Cards.attachSchema(new SimpleSchema({
   listId: {
     type: String,
   },
-  // The system could work without this `boardId` information (we could deduce
-  // the board identifier from the card), but it would make the system more
-  // difficult to manage and less efficient.
+    // The system could work without this `boardId` information (we could deduce
+    // the board identifier from the card), but it would make the system more
+    // difficult to manage and less efficient.
   boardId: {
     type: String,
   },
@@ -64,8 +64,8 @@ Cards.attachSchema(new SimpleSchema({
     type: Date,
     optional: true,
   },
-  // XXX Should probably be called `authorId`. Is it even needed since we have
-  // the `members` field?
+    // XXX Should probably be called `authorId`. Is it even needed since we have
+    // the `members` field?
   userId: {
     type: String,
     autoValue() { // eslint-disable-line consistent-return
@@ -123,26 +123,26 @@ Cards.helpers({
   },
 
   activities() {
-    return Activities.find({ cardId: this._id }, { sort: { createdAt: -1 } });
+    return Activities.find({cardId: this._id}, {sort: {createdAt: -1}});
   },
 
   comments() {
-    return CardComments.find({ cardId: this._id }, { sort: { createdAt: -1 } });
+    return CardComments.find({cardId: this._id}, {sort: {createdAt: -1}});
   },
 
   attachments() {
-    return Attachments.find({ cardId: this._id }, { sort: { uploadedAt: -1 } });
+    return Attachments.find({cardId: this._id}, {sort: {uploadedAt: -1}});
   },
 
   cover() {
     const cover = Attachments.findOne(this.coverId);
-    // if we return a cover before it is fully stored, we will get errors when we try to display it
-    // todo XXX we could return a default "upload pending" image in the meantime?
+        // if we return a cover before it is fully stored, we will get errors when we try to display it
+        // todo XXX we could return a default "upload pending" image in the meantime?
     return cover && cover.url() && cover;
   },
 
   checklists() {
-    return Checklists.find({ cardId: this._id }, { sort: { createdAt: 1 } });
+    return Checklists.find({cardId: this._id}, {sort: {createdAt: 1}});
   },
 
   checklistItemCount() {
@@ -183,35 +183,35 @@ Cards.helpers({
 
 Cards.mutations({
   archive() {
-    return { $set: { archived: true } };
+    return {$set: {archived: true}};
   },
 
   restore() {
-    return { $set: { archived: false } };
+    return {$set: {archived: false}};
   },
 
   setTitle(title) {
-    return { $set: { title } };
+    return {$set: {title}};
   },
 
   setDescription(description) {
-    return { $set: { description } };
+    return {$set: {description}};
   },
 
   move(listId, sortIndex) {
-    const mutatedFields = { listId };
+    const mutatedFields = {listId};
     if (sortIndex) {
       mutatedFields.sort = sortIndex;
     }
-    return { $set: mutatedFields };
+    return {$set: mutatedFields};
   },
 
   addLabel(labelId) {
-    return { $addToSet: { labelIds: labelId } };
+    return {$addToSet: {labelIds: labelId}};
   },
 
   removeLabel(labelId) {
-    return { $pull: { labelIds: labelId } };
+    return {$pull: {labelIds: labelId}};
   },
 
   toggleLabel(labelId) {
@@ -223,11 +223,11 @@ Cards.mutations({
   },
 
   assignMember(memberId) {
-    return { $addToSet: { members: memberId } };
+    return {$addToSet: {members: memberId}};
   },
 
   unassignMember(memberId) {
-    return { $pull: { members: memberId } };
+    return {$pull: {members: memberId}};
   },
 
   toggleMember(memberId) {
@@ -239,135 +239,159 @@ Cards.mutations({
   },
 
   setCover(coverId) {
-    return { $set: { coverId } };
+    return {$set: {coverId}};
   },
 
   unsetCover() {
-    return { $unset: { coverId: '' } };
+    return {$unset: {coverId: ''}};
   },
 
   setStart(startAt) {
-    return { $set: { startAt } };
+    return {$set: {startAt}};
   },
 
   unsetStart() {
-    return { $unset: { startAt: '' } };
+    return {$unset: {startAt: ''}};
   },
 
   setDue(dueAt) {
-    return { $set: { dueAt } };
+    return {$set: {dueAt}};
   },
 
   unsetDue() {
-    return { $unset: { dueAt: '' } };
+    return {$unset: {dueAt: ''}};
   },
 });
 
-if (Meteor.isServer) {
-  // Cards are often fetched within a board, so we create an index to make these
-  // queries more efficient.
-  Meteor.startup(() => {
-    Cards._collection._ensureIndex({ boardId: 1, createdAt: -1 });
-  });
 
-  Cards.after.insert((userId, doc) => {
+//FUNCTIONS FOR creation of Activities
+
+function cardMove(userId, doc, fieldNames, oldListId) {
+  if (_.contains(fieldNames, 'listId') && doc.listId !== oldListId) {
     Activities.insert({
       userId,
-      activityType: 'createCard',
-      boardId: doc.boardId,
+      oldListId,
+      activityType: 'moveCard',
       listId: doc.listId,
+      boardId: doc.boardId,
       cardId: doc._id,
     });
-  });
-
-  // New activity for card (un)archivage
-  Cards.after.update((userId, doc, fieldNames) => {
-    if (_.contains(fieldNames, 'archived')) {
-      if (doc.archived) {
-        Activities.insert({
-          userId,
-          activityType: 'archivedCard',
-          boardId: doc.boardId,
-          listId: doc.listId,
-          cardId: doc._id,
-        });
-      } else {
-        Activities.insert({
-          userId,
-          activityType: 'restoredCard',
-          boardId: doc.boardId,
-          listId: doc.listId,
-          cardId: doc._id,
-        });
-      }
-    }
-  });
+  }
+}
 
-  // New activity for card moves
-  Cards.after.update(function (userId, doc, fieldNames) {
-    const oldListId = this.previous.listId;
-    if (_.contains(fieldNames, 'listId') && doc.listId !== oldListId) {
+function cardState(userId, doc, fieldNames) {
+  if (_.contains(fieldNames, 'archived')) {
+    if (doc.archived) {
       Activities.insert({
         userId,
-        oldListId,
-        activityType: 'moveCard',
+        activityType: 'archivedCard',
+        boardId: doc.boardId,
         listId: doc.listId,
+        cardId: doc._id,
+      });
+    } else {
+      Activities.insert({
+        userId,
+        activityType: 'restoredCard',
         boardId: doc.boardId,
+        listId: doc.listId,
         cardId: doc._id,
       });
     }
-  });
+  }
+}
 
-  // Add a new activity if we add or remove a member to the card
-  Cards.before.update((userId, doc, fieldNames, modifier) => {
-    if (!_.contains(fieldNames, 'members'))
-      return;
-    let memberId;
+function cardMembers(userId, doc, fieldNames, modifier) {
+  if (!_.contains(fieldNames, 'members'))
+    return;
+  let memberId;
     // Say hello to the new member
-    if (modifier.$addToSet && modifier.$addToSet.members) {
-      memberId = modifier.$addToSet.members;
-      if (!_.contains(doc.members, memberId)) {
-        Activities.insert({
-          userId,
-          memberId,
-          activityType: 'joinMember',
-          boardId: doc.boardId,
-          cardId: doc._id,
-        });
-      }
+  if (modifier.$addToSet && modifier.$addToSet.members) {
+    memberId = modifier.$addToSet.members;
+    if (!_.contains(doc.members, memberId)) {
+      Activities.insert({
+        userId,
+        memberId,
+        activityType: 'joinMember',
+        boardId: doc.boardId,
+        cardId: doc._id,
+      });
     }
+  }
 
     // Say goodbye to the former member
-    if (modifier.$pull && modifier.$pull.members) {
-      memberId = modifier.$pull.members;
-      // Check that the former member is member of the card
-      if (_.contains(doc.members, memberId)) {
-        Activities.insert({
-          userId,
-          memberId,
-          activityType: 'unjoinMember',
-          boardId: doc.boardId,
-          cardId: doc._id,
-        });
-      }
+  if (modifier.$pull && modifier.$pull.members) {
+    memberId = modifier.$pull.members;
+        // Check that the former member is member of the card
+    if (_.contains(doc.members, memberId)) {
+      Activities.insert({
+        userId,
+        memberId,
+        activityType: 'unjoinMember',
+        boardId: doc.boardId,
+        cardId: doc._id,
+      });
     }
+  }
+}
+
+function cardCreation(userId, doc) {
+  Activities.insert({
+    userId,
+    activityType: 'createCard',
+    boardId: doc.boardId,
+    listId: doc.listId,
+    cardId: doc._id,
+  });
+}
+
+function cardRemover(userId, doc) {
+  Activities.remove({
+    cardId: doc._id,
+  });
+  Checklists.remove({
+    cardId: doc._id,
+  });
+  CardComments.remove({
+    cardId: doc._id,
+  });
+  Attachments.remove({
+    cardId: doc._id,
+  });
+}
+
+
+if (Meteor.isServer) {
+    // Cards are often fetched within a board, so we create an index to make these
+    // queries more efficient.
+  Meteor.startup(() => {
+    Cards._collection._ensureIndex({boardId: 1, createdAt: -1});
+  });
+
+  Cards.after.insert((userId, doc) => {
+    cardCreation(userId, doc);
   });
 
-  // Remove all activities associated with a card if we remove the card
-  // Remove also card_comments / checklists / attachments
+    // New activity for card (un)archivage
+  Cards.after.update((userId, doc, fieldNames) => {
+    cardState(userId, doc, fieldNames);
+  });
+
+    //New activity for card moves
+  Cards.after.update(function (userId, doc, fieldNames) {
+    const oldListId = this.previous.listId;
+    cardMove(userId, doc, fieldNames, oldListId);
+  });
+
+    // Add a new activity if we add or remove a member to the card
+  Cards.before.update((userId, doc, fieldNames, modifier) => {
+    cardMembers(userId, doc, fieldNames, modifier);
+  });
+
+    // Remove all activities associated with a card if we remove the card
+    // Remove also card_comments / checklists / attachments
   Cards.after.remove((userId, doc) => {
-    Activities.remove({
-      cardId: doc._id,
-    });
-    Checklists.remove({
-      cardId: doc._id,
-    });
-    CardComments.remove({
-      cardId: doc._id,
-    });
-    Attachments.remove({
-      cardId: doc._id,
-    });
+    cardRemover(userId, doc);
   });
 }
 //LISTS REST API
@@ -375,10 +399,10 @@ if (Meteor.isServer) {
   JsonRoutes.add('GET', '/api/boards/:boardId/lists/:listId/cards', function (req, res, next) {
     const paramBoardId = req.params.boardId;
     const paramListId = req.params.listId;
-    Authentication.checkBoardAccess( req.userId, paramBoardId);
+    Authentication.checkBoardAccess(req.userId, paramBoardId);
     JsonRoutes.sendResult(res, {
       code: 200,
-      data: Cards.find({ boardId: paramBoardId, listId: paramListId, archived: false }).map(function (doc) {
+      data: Cards.find({boardId: paramBoardId, listId: paramListId, archived: false}).map(function (doc) {
         return {
           _id: doc._id,
           title: doc.title,
@@ -392,53 +416,69 @@ if (Meteor.isServer) {
     const paramBoardId = req.params.boardId;
     const paramListId = req.params.listId;
     const paramCardId = req.params.cardId;
-    Authentication.checkBoardAccess( req.userId, paramBoardId);
+    Authentication.checkBoardAccess(req.userId, paramBoardId);
     JsonRoutes.sendResult(res, {
       code: 200,
-      data: Cards.findOne({ _id: paramCardId, listId: paramListId, boardId: paramBoardId, archived: false }),
+      data: Cards.findOne({_id: paramCardId, listId: paramListId, boardId: paramBoardId, archived: false}),
     });
   });
 
   JsonRoutes.add('POST', '/api/boards/:boardId/lists/:listId/cards', function (req, res, next) {
-    Authentication.checkUserId( req.userId);
+    Authentication.checkUserId(req.userId);
     const paramBoardId = req.params.boardId;
     const paramListId = req.params.listId;
-    const id = Cards.insert({
-      title: req.body.title,
-      boardId: paramBoardId,
-      listId: paramListId,
-      description: req.body.description,
-      userId : req.body.authorId,
-      sort: 0,
-      members:[ req.body.authorId ],
-    });
-    JsonRoutes.sendResult(res, {
-      code: 200,
-      data: {
-        _id: id,
-      },
-    });
+    const check = Users.findOne({_id: req.body.authorId});
+    if (typeof  check !== 'undefined') {
+      const id = Cards.direct.insert({
+        title: req.body.title,
+        boardId: paramBoardId,
+        listId: paramListId,
+        description: req.body.description,
+        userId: req.body.authorId,
+        sort: 0,
+        members: [req.body.authorId],
+      });
+      JsonRoutes.sendResult(res, {
+        code: 200,
+        data: {
+          _id: id,
+        },
+      });
+
+      const card = Cards.findOne({_id:id});
+      cardCreation(req.body.authorId, card);
+
+    } else {
+      JsonRoutes.sendResult(res, {
+        code: 401,
+      });
+    }
   });
 
   JsonRoutes.add('PUT', '/api/boards/:boardId/lists/:listId/cards/:cardId', function (req, res, next) {
-    Authentication.checkUserId( req.userId);
+    Authentication.checkUserId(req.userId);
     const paramBoardId = req.params.boardId;
     const paramCardId = req.params.cardId;
     const paramListId = req.params.listId;
-    if(req.body.hasOwnProperty('title')){
+
+    if (req.body.hasOwnProperty('title')) {
       const newTitle = req.body.title;
-      Cards.update({ _id: paramCardId, listId: paramListId, boardId: paramBoardId, archived: false },
-                {$set:{title:newTitle}});
+      Cards.direct.update({_id: paramCardId, listId: paramListId, boardId: paramBoardId, archived: false},
+                {$set: {title: newTitle}});
     }
-    if(req.body.hasOwnProperty('listId')){
+    if (req.body.hasOwnProperty('listId')) {
       const newParamListId = req.body.listId;
-      Cards.update({ _id: paramCardId, listId: paramListId, boardId: paramBoardId, archived: false },
-                {$set:{listId:newParamListId}});
+      Cards.direct.update({_id: paramCardId, listId: paramListId, boardId: paramBoardId, archived: false},
+                {$set: {listId: newParamListId}});
+
+      const card = Cards.findOne({_id: paramCardId} );
+      cardMove(req.body.authorId, card, {fieldName: 'listId'}, paramListId);
+
     }
-    if(req.body.hasOwnProperty('description')){
+    if (req.body.hasOwnProperty('description')) {
       const newDescription = req.body.description;
-      Cards.update({ _id: paramCardId, listId: paramListId, boardId: paramBoardId, archived: false },
-                {$set:{description:newDescription}});
+      Cards.direct.update({_id: paramCardId, listId: paramListId, boardId: paramBoardId, archived: false},
+                {$set: {description: newDescription}});
     }
     JsonRoutes.sendResult(res, {
       code: 200,
@@ -450,16 +490,20 @@ if (Meteor.isServer) {
 
 
   JsonRoutes.add('DELETE', '/api/boards/:boardId/lists/:listId/cards/:cardId', function (req, res, next) {
-    Authentication.checkUserId( req.userId);
+    Authentication.checkUserId(req.userId);
     const paramBoardId = req.params.boardId;
     const paramListId = req.params.listId;
     const paramCardId = req.params.cardId;
-    Cards.remove({ _id: paramCardId, listId: paramListId, boardId: paramBoardId });
+
+    Cards.direct.remove({_id: paramCardId, listId: paramListId, boardId: paramBoardId});
+    const card = Cards.find({_id: paramCardId} );
+    cardRemover(req.body.authorId, card);
     JsonRoutes.sendResult(res, {
       code: 200,
       data: {
         _id: paramCardId,
       },
     });
+
   });
 }