code review fixes

.meteor/release

@@ -1 +1 @@
-METEOR@1.2.2-cdn-url
+METEOR@1.2.1

.meteor/versions

@@ -14,7 +14,7 @@ binary-heap@1.0.4
 blaze@2.1.3
 blaze-html-templates@1.0.1
 blaze-tools@1.0.4
-boilerplate-generator@1.0.5-cdn-url
+boilerplate-generator@1.0.4
 caching-compiler@1.0.0
 caching-html-compiler@1.0.2
 callback-hook@1.0.4
@@ -142,6 +142,6 @@ useraccounts:core@1.12.4
 useraccounts:flow-routing@1.12.4
 useraccounts:unstyled@1.12.4
 verron:autosize@3.0.8
-webapp@1.2.4-cdn-url
+webapp@1.2.3
 webapp-hashing@1.0.5
 zimme:active-route@2.3.2

client/components/import/import.js

@@ -33,12 +33,6 @@ const ImportPopup = BlazeComponent.extendComponent({
     Popup.open('mapMembers')(evt);
   },
 
-  _storeText() {
-    const dataJson = this.$('.js-import-json').val();
-    Session.set('import.text', dataJson);
-    return dataJson;
-  },
-
   onSubmit(evt){
     evt.preventDefault();
     const dataJson = this._storeText(evt);
@@ -50,47 +44,12 @@ const ImportPopup = BlazeComponent.extendComponent({
       this.setError('error-json-malformed');
       return;
     }
-    // if there are members listed in the import and we have no mapping for them...
-    if(dataObject.members.length > 0 && !this.membersMapping()) {
-      // we will work on the list itself (an ordered array of POJO)
-      // when a mapping is done, we add a 'wekan' field to the POJO representing the imported member
-      const membersToMap = dataObject.members;
-      // auto-map based on username
-      membersToMap.forEach((importedMember) => {
-        const wekanUser = Users.findOne({username: importedMember.username});
-        if(wekanUser) {
-          importedMember.wekan = wekanUser;
-        }
-      });
-      // store members data and mapping in Session
-      // (we go deep and 2-way, so storing in data context is not a viable option)
-      Session.set('import.membersToMap', membersToMap);
-      Popup.open('mapMembers')(evt);
+    if(this._hasAllNeededData(dataObject)) {
+      this._import(dataObject);
     } else {
-      const additionalData = this.getAdditionalData();
-      const membersMapping = this.membersMapping();
-      if(membersMapping) {
-        const mappingById = {};
-        membersMapping.forEach((member) => {
-          if (member.wekan) {
-            mappingById[member.id] = member.wekan._id;
-          }
-        });
-        additionalData.membersMapping = mappingById;
-      }
-      Session.set('import.membersToMap', null);
-      Session.set('import.text', null);
-      Meteor.call(this.getMethodName(), dataObject, additionalData,
-        (error, response) => {
-          if (error) {
-            this.setError(error.error);
-          } else {
-            // ensure will display what we just imported
-            Filter.addException(response);
-            this.onFinish(response);
-          }
-        }
-      );
+      this._prepareAdditionalData(dataObject);
+      Popup.open(this._screenAdditionalData())(evt);
+
     }
   },
 
@@ -105,6 +64,64 @@ const ImportPopup = BlazeComponent.extendComponent({
     this.error.set(error);
   },
 
+  _import: function (dataObject) {
+    const additionalData = this.getAdditionalData();
+    const membersMapping = this.membersMapping();
+    if (membersMapping) {
+      const mappingById = {};
+      membersMapping.forEach((member) => {
+        if (member.wekan) {
+          mappingById[member.id] = member.wekan._id;
+        }
+      });
+      additionalData.membersMapping = mappingById;
+    }
+    Session.set('import.membersToMap', null);
+    Session.set('import.text', null);
+    Meteor.call(this.getMethodName(), dataObject, additionalData,
+      (error, response) => {
+        if (error) {
+          this.setError(error.error);
+        } else {
+          // ensure will display what we just imported
+          Filter.addException(response);
+          this.onFinish(response);
+        }
+      }
+    );
+  },
+
+  _hasAllNeededData(dataObject) {
+    // import has no members or they are already mapped
+    return dataObject.members.length === 0 || this.membersMapping();
+  },
+
+  _prepareAdditionalData(dataObject) {
+    // we will work on the list itself (an ordered array of objects)
+    // when a mapping is done, we add a 'wekan' field to the object representing the imported member
+    const membersToMap = dataObject.members;
+    // auto-map based on username
+    membersToMap.forEach((importedMember) => {
+      const wekanUser = Users.findOne({username: importedMember.username});
+      if(wekanUser) {
+        importedMember.wekan = wekanUser;
+      }
+    });
+    // store members data and mapping in Session
+    // (we go deep and 2-way, so storing in data context is not a viable option)
+    Session.set('import.membersToMap', membersToMap);
+    return membersToMap;
+  },
+
+  _screenAdditionalData() {
+    return 'mapMembers';
+  },
+
+  _storeText() {
+    const dataJson = this.$('.js-import-json').val();
+    Session.set('import.text', dataJson);
+    return dataJson;
+  },
 });
 
 ImportPopup.extendComponent({

models/import.js

@@ -103,7 +103,7 @@ class TrelloCreator {
         if(this.members[trelloId]) {
           const wekanId = this.members[trelloId];
           // do we already have it in our list?
-          if(!boardToCreate.members.find((wekanMember) => {return (wekanMember.userId === wekanId);})) {
+          if(!boardToCreate.members.find((wekanMember) => wekanMember.userId === wekanId)) {
             boardToCreate.members.push({
               userId: wekanId,
               isAdmin: false,
@@ -181,7 +181,7 @@ class TrelloCreator {
             const wekanId = this.members[trelloId];
             // we may map multiple Trello members to the same wekan user
             // in which case we risk adding the same user multiple times
-            if(!wekanMembers.find((wId) => {return (wId === wekanId);})){
+            if(!wekanMembers.find((wId) => wId === wekanId)){
               wekanMembers.push(wekanId);
             }
           }