Kaynağa Gözat

Prevent duplicate board labels

43de3b8 did prevent empty labels with the same color, but we also want
to prevent label with the same non-empty name and same color because
the rationale is identical.
Maxime Quandalle 9 yıl önce
ebeveyn
işleme
6dedf673d5
2 değiştirilmiş dosya ile 31 ekleme ve 42 silme
  1. 15 18
      models/boards.js
  2. 16 24
      models/import.js

+ 15 - 18
models/boards.js

@@ -135,29 +135,26 @@ Boards.mutations({
   },
   },
 
 
   addLabel(name, color) {
   addLabel(name, color) {
-    const _id = Random.id(6);
-
-    // If an empty label of a given color already exists we don't want to create
-    // an other one because they would be indistinguishable in the UI (they
-    // would still have different `_id` but that is not exposed to the user).
-    if (name === '' && this.getLabel(name, color)) {
-      return {};
+    // If label with the same name and color already exists we don't want to
+    // create another one because they would be indistinguishable in the UI
+    // (they would still have different `_id` but that is not exposed to the
+    // user).
+    if (!this.getLabel(name, color)) {
+      const _id = Random.id(6);
+      return { $push: {labels: { _id, name, color }}};
     }
     }
-    return { $push: {labels: { _id, name, color }}};
   },
   },
 
 
   editLabel(labelId, name, color) {
   editLabel(labelId, name, color) {
-    const labelIndex = this.labelIndex(labelId);
-
-    if (name === '' && this.getLabel(name, color)) {
-      return {};
+    if (!this.getLabel(name, color)) {
+      const labelIndex = this.labelIndex(labelId);
+      return {
+        $set: {
+          [`labels.${labelIndex}.name`]: name,
+          [`labels.${labelIndex}.color`]: color,
+        },
+      };
     }
     }
-    return {
-      $set: {
-        [`labels.${labelIndex}.name`]: name,
-        [`labels.${labelIndex}.color`]: color,
-      },
-    };
   },
   },
 
 
   removeLabel(labelId) {
   removeLabel(labelId) {

+ 16 - 24
models/import.js

@@ -24,16 +24,17 @@ Meteor.methods({
       }));
       }));
       check(listId, String);
       check(listId, String);
       check(sortIndex, Number);
       check(sortIndex, Number);
-    } catch(e) {
+    } catch (e) {
       throw new Meteor.Error('error-json-schema');
       throw new Meteor.Error('error-json-schema');
     }
     }
 
 
-    // 2. check parameters are ok from a business point of view (exist & authorized)
+    // 2. check parameters are ok from a business point of view (exist &
+    // authorized)
     const list = Lists.findOne(listId);
     const list = Lists.findOne(listId);
-    if(!list) {
+    if (!list) {
       throw new Meteor.Error('error-list-doesNotExist');
       throw new Meteor.Error('error-list-doesNotExist');
     }
     }
-    if(Meteor.isServer) {
+    if (Meteor.isServer) {
       if (!allowIsBoardMember(Meteor.userId(), Boards.findOne(list.boardId))) {
       if (!allowIsBoardMember(Meteor.userId(), Boards.findOne(list.boardId))) {
         throw new Meteor.Error('error-board-notAMember');
         throw new Meteor.Error('error-board-notAMember');
       }
       }
@@ -48,6 +49,7 @@ Meteor.methods({
       createdAt: dateOfImport,
       createdAt: dateOfImport,
       dateLastActivity: dateOfImport,
       dateLastActivity: dateOfImport,
       description: trelloCard.desc,
       description: trelloCard.desc,
+      labelIds: [],
       listId: list._id,
       listId: list._id,
       sort: sortIndex,
       sort: sortIndex,
       title: trelloCard.name,
       title: trelloCard.name,
@@ -59,29 +61,18 @@ Meteor.methods({
     const creationAction = trelloCard.actions.find((action) => {
     const creationAction = trelloCard.actions.find((action) => {
       return action.type === 'createCard';
       return action.type === 'createCard';
     });
     });
-    if(creationAction) {
+    if (creationAction) {
       cardToCreate.createdAt = creationAction.date;
       cardToCreate.createdAt = creationAction.date;
     }
     }
 
 
     // 5. map labels - create missing ones
     // 5. map labels - create missing ones
     trelloCard.labels.forEach((currentLabel) => {
     trelloCard.labels.forEach((currentLabel) => {
-      const color = currentLabel.color;
-      const name = currentLabel.name;
-      const existingLabel = list.board().getLabel(name, color);
-      let labelId = undefined;
-      if (existingLabel) {
-        labelId = existingLabel._id;
-      } else {
-        let labelCreated = list.board().addLabel(name, color);
-        // XXX currently mutations return no value so we have to fetch the label we just created
-        // waiting on https://github.com/mquandalle/meteor-collection-mutations/issues/1 to remove...
-        labelCreated = list.board().getLabel(name, color);
-        labelId = labelCreated._id;
-      }
-      if(labelId) {
-        if (!cardToCreate.labelIds) {
-          cardToCreate.labelIds = [];
-        }
+      const { name, color } = currentLabel;
+      // `addLabel` won't create dublicate labels (ie labels with the same name
+      // and color) so here it is used more in a "enforceLabelExistence" way.
+      list.board().addLabel(name, color);
+      const { _id: labelId } = list.board().getLabel(name, color);
+      if (labelId) {
         cardToCreate.labelIds.push(labelId);
         cardToCreate.labelIds.push(labelId);
       }
       }
     });
     });
@@ -99,13 +90,14 @@ Meteor.methods({
         system: 'Trello',
         system: 'Trello',
         url: trelloCard.url,
         url: trelloCard.url,
       },
       },
-      // we attribute the import to current user, not the one from the original card
+      // we attribute the import to current user, not the one from the original
+      // card
       userId: Meteor.userId(),
       userId: Meteor.userId(),
     });
     });
 
 
     // 7. parse actions and add comments
     // 7. parse actions and add comments
     trelloCard.actions.forEach((currentAction) => {
     trelloCard.actions.forEach((currentAction) => {
-      if(currentAction.type === 'commentCard') {
+      if (currentAction.type === 'commentCard') {
         const commentToCreate = {
         const commentToCreate = {
           boardId: list.boardId,
           boardId: list.boardId,
           cardId,
           cardId,