Merge branch '51994-disable-merging-labels-in-dropdowns' into 'master'

Resolve "Fix labels dropdown with multiple same names"

Closes #51994

See merge request gitlab-org/gitlab-ce!23265

11 files changed, 41 insertions, 284 deletions

diff --git a/app/assets/javascripts/boards/components/new_list_dropdown.js b/app/assets/javascripts/boards/components/new_list_dropdown.js
index f7016561f93..10577da9305 100644
--- a/app/assets/javascripts/boards/components/new_list_dropdown.js
+++ b/app/assets/javascripts/boards/components/new_list_dropdown.js
@@ -37,7 +37,7 @@ export default function initNewListDropdown() {
         });
       },
       renderRow(label) {
-        const active = boardsStore.findList('title', label.title);
+        const active = boardsStore.findListByLabelId(label.id);
         const $li = $('<li />');
         const $a = $('<a />', {
           class: active ? `is-active js-board-list-${active.id}` : '',
@@ -63,7 +63,7 @@ export default function initNewListDropdown() {
         const label = options.selectedObj;
         e.preventDefault();
 
-        if (!boardsStore.findList('title', label.title)) {
+        if (!boardsStore.findListByLabelId(label.id)) {
           boardsStore.new({
             title: label.title,
             position: boardsStore.state.lists.length - 2,
diff --git a/app/assets/javascripts/boards/models/issue.js b/app/assets/javascripts/boards/models/issue.js
index 5e0f0b07247..dd92d3c8552 100644
--- a/app/assets/javascripts/boards/models/issue.js
+++ b/app/assets/javascripts/boards/models/issue.js
@@ -55,12 +55,12 @@ class ListIssue {
   }
 
   findLabel(findLabel) {
-    return this.labels.filter(label => label.title === findLabel.title)[0];
+    return this.labels.find(label => label.id === findLabel.id);
   }
 
   removeLabel(removeLabel) {
     if (removeLabel) {
-      this.labels = this.labels.filter(label => removeLabel.title !== label.title);
+      this.labels = this.labels.filter(label => removeLabel.id !== label.id);
     }
   }
 
@@ -75,7 +75,7 @@ class ListIssue {
   }
 
   findAssignee(findAssignee) {
-    return this.assignees.filter(assignee => assignee.id === findAssignee.id)[0];
+    return this.assignees.find(assignee => assignee.id === findAssignee.id);
   }
 
   removeAssignee(removeAssignee) {
diff --git a/app/assets/javascripts/boards/stores/boards_store.js b/app/assets/javascripts/boards/stores/boards_store.js
index cf88a973d33..802796208c2 100644
--- a/app/assets/javascripts/boards/stores/boards_store.js
+++ b/app/assets/javascripts/boards/stores/boards_store.js
@@ -166,6 +166,9 @@ const boardsStore = {
     });
     return filteredList[0];
   },
+  findListByLabelId(id) {
+    return this.state.lists.find(list => list.type === 'label' && list.label.id === id);
+  },
   updateFiltersUrl() {
     window.history.pushState(null, null, `?${this.filter.path}`);
   },
diff --git a/app/assets/javascripts/filtered_search/dropdown_utils.js b/app/assets/javascripts/filtered_search/dropdown_utils.js
index 1b79a3320c6..8d92af2cf7e 100644
--- a/app/assets/javascripts/filtered_search/dropdown_utils.js
+++ b/app/assets/javascripts/filtered_search/dropdown_utils.js
@@ -54,67 +54,6 @@ export default class DropdownUtils {
     return updatedItem;
   }
 
-  static mergeDuplicateLabels(dataMap, newLabel) {
-    const updatedMap = dataMap;
-    const key = newLabel.title;
-
-    const hasKeyProperty = Object.prototype.hasOwnProperty.call(updatedMap, key);
-
-    if (!hasKeyProperty) {
-      updatedMap[key] = newLabel;
-    } else {
-      const existing = updatedMap[key];
-
-      if (!existing.multipleColors) {
-        existing.multipleColors = [existing.color];
-      }
-
-      existing.multipleColors.push(newLabel.color);
-    }
-
-    return updatedMap;
-  }
-
-  static duplicateLabelColor(labelColors) {
-    const colors = labelColors;
-    const spacing = 100 / colors.length;
-
-    // Reduce the colors to 4
-    colors.length = Math.min(colors.length, 4);
-
-    const color = colors
-      .map((c, i) => {
-        const percentFirst = Math.floor(spacing * i);
-        const percentSecond = Math.floor(spacing * (i + 1));
-        return `${c} ${percentFirst}%, ${c} ${percentSecond}%`;
-      })
-      .join(', ');
-
-    return `linear-gradient(${color})`;
-  }
-
-  static duplicateLabelPreprocessing(data) {
-    const results = [];
-    const dataMap = {};
-
-    data.forEach(DropdownUtils.mergeDuplicateLabels.bind(null, dataMap));
-
-    Object.keys(dataMap).forEach(key => {
-      const label = dataMap[key];
-
-      if (label.multipleColors) {
-        label.color = DropdownUtils.duplicateLabelColor(label.multipleColors);
-        label.text_color = '#000000';
-      }
-
-      results.push(label);
-    });
-
-    results.preprocessed = true;
-
-    return results;
-  }
-
   static filterHint(config, item) {
     const { input, allowedKeys } = config;
     const updatedItem = item;
diff --git a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js
index 89dcff74d0e..fba31f16d65 100644
--- a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js
+++ b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js
@@ -79,11 +79,7 @@ export default class FilteredSearchVisualTokens {
   static setTokenStyle(tokenContainer, backgroundColor, textColor) {
     const token = tokenContainer;
 
-    // Labels with linear gradient should not override default background color
-    if (backgroundColor.indexOf('linear-gradient') === -1) {
-      token.style.backgroundColor = backgroundColor;
-    }
-
+    token.style.backgroundColor = backgroundColor;
     token.style.color = textColor;
 
     if (textColor === '#FFFFFF') {
@@ -94,18 +90,6 @@ export default class FilteredSearchVisualTokens {
     return token;
   }
 
-  static preprocessLabel(labelsEndpoint, labels) {
-    let processed = labels;
-
-    if (!labels.preprocessed) {
-      processed = DropdownUtils.duplicateLabelPreprocessing(labels);
-      AjaxCache.override(labelsEndpoint, processed);
-      processed.preprocessed = true;
-    }
-
-    return processed;
-  }
-
   static updateLabelTokenColor(tokenValueContainer, tokenValue) {
     const filteredSearchInput = FilteredSearchContainer.container.querySelector('.filtered-search');
     const { baseEndpoint } = filteredSearchInput.dataset;
@@ -115,7 +99,6 @@ export default class FilteredSearchVisualTokens {
     );
 
     return AjaxCache.retrieve(labelsEndpoint)
-      .then(FilteredSearchVisualTokens.preprocessLabel.bind(null, labelsEndpoint))
       .then(labels => {
         const matchingLabel = (labels || []).find(
           label => `~${DropdownUtils.getEscapedText(label.title)}` === tokenValue,
diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js
index c0a76814102..f7a611fbca0 100644
--- a/app/assets/javascripts/labels_select.js
+++ b/app/assets/javascripts/labels_select.js
@@ -7,7 +7,6 @@ import _ from 'underscore';
 import { sprintf, __ } from './locale';
 import axios from './lib/utils/axios_utils';
 import IssuableBulkUpdateActions from './issuable_bulk_update_actions';
-import DropdownUtils from './filtered_search/dropdown_utils';
 import CreateLabelDropdown from './create_label';
 import flash from './flash';
 import ModalStore from './boards/stores/modal_store';
@@ -171,23 +170,7 @@ export default class LabelsSelect {
           axios
             .get(labelUrl)
             .then(res => {
-              let data = _.chain(res.data)
-                .groupBy(function(label) {
-                  return label.title;
-                })
-                .map(function(label) {
-                  var color;
-                  color = _.map(label, function(dup) {
-                    return dup.color;
-                  });
-                  return {
-                    id: label[0].id,
-                    title: label[0].title,
-                    color: color,
-                    duplicate: color.length > 1,
-                  };
-                })
-                .value();
+              let { data } = res;
               if ($dropdown.hasClass('js-extra-options')) {
                 var extraData = [];
                 if (showNo) {
@@ -272,15 +255,9 @@ export default class LabelsSelect {
               selectedClass.push('dropdown-clear-active');
             }
           }
-          if (label.duplicate) {
-            color = DropdownUtils.duplicateLabelColor(label.color);
-          } else {
-            if (label.color != null) {
-              [color] = label.color;
-            }
-          }
-          if (color) {
-            colorEl = "<span class='dropdown-label-box' style='background: " + color + "'></span>";
+          if (label.color) {
+            colorEl =
+              "<span class='dropdown-label-box' style='background: " + label.color + "'></span>";
           } else {
             colorEl = '';
           }
@@ -435,7 +412,7 @@ export default class LabelsSelect {
                 new ListLabel({
                   id: label.id,
                   title: label.title,
-                  color: label.color[0],
+                  color: label.color,
                   textColor: '#fff',
                 }),
               );
diff --git a/changelogs/unreleased/51994-disable-merging-labels-in-dropdowns.yml b/changelogs/unreleased/51994-disable-merging-labels-in-dropdowns.yml
new file mode 100644
index 00000000000..2d54cf814b7
--- /dev/null
+++ b/changelogs/unreleased/51994-disable-merging-labels-in-dropdowns.yml
@@ -0,0 +1,5 @@
+---
+title: Disable merging of labels with same names
+merge_request: 23265
+author:
+type: changed
diff --git a/spec/javascripts/boards/boards_store_spec.js b/spec/javascripts/boards/boards_store_spec.js
index 54f1edfb1f9..22f192bc7f3 100644
--- a/spec/javascripts/boards/boards_store_spec.js
+++ b/spec/javascripts/boards/boards_store_spec.js
@@ -65,6 +65,13 @@ describe('Store', () => {
       expect(list).toBeDefined();
     });
 
+    it('finds list by label ID', () => {
+      boardsStore.addList(listObj);
+      const list = boardsStore.findListByLabelId(listObj.label.id);
+
+      expect(list.id).toBe(listObj.id);
+    });
+
     it('gets issue when new list added', done => {
       boardsStore.addList(listObj);
       const list = boardsStore.findList('id', listObj.id);
diff --git a/spec/javascripts/boards/issue_spec.js b/spec/javascripts/boards/issue_spec.js
index 437ab4bb3df..54fb0e8228b 100644
--- a/spec/javascripts/boards/issue_spec.js
+++ b/spec/javascripts/boards/issue_spec.js
@@ -55,15 +55,27 @@ describe('Issue model', () => {
     expect(issue.labels.length).toBe(2);
   });
 
-  it('does not add existing label', () => {
+  it('does not add label if label id exists', () => {
+    issue.addLabel({
+      id: 1,
+      title: 'test 2',
+      color: 'blue',
+      description: 'testing',
+    });
+
+    expect(issue.labels.length).toBe(1);
+    expect(issue.labels[0].color).toBe('red');
+  });
+
+  it('adds other label with same title', () => {
     issue.addLabel({
       id: 2,
       title: 'test',
       color: 'blue',
-      description: 'bugs!',
+      description: 'other test',
     });
 
-    expect(issue.labels.length).toBe(1);
+    expect(issue.labels.length).toBe(2);
   });
 
   it('finds label', () => {
diff --git a/spec/javascripts/filtered_search/dropdown_utils_spec.js b/spec/javascripts/filtered_search/dropdown_utils_spec.js
index 6605b0a30d7..cfd0b96ec43 100644
--- a/spec/javascripts/filtered_search/dropdown_utils_spec.js
+++ b/spec/javascripts/filtered_search/dropdown_utils_spec.js
@@ -211,132 +211,6 @@ describe('Dropdown Utils', () => {
     });
   });
 
-  describe('mergeDuplicateLabels', () => {
-    const dataMap = {
-      label: {
-        title: 'label',
-        color: '#FFFFFF',
-      },
-    };
-
-    it('should add label to dataMap if it is not a duplicate', () => {
-      const newLabel = {
-        title: 'new-label',
-        color: '#000000',
-      };
-
-      const updated = DropdownUtils.mergeDuplicateLabels(dataMap, newLabel);
-
-      expect(updated[newLabel.title]).toEqual(newLabel);
-    });
-
-    it('should merge colors if label is a duplicate', () => {
-      const duplicate = {
-        title: 'label',
-        color: '#000000',
-      };
-
-      const updated = DropdownUtils.mergeDuplicateLabels(dataMap, duplicate);
-
-      expect(updated.label.multipleColors).toEqual([dataMap.label.color, duplicate.color]);
-    });
-  });
-
-  describe('duplicateLabelColor', () => {
-    it('should linear-gradient 2 colors', () => {
-      const gradient = DropdownUtils.duplicateLabelColor(['#FFFFFF', '#000000']);
-
-      expect(gradient).toEqual(
-        'linear-gradient(#FFFFFF 0%, #FFFFFF 50%, #000000 50%, #000000 100%)',
-      );
-    });
-
-    it('should linear-gradient 3 colors', () => {
-      const gradient = DropdownUtils.duplicateLabelColor(['#FFFFFF', '#000000', '#333333']);
-
-      expect(gradient).toEqual(
-        'linear-gradient(#FFFFFF 0%, #FFFFFF 33%, #000000 33%, #000000 66%, #333333 66%, #333333 100%)',
-      );
-    });
-
-    it('should linear-gradient 4 colors', () => {
-      const gradient = DropdownUtils.duplicateLabelColor([
-        '#FFFFFF',
-        '#000000',
-        '#333333',
-        '#DDDDDD',
-      ]);
-
-      expect(gradient).toEqual(
-        'linear-gradient(#FFFFFF 0%, #FFFFFF 25%, #000000 25%, #000000 50%, #333333 50%, #333333 75%, #DDDDDD 75%, #DDDDDD 100%)',
-      );
-    });
-
-    it('should not linear-gradient more than 4 colors', () => {
-      const gradient = DropdownUtils.duplicateLabelColor([
-        '#FFFFFF',
-        '#000000',
-        '#333333',
-        '#DDDDDD',
-        '#EEEEEE',
-      ]);
-
-      expect(gradient.indexOf('#EEEEEE')).toBe(-1);
-    });
-  });
-
-  describe('duplicateLabelPreprocessing', () => {
-    it('should set preprocessed to true', () => {
-      const results = DropdownUtils.duplicateLabelPreprocessing([]);
-
-      expect(results.preprocessed).toEqual(true);
-    });
-
-    it('should not mutate existing data if there are no duplicates', () => {
-      const data = [
-        {
-          title: 'label1',
-          color: '#FFFFFF',
-        },
-        {
-          title: 'label2',
-          color: '#000000',
-        },
-      ];
-      const results = DropdownUtils.duplicateLabelPreprocessing(data);
-
-      expect(results.length).toEqual(2);
-      expect(results[0]).toEqual(data[0]);
-      expect(results[1]).toEqual(data[1]);
-    });
-
-    describe('duplicate labels', () => {
-      const data = [
-        {
-          title: 'label',
-          color: '#FFFFFF',
-        },
-        {
-          title: 'label',
-          color: '#000000',
-        },
-      ];
-      const results = DropdownUtils.duplicateLabelPreprocessing(data);
-
-      it('should merge duplicate labels', () => {
-        expect(results.length).toEqual(1);
-      });
-
-      it('should convert multiple colored labels into linear-gradient', () => {
-        expect(results[0].color).toEqual(DropdownUtils.duplicateLabelColor(['#FFFFFF', '#000000']));
-      });
-
-      it('should set multiple colored label text color to black', () => {
-        expect(results[0].text_color).toEqual('#000000');
-      });
-    });
-  });
-
   describe('setDataValueIfSelected', () => {
     beforeEach(() => {
       spyOn(FilteredSearchDropdownManager, 'addWordToInput').and.callFake(() => {});
diff --git a/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js b/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js
index 4f561df7943..9aa3cbaa231 100644
--- a/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js
+++ b/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js
@@ -909,16 +909,6 @@ describe('Filtered Search Visual Tokens', () => {
       expect(token.style.backgroundColor).not.toEqual(originalBackgroundColor);
     });
 
-    it('should not set backgroundColor when it is a linear-gradient', () => {
-      const token = subject.setTokenStyle(
-        bugLabelToken,
-        'linear-gradient(135deg, red, blue)',
-        'white',
-      );
-
-      expect(token.style.backgroundColor).toEqual(bugLabelToken.style.backgroundColor);
-    });
-
     it('should set textColor', () => {
       const token = subject.setTokenStyle(bugLabelToken, 'white', 'black');
 
@@ -935,39 +925,6 @@ describe('Filtered Search Visual Tokens', () => {
     });
   });
 
-  describe('preprocessLabel', () => {
-    const endpoint = 'endpoint';
-
-    it('does not preprocess more than once', () => {
-      let labels = [];
-
-      spyOn(DropdownUtils, 'duplicateLabelPreprocessing').and.callFake(() => []);
-
-      labels = FilteredSearchVisualTokens.preprocessLabel(endpoint, labels);
-      FilteredSearchVisualTokens.preprocessLabel(endpoint, labels);
-
-      expect(DropdownUtils.duplicateLabelPreprocessing.calls.count()).toEqual(1);
-    });
-
-    describe('not preprocessed before', () => {
-      it('returns preprocessed labels', () => {
-        let labels = [];
-
-        expect(labels.preprocessed).not.toEqual(true);
-        labels = FilteredSearchVisualTokens.preprocessLabel(endpoint, labels);
-
-        expect(labels.preprocessed).toEqual(true);
-      });
-
-      it('overrides AjaxCache with preprocessed results', () => {
-        spyOn(AjaxCache, 'override').and.callFake(() => {});
-        FilteredSearchVisualTokens.preprocessLabel(endpoint, []);
-
-        expect(AjaxCache.override.calls.count()).toEqual(1);
-      });
-    });
-  });
-
   describe('updateLabelTokenColor', () => {
     const jsonFixtureName = 'labels/project_labels.json';
     const dummyEndpoint = '/dummy/endpoint';