Merge branch 'iterations-dates-validations' into 'master'

Iterations dates validations

See merge request gitlab-org/gitlab!52403

app/models/iteration.rb

@@ -13,6 +13,7 @@ class Iteration < ApplicationRecord
   }.with_indifferent_access.freeze
 
   include AtomicInternalId
+  include Timebox
 
   belongs_to :project
   belongs_to :group
@@ -23,22 +24,22 @@ class Iteration < ApplicationRecord
 
   validates :start_date, presence: true
   validates :due_date, presence: true
+  validates :iterations_cadence, presence: true, unless: -> { project_id.present? }
 
   validate :dates_do_not_overlap, if: :start_or_due_dates_changed?
   validate :future_date, if: :start_or_due_dates_changed?, unless: :skip_future_date_validation
   validate :no_project, unless: :skip_project_validation
   validate :validate_group
 
-  before_create :set_iterations_cadence
+  before_validation :set_iterations_cadence, unless: -> { project_id.present? }
+  before_create :set_past_iteration_state
 
   scope :upcoming, -> { with_state(:upcoming) }
   scope :started, -> { with_state(:started) }
   scope :closed, -> { with_state(:closed) }
 
   scope :within_timeframe, -> (start_date, end_date) do
-    where('start_date IS NOT NULL OR due_date IS NOT NULL')
-      .where('start_date IS NULL OR start_date <= ?', end_date)
-      .where('due_date IS NULL OR due_date >= ?', start_date)
+    where('start_date <= ?', end_date).where('due_date >= ?', start_date)
   end
 
   scope :start_date_passed, -> { where('start_date <= ?', Date.current).where('due_date >= ?', Date.current) }
@@ -106,30 +107,20 @@ class Iteration < ApplicationRecord
     start_date_changed? || due_date_changed?
   end
 
-  # ensure dates do not overlap with other Iterations in the same group/project tree
+  # ensure dates do not overlap with other Iterations in the same cadence tree
   def dates_do_not_overlap
-    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?
+    return unless iterations_cadence
+    return unless iterations_cadence.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"))
+    # for now we only have a single default cadence within a group just to wrap the iterations into a set.
+    # once we introduce multiple cadences per group we need to change this message.
+    # related issue: https://gitlab.com/gitlab-org/gitlab/-/issues/299312
+    errors.add(:base, s_("Iteration|Dates cannot overlap with other existing Iterations within this group"))
   end
 
-  # ensure dates are in the future
   def future_date
-    if start_date_changed?
-      errors.add(:start_date, s_("Iteration|cannot be in the past")) if start_date < Date.current
+    if start_or_due_dates_changed?
       errors.add(:start_date, s_("Iteration|cannot be more than 500 years in the future")) if start_date > 500.years.from_now
-    end
-
-    if due_date_changed?
-      errors.add(:due_date, s_("Iteration|cannot be in the past")) if due_date < Date.current
       errors.add(:due_date, s_("Iteration|cannot be more than 500 years in the future")) if due_date > 500.years.from_now
     end
   end
@@ -140,6 +131,12 @@ class Iteration < ApplicationRecord
     errors.add(:project_id, s_("is not allowed. We do not currently support project-level iterations"))
   end
 
+  def set_past_iteration_state
+    # if we create an iteration in the past, we set the state to closed right away,
+    # no need to wait for IterationsUpdateStatusWorker to do so.
+    self.state = :closed if due_date < Date.current
+  end
+
   # TODO: this method should be removed as part of https://gitlab.com/gitlab-org/gitlab/-/issues/296099
   def set_iterations_cadence
     return if iterations_cadence
@@ -147,13 +144,20 @@ class Iteration < ApplicationRecord
     # issue to clarify project iterations: https://gitlab.com/gitlab-org/gitlab/-/issues/299864
     return unless group
 
-    self.iterations_cadence = group.iterations_cadences.first || create_default_cadence
+    # we need this as we use the cadence to validate the dates overlap for this iteration,
+    # so in the case this runs before background migration we need to first set all iterations
+    # in this group to a cadence before we can validate the dates overlap.
+    default_cadence = find_or_create_default_cadence
+    group.iterations.where(iterations_cadence_id: nil).update_all(iterations_cadence_id: default_cadence.id)
+
+    self.iterations_cadence = default_cadence
   end
 
-  def create_default_cadence
+  def find_or_create_default_cadence
     cadence_title = "#{group.name} Iterations"
+    start_date = self.start_date || Date.today
 
-    Iterations::Cadence.create!(group: group, title: cadence_title, start_date: start_date)
+    ::Iterations::Cadence.create_with(title: cadence_title, start_date: start_date).safe_find_or_create_by!(group: group)
   end
 
   # TODO: remove this as part of https://gitlab.com/gitlab-org/gitlab/-/issues/296100

changelogs/unreleased/iterations-dates-validations.yml

+---
+title: Allow overlapping iteration dates with ancestor group iterations and restrict dates overlapping for iterations within same group
+merge_request: 52403
+author:
+type: changed

changelogs/unreleased/iterations-in-the-past.yml

+---
+title: Allow creation of iterations in the past
+merge_request: 52403
+author:
+type: changed

db/migrate/20210127152613_add_iterations_cadence_date_range_constraint.rb

+# frozen_string_literal: true
+
+class AddIterationsCadenceDateRangeConstraint < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      execute <<~SQL
+        ALTER TABLE sprints
+          ADD CONSTRAINT iteration_start_and_due_date_iterations_cadence_id_constraint
+          EXCLUDE USING gist
+          ( iterations_cadence_id WITH =,
+            daterange(start_date, due_date, '[]') WITH &&
+          )
+          WHERE (group_id IS NOT NULL)
+      SQL
+    end
+  end
+
+  def down
+    with_lock_retries do
+      execute <<~SQL
+        ALTER TABLE sprints
+          DROP CONSTRAINT IF EXISTS iteration_start_and_due_date_iterations_cadence_id_constraint
+      SQL
+    end
+  end
+end

db/migrate/20210127202613_remove_iteration_group_date_range_constraint.rb

+# frozen_string_literal: true
+
+class RemoveIterationGroupDateRangeConstraint < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      execute <<~SQL
+        ALTER TABLE sprints
+          DROP CONSTRAINT IF EXISTS iteration_start_and_due_daterange_group_id_constraint
+      SQL
+    end
+  end
+
+  def down
+    with_lock_retries do
+      execute <<~SQL
+        ALTER TABLE sprints
+          ADD CONSTRAINT iteration_start_and_due_daterange_group_id_constraint
+          EXCLUDE USING gist
+          ( group_id WITH =,
+            daterange(start_date, due_date, '[]') WITH &&
+          )
+          WHERE (group_id IS NOT NULL)
+      SQL
+    end
+  end
+end

db/migrate/20210218144056_add_sprints_start_date_not_null_check_constraint.rb

+# frozen_string_literal: true
+
+class AddSprintsStartDateNotNullCheckConstraint < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_not_null_constraint(:sprints, :start_date, validate: false)
+  end
+
+  def down
+    remove_not_null_constraint(:sprints, :start_date)
+  end
+end

db/migrate/20210218144656_add_sprints_due_date_not_null_check_constraint.rb

+# frozen_string_literal: true
+
+class AddSprintsDueDateNotNullCheckConstraint < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_not_null_constraint(:sprints, :due_date, validate: false)
+  end
+
+  def down
+    remove_not_null_constraint(:sprints, :due_date)
+  end
+end

db/schema_migrations/20210127152613

+32f636ffad4d2c6a002129d6e3eaeaf5d8f420dcc1273665129dc4d23f2e0dbe
\ No newline at end of file

db/schema_migrations/20210127202613

+951f46f88c1b07505e0b560e802a8bd701db7d379342d97b0bff3ad90e81fb02
\ No newline at end of file

db/schema_migrations/20210218144056

+545747e86481c74832a6df55764ab97ecfefc4446df9cc2366a8ce9d9c400ea4
\ No newline at end of file

db/schema_migrations/20210218144656

+91969bfc791cd7bc78b940aa6fed345b13a3186db0b89828428b798aa4f7949e
\ No newline at end of file

db/structure.sql

@@ -19889,9 +19889,15 @@ ALTER TABLE ONLY chat_teams
 ALTER TABLE vulnerability_scanners
     ADD CONSTRAINT check_37608c9db5 CHECK ((char_length(vendor) <= 255)) NOT VALID;
 
+ALTER TABLE sprints
+    ADD CONSTRAINT check_ccd8a1eae0 CHECK ((start_date IS NOT NULL)) NOT VALID;
+
 ALTER TABLE group_import_states
     ADD CONSTRAINT check_cda75c7c3f CHECK ((user_id IS NOT NULL)) NOT VALID;
 
+ALTER TABLE sprints
+    ADD CONSTRAINT check_df3816aed7 CHECK ((due_date IS NOT NULL)) NOT VALID;
+
 ALTER TABLE ONLY ci_build_needs
     ADD CONSTRAINT ci_build_needs_pkey PRIMARY KEY (id);
 
@@ -20400,7 +20406,7 @@ ALTER TABLE ONLY issues_self_managed_prometheus_alert_events
     ADD CONSTRAINT issues_self_managed_prometheus_alert_events_pkey PRIMARY KEY (issue_id, self_managed_prometheus_alert_event_id);
 
 ALTER TABLE ONLY sprints
-    ADD CONSTRAINT iteration_start_and_due_daterange_group_id_constraint EXCLUDE USING gist (group_id WITH =, daterange(start_date, due_date, '[]'::text) WITH &&) WHERE ((group_id IS NOT NULL));
+    ADD CONSTRAINT iteration_start_and_due_date_iterations_cadence_id_constraint EXCLUDE USING gist (iterations_cadence_id WITH =, daterange(start_date, due_date, '[]'::text) WITH &&) WHERE ((group_id IS NOT NULL));
 
 ALTER TABLE ONLY sprints
     ADD CONSTRAINT iteration_start_and_due_daterange_project_id_constraint EXCLUDE USING gist (project_id WITH =, daterange(start_date, due_date, '[]'::text) WITH &&) WHERE ((project_id IS NOT NULL));

ee/app/models/ee/iteration.rb

@@ -14,8 +14,6 @@ module EE
     end
 
     prepended do
-      include Timebox
-
       has_many :issues, foreign_key: 'sprint_id'
       has_many :merge_requests, foreign_key: 'sprint_id'
     end

ee/spec/features/groups/iterations/iterations_list_spec.rb

@@ -3,14 +3,15 @@
 require 'spec_helper'
 
 RSpec.describe 'Iterations list', :js do
-  let(:now) { Time.now }
+  let_it_be(:now) { Time.now }
   let_it_be(:group) { create(:group) }
   let_it_be(:subgroup) { create(:group, parent: group) }
   let_it_be(:user) { create(:user) }
-  let!(:started_iteration) { create(:iteration, :skip_future_date_validation, group: group, start_date: now - 1.day, due_date: now, title: 'Started iteration') }
-  let!(:upcoming_iteration) { create(:iteration, group: group, start_date: now + 1.day, due_date: now + 2.days) }
-  let!(:closed_iteration) { create(:closed_iteration, :skip_future_date_validation, group: group, start_date: now - 3.days, due_date: now - 2.days) }
-  let!(:subgroup_iteration) { create(:iteration, :skip_future_date_validation, group: subgroup, start_date: now - 5.days, due_date: now - 4.days) }
+  let_it_be(:started_iteration) { create(:iteration, :skip_future_date_validation, group: group, start_date: now - 1.day, due_date: now, title: 'Started iteration') }
+  let_it_be(:upcoming_iteration) { create(:iteration, group: group, start_date: now + 1.day, due_date: now + 2.days) }
+  let_it_be(:closed_iteration) { create(:closed_iteration, :skip_future_date_validation, group: group, start_date: now - 3.days, due_date: now - 2.days) }
+  let_it_be(:subgroup_iteration) { create(:iteration, :skip_future_date_validation, group: subgroup, start_date: now - 3.days, due_date: now + 4.days) }
+  let_it_be(:subgroup_closed_iteration) { create(:iteration, :skip_future_date_validation, group: subgroup, start_date: now - 5.days, due_date: now - 4.days) }
 
   context 'as guest' do
     context 'when in group' do
@@ -28,6 +29,7 @@ RSpec.describe 'Iterations list', :js do
           expect(page).to have_link(upcoming_iteration.title)
           expect(page).not_to have_link(closed_iteration.title)
           expect(page).not_to have_link(subgroup_iteration.title)
+          expect(page).not_to have_link(subgroup_closed_iteration.title)
         end
 
         click_link('Closed')
@@ -37,6 +39,7 @@ RSpec.describe 'Iterations list', :js do
           expect(page).not_to have_link(started_iteration.title)
           expect(page).not_to have_link(upcoming_iteration.title)
           expect(page).not_to have_link(subgroup_iteration.title)
+          expect(page).not_to have_link(subgroup_closed_iteration.title)
         end
 
         click_link('All')
@@ -46,6 +49,7 @@ RSpec.describe 'Iterations list', :js do
           expect(page).to have_link(upcoming_iteration.title)
           expect(page).to have_link(closed_iteration.title)
           expect(page).not_to have_link(subgroup_iteration.title)
+          expect(page).not_to have_link(subgroup_closed_iteration.title)
         end
       end
 
@@ -71,12 +75,14 @@ RSpec.describe 'Iterations list', :js do
           expect(page).to have_link(upcoming_iteration.title)
           expect(page).not_to have_link(closed_iteration.title)
           expect(page).to have_link(subgroup_iteration.title)
+          expect(page).not_to have_link(subgroup_closed_iteration.title)
         end
 
         click_link('Closed')
 
         aggregate_failures do
           expect(page).to have_link(closed_iteration.title)
+          expect(page).to have_link(subgroup_closed_iteration.title)
           expect(page).not_to have_link(started_iteration.title)
           expect(page).not_to have_link(upcoming_iteration.title)
           expect(page).not_to have_link(subgroup_iteration.title)
@@ -89,6 +95,7 @@ RSpec.describe 'Iterations list', :js do
           expect(page).to have_link(upcoming_iteration.title)
           expect(page).to have_link(closed_iteration.title)
           expect(page).to have_link(subgroup_iteration.title)
+          expect(page).to have_link(subgroup_closed_iteration.title)
         end
       end
     end

ee/spec/requests/api/graphql/mutations/iterations/update_spec.rb

@@ -84,15 +84,33 @@ RSpec.describe 'Updating an Iteration' do
         expect(iteration.due_date).to eq(end_date.to_date)
       end
 
-      context 'when there are ActiveRecord validation errors' do
-        let(:attributes) { { start_date: 1.month.ago.strftime('%F') } }
+      context 'when updating dates' do
+        let_it_be(:start_date) { 1.month.ago }
+        let_it_be(:end_date) { 1.month.ago + 1.day }
+        let_it_be(:attributes) { { start_date: start_date.strftime('%F') } }
 
-        it_behaves_like 'a mutation that returns errors in the response',
-                        errors: ["Start date cannot be in the past"]
+        it 'updates the iteration with date in the past', :aggregate_failures do
+          post_graphql_mutation(mutation, current_user: current_user)
 
-        it 'does not update the iteration' do
+          # Let's check that the mutation response is good
+          iteration_hash = mutation_response['iteration']
+          expect(iteration_hash['startDate'].to_date).to eq(start_date.to_date)
+
+          # Let's also check that the object was updated properly
+          iteration.reload
+          expect(iteration.start_date).to eq(start_date.to_date)
+        end
+
+        it 'does not update the iteration title' do
           expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(iteration, :title)
         end
+
+        context 'when another iteration with given dates overlap' do
+          let_it_be(:another_iteration) { create(:iteration, group: group, start_date: start_date.strftime('%F'), due_date: end_date.strftime('%F') ) }
+
+          it_behaves_like 'a mutation that returns errors in the response',
+                          errors: ["Dates cannot overlap with other existing Iterations within this group"]
+        end
       end
 
       context 'when given a raw model id (backward compatibility)' do

ee/spec/workers/iterations_update_status_worker_spec.rb

@@ -13,11 +13,12 @@ RSpec.describe IterationsUpdateStatusWorker do
       let_it_be(:upcoming_iteration) { create(:iteration, start_date: 11.days.from_now, due_date: 13.days.from_now) }
 
       it 'updates the status of iterations that require it', :aggregate_failures do
-        expect(closed_iteration1.state).to eq('upcoming')
-        expect(closed_iteration2.state).to eq('upcoming')
+        expect(closed_iteration1.state).to eq('closed')
+        expect(closed_iteration2.state).to eq('closed')
         expect(started_iteration.state).to eq('upcoming')
         expect(upcoming_iteration.state).to eq('upcoming')
 
+        closed_iteration2.update!(state: 'upcoming')
         worker.perform
 
         expect(closed_iteration1.reload.state).to eq('closed')
@@ -28,13 +29,14 @@ RSpec.describe IterationsUpdateStatusWorker do
     end
 
     context 'when iterations are in `started` state' do
-      let_it_be(:iteration1) { create(:iteration, :skip_future_date_validation, start_date: 10.days.ago, due_date: 2.days.ago, state_enum: ::Iteration::STATE_ENUM_MAP[:started]) }
+      let_it_be(:iteration1) { create(:iteration, :skip_future_date_validation, start_date: 10.days.ago, due_date: 2.days.ago) }
       let_it_be(:iteration2) { create(:iteration, :skip_future_date_validation, start_date: 1.day.ago, due_date: Date.today, state_enum: ::Iteration::STATE_ENUM_MAP[:started]) }
 
       it 'updates from started to closed when due date is past, does not touch others', :aggregate_failures do
-        expect(iteration1.state).to eq('started')
+        expect(iteration1.state).to eq('closed')
         expect(iteration2.state).to eq('started')
 
+        iteration1.update!(state: 'started')
         worker.perform
 
         expect(iteration1.reload.state).to eq('closed')

locale/gitlab.pot

@@ -16849,10 +16849,7 @@ msgstr ""
 msgid "Iterations"
 msgstr ""
 
-msgid "Iteration|Dates cannot overlap with other existing Iterations"
-msgstr ""
-
-msgid "Iteration|cannot be in the past"
+msgid "Iteration|Dates cannot overlap with other existing Iterations within this group"
 msgstr ""
 
 msgid "Iteration|cannot be more than 500 years in the future"

spec/lib/gitlab/background_migration/set_default_iteration_cadences_spec.rb

@@ -58,12 +58,10 @@ RSpec.describe Gitlab::BackgroundMigration::SetDefaultIterationCadences, schema:
     context 'when an iteration cadence exists for a group' do
       let!(:group) { namespaces.create!(name: 'group', path: 'group') }
 
-      let!(:iterations_cadence_1) { iterations_cadences.create!(group_id: group.id, start_date: 5.days.ago, title: 'Cadence 1') }
-      let!(:iterations_cadence_2) { iterations_cadences.create!(group_id: group.id, start_date: 2.days.ago, title: 'Cadence 2') }
+      let!(:iterations_cadence_1) { iterations_cadences.create!(group_id: group.id, start_date: 2.days.ago, title: 'Cadence 1') }
 
       let!(:iteration_1) { iterations.create!(group_id: group.id, iid: 1, title: 'Iteration 1', start_date: 10.days.ago, due_date: 8.days.ago) }
       let!(:iteration_2) { iterations.create!(group_id: group.id, iterations_cadence_id: iterations_cadence_1.id, iid: 2, title: 'Iteration 2', start_date: 5.days.ago, due_date: 3.days.ago) }
-      let!(:iteration_3) { iterations.create!(group_id: group.id, iterations_cadence_id: iterations_cadence_2.id, iid: 3, title: 'Iteration 3', start_date: 2.days.ago, due_date: 1.day.ago) }
 
       subject { described_class.new.perform(group.id) }
 
@@ -76,7 +74,6 @@ RSpec.describe Gitlab::BackgroundMigration::SetDefaultIterationCadences, schema:
 
         expect(iteration_1.reload.iterations_cadence_id).to eq(iterations_cadence_1.id)
         expect(iteration_2.reload.iterations_cadence_id).to eq(iterations_cadence_1.id)
-        expect(iteration_3.reload.iterations_cadence_id).to eq(iterations_cadence_2.id)
       end
     end
   end

spec/migrations/reschedule_set_default_iteration_cadences_spec.rb

@@ -16,13 +16,13 @@ RSpec.describe RescheduleSetDefaultIterationCadences do
   let(:group_7) { namespaces.create!(name: 'test_7', path: 'test_7') }
   let(:group_8) { namespaces.create!(name: 'test_8', path: 'test_8') }
 
-  let!(:iteration_1) { iterations.create!(iid: 1, title: 'iteration 1', group_id: group_1.id) }
-  let!(:iteration_2) { iterations.create!(iid: 1, title: 'iteration 2', group_id: group_3.id) }
-  let!(:iteration_3) { iterations.create!(iid: 1, title: 'iteration 2', group_id: group_4.id) }
-  let!(:iteration_4) { iterations.create!(iid: 1, title: 'iteration 2', group_id: group_5.id) }
-  let!(:iteration_5) { iterations.create!(iid: 1, title: 'iteration 2', group_id: group_6.id) }
-  let!(:iteration_6) { iterations.create!(iid: 1, title: 'iteration 2', group_id: group_7.id) }
-  let!(:iteration_7) { iterations.create!(iid: 1, title: 'iteration 2', group_id: group_8.id) }
+  let!(:iteration_1) { iterations.create!(iid: 1, title: 'iteration 1', group_id: group_1.id, start_date: 2.days.from_now, due_date: 3.days.from_now) }
+  let!(:iteration_2) { iterations.create!(iid: 1, title: 'iteration 2', group_id: group_3.id, start_date: 2.days.from_now, due_date: 3.days.from_now) }
+  let!(:iteration_3) { iterations.create!(iid: 1, title: 'iteration 2', group_id: group_4.id, start_date: 2.days.from_now, due_date: 3.days.from_now) }
+  let!(:iteration_4) { iterations.create!(iid: 1, title: 'iteration 2', group_id: group_5.id, start_date: 2.days.from_now, due_date: 3.days.from_now) }
+  let!(:iteration_5) { iterations.create!(iid: 1, title: 'iteration 2', group_id: group_6.id, start_date: 2.days.from_now, due_date: 3.days.from_now) }
+  let!(:iteration_6) { iterations.create!(iid: 1, title: 'iteration 2', group_id: group_7.id, start_date: 2.days.from_now, due_date: 3.days.from_now) }
+  let!(:iteration_7) { iterations.create!(iid: 1, title: 'iteration 2', group_id: group_8.id, start_date: 2.days.from_now, due_date: 3.days.from_now) }
 
   around do |example|
     freeze_time { Sidekiq::Testing.fake! { example.run } }

spec/models/iteration_spec.rb

@@ -3,10 +3,11 @@
 require 'spec_helper'
 
 RSpec.describe Iteration do
-  let_it_be(:project) { create(:project) }
-  let_it_be(:group) { create(:group) }
   let(:set_cadence) { nil }
 
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, group: group) }
+
   describe 'associations' do
     it { is_expected.to belong_to(:project) }
     it { is_expected.to belong_to(:group) }
@@ -67,7 +68,7 @@ RSpec.describe Iteration do
         expect { iteration.save! }.to change { Iterations::Cadence.count }.by(1)
       end
 
-      it 'sets the newly created iterations_cadence to the reecord' do
+      it 'sets the newly created iterations_cadence to the record' do
         iteration.save!
 
         expect(iteration.iterations_cadence).to eq(Iterations::Cadence.last)
@@ -148,7 +149,7 @@ RSpec.describe Iteration do
   context 'Validations' do
     subject { build(:iteration, group: group, start_date: start_date, due_date: due_date) }
 
-    describe '#not_belonging_to_project' do
+    describe 'when iteration belongs to project' do
       subject { build(:iteration, project: project, start_date: Time.current, due_date: 1.day.from_now) }
 
       it 'is invalid' do
@@ -180,13 +181,13 @@ RSpec.describe Iteration do
         let(:due_date) { 6.days.from_now }
 
         shared_examples_for 'overlapping dates' do |skip_constraint_test: false|
-          context 'when start_date is in range' do
+          context 'when start_date overlaps' do
             let(:start_date) { 5.days.from_now }
             let(:due_date) { 3.weeks.from_now }
 
             it 'is not valid' do
               expect(subject).not_to be_valid
-              expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations')
+              expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations within this group')
             end
 
             unless skip_constraint_test
@@ -197,13 +198,13 @@ RSpec.describe Iteration do
             end
           end
 
-          context 'when end_date is in range' do
+          context 'when due_date overlaps' do
             let(:start_date) { Time.current }
             let(:due_date) { 6.days.from_now }
 
             it 'is not valid' do
               expect(subject).not_to be_valid
-              expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations')
+              expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations within this group')
             end
 
             unless skip_constraint_test
@@ -217,7 +218,7 @@ RSpec.describe Iteration do
           context 'when both overlap' do
             it 'is not valid' do
               expect(subject).not_to be_valid
-              expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations')
+              expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations within this group')
             end
 
             unless skip_constraint_test
@@ -231,7 +232,7 @@ RSpec.describe Iteration do
 
         context 'group' do
           it_behaves_like 'overlapping dates' do
-            let(:constraint_name) { 'iteration_start_and_due_daterange_group_id_constraint' }
+            let(:constraint_name) { 'iteration_start_and_due_date_iterations_cadence_id_constraint' }
           end
 
           context 'different group' do
@@ -249,11 +250,12 @@ RSpec.describe Iteration do
 
             subject { build(:iteration, group: subgroup, start_date: start_date, due_date: due_date) }
 
-            it_behaves_like 'overlapping dates', skip_constraint_test: true
+            it { is_expected.to be_valid }
           end
         end
 
-        context 'project' do
+        # Skipped. Pending https://gitlab.com/gitlab-org/gitlab/-/issues/299864
+        xcontext 'project' do
           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) }
@@ -283,16 +285,16 @@ RSpec.describe Iteration do
               expect { subject.save! }.not_to raise_exception
             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) }
+          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) }
+            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' }
+            it_behaves_like 'overlapping dates' do
+              let(:constraint_name) { 'iteration_start_and_due_daterange_project_id_constraint' }
+            end
           end
         end
       end
@@ -310,19 +312,23 @@ RSpec.describe Iteration do
         let(:start_date) { 1.week.ago }
         let(:due_date) { 1.week.from_now }
 
-        it 'is not valid' do
-          expect(subject).not_to be_valid
-          expect(subject.errors[:start_date]).to include('cannot be in the past')
-        end
+        it { is_expected.to be_valid }
       end
 
       context 'when due_date is in the past' do
+        let(:start_date) { 2.weeks.ago }
+        let(:due_date) { 1.week.ago }
+
+        it { is_expected.to be_valid }
+      end
+
+      context 'when due_date is before start date' do
         let(:start_date) { Time.current }
         let(:due_date) { 1.week.ago }
 
         it 'is not valid' do
           expect(subject).not_to be_valid
-          expect(subject.errors[:due_date]).to include('cannot be in the past')
+          expect(subject.errors[:due_date]).to include('must be greater than start date')
         end
       end
 

spec/support/shared_examples/graphql/mutation_shared_examples.rb

@@ -48,6 +48,6 @@ RSpec.shared_examples 'a mutation that returns errors in the response' do |error
   it do
     post_graphql_mutation(mutation, current_user: current_user)
 
-    expect(mutation_response['errors']).to eq(errors)
+    expect(mutation_response['errors']).to match_array(errors)
   end
 end

spec/support/shared_examples/models/concerns/timebox_shared_examples.rb

@@ -18,7 +18,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
     context 'with a project' do
       it_behaves_like 'AtomicInternalId' do
         let(:internal_id_attribute) { :iid }
-        let(:instance) { build(timebox_type, *timebox_args, project: build(:project), group: nil) }
+        let(:instance) { build(timebox_type, *timebox_args, project: create(:project), group: nil) }
         let(:scope) { :project }
         let(:scope_attrs) { { project: instance.project } }
         let(:usage) { timebox_table_name }
@@ -28,7 +28,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
     context 'with a group' do
       it_behaves_like 'AtomicInternalId' do
         let(:internal_id_attribute) { :iid }
-        let(:instance) { build(timebox_type, *timebox_args, project: nil, group: build(:group)) }
+        let(:instance) { build(timebox_type, *timebox_args, project: nil, group: create(:group)) }
         let(:scope) { :group }
         let(:scope_attrs) { { namespace: instance.group } }
         let(:usage) { timebox_table_name }