Merge branch '28424-labels-support-color-names-in-backend' into 'master'

Labels support color names in backend

Closes #28424

See merge request !9725

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

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

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

app/services/labels/base_service.rb

+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

app/services/labels/create_service.rb

+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

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

app/services/labels/update_service.rb

+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

changelogs/unreleased/28424-labels-support-color-names-in-backend.yml

+---
+title: Labels support color names in backend
+merge_request: 9725
+author: Dongqing Hu

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. |
 

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?

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
 

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

spec/services/labels/create_service_spec.rb

+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

spec/services/labels/update_service_spec.rb

+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