Merge branch '346039-add-tast-to-work-item-types' into 'master'

Add Task work item type to the database

See merge request gitlab-org/gitlab!75447

app/graphql/types/issue_type_enum.rb

@@ -5,7 +5,7 @@ module Types
     graphql_name 'IssueType'
     description 'Issue type'
 
-    ::WorkItem::Type.base_types.keys.each do |issue_type|
+    ::WorkItem::Type.allowed_types_for_issues.each do |issue_type|
       value issue_type.upcase, value: issue_type, description: "#{issue_type.titleize} issue type"
     end
   end

app/models/work_item/type.rb

@@ -15,7 +15,8 @@ class WorkItem::Type < ApplicationRecord
     issue:       { name: 'Issue', icon_name: 'issue-type-issue', enum_value: 0 },
     incident:    { name: 'Incident', icon_name: 'issue-type-incident', enum_value: 1 },
     test_case:   { name: 'Test Case', icon_name: 'issue-type-test-case', enum_value: 2 }, ## EE-only
-    requirement: { name: 'Requirement', icon_name: 'issue-type-requirements', enum_value: 3 } ## EE-only
+    requirement: { name: 'Requirement', icon_name: 'issue-type-requirements', enum_value: 3 }, ## EE-only
+    task:        { name: 'Task', icon_name: 'issue-type-task', enum_value: 4 }
   }.freeze
 
   cache_markdown_field :description, pipeline: :single_line
@@ -42,6 +43,10 @@ class WorkItem::Type < ApplicationRecord
     default_by_type(:issue)
   end
 
+  def self.allowed_types_for_issues
+    base_types.keys.excluding('task')
+  end
+
   private
 
   def strip_whitespace

db/migrate/20211126204445_add_task_to_work_item_types.rb

+# frozen_string_literal: true
+
+class AddTaskToWorkItemTypes < Gitlab::Database::Migration[1.0]
+  TASK_ENUM_VALUE = 4
+
+  class WorkItemType < ActiveRecord::Base
+    self.inheritance_column = :_type_disabled
+    self.table_name = 'work_item_types'
+
+    validates :name, uniqueness: { case_sensitive: false, scope: [:namespace_id] }
+  end
+
+  def up
+    # New instances will not run this migration and add this type via fixtures
+    # checking if record exists mostly because migration specs will run all migrations
+    # and that will conflict with the preloaded base work item types
+    task_work_item = WorkItemType.find_by(name: 'Task', namespace_id: nil)
+
+    if task_work_item
+      say('Task item record exist, skipping creation')
+    else
+      WorkItemType.create(name: 'Task', namespace_id: nil, base_type: TASK_ENUM_VALUE, icon_name: 'issue-type-task')
+    end
+  end
+
+  def down
+    # There's the remote possibility that issues could already be
+    # using this issue type, with a tight foreign constraint.
+    # Therefore we will not attempt to remove any data.
+  end
+end

db/schema_migrations/20211126204445

+e31592bbeb6ba6175f19cfceaafb37672633028dd021052542909999b46eac38
\ No newline at end of file

ee/spec/requests/api/issues_spec.rb

@@ -491,6 +491,15 @@ RSpec.describe API::Issues, :mailer do
       expect(Issue.last.work_item_type.base_type).to eq('issue')
     end
 
+    it 'does not allow the creation of an issue of type task' do
+      expect do
+        post api("/projects/#{project.id}/issues", user),
+          params: { title: 'new issue', issue_type: 'task' }
+      end.to not_change(Issue, :count)
+
+      expect(response).to have_gitlab_http_status(:bad_request)
+    end
+
     it_behaves_like 'with epic parameter' do
       let(:request) { post api("/projects/#{target_project.id}/issues", user), params: params }
     end

ee/spec/services/issues/build_service_spec.rb

@@ -56,11 +56,11 @@ RSpec.describe Issues::BuildService do
     end
 
     context 'as developer' do
-      WorkItem::Type.base_types.each_key do |issue_type|
+      WorkItem::Type.allowed_types_for_issues do |issue_type|
         it "sets the issue type to #{issue_type}" do
           issue = build_issue(issue_type: issue_type)
 
-          expect(issue.issue_type).to eq(issue_type.to_s)
+          expect(issue.issue_type).to eq(issue_type)
         end
       end
     end

lib/api/entities/issue_basic.rb

@@ -23,7 +23,7 @@ module API
       expose :issue_type,
              as: :type,
              format_with: :upcase,
-             documentation: { type: "String", desc: "One of #{::WorkItem::Type.base_types.keys.map(&:upcase)}" }
+             documentation: { type: "String", desc: "One of #{::WorkItem::Type.allowed_types_for_issues.map(&:upcase)}" }
 
       expose :assignee, using: ::API::Entities::UserBasic do |issue|
         issue.assignees.first

lib/api/issues.rb

@@ -82,7 +82,7 @@ module API
                  desc: 'Return issues sorted in `asc` or `desc` order.'
         optional :due_date, type: String, values: %w[0 overdue week month next_month_and_previous_two_weeks] << '',
                  desc: 'Return issues that have no due date (`0`), or whose due date is this week, this month, between two weeks ago and next month, or which are overdue. Accepts: `overdue`, `week`, `month`, `next_month_and_previous_two_weeks`, `0`'
-        optional :issue_type, type: String, values: WorkItem::Type.base_types.keys, desc: "The type of the issue. Accepts: #{WorkItem::Type.base_types.keys.join(', ')}"
+        optional :issue_type, type: String, values: WorkItem::Type.allowed_types_for_issues, desc: "The type of the issue. Accepts: #{WorkItem::Type.allowed_types_for_issues.join(', ')}"
 
         use :issues_stats_params
         use :pagination
@@ -99,7 +99,7 @@ module API
         optional :due_date, type: String, desc: 'Date string in the format YEAR-MONTH-DAY'
         optional :confidential, type: Boolean, desc: 'Boolean parameter if the issue should be confidential'
         optional :discussion_locked, type: Boolean, desc: " Boolean parameter indicating if the issue's discussion is locked"
-        optional :issue_type, type: String, values: WorkItem::Type.base_types.keys, desc: "The type of the issue. Accepts: #{WorkItem::Type.base_types.keys.join(', ')}"
+        optional :issue_type, type: String, values: WorkItem::Type.allowed_types_for_issues, desc: "The type of the issue. Accepts: #{WorkItem::Type.allowed_types_for_issues.join(', ')}"
 
         use :optional_issue_params_ee
       end

spec/controllers/projects/issues_controller_spec.rb

@@ -1197,6 +1197,15 @@ RSpec.describe Projects::IssuesController do
       end
     end
 
+    context 'when trying to create a task' do
+      it 'defaults to issue type' do
+        issue = post_new_issue(issue_type: 'task')
+
+        expect(issue.issue_type).to eq('issue')
+        expect(issue.work_item_type.base_type).to eq('issue')
+      end
+    end
+
     it 'creates the issue successfully', :aggregate_failures do
       issue = post_new_issue
 

spec/graphql/types/issue_type_enum_spec.rb

@@ -5,9 +5,9 @@ require 'spec_helper'
 RSpec.describe Types::IssueTypeEnum do
   specify { expect(described_class.graphql_name).to eq('IssueType') }
 
-  it 'exposes all the existing issue type values' do
-    expect(described_class.values.keys).to include(
-      *%w[ISSUE INCIDENT]
+  it 'exposes all the existing issue type values except for task' do
+    expect(described_class.values.keys).to match_array(
+      %w[ISSUE INCIDENT TEST_CASE REQUIREMENT]
     )
   end
 end

spec/migrations/20210804150320_create_base_work_item_types_spec.rb

@@ -4,18 +4,28 @@ require 'spec_helper'
 require_migration!
 
 RSpec.describe CreateBaseWorkItemTypes, :migration do
-  let!(:work_item_types) { table(:work_item_types) }
+  include MigrationHelpers::WorkItemTypesHelper
+
+  let_it_be(:work_item_types) { table(:work_item_types) }
+
+  let(:base_types) do
+    {
+      issue:       0,
+      incident:    1,
+      test_case:   2,
+      requirement: 3
+    }
+  end
 
   after(:all) do
     # Make sure base types are recreated after running the migration
     # because migration specs are not run in a transaction
-    WorkItem::Type.delete_all
-    Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter.import
+    reset_work_item_types
   end
 
   it 'creates default data' do
     # Need to delete all as base types are seeded before entire test suite
-    WorkItem::Type.delete_all
+    work_item_types.delete_all
 
     reversible_migration do |migration|
       migration.before -> {
@@ -24,8 +34,8 @@ RSpec.describe CreateBaseWorkItemTypes, :migration do
       }
 
       migration.after -> {
-        expect(work_item_types.count).to eq 4
-        expect(work_item_types.all.pluck(:base_type)).to match_array WorkItem::Type.base_types.values
+        expect(work_item_types.count).to eq(4)
+        expect(work_item_types.all.pluck(:base_type)).to match_array(base_types.values)
       }
     end
   end

spec/migrations/20210831203408_upsert_base_work_item_types_spec.rb

@@ -4,19 +4,29 @@ require 'spec_helper'
 require_migration!
 
 RSpec.describe UpsertBaseWorkItemTypes, :migration do
-  let!(:work_item_types) { table(:work_item_types) }
+  include MigrationHelpers::WorkItemTypesHelper
+
+  let_it_be(:work_item_types) { table(:work_item_types) }
+
+  let(:base_types) do
+    {
+      issue:       0,
+      incident:    1,
+      test_case:   2,
+      requirement: 3
+    }
+  end
 
   after(:all) do
     # Make sure base types are recreated after running the migration
     # because migration specs are not run in a transaction
-    WorkItem::Type.delete_all
-    Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter.import
+    reset_work_item_types
   end
 
   context 'when no default types exist' do
     it 'creates default data' do
       # Need to delete all as base types are seeded before entire test suite
-      WorkItem::Type.delete_all
+      work_item_types.delete_all
 
       expect(work_item_types.count).to eq(0)
 
@@ -29,7 +39,7 @@ RSpec.describe UpsertBaseWorkItemTypes, :migration do
 
         migration.after -> {
           expect(work_item_types.count).to eq(4)
-          expect(work_item_types.all.pluck(:base_type)).to match_array(WorkItem::Type.base_types.values)
+          expect(work_item_types.all.pluck(:base_type)).to match_array(base_types.values)
         }
       end
     end
@@ -37,16 +47,21 @@ RSpec.describe UpsertBaseWorkItemTypes, :migration do
 
   context 'when default types already exist' do
     it 'does not create default types again' do
-      expect(work_item_types.all.pluck(:base_type)).to match_array(WorkItem::Type.base_types.values)
+      # Database needs to be in a similar state as when this migration was created
+      work_item_types.delete_all
+      work_item_types.find_or_create_by!(name: 'Issue', namespace_id: nil, base_type: base_types[:issue], icon_name: 'issue-type-issue')
+      work_item_types.find_or_create_by!(name: 'Incident', namespace_id: nil, base_type: base_types[:incident], icon_name: 'issue-type-incident')
+      work_item_types.find_or_create_by!(name: 'Test Case', namespace_id: nil, base_type: base_types[:test_case], icon_name: 'issue-type-test-case')
+      work_item_types.find_or_create_by!(name: 'Requirement', namespace_id: nil, base_type: base_types[:requirement], icon_name: 'issue-type-requirements')
 
       reversible_migration do |migration|
         migration.before -> {
-          expect(work_item_types.all.pluck(:base_type)).to match_array(WorkItem::Type.base_types.values)
+          expect(work_item_types.all.pluck(:base_type)).to match_array(base_types.values)
         }
 
         migration.after -> {
           expect(work_item_types.count).to eq(4)
-          expect(work_item_types.all.pluck(:base_type)).to match_array(WorkItem::Type.base_types.values)
+          expect(work_item_types.all.pluck(:base_type)).to match_array(base_types.values)
         }
       end
     end

spec/migrations/20211126204445_add_task_to_work_item_types_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe AddTaskToWorkItemTypes, :migration do
+  include MigrationHelpers::WorkItemTypesHelper
+
+  let_it_be(:work_item_types) { table(:work_item_types) }
+
+  let(:base_types) do
+    {
+      issue:       0,
+      incident:    1,
+      test_case:   2,
+      requirement: 3,
+      task:        4
+    }
+  end
+
+  after(:all) do
+    # Make sure base types are recreated after running the migration
+    # because migration specs are not run in a transaction
+    reset_work_item_types
+  end
+
+  it 'skips creating the record if it already exists' do
+    reset_db_state_prior_to_migration
+    work_item_types.find_or_create_by!(name: 'Task', namespace_id: nil, base_type: base_types[:task], icon_name: 'issue-type-task')
+
+    expect do
+      migrate!
+    end.to not_change(work_item_types, :count)
+  end
+
+  it 'adds task to base work item types' do
+    reset_db_state_prior_to_migration
+
+    expect do
+      migrate!
+    end.to change(work_item_types, :count).from(4).to(5)
+
+    expect(work_item_types.all.pluck(:base_type)).to include(base_types[:task])
+  end
+
+  def reset_db_state_prior_to_migration
+    # Database needs to be in a similar state as when this migration was created
+    work_item_types.delete_all
+    work_item_types.find_or_create_by!(name: 'Issue', namespace_id: nil, base_type: base_types[:issue], icon_name: 'issue-type-issue')
+    work_item_types.find_or_create_by!(name: 'Incident', namespace_id: nil, base_type: base_types[:incident], icon_name: 'issue-type-incident')
+    work_item_types.find_or_create_by!(name: 'Test Case', namespace_id: nil, base_type: base_types[:test_case], icon_name: 'issue-type-test-case')
+    work_item_types.find_or_create_by!(name: 'Requirement', namespace_id: nil, base_type: base_types[:requirement], icon_name: 'issue-type-requirements')
+  end
+end

spec/models/work_item/type_spec.rb

@@ -19,10 +19,10 @@ RSpec.describe WorkItem::Type do
       it 'deletes type but not unrelated issues' do
         type = create(:work_item_type)
 
-        expect(WorkItem::Type.count).to eq(5)
+        expect(WorkItem::Type.count).to eq(6)
 
         expect { type.destroy! }.not_to change(Issue, :count)
-        expect(WorkItem::Type.count).to eq(4)
+        expect(WorkItem::Type.count).to eq(5)
       end
     end
 

spec/support/helpers/migrations_helpers/work_item_types_helper.rb

+# frozen_string_literal: true
+
+module MigrationHelpers
+  module WorkItemTypesHelper
+    DEFAULT_WORK_ITEM_TYPES = {
+      issue:       { name: 'Issue', icon_name: 'issue-type-issue', enum_value: 0 },
+      incident:    { name: 'Incident', icon_name: 'issue-type-incident', enum_value: 1 },
+      test_case:   { name: 'Test Case', icon_name: 'issue-type-test-case', enum_value: 2 },
+      requirement: { name: 'Requirement', icon_name: 'issue-type-requirements', enum_value: 3 },
+      task:        { name: 'Task', icon_name: 'issue-type-task', enum_value: 4 }
+    }.freeze
+
+    def reset_work_item_types
+      work_item_types_table.delete_all
+
+      DEFAULT_WORK_ITEM_TYPES.each do |type, attributes|
+        work_item_types_table.create!(base_type: attributes[:enum_value], **attributes.slice(:name, :icon_name))
+      end
+    end
+
+    private
+
+    def work_item_types_table
+      table(:work_item_types)
+    end
+  end
+end