Merge branch...

Merge branch '323279-move-issue_type-to-separate-table-to-support-user-creation-of-types' into 'master'

Create work_item_types table

See merge request gitlab-org/gitlab!55705

app/models/issue.rb

@@ -48,6 +48,7 @@ class Issue < ApplicationRecord
   belongs_to :duplicated_to, class_name: 'Issue'
   belongs_to :closed_by, class_name: 'User'
   belongs_to :iteration, foreign_key: 'sprint_id'
+  belongs_to :work_item_type, class_name: 'WorkItem::Type', inverse_of: :work_items
 
   belongs_to :moved_to, class_name: 'Issue'
   has_one :moved_from, class_name: 'Issue', foreign_key: :moved_to_id

app/models/work_item/type.rb

+# frozen_string_literal: true
+
+# Note: initial thinking behind `icon_name` is for it to do triple duty:
+# 1. one of our svg icon names, such as `external-link` or a new one `bug`
+# 2. if it's an absolute url, then url to a user uploaded icon/image
+# 3. an emoji, with the format of `:smile:`
+class WorkItem::Type < ApplicationRecord
+  self.table_name = 'work_item_types'
+
+  include CacheMarkdownField
+
+  cache_markdown_field :description, pipeline: :single_line
+
+  enum base_type: Issue.issue_types
+
+  belongs_to :group, foreign_key: :namespace_id, optional: true
+  has_many :work_items, class_name: 'Issue', foreign_key: :work_item_type_id, inverse_of: :work_item_type
+
+  before_validation :strip_whitespace
+
+  # TODO: review validation rules
+  # https://gitlab.com/gitlab-org/gitlab/-/issues/336919
+  validates :name, presence: true
+  validates :name, uniqueness: { case_sensitive: false, scope: [:namespace_id] }
+  validates :name, length: { maximum: 255 }
+  validates :icon_name, length: { maximum: 255 }
+
+  private
+
+  def strip_whitespace
+    name&.strip!
+  end
+end

db/migrate/20210709221051_create_work_item_types.rb

+# frozen_string_literal: true
+
+class CreateWorkItemTypes < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def up
+    create_table_with_constraints :work_item_types do |t|
+      t.integer :base_type, limit: 2, default: 0, null: false
+      t.integer :cached_markdown_version
+      t.text :name, null: false
+      t.text :description # rubocop:disable Migration/AddLimitToTextColumns
+      t.text :description_html # rubocop:disable Migration/AddLimitToTextColumns
+      t.text :icon_name, null: true
+      t.references :namespace, foreign_key: { on_delete: :cascade }, index: false, null: true
+      t.timestamps_with_timezone null: false
+
+      t.text_limit :name, 255
+      t.text_limit :icon_name, 255
+    end
+
+    add_concurrent_index :work_item_types,
+                         'namespace_id, TRIM(BOTH FROM LOWER(name))',
+                         unique: true,
+                         name: :work_item_types_namespace_id_and_name_unique
+  end
+
+  def down
+    with_lock_retries do
+      drop_table :work_item_types
+    end
+  end
+end

db/migrate/20210709221659_add_work_item_type_id_to_issue.rb

+# frozen_string_literal: true
+
+# See https://docs.gitlab.com/ee/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddWorkItemTypeIdToIssue < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def up
+    unless column_exists?(:issues, :work_item_type_id)
+      with_lock_retries do
+        add_column :issues, :work_item_type_id, :bigint
+      end
+    end
+
+    add_concurrent_index :issues, :work_item_type_id
+    add_concurrent_foreign_key :issues, :work_item_types, column: :work_item_type_id, on_delete: nil
+  end
+
+  def down
+    if foreign_key_exists?(:issues, :work_item_types)
+      remove_foreign_key :issues, column: :work_item_type_id
+    end
+
+    with_lock_retries do
+      remove_column :issues, :work_item_type_id
+    end
+  end
+end

db/schema_migrations/20210709221051

+7847339fb7b143845e2715b15505016dc8e6de3fbd2c5cb4bae55da4f25a5a5f
\ No newline at end of file

db/schema_migrations/20210709221659

+5bec34d517f3f2bbb9735f73fb5641512c9f5286ee5d7a59b17c976dd1459347
\ No newline at end of file

db/structure.sql

@@ -14341,6 +14341,7 @@ CREATE TABLE issues (
     issue_type smallint DEFAULT 0 NOT NULL,
     blocking_issues_count integer DEFAULT 0 NOT NULL,
     upvotes_count integer DEFAULT 0 NOT NULL,
+    work_item_type_id bigint,
     CONSTRAINT check_fba63f706d CHECK ((lock_version IS NOT NULL))
 );
 
@@ -19769,6 +19770,30 @@ CREATE SEQUENCE wiki_page_slugs_id_seq
 
 ALTER SEQUENCE wiki_page_slugs_id_seq OWNED BY wiki_page_slugs.id;
 
+CREATE TABLE work_item_types (
+    id bigint NOT NULL,
+    base_type smallint DEFAULT 0 NOT NULL,
+    cached_markdown_version integer,
+    name text NOT NULL,
+    description text,
+    description_html text,
+    icon_name text,
+    namespace_id bigint,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    CONSTRAINT check_104d2410f6 CHECK ((char_length(name) <= 255)),
+    CONSTRAINT check_fecb3a98d1 CHECK ((char_length(icon_name) <= 255))
+);
+
+CREATE SEQUENCE work_item_types_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE work_item_types_id_seq OWNED BY work_item_types.id;
+
 CREATE TABLE x509_certificates (
     id bigint NOT NULL,
     created_at timestamp with time zone NOT NULL,
@@ -20748,6 +20773,8 @@ ALTER TABLE ONLY wiki_page_meta ALTER COLUMN id SET DEFAULT nextval('wiki_page_m
 
 ALTER TABLE ONLY wiki_page_slugs ALTER COLUMN id SET DEFAULT nextval('wiki_page_slugs_id_seq'::regclass);
 
+ALTER TABLE ONLY work_item_types ALTER COLUMN id SET DEFAULT nextval('work_item_types_id_seq'::regclass);
+
 ALTER TABLE ONLY x509_certificates ALTER COLUMN id SET DEFAULT nextval('x509_certificates_id_seq'::regclass);
 
 ALTER TABLE ONLY x509_commit_signatures ALTER COLUMN id SET DEFAULT nextval('x509_commit_signatures_id_seq'::regclass);
@@ -22499,6 +22526,9 @@ ALTER TABLE ONLY wiki_page_meta
 ALTER TABLE ONLY wiki_page_slugs
     ADD CONSTRAINT wiki_page_slugs_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY work_item_types
+    ADD CONSTRAINT work_item_types_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY x509_certificates
     ADD CONSTRAINT x509_certificates_pkey PRIMARY KEY (id);
 
@@ -24035,6 +24065,8 @@ CREATE INDEX index_issues_on_updated_at ON issues USING btree (updated_at);
 
 CREATE INDEX index_issues_on_updated_by_id ON issues USING btree (updated_by_id) WHERE (updated_by_id IS NOT NULL);
 
+CREATE INDEX index_issues_on_work_item_type_id ON issues USING btree (work_item_type_id);
+
 CREATE INDEX index_iterations_cadences_on_group_id ON iterations_cadences USING btree (group_id);
 
 CREATE UNIQUE INDEX index_jira_connect_installations_on_client_key ON jira_connect_installations USING btree (client_key);
@@ -25547,6 +25579,8 @@ CREATE UNIQUE INDEX vulnerability_feedback_unique_idx ON vulnerability_feedback
 
 CREATE UNIQUE INDEX vulnerability_occurrence_pipelines_on_unique_keys ON vulnerability_occurrence_pipelines USING btree (occurrence_id, pipeline_id);
 
+CREATE UNIQUE INDEX work_item_types_namespace_id_and_name_unique ON work_item_types USING btree (namespace_id, btrim(lower(name)));
+
 ALTER INDEX index_product_analytics_events_experimental_project_and_time ATTACH PARTITION gitlab_partitions_static.product_analytics_events_expe_project_id_collector_tstamp_idx10;
 
 ALTER INDEX index_product_analytics_events_experimental_project_and_time ATTACH PARTITION gitlab_partitions_static.product_analytics_events_expe_project_id_collector_tstamp_idx11;
@@ -26372,6 +26406,9 @@ ALTER TABLE ONLY vulnerabilities
 ALTER TABLE ONLY project_access_tokens
     ADD CONSTRAINT fk_b27801bfbf FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY issues
+    ADD CONSTRAINT fk_b37be69be6 FOREIGN KEY (work_item_type_id) REFERENCES work_item_types(id);
+
 ALTER TABLE ONLY protected_tag_create_access_levels
     ADD CONSTRAINT fk_b4eb82fe3c FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE;
 
@@ -26849,6 +26886,9 @@ ALTER TABLE ONLY approval_merge_request_rules_groups
 ALTER TABLE ONLY vulnerability_feedback
     ADD CONSTRAINT fk_rails_20976e6fd9 FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE SET NULL;
 
+ALTER TABLE ONLY work_item_types
+    ADD CONSTRAINT fk_rails_20f694a960 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY user_statuses
     ADD CONSTRAINT fk_rails_2178592333 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE;
 

lib/gitlab/import_export/project/import_export.yml

@@ -230,6 +230,7 @@ excluded_attributes:
     - :blocking_issues_count
     - :service_desk_reply_to
     - :upvotes_count
+    - :work_item_type_id
   merge_request:
     - :milestone_id
     - :sprint_id

spec/factories/sequences.rb

@@ -20,4 +20,5 @@ FactoryBot.define do
   sequence(:jira_title) { |n| "[PROJ-#{n}]: fix bug" }
   sequence(:jira_branch) { |n| "feature/PROJ-#{n}" }
   sequence(:job_name) { |n| "job #{n}" }
+  sequence(:work_item_type_name) { |n| "bug#{n}" }
 end

spec/factories/work_item/work_item_types.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :work_item_type, class: 'WorkItem::Type' do
+    group
+
+    name      { generate(:work_item_type_name) }
+    icon_name { 'issue' }
+    base_type { Issue.issue_types['issue'] }
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -7,6 +7,7 @@ issues:
 - updated_by
 - milestone
 - iteration
+- work_item_type
 - notes
 - resource_label_events
 - resource_weight_events
@@ -56,6 +57,8 @@ issues:
 - issue_email_participants
 - test_reports
 - requirement
+work_item_type:
+- issues
 events:
 - author
 - project

spec/models/issue_spec.rb

@@ -15,6 +15,7 @@ RSpec.describe Issue do
     it { is_expected.to belong_to(:iteration) }
     it { is_expected.to belong_to(:project) }
     it { is_expected.to have_one(:namespace).through(:project) }
+    it { is_expected.to belong_to(:work_item_type).class_name('WorkItem::Type') }
     it { is_expected.to belong_to(:moved_to).class_name('Issue') }
     it { is_expected.to have_one(:moved_from).class_name('Issue') }
     it { is_expected.to belong_to(:duplicated_to).class_name('Issue') }

spec/models/work_item/type_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe WorkItem::Type do
+  describe 'modules' do
+    it { is_expected.to include_module(CacheMarkdownField) }
+  end
+
+  describe 'associations' do
+    it { is_expected.to have_many(:work_items).with_foreign_key('work_item_type_id') }
+    it { is_expected.to belong_to(:group).with_foreign_key('namespace_id') }
+  end
+
+  describe '#destroy' do
+    let!(:work_item) { create :issue }
+
+    context 'when there are no work items of that type' do
+      it 'deletes type but not unrelated issues' do
+        type = create(:work_item_type)
+
+        expect { type.destroy! }.not_to change(Issue, :count)
+        expect(WorkItem::Type.count).to eq 0
+      end
+    end
+
+    it 'does not delete type when there are related issues' do
+      type = create(:work_item_type, work_items: [work_item])
+
+      expect { type.destroy! }.to raise_error(ActiveRecord::InvalidForeignKey)
+      expect(Issue.count).to eq 1
+    end
+  end
+
+  describe 'validation' do
+    describe 'name uniqueness' do
+      subject { create(:work_item_type) }
+
+      it { is_expected.to validate_uniqueness_of(:name).case_insensitive.scoped_to([:namespace_id]) }
+    end
+
+    it { is_expected.not_to allow_value('s' * 256).for(:icon_name) }
+  end
+
+  describe '#name' do
+    it 'strips name' do
+      work_item_type = described_class.new(name: '   label😸   ')
+      work_item_type.valid?
+
+      expect(work_item_type.name).to eq('label😸')
+    end
+  end
+end