Merge branch '250355-iterations-overlap-validation-bug' into 'master'

Validate Iteration dates across group tree

See merge request gitlab-org/gitlab!43234

app/models/iteration.rb

@@ -94,13 +94,25 @@ class Iteration < ApplicationRecord
 
   private
 
+  def parent_group
+    group || project.group
+  end
+
   def start_or_due_dates_changed?
     start_date_changed? || due_date_changed?
   end
 
-  # ensure dates do not overlap with other Iterations in the same group/project
+  # ensure dates do not overlap with other Iterations in the same group/project tree
   def dates_do_not_overlap
-    return unless resource_parent.iterations.where.not(id: self.id).within_timeframe(start_date, due_date).exists?
+    iterations = if parent_group.present? && resource_parent.is_a?(Project)
+                   Iteration.where(group: parent_group.self_and_ancestors).or(project.iterations)
+                 elsif parent_group.present?
+                   Iteration.where(group: parent_group.self_and_ancestors)
+                 else
+                   project.iterations
+                 end
+
+    return unless iterations.where.not(id: self.id).within_timeframe(start_date, due_date).exists?
 
     errors.add(:base, s_("Iteration|Dates cannot overlap with other existing Iterations"))
   end

changelogs/unreleased/250355-iterations-overlap-validation-bug.yml

+---
+title: Fix iteration validation not checking parent groups
+merge_request: 43234
+author:
+type: fixed

ee/spec/finders/iterations_finder_spec.rb

@@ -10,8 +10,8 @@ RSpec.describe IterationsFinder do
   let_it_be(:user) { create(:user) }
   let!(:started_group_iteration) { create(:started_iteration, :skip_future_date_validation, group: group, title: 'one test', start_date: now - 1.day, due_date: now) }
   let!(:upcoming_group_iteration) { create(:iteration, group: group, start_date: 1.day.from_now, due_date: 2.days.from_now) }
-  let!(:iteration_from_project_1) { create(:started_iteration, :skip_project_validation, project: project_1, start_date: 2.days.from_now, due_date: 3.days.from_now) }
-  let!(:iteration_from_project_2) { create(:started_iteration, :skip_project_validation, project: project_2, start_date: 4.days.from_now, due_date: 5.days.from_now) }
+  let!(:iteration_from_project_1) { create(:started_iteration, :skip_project_validation, project: project_1, start_date: 3.days.from_now, due_date: 4.days.from_now) }
+  let!(:iteration_from_project_2) { create(:started_iteration, :skip_project_validation, project: project_2, start_date: 5.days.from_now, due_date: 6.days.from_now) }
   let(:project_ids) { [project_1.id, project_2.id] }
 
   subject { described_class.new(user, params).execute }
@@ -130,8 +130,8 @@ RSpec.describe IterationsFinder do
         end
 
         it 'returns iterations which end after the timeframe' do
-          iteration = create(:iteration, :skip_project_validation, project: project_2, start_date: 6.days.from_now, due_date: 2.weeks.from_now)
-          params.merge!(start_date: 6.days.from_now, end_date: 7.days.from_now)
+          iteration = create(:iteration, :skip_project_validation, project: project_2, start_date: 9.days.from_now, due_date: 2.weeks.from_now)
+          params.merge!(start_date: 9.days.from_now, end_date: 10.days.from_now)
 
           expect(subject).to match_array([iteration])
         end

spec/models/iteration_spec.rb

@@ -119,7 +119,7 @@ RSpec.describe Iteration do
         let(:start_date) { 5.days.from_now }
         let(:due_date) { 6.days.from_now }
 
-        shared_examples_for 'overlapping dates' do
+        shared_examples_for 'overlapping dates' do |skip_constraint_test: false|
           context 'when start_date is in range' do
             let(:start_date) { 5.days.from_now }
             let(:due_date) { 3.weeks.from_now }
@@ -129,9 +129,11 @@ RSpec.describe Iteration do
               expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations')
             end
 
-            it 'is not valid even if forced' do
-              subject.validate # to generate iid/etc
-              expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/)
+            unless skip_constraint_test
+              it 'is not valid even if forced' do
+                subject.validate # to generate iid/etc
+                expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/)
+              end
             end
           end
 
@@ -144,9 +146,11 @@ RSpec.describe Iteration do
               expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations')
             end
 
-            it 'is not valid even if forced' do
-              subject.validate # to generate iid/etc
-              expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/)
+            unless skip_constraint_test
+              it 'is not valid even if forced' do
+                subject.validate # to generate iid/etc
+                expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/)
+              end
             end
           end
 
@@ -156,9 +160,11 @@ RSpec.describe Iteration do
               expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations')
             end
 
-            it 'is not valid even if forced' do
-              subject.validate # to generate iid/etc
-              expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/)
+            unless skip_constraint_test
+              it 'is not valid even if forced' do
+                subject.validate # to generate iid/etc
+                expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/)
+              end
             end
           end
         end
@@ -177,6 +183,14 @@ RSpec.describe Iteration do
               expect { subject.save! }.not_to raise_exception
             end
           end
+
+          context 'sub-group' do
+            let(:subgroup) { create(:group, parent: group) }
+
+            subject { build(:iteration, group: subgroup, start_date: start_date, due_date: due_date) }
+
+            it_behaves_like 'overlapping dates', skip_constraint_test: true
+          end
         end
 
         context 'project' do
@@ -210,6 +224,17 @@ RSpec.describe Iteration do
             end
           end
         end
+
+        context 'project in a group' do
+          let_it_be(:project) { create(:project, group: create(:group)) }
+          let_it_be(:existing_iteration) { create(:iteration, :skip_project_validation, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) }
+
+          subject { build(:iteration, :skip_project_validation, project: project, start_date: start_date, due_date: due_date) }
+
+          it_behaves_like 'overlapping dates' do
+            let(:constraint_name) { 'iteration_start_and_due_daterange_project_id_constraint' }
+          end
+        end
       end
     end