diff options
| -rw-r--r-- | app/controllers/admin/labels_controller.rb | 9 | ||||
| -rw-r--r-- | app/controllers/groups/labels_controller.rb | 6 | ||||
| -rw-r--r-- | app/controllers/projects/labels_controller.rb | 6 | ||||
| -rw-r--r-- | app/services/labels/base_service.rb | 161 | ||||
| -rw-r--r-- | app/services/labels/create_service.rb | 25 | ||||
| -rw-r--r-- | app/services/labels/find_or_create_service.rb | 4 | ||||
| -rw-r--r-- | app/services/labels/update_service.rb | 15 | ||||
| -rw-r--r-- | changelogs/unreleased/28424-labels-support-color-names-in-backend.yml | 4 | ||||
| -rw-r--r-- | doc/api/labels.md | 4 | ||||
| -rw-r--r-- | lib/api/labels.rb | 9 | ||||
| -rw-r--r-- | lib/gitlab/bitbucket_import/importer.rb | 9 | ||||
| -rw-r--r-- | spec/lib/gitlab/github_import/label_formatter_spec.rb | 2 | ||||
| -rw-r--r-- | spec/services/labels/create_service_spec.rb | 186 | ||||
| -rw-r--r-- | spec/services/labels/update_service_spec.rb | 80 |
14 files changed, 501 insertions, 19 deletions
diff --git a/app/controllers/admin/labels_controller.rb b/app/controllers/admin/labels_controller.rb index d496f08a598..4531657268c 100644 --- a/app/controllers/admin/labels_controller.rb +++ b/app/controllers/admin/labels_controller.rb @@ -16,10 +16,9 @@ class Admin::LabelsController < Admin::ApplicationController end def create - @label = Label.new(label_params) - @label.template = true + @label = Labels::CreateService.new(label_params).execute(template: true) - if @label.save + if @label.persisted? redirect_to admin_labels_url, notice: "Label was created" else render :new @@ -27,7 +26,9 @@ class Admin::LabelsController < Admin::ApplicationController end def update - if @label.update(label_params) + @label = Labels::UpdateService.new(label_params).execute(@label) + + if @label.valid? redirect_to admin_labels_path, notice: 'label was successfully updated.' else render :edit diff --git a/app/controllers/groups/labels_controller.rb b/app/controllers/groups/labels_controller.rb index 587898a8634..facb25525b5 100644 --- a/app/controllers/groups/labels_controller.rb +++ b/app/controllers/groups/labels_controller.rb @@ -26,7 +26,7 @@ class Groups::LabelsController < Groups::ApplicationController end def create - @label = @group.labels.create(label_params) + @label = Labels::CreateService.new(label_params).execute(group: group) if @label.valid? redirect_to group_labels_path(@group) @@ -40,7 +40,9 @@ class Groups::LabelsController < Groups::ApplicationController end def update - if @label.update_attributes(label_params) + @label = Labels::UpdateService.new(label_params).execute(@label) + + if @label.valid? redirect_back_or_group_labels_path else render :edit diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index 1593b5c1afb..2f55ba4e700 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -29,7 +29,7 @@ class Projects::LabelsController < Projects::ApplicationController end def create - @label = @project.labels.create(label_params) + @label = Labels::CreateService.new(label_params).execute(project: @project) if @label.valid? respond_to do |format| @@ -48,7 +48,9 @@ class Projects::LabelsController < Projects::ApplicationController end def update - if @label.update_attributes(label_params) + @label = Labels::UpdateService.new(label_params).execute(@label) + + if @label.valid? redirect_to namespace_project_labels_path(@project.namespace, @project) else render :edit diff --git a/app/services/labels/base_service.rb b/app/services/labels/base_service.rb new file mode 100644 index 00000000000..91d72a57b4e --- /dev/null +++ b/app/services/labels/base_service.rb @@ -0,0 +1,161 @@ +module Labels + class BaseService < ::BaseService + COLOR_NAME_TO_HEX = { + black: '#000000', + silver: '#C0C0C0', + gray: '#808080', + white: '#FFFFFF', + maroon: '#800000', + red: '#FF0000', + purple: '#800080', + fuchsia: '#FF00FF', + green: '#008000', + lime: '#00FF00', + olive: '#808000', + yellow: '#FFFF00', + navy: '#000080', + blue: '#0000FF', + teal: '#008080', + aqua: '#00FFFF', + orange: '#FFA500', + aliceblue: '#F0F8FF', + antiquewhite: '#FAEBD7', + aquamarine: '#7FFFD4', + azure: '#F0FFFF', + beige: '#F5F5DC', + bisque: '#FFE4C4', + blanchedalmond: '#FFEBCD', + blueviolet: '#8A2BE2', + brown: '#A52A2A', + burlywood: '#DEB887', + cadetblue: '#5F9EA0', + chartreuse: '#7FFF00', + chocolate: '#D2691E', + coral: '#FF7F50', + cornflowerblue: '#6495ED', + cornsilk: '#FFF8DC', + crimson: '#DC143C', + darkblue: '#00008B', + darkcyan: '#008B8B', + darkgoldenrod: '#B8860B', + darkgray: '#A9A9A9', + darkgreen: '#006400', + darkgrey: '#A9A9A9', + darkkhaki: '#BDB76B', + darkmagenta: '#8B008B', + darkolivegreen: '#556B2F', + darkorange: '#FF8C00', + darkorchid: '#9932CC', + darkred: '#8B0000', + darksalmon: '#E9967A', + darkseagreen: '#8FBC8F', + darkslateblue: '#483D8B', + darkslategray: '#2F4F4F', + darkslategrey: '#2F4F4F', + darkturquoise: '#00CED1', + darkviolet: '#9400D3', + deeppink: '#FF1493', + deepskyblue: '#00BFFF', + dimgray: '#696969', + dimgrey: '#696969', + dodgerblue: '#1E90FF', + firebrick: '#B22222', + floralwhite: '#FFFAF0', + forestgreen: '#228B22', + gainsboro: '#DCDCDC', + ghostwhite: '#F8F8FF', + gold: '#FFD700', + goldenrod: '#DAA520', + greenyellow: '#ADFF2F', + grey: '#808080', + honeydew: '#F0FFF0', + hotpink: '#FF69B4', + indianred: '#CD5C5C', + indigo: '#4B0082', + ivory: '#FFFFF0', + khaki: '#F0E68C', + lavender: '#E6E6FA', + lavenderblush: '#FFF0F5', + lawngreen: '#7CFC00', + lemonchiffon: '#FFFACD', + lightblue: '#ADD8E6', + lightcoral: '#F08080', + lightcyan: '#E0FFFF', + lightgoldenrodyellow: '#FAFAD2', + lightgray: '#D3D3D3', + lightgreen: '#90EE90', + lightgrey: '#D3D3D3', + lightpink: '#FFB6C1', + lightsalmon: '#FFA07A', + lightseagreen: '#20B2AA', + lightskyblue: '#87CEFA', + lightslategray: '#778899', + lightslategrey: '#778899', + lightsteelblue: '#B0C4DE', + lightyellow: '#FFFFE0', + limegreen: '#32CD32', + linen: '#FAF0E6', + mediumaquamarine: '#66CDAA', + mediumblue: '#0000CD', + mediumorchid: '#BA55D3', + mediumpurple: '#9370DB', + mediumseagreen: '#3CB371', + mediumslateblue: '#7B68EE', + mediumspringgreen: '#00FA9A', + mediumturquoise: '#48D1CC', + mediumvioletred: '#C71585', + midnightblue: '#191970', + mintcream: '#F5FFFA', + mistyrose: '#FFE4E1', + moccasin: '#FFE4B5', + navajowhite: '#FFDEAD', + oldlace: '#FDF5E6', + olivedrab: '#6B8E23', + orangered: '#FF4500', + orchid: '#DA70D6', + palegoldenrod: '#EEE8AA', + palegreen: '#98FB98', + paleturquoise: '#AFEEEE', + palevioletred: '#DB7093', + papayawhip: '#FFEFD5', + peachpuff: '#FFDAB9', + peru: '#CD853F', + pink: '#FFC0CB', + plum: '#DDA0DD', + powderblue: '#B0E0E6', + rosybrown: '#BC8F8F', + royalblue: '#4169E1', + saddlebrown: '#8B4513', + salmon: '#FA8072', + sandybrown: '#F4A460', + seagreen: '#2E8B57', + seashell: '#FFF5EE', + sienna: '#A0522D', + skyblue: '#87CEEB', + slateblue: '#6A5ACD', + slategray: '#708090', + slategrey: '#708090', + snow: '#FFFAFA', + springgreen: '#00FF7F', + steelblue: '#4682B4', + tan: '#D2B48C', + thistle: '#D8BFD8', + tomato: '#FF6347', + turquoise: '#40E0D0', + violet: '#EE82EE', + wheat: '#F5DEB3', + whitesmoke: '#F5F5F5', + yellowgreen: '#9ACD32', + rebeccapurple: '#663399' + }.freeze + + def convert_color_name_to_hex + color = params[:color] + color_name = color.strip.downcase + + return color if color_name.start_with?('#') + + COLOR_NAME_TO_HEX[color_name.to_sym] || color + end + end +end diff --git a/app/services/labels/create_service.rb b/app/services/labels/create_service.rb new file mode 100644 index 00000000000..6c399c92377 --- /dev/null +++ b/app/services/labels/create_service.rb @@ -0,0 +1,25 @@ +module Labels + class CreateService < Labels::BaseService + def initialize(params = {}) + @params = params.dup.with_indifferent_access + end + + # returns the created label + def execute(target_params) + params[:color] = convert_color_name_to_hex if params[:color].present? + + project_or_group = target_params[:project] || target_params[:group] + + if project_or_group.present? + project_or_group.labels.create(params) + elsif target_params[:template] + label = Label.new(params) + label.template = true + label.save + label + else + Rails.logger.warn("target_params should contain :project or :group or :template, actual value: #{target_params}") + end + end + end +end diff --git a/app/services/labels/find_or_create_service.rb b/app/services/labels/find_or_create_service.rb index cf4f7606c94..940c8b333d3 100644 --- a/app/services/labels/find_or_create_service.rb +++ b/app/services/labels/find_or_create_service.rb @@ -3,7 +3,7 @@ module Labels def initialize(current_user, project, params = {}) @current_user = current_user @project = project - @params = params.dup + @params = params.dup.with_indifferent_access end def execute(skip_authorization: false) @@ -28,7 +28,7 @@ module Labels new_label = available_labels.find_by(title: title) if new_label.nil? && (skip_authorization || Ability.allowed?(current_user, :admin_label, project)) - new_label = project.labels.create(params) + new_label = Labels::CreateService.new(params).execute(project: project) end new_label diff --git a/app/services/labels/update_service.rb b/app/services/labels/update_service.rb new file mode 100644 index 00000000000..28dcabf9541 --- /dev/null +++ b/app/services/labels/update_service.rb @@ -0,0 +1,15 @@ +module Labels + class UpdateService < Labels::BaseService + def initialize(params = {}) + @params = params.dup.with_indifferent_access + end + + # returns the updated label + def execute(label) + params[:color] = convert_color_name_to_hex if params[:color].present? + + label.update(params) + label + end + end +end diff --git a/changelogs/unreleased/28424-labels-support-color-names-in-backend.yml b/changelogs/unreleased/28424-labels-support-color-names-in-backend.yml new file mode 100644 index 00000000000..00da1e0fa60 --- /dev/null +++ b/changelogs/unreleased/28424-labels-support-color-names-in-backend.yml @@ -0,0 +1,4 @@ +--- +title: Labels support color names in backend +merge_request: 9725 +author: Dongqing Hu diff --git a/doc/api/labels.md b/doc/api/labels.md index e8c220f6809..839000a4f48 100644 --- a/doc/api/labels.md +++ b/doc/api/labels.md @@ -90,7 +90,7 @@ POST /projects/:id/labels | ------------- | ------- | -------- | ---------------------------- | | `id` | integer | yes | The ID of the project | | `name` | string | yes | The name of the label | -| `color` | string | yes | The color of the label in 6-digit hex notation with leading `#` sign | +| `color` | string | yes | The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB) or one of the [CSS color names](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#Color_keywords) | | `description` | string | no | The description of the label | | `priority` | integer | no | The priority of the label. Must be greater or equal than zero or `null` to remove the priority. | @@ -145,7 +145,7 @@ PUT /projects/:id/labels | `id` | integer | yes | The ID of the project | | `name` | string | yes | The name of the existing label | | `new_name` | string | yes if `color` is not provided | The new name of the label | -| `color` | string | yes if `new_name` is not provided | The new color of the label in 6-digit hex notation with leading `#` sign | +| `color` | string | yes if `new_name` is not provided | The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB) or one of the [CSS color names](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#Color_keywords) | | `description` | string | no | The new description of the label | | `priority` | integer | no | The new priority of the label. Must be greater or equal than zero or `null` to remove the priority. | diff --git a/lib/api/labels.rb b/lib/api/labels.rb index d9a3cb7bb6b..20b25529d0c 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -23,7 +23,7 @@ module API end params do requires :name, type: String, desc: 'The name of the label to be created' - requires :color, type: String, desc: "The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)" + requires :color, type: String, desc: "The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB) or one of the allowed CSS color names" optional :description, type: String, desc: 'The description of label to be created' optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true end @@ -34,7 +34,7 @@ module API conflict!('Label already exists') if label priority = params.delete(:priority) - label = user_project.labels.create(declared_params(include_missing: false)) + label = ::Labels::CreateService.new(declared_params(include_missing: false)).execute(project: user_project) if label.valid? label.prioritize!(user_project, priority) if priority @@ -65,7 +65,7 @@ module API params do requires :name, type: String, desc: 'The name of the label to be updated' optional :new_name, type: String, desc: 'The new name of the label' - optional :color, type: String, desc: "The new color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)" + optional :color, type: String, desc: "The new color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB) or one of the allowed CSS color names" optional :description, type: String, desc: 'The new description of label' optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true at_least_one_of :new_name, :color, :description, :priority @@ -82,7 +82,8 @@ module API # Rename new name to the actual label attribute name label_params[:name] = label_params.delete(:new_name) if label_params.key?(:new_name) - render_validation_error!(label) unless label.update(label_params) + label = ::Labels::UpdateService.new(label_params).execute(label) + render_validation_error!(label) unless label.valid? if update_priority if priority.nil? diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index 44323b47dca..f4efa20374a 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -130,8 +130,13 @@ module Gitlab end def create_labels - LABELS.each do |label| - @labels[label[:title]] = project.labels.create!(label) + LABELS.each do |label_params| + label = ::Labels::CreateService.new(label_params).execute(project: project) + if label.valid? + @labels[label_params[:title]] = label + else + raise "Failed to create label \"#{label_params[:title]}\" for project \"#{project.name_with_namespace}\"" + end end end diff --git a/spec/lib/gitlab/github_import/label_formatter_spec.rb b/spec/lib/gitlab/github_import/label_formatter_spec.rb index 10449ef5fcb..565435824fd 100644 --- a/spec/lib/gitlab/github_import/label_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/label_formatter_spec.rb @@ -25,7 +25,7 @@ describe Gitlab::GithubImport::LabelFormatter, lib: true do context 'when label exists' do it 'does not create a new label' do - project.labels.create(name: raw.name) + Labels::CreateService.new(name: raw.name).execute(project: project) expect { subject.create! }.not_to change(Label, :count) end diff --git a/spec/services/labels/create_service_spec.rb b/spec/services/labels/create_service_spec.rb new file mode 100644 index 00000000000..0ecab0c3475 --- /dev/null +++ b/spec/services/labels/create_service_spec.rb @@ -0,0 +1,186 @@ +require 'spec_helper' + +describe Labels::CreateService, services: true do + describe '#execute' do + let(:project) { create(:project) } + let(:group) { create(:group) } + + let(:hex_color) { '#FF0000' } + let(:named_color) { 'red' } + let(:upcase_color) { 'RED' } + let(:spaced_color) { ' red ' } + let(:unknown_color) { 'unknown' } + let(:no_color) { '' } + + let(:expected_saved_color) { hex_color } + + context 'in a project' do + context 'with color in hex-code' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(hex_color)).execute(project: project) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color in allowed name' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(named_color)).execute(project: project) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color in up-case allowed name' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(upcase_color)).execute(project: project) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color surrounded by spaces' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(spaced_color)).execute(project: project) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with unknown color' do + it 'doesn\'t create a label' do + label = Labels::CreateService.new(params_with(unknown_color)).execute(project: project) + + expect(label).not_to be_persisted + end + end + + context 'with no color' do + it 'doesn\'t create a label' do + label = Labels::CreateService.new(params_with(no_color)).execute(project: project) + + expect(label).not_to be_persisted + end + end + end + + context 'in a group' do + context 'with color in hex-code' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(hex_color)).execute(group: group) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color in allowed name' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(named_color)).execute(group: group) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color in up-case allowed name' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(upcase_color)).execute(group: group) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color surrounded by spaces' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(spaced_color)).execute(group: group) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with unknown color' do + it 'doesn\'t create a label' do + label = Labels::CreateService.new(params_with(unknown_color)).execute(group: group) + + expect(label).not_to be_persisted + end + end + + context 'with no color' do + it 'doesn\'t create a label' do + label = Labels::CreateService.new(params_with(no_color)).execute(group: group) + + expect(label).not_to be_persisted + end + end + end + + context 'in admin area' do + context 'with color in hex-code' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(hex_color)).execute(template: true) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color in allowed name' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(named_color)).execute(template: true) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color in up-case allowed name' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(upcase_color)).execute(template: true) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color surrounded by spaces' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(spaced_color)).execute(template: true) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with unknown color' do + it 'doesn\'t create a label' do + label = Labels::CreateService.new(params_with(unknown_color)).execute(template: true) + + expect(label).not_to be_persisted + end + end + + context 'with no color' do + it 'doesn\'t create a label' do + label = Labels::CreateService.new(params_with(no_color)).execute(template: true) + + expect(label).not_to be_persisted + end + end + end + end + + def params_with(color) + { + title: 'A Label', + color: color + } + end +end diff --git a/spec/services/labels/update_service_spec.rb b/spec/services/labels/update_service_spec.rb new file mode 100644 index 00000000000..f2498ea6990 --- /dev/null +++ b/spec/services/labels/update_service_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +describe Labels::UpdateService, services: true do + describe '#execute' do + let(:project) { create(:project) } + + let(:hex_color) { '#FF0000' } + let(:named_color) { 'red' } + let(:upcase_color) { 'RED' } + let(:spaced_color) { ' red ' } + let(:unknown_color) { 'unknown' } + let(:no_color) { '' } + + let(:expected_saved_color) { hex_color } + + before(:each) do + @label = Labels::CreateService.new(title: 'Initial', color: '#000000').execute(project: project) + expect(@label).to be_persisted + end + + context 'with color in hex-code' do + it 'updates the label' do + label = Labels::UpdateService.new(params_with(hex_color)).execute(@label) + + expect(label).to be_valid + expect(label.reload.color).to eq expected_saved_color + end + end + + context 'with color in allowed name' do + it 'updates the label' do + label = Labels::UpdateService.new(params_with(named_color)).execute(@label) + + expect(label).to be_valid + expect(label.reload.color).to eq expected_saved_color + end + end + + context 'with color in up-case allowed name' do + it 'updates the label' do + label = Labels::UpdateService.new(params_with(upcase_color)).execute(@label) + + expect(label).to be_valid + expect(label.reload.color).to eq expected_saved_color + end + end + + context 'with color surrounded by spaces' do + it 'updates the label' do + label = Labels::UpdateService.new(params_with(spaced_color)).execute(@label) + + expect(label).to be_valid + expect(label.reload.color).to eq expected_saved_color + end + end + + context 'with unknown color' do + it 'doesn\'t update the label' do + label = Labels::UpdateService.new(params_with(unknown_color)).execute(@label) + + expect(label).not_to be_valid + end + end + + context 'with no color' do + it 'doesn\'t update the label' do + label = Labels::UpdateService.new(params_with(no_color)).execute(@label) + + expect(label).not_to be_valid + end + end + end + + def params_with(color) + { + title: 'A Label', + color: color + } + end +end |