Merge branch '342758-do-not-require-start-date-on-cadence-when-manual' into 'master'

Make Iteration Cadence Start Date Not Required When Manual

See merge request gitlab-org/gitlab!75147

db/migrate/20211123182614_make_iteration_cadences_start_date_nullable.rb

+# frozen_string_literal: true
+
+class MakeIterationCadencesStartDateNullable < Gitlab::Database::Migration[1.0]
+  def change
+    change_column_null :iterations_cadences, :start_date, true
+  end
+end

db/schema_migrations/20211123182614

+9a3ba69a1df02059b240393cc381c4a5ba9db0f116818aa9f3d4f1009f055b09
\ No newline at end of file

db/structure.sql

@@ -15439,7 +15439,7 @@ CREATE TABLE iterations_cadences (
     group_id bigint NOT NULL,
     created_at timestamp with time zone NOT NULL,
     updated_at timestamp with time zone NOT NULL,
-    start_date date NOT NULL,
+    start_date date,
     last_run_date date,
     duration_in_weeks integer,
     iterations_in_advance integer,

ee/app/models/iterations/cadence.rb

@@ -13,7 +13,7 @@ module Iterations
     has_many :iterations, foreign_key: :iterations_cadence_id, inverse_of: :iterations_cadence
 
     validates :title, presence: true
-    validates :start_date, presence: true
+    validates :start_date, presence: true, if: :automatic?
     validates :group_id, presence: true
     validates :duration_in_weeks, inclusion: { in: 0..4 }, allow_nil: true
     validates :duration_in_weeks, presence: true, if: :automatic?

ee/spec/models/ee/iterations/cadence_spec.rb

@@ -11,13 +11,26 @@ RSpec.describe ::Iterations::Cadence do
   end
 
   describe 'validations' do
-    subject { build(:iterations_cadence) }
+    let(:instance_attributes) { {} }
+
+    subject { build(:iterations_cadence, **instance_attributes) }
 
     it { is_expected.to validate_presence_of(:title) }
-    it { is_expected.to validate_presence_of(:start_date) }
     it { is_expected.to validate_presence_of(:group_id) }
     it { is_expected.not_to allow_value(nil).for(:active) }
     it { is_expected.not_to allow_value(nil).for(:automatic) }
     it { is_expected.to validate_length_of(:description).is_at_most(5000) }
+
+    context 'when iteration cadence is automatic' do
+      let(:instance_attributes) { { automatic: true } }
+
+      it { is_expected.to validate_presence_of(:start_date) }
+    end
+
+    context 'when iteration cadence is manual' do
+      let(:instance_attributes) { { automatic: false } }
+
+      it { is_expected.not_to validate_presence_of(:start_date) }
+    end
   end
 end

ee/spec/requests/api/graphql/mutations/iterations/cadences/create_spec.rb

@@ -78,6 +78,16 @@ RSpec.describe 'Creating an iteration cadence' do
         end
       end
 
+      context 'when creating a manual iteration cadence' do
+        context 'when start date is not provided' do
+          let(:attributes) { { title: 'automatic cadence', duration_in_weeks: 1, active: true, automatic: false } }
+
+          it 'creates an iteration cadence' do
+            expect { post_graphql_mutation(mutation, current_user: current_user) }.to change(Iterations::Cadence, :count).by(1)
+          end
+        end
+      end
+
       context 'when iteration_cadences feature flag is disabled' do
         before do
           stub_feature_flags(iteration_cadences: false)
@@ -87,10 +97,10 @@ RSpec.describe 'Creating an iteration cadence' do
       end
 
       context 'when there are ActiveRecord validation errors' do
-        let(:attributes) { { title: '', duration_in_weeks: 1, active: false, automatic: false } }
+        let(:attributes) { { title: '', duration_in_weeks: 1, active: false, automatic: true } }
 
         it_behaves_like 'a mutation that returns errors in the response',
-                        errors: ["Start date can't be blank", "Title can't be blank"]
+                        errors: ["Iterations in advance can't be blank", "Start date can't be blank", "Title can't be blank"]
 
         it 'does not create the iteration cadence' do
           expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(Iterations::Cadence, :count)

ee/spec/services/iterations/cadences/create_service_spec.rb

@@ -10,6 +10,13 @@ RSpec.describe Iterations::Cadences::CreateService do
     group.add_developer(user)
   end
 
+  shared_examples 'does not create an interation cadence' do |errors|
+    it 'does not create an iteration cadence and returns errors' do
+      expect(response).to be_error
+      expect(errors).to match_array(errors)
+    end
+  end
+
   context 'iterations feature enabled' do
     before do
       stub_licensed_features(iterations: true)
@@ -33,7 +40,7 @@ RSpec.describe Iterations::Cadences::CreateService do
 
       context 'valid params' do
         it 'creates an iteration cadence' do
-          expect(response.success?).to be_truthy
+          expect(response).to be_success
           expect(iteration_cadence).to be_persisted
           expect(iteration_cadence.title).to eq('My iteration cadence')
           expect(iteration_cadence.duration_in_weeks).to eq(1)
@@ -43,19 +50,20 @@ RSpec.describe Iterations::Cadences::CreateService do
         end
 
         context 'create manual cadence' do
-          context 'when duration_in_weeks: nil and iterations_in_advance: nil' do
+          context 'when duration_in_weeks: nil, start_date: nil and iterations_in_advance: nil' do
             before do
-              params.merge!(automatic: false, duration_in_weeks: nil, iterations_in_advance: nil)
+              params.merge!(automatic: false, duration_in_weeks: nil, iterations_in_advance: nil, start_date: nil)
             end
 
             it 'creates an iteration cadence' do
-              expect(response.success?).to be_truthy
+              expect(response).to be_success
               expect(iteration_cadence).to be_persisted
               expect(iteration_cadence.title).to eq('My iteration cadence')
               expect(iteration_cadence.duration_in_weeks).to be_nil
               expect(iteration_cadence.iterations_in_advance).to be_nil
               expect(iteration_cadence.active).to eq(true)
               expect(iteration_cadence.automatic).to eq(false)
+              expect(iteration_cadence.start_date).to be_nil
             end
           end
 
@@ -73,6 +81,16 @@ RSpec.describe Iterations::Cadences::CreateService do
             end
           end
         end
+
+        context 'create automatic cadence' do
+          context 'when start_date is not provided' do
+            before do
+              params.merge!(automatic: true, duration_in_weeks: 2, iterations_in_advance: 1, start_date: nil)
+            end
+
+            it_behaves_like 'does not create an interation cadence', ["Start date can't be blank"]
+          end
+        end
       end
 
       context 'invalid params' do
@@ -83,29 +101,23 @@ RSpec.describe Iterations::Cadences::CreateService do
         end
 
         context 'when duration_in_weeks: nil and iterations_in_advance: nil' do
-          it 'does not create an iteration cadence but returns errors' do
-            expect(response.error?).to be_truthy
-            expect(errors).to match([
-              "Start date can't be blank",
-              "Duration in weeks can't be blank",
-              "Iterations in advance can't be blank"
-            ])
-          end
+          it_behaves_like 'does not create an interation cadence', [
+            "Start date can't be blank",
+            "Duration in weeks can't be blank",
+            "Iterations in advance can't be blank"
+          ]
         end
 
-        context 'with out of list values for duration_in_weeks and iterations_in_advance' do
+        context 'without of list values for duration_in_weeks and iterations_in_advance' do
           before do
             params.merge!(duration_in_weeks: 15, iterations_in_advance: 15)
           end
 
-          it 'does not create an iteration cadence but returns errors' do
-            expect(response.error?).to be_truthy
-            expect(errors).to match([
-              "Start date can't be blank",
-              "Duration in weeks is not included in the list",
-              "Iterations in advance is not included in the list"
-            ])
-          end
+          it_behaves_like 'does not create an interation cadence', [
+            "Start date can't be blank",
+            "Duration in weeks is not included in the list",
+            "Iterations in advance is not included in the list"
+          ]
         end
       end