Merge branch 'GhassenRjab-feature/manual-attachments-activities' into devel

Import attachments related activities from Wekan and Trello.
Thanks to GhassenRjab ! Closes #1157

5 files changed, 89 insertions, 65 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index adf022f7..edad19db 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,12 @@
+# Upcoming Wekan release
+
+This release adds the following new features:
+
+* [Import attachments related activities from Wekan and
+   Trello](https://github.com/wekan/wekan/pull/1202).
+
+Thanks to GitHub user GhassenRjab for contributions.
+
 # v0.35 2017-09-02 Wekan release
 
 This release adds the following new features:
diff --git a/client/components/cards/attachments.js b/client/components/cards/attachments.js
index 4e3e0b19..95cb9f55 100644
--- a/client/components/cards/attachments.js
+++ b/client/components/cards/attachments.js
@@ -62,7 +62,7 @@ Template.cardAttachmentsPopup.events({
       const file = new FS.File(f);
       file.boardId = card.boardId;
       file.cardId = card._id;
-
+      file.userId = Meteor.userId();
       Attachments.insert(file);
       Popup.close();
     });
@@ -109,6 +109,7 @@ Template.previewClipboardImagePopup.events({
       file.updatedAt(new Date());
       file.boardId = card.boardId;
       file.cardId = card._id;
+      file.userId = Meteor.userId();
       Attachments.insert(file);
       pastedResults = null;
       $(document.body).pasteImageReader(() => {});
diff --git a/models/attachments.js b/models/attachments.js
index 1c9878c7..560bec99 100644
--- a/models/attachments.js
+++ b/models/attachments.js
@@ -3,7 +3,25 @@ Attachments = new FS.Collection('attachments', {
 
     // XXX Add a new store for cover thumbnails so we don't load big images in
     // the general board view
-    new FS.Store.GridFS('attachments'),
+    new FS.Store.GridFS('attachments', {
+      // If the uploaded document is not an image we need to enforce browser
+      // download instead of execution. This is particularly important for HTML
+      // files that the browser will just execute if we don't serve them with the
+      // appropriate `application/octet-stream` MIME header which can lead to user
+      // data leaks. I imagine other formats (like PDF) can also be attack vectors.
+      // See https://github.com/wekan/wekan/issues/99
+      // XXX Should we use `beforeWrite` option of CollectionFS instead of
+      // collection-hooks?
+      // We should use `beforeWrite`.
+      beforeWrite: (fileObj) => {
+        if (!fileObj.isImage()) {
+          return {
+            type: 'application/octet-stream',
+          };
+        }
+        return {};
+      },
+    }),
   ],
 });
 
@@ -36,33 +54,25 @@ if (Meteor.isServer) {
 
 // XXX Enforce a schema for the Attachments CollectionFS
 
-Attachments.files.before.insert((userId, doc) => {
-  const file = new FS.File(doc);
-  doc.userId = userId;
-
-  // If the uploaded document is not an image we need to enforce browser
-  // download instead of execution. This is particularly important for HTML
-  // files that the browser will just execute if we don't serve them with the
-  // appropriate `application/octet-stream` MIME header which can lead to user
-  // data leaks. I imagine other formats (like PDF) can also be attack vectors.
-  // See https://github.com/wekan/wekan/issues/99
-  // XXX Should we use `beforeWrite` option of CollectionFS instead of
-  // collection-hooks?
-  if (!file.isImage()) {
-    file.original.type = 'application/octet-stream';
-  }
-});
-
 if (Meteor.isServer) {
   Attachments.files.after.insert((userId, doc) => {
-    Activities.insert({
-      userId,
-      type: 'card',
-      activityType: 'addAttachment',
-      attachmentId: doc._id,
-      boardId: doc.boardId,
-      cardId: doc.cardId,
-    });
+    // If the attachment doesn't have a source field
+    // or its source is different than import
+    if (!doc.source || doc.source !== 'import') {
+      // Add activity about adding the attachment
+      Activities.insert({
+        userId,
+        type: 'card',
+        activityType: 'addAttachment',
+        attachmentId: doc._id,
+        boardId: doc.boardId,
+        cardId: doc.cardId,
+      });
+    } else {
+      // Don't add activity about adding the attachment as the activity
+      // be imported and delete source field
+      Attachments.update( {_id: doc._id}, {$unset: { source : '' } } );
+    }
   });
 
   Attachments.files.after.remove((userId, doc) => {
diff --git a/models/trelloCreator.js b/models/trelloCreator.js
index 60207546..b296efdf 100644
--- a/models/trelloCreator.js
+++ b/models/trelloCreator.js
@@ -318,17 +318,22 @@ export class TrelloCreator {
           // - HEAD returns null, which causes exception down the line
           // - the template then tries to display the url to the attachment which causes other errors
           // so we make it server only, and let UI catch up once it is done, forget about latency comp.
+          const self = this;
           if(Meteor.isServer) {
             file.attachData(att.url, function (error) {
               file.boardId = boardId;
               file.cardId = cardId;
+              file.userId = self._user(att.idMemberCreator);
+              // The field source will only be used to prevent adding
+              // attachments' related activities automatically
+              file.source = 'import';
               if (error) {
                 throw(error);
               } else {
                 const wekanAtt = Attachments.insert(file, () => {
                   // we do nothing
                 });
-                this.attachmentIds[att.id] = wekanAtt._id;
+                self.attachmentIds[att.id] = wekanAtt._id;
                 //
                 if(trelloCoverId === att.id) {
                   Cards.direct.update(cardId, { $set: {coverId: wekanAtt._id}});
@@ -452,6 +457,8 @@ export class TrelloCreator {
         // In that case Trello still reports its addition, but removes its 'url' field.
         // So we test for that
         const trelloAttachment = action.data.attachment;
+        // We need the idMemberCreator
+        trelloAttachment.idMemberCreator = action.idMemberCreator;
         if(trelloAttachment.url) {
           // we cannot actually create the Wekan attachment, because we don't yet
           // have the cards to attach it to, so we store it in the instance variable.
@@ -540,24 +547,18 @@ export class TrelloCreator {
       // Comment related activities
       // Trello doesn't export the comment id
       // Attachment related activities
-      // TODO: We can't add activities related to adding attachments
-      //       because when we import an attachment, an activity is
-      //       autmatically created. We need to directly insert the attachment
-      //       without calling the "Attachments.files.after.insert" hook first,
-      //       then we can uncomment the code below
-      // case 'addAttachment': {
-      //   console.log(this.attachmentIds);
-      //   Activities.direct.insert({
-      //     userId: this._user(activity.userId),
-      //     type: 'card',
-      //     activityType: activity.activityType,
-      //     attachmentId: this.attachmentIds[activity.attachmentId],
-      //     cardId: this.cards[activity.cardId],
-      //     boardId,
-      //     createdAt: this._now(activity.createdAt),
-      //   });
-      //   break;
-      // }
+      case 'addAttachmentToCard': {
+        Activities.direct.insert({
+          userId: this._user(action.idMemberCreator),
+          type: 'card',
+          activityType: 'addAttachment',
+          attachmentId: this.attachmentIds[action.data.attachment.id],
+          cardId: this.cards[action.data.card.id],
+          boardId,
+          createdAt: this._now(action.date),
+        });
+        break;
+      }
       // Checklist related activities
       case 'addChecklistToCard': {
         Activities.direct.insert({
diff --git a/models/wekanCreator.js b/models/wekanCreator.js
index 1a02339a..9c7f50ec 100644
--- a/models/wekanCreator.js
+++ b/models/wekanCreator.js
@@ -307,18 +307,23 @@ export class WekanCreator {
           // - HEAD returns null, which causes exception down the line
           // - the template then tries to display the url to the attachment which causes other errors
           // so we make it server only, and let UI catch up once it is done, forget about latency comp.
+          const self = this;
           if(Meteor.isServer) {
             if (att.url) {
               file.attachData(att.url, function (error) {
                 file.boardId = boardId;
                 file.cardId = cardId;
+                file.userId = self._user(att.userId);
+                // The field source will only be used to prevent adding
+                // attachments' related activities automatically
+                file.source = 'import';
                 if (error) {
                   throw(error);
                 } else {
                   const wekanAtt = Attachments.insert(file, () => {
                     // we do nothing
                   });
-                  this.attachmentIds[att._id] = wekanAtt._id;
+                  self.attachmentIds[att._id] = wekanAtt._id;
                   //
                   if(wekanCoverId === att._id) {
                     Cards.direct.update(cardId, { $set: {coverId: wekanAtt._id}});
@@ -330,6 +335,10 @@ export class WekanCreator {
                 file.name(att.name);
                 file.boardId = boardId;
                 file.cardId = cardId;
+                file.userId = self._user(att.userId);
+                // The field source will only be used to prevent adding
+                // attachments' related activities automatically
+                file.source = 'import';
                 if (error) {
                   throw(error);
                 } else {
@@ -541,24 +550,18 @@ export class WekanCreator {
         break;
       }
       // Attachment related activities
-      // TODO: We can't add activities related to adding attachments
-      //       because when we import an attachment, an activity is
-      //       autmatically created. We need to directly insert the attachment
-      //       without calling the "Attachments.files.after.insert" hook first,
-      //       then we can uncomment the code below
-      // case 'addAttachment': {
-      //   console.log(this.attachmentIds);
-      //   Activities.direct.insert({
-      //     userId: this._user(activity.userId),
-      //     type: 'card',
-      //     activityType: activity.activityType,
-      //     attachmentId: this.attachmentIds[activity.attachmentId],
-      //     cardId: this.cards[activity.cardId],
-      //     boardId,
-      //     createdAt: this._now(activity.createdAt),
-      //   });
-      //   break;
-      // }
+      case 'addAttachment': {
+        Activities.direct.insert({
+          userId: this._user(activity.userId),
+          type: 'card',
+          activityType: activity.activityType,
+          attachmentId: this.attachmentIds[activity.attachmentId],
+          cardId: this.cards[activity.cardId],
+          boardId,
+          createdAt: this._now(activity.createdAt),
+        });
+        break;
+      }
       // Checklist related activities
       case 'addChecklist': {
         Activities.direct.insert({