Merge branch...

Merge branch 'ms/custom-status/nm-relation-between-system-defined-lifecycle-and-status' into 'master' 

Allow has many association for status and lifecycle

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/183385



Merged-by: Marc Saleiko <msaleiko@gitlab.com>
Approved-by: Agnes Slota <aslota@gitlab.com>
Approved-by: Alexandru Croitor <acroitor@gitlab.com>
Reviewed-by: Marc Saleiko <msaleiko@gitlab.com>
Reviewed-by: Agnes Slota <aslota@gitlab.com>
Co-authored-by: Marc Saleiko <marc.saleiko@komadesign.de>

ee/app/models/work_items/statuses/system_defined/lifecycle.rb

@@ -13,13 +13,21 @@ class Lifecycle
           {
             id: 1,
             name: 'Default',
-            work_item_base_types: [:issue, :task]
+            work_item_base_types: [:issue, :task],
+            status_ids: [1, 2, 3, 4, 5],
+            default_open_status_id: 1,
+            default_closed_status_id: 3,
+            default_duplicate_status_id: 5
           }
         ].freeze
 
         attribute :id, :integer
         attribute :name, :string
         attribute :work_item_base_types
+        attribute :status_ids
+        attribute :default_open_status_id, :integer
+        attribute :default_closed_status_id, :integer
+        attribute :default_duplicate_status_id, :integer
 
         class << self
           def of_work_item_base_type(base_type)
@@ -36,12 +44,24 @@ def work_item_types
         end
 
         def statuses
-          Status.where(lifecycle_id: id)
+          Status.where(id: status_ids)
         end
 
         def find_available_status_by_name(name)
           statuses.find { |status| status.matches_name?(name) }
         end
+
+        def default_open_status
+          Status.find(default_open_status_id)
+        end
+
+        def default_closed_status
+          Status.find(default_closed_status_id)
+        end
+
+        def default_duplicate_status
+          Status.find(default_duplicate_status_id)
+        end
       end
     end
   end

ee/app/models/work_items/statuses/system_defined/status.rb

@@ -15,40 +15,32 @@ class Status
             id: 1,
             name: 'To do',
             color: '#737278',
-            category: :to_do,
-            default_open: true,
-            lifecycle_id: 1
+            category: :to_do
           },
           {
             id: 2,
             name: 'In progress',
             color: '#1f75cb',
-            category: :in_progress,
-            lifecycle_id: 1
+            category: :in_progress
           },
           {
             id: 3,
             name: 'Done',
             color: '#108548',
-            category: :done,
-            default_closed: true,
-            lifecycle_id: 1
+            category: :done
           },
           {
             id: 4,
             name: "Won't do",
             color: '#DD2B0E',
-            category: :cancelled,
-            lifecycle_id: 1
+            category: :cancelled
           },
           {
             id: 5,
             name: 'Duplicate',
             color: '#DD2B0E',
             category: :cancelled,
-            default_duplicated: true,
-            position: 10,
-            lifecycle_id: 1
+            position: 10
           }
         ].freeze
 
@@ -56,11 +48,10 @@ class Status
         attribute :name, :string
         attribute :color, :string
         attribute :category
+        # For custom statuses position will be on the join model between
+        # custom lifecycle and custom status to allow modification per lifecycle.
+        # We don't plan to change the position of the status for system defined lifecycles.
         attribute :position, :integer, default: 0
-        attribute :default_open, :boolean, default: false
-        attribute :default_closed, :boolean, default: false
-        attribute :default_duplicated, :boolean, default: false
-        attribute :lifecycle_id, :integer
 
         def icon_name
           CATEGORY_ICONS[category]

ee/spec/models/work_items/statuses/system_defined/lifecycle_spec.rb

@@ -7,8 +7,14 @@
 
   describe 'validations' do
     it 'has the correct structure for each item' do
+      expected_attributes = [
+        :id, :name, :work_item_base_types, :status_ids,
+        :default_open_status_id, :default_closed_status_id,
+        :default_duplicate_status_id
+      ]
       described_class::ITEMS.each do |item|
-        expect(item).to include(:id, :name, :work_item_base_types)
+        expect(item).to include(*expected_attributes)
+        expect(item[:status_ids]).to be_an(Array)
         expect(item[:work_item_base_types]).to be_an(Array)
       end
     end
@@ -44,10 +50,9 @@
     let(:statuses) { lifecycle.statuses }
 
     it 'returns statuses for the lifecycle' do
-      # We could make this more explicit and check for concrete statuses
-      # but this would increase coupling.
       expect(statuses).to be_an(Array)
       expect(statuses.first).to be_an(WorkItems::Statuses::SystemDefined::Status)
+      expect(statuses.map(&:id)).to contain_exactly(1, 2, 3, 4, 5)
     end
   end
 
@@ -62,6 +67,20 @@
     end
   end
 
+  describe 'default status methods' do
+    {
+      default_open_status: 1,
+      default_closed_status: 3,
+      default_duplicate_status: 5
+    }.each do |method_name, expected_id|
+      it "returns assigned status for ##{method_name}" do
+        status = lifecycle.public_send(method_name)
+        expect(status).to be_an(WorkItems::Statuses::SystemDefined::Status)
+        expect(status.id).to eq(expected_id)
+      end
+    end
+  end
+
   it 'has the correct attributes' do
     is_expected.to have_attributes(
       id: 1,

ee/spec/models/work_items/statuses/system_defined/status_spec.rb

@@ -7,7 +7,7 @@
 
   describe 'validations' do
     it 'has the correct minimal structure for each item' do
-      expect(described_class::ITEMS).to all(include(:id, :name, :color, :category, :lifecycle_id))
+      expect(described_class::ITEMS).to all(include(:id, :name, :color, :category))
     end
 
     it 'has unique names for all statuses' do
@@ -46,19 +46,12 @@
       name: 'To do',
       color: '#737278',
       category: :to_do,
-      position: 0,
-      default_open: true,
-      lifecycle_id: 1
+      position: 0
     )
   end
 
-  it 'has default values for some attributes' do
-    expect(described_class.new).to have_attributes(
-      position: 0,
-      default_open: false,
-      default_closed: false,
-      default_duplicated: false
-    )
+  it 'has default value for position' do
+    expect(described_class.new.position).to eq(0)
   end
 
   describe 'included modules' do