Commit 81ddb692 authored by Fatih Acet's avatar Fatih Acet

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
parents 3b14a44c f57b1323
......@@ -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,
......
......@@ -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) {
......
......@@ -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}`);
},
......
......@@ -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;
......
......@@ -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,
......
......@@ -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',
}),
);
......
---
title: Disable merging of labels with same names
merge_request: 23265
author:
type: changed
......@@ -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);
......
......@@ -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', () => {
......
......@@ -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(() => {});
......
......@@ -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';
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment