Merge branch '328457-set-iteration-status-on-dates-update' into 'master'

Set iteration state based on updated dates

See merge request gitlab-org/gitlab!62558

ee/app/models/ee/iteration.rb

@@ -46,7 +46,7 @@ module EE
       validate :validate_group
 
       before_validation :set_iterations_cadence, unless: -> { project_id.present? }
-      before_create :set_past_iteration_state
+      before_save :set_iteration_state
       before_destroy :check_if_can_be_destroyed
 
       scope :upcoming, -> { with_state(:upcoming) }
@@ -197,10 +197,20 @@ module EE
       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
+    def set_iteration_state
+      self.state = compute_state
+    end
+
+    def compute_state
+      today = Date.today
+
+      if start_date > today
+        :upcoming
+      elsif due_date < today
+        :closed
+      else
+        :started
+      end
     end
 
     # TODO: this method should be removed as part of https://gitlab.com/gitlab-org/gitlab/-/issues/296099

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

@@ -8,7 +8,7 @@ RSpec.describe 'User views iteration cadences', :js do
   let_it_be(:cadence) { create(:iterations_cadence, group: group) }
   let_it_be(:other_cadence) { create(:iterations_cadence, group: group) }
   let_it_be(:iteration_in_cadence) { create(:iteration, group: group, iterations_cadence: cadence) }
-  let_it_be(:closed_iteration_in_cadence) { create(:closed_iteration, group: group, iterations_cadence: cadence) }
+  let_it_be(:closed_iteration_in_cadence) { create(:iteration, group: group, iterations_cadence: cadence, start_date: 2.weeks.ago, due_date: 1.week.ago) }
   let_it_be(:iteration_in_other_cadence) { create(:iteration, group: group, iterations_cadence: other_cadence) }
 
   before do

ee/spec/finders/iterations_finder_spec.rb

@@ -13,8 +13,8 @@ RSpec.describe IterationsFinder do
   let_it_be(:closed_iteration) { create(:closed_iteration, :skip_future_date_validation, iterations_cadence: iteration_cadence2, group: iteration_cadence2.group, start_date: 7.days.ago, due_date: 2.days.ago) }
   let_it_be(:started_group_iteration) { create(:started_iteration, :skip_future_date_validation, iterations_cadence: iteration_cadence2, group: iteration_cadence2.group, title: 'one test', start_date: 1.day.ago, due_date: Date.today) }
   let_it_be(:upcoming_group_iteration) { create(:iteration, iterations_cadence: iteration_cadence1, group: iteration_cadence1.group, start_date: 1.day.from_now, due_date: 3.days.from_now) }
-  let_it_be(:root_group_iteration) { create(:started_iteration, iterations_cadence: iteration_cadence3, group: iteration_cadence3.group, start_date: 1.day.from_now, due_date: 2.days.from_now) }
-  let_it_be(:root_closed_iteration) { create(:closed_iteration, iterations_cadence: iteration_cadence3, group: iteration_cadence3.group, start_date: 1.week.ago, due_date: 1.day.ago) }
+  let_it_be(:root_group_iteration) { create(:started_iteration, iterations_cadence: iteration_cadence3, group: iteration_cadence3.group, start_date: 1.day.ago, due_date: 2.days.from_now) }
+  let_it_be(:root_closed_iteration) { create(:closed_iteration, iterations_cadence: iteration_cadence3, group: iteration_cadence3.group, start_date: 1.week.ago, due_date: 2.days.ago) }
 
   let(:parent) { project_1 }
   let(:params) { { parent: parent, include_ancestors: true } }
@@ -132,7 +132,7 @@ RSpec.describe IterationsFinder do
         it 'returns iterations with start_date and due_date between timeframe' do
           params.merge!(start_date: 1.day.ago, end_date: 3.days.from_now)
 
-          expect(subject).to match_array([started_group_iteration, upcoming_group_iteration, root_group_iteration, root_closed_iteration])
+          expect(subject).to match_array([started_group_iteration, upcoming_group_iteration, root_group_iteration])
         end
 
         it 'returns iterations which start before the timeframe' do

ee/spec/models/ee/iteration_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Iteration do
+  include ActiveSupport::Testing::TimeHelpers
+
   let(:set_cadence) { nil }
 
   let_it_be(:group) { create(:group) }
@@ -452,6 +454,98 @@ RSpec.describe Iteration do
     end
   end
 
+  context 'sets correct state based on iteration dates' do
+    around do |example|
+      travel_to(Time.utc(2019, 12, 30)) { example.run }
+    end
+
+    let_it_be(:iterations_cadence) { create(:iterations_cadence, group: group, start_date: 10.days.ago.utc.to_date) }
+
+    let(:iteration) { build(:iteration, group: iterations_cadence.group, iterations_cadence: iterations_cadence, start_date: start_date, due_date: 2.weeks.after(start_date).to_date) }
+
+    context 'start_date is in the future' do
+      let(:start_date) { 1.day.from_now.utc.to_date }
+
+      it 'sets state to started' do
+        iteration.save!
+
+        expect(iteration.state).to eq('upcoming')
+      end
+    end
+
+    context 'start_date is today' do
+      let(:start_date) { Time.now.utc.to_date }
+
+      it 'sets state to started' do
+        iteration.save!
+
+        expect(iteration.state).to eq('started')
+      end
+    end
+
+    context 'start_date is in the past and due date is still in the future' do
+      let(:start_date) { 1.week.ago.utc.to_date }
+
+      it 'sets state to started' do
+        iteration.save!
+
+        expect(iteration.state).to eq('started')
+      end
+    end
+
+    context 'start_date is in the past and due date is also in the past' do
+      let(:start_date) { 3.weeks.ago.utc.to_date }
+
+      it 'sets state to started' do
+        iteration.save!
+
+        expect(iteration.state).to eq('closed')
+      end
+    end
+
+    context 'when dates for an existing iteration change' do
+      context 'when iteration dates go from future to past' do
+        let(:iteration) { create(:iteration, group: iterations_cadence.group, iterations_cadence: iterations_cadence, start_date: 2.weeks.from_now.utc.to_date, due_date: 3.weeks.from_now.utc.to_date)}
+
+        it 'sets state to closed' do
+          expect(iteration.state).to eq('upcoming')
+
+          iteration.start_date -= 4.weeks
+          iteration.due_date -= 4.weeks
+          iteration.save!
+
+          expect(iteration.state).to eq('closed')
+        end
+      end
+
+      context 'when iteration dates go from past to future' do
+        let(:iteration) { create(:iteration, group: iterations_cadence.group, iterations_cadence: iterations_cadence, start_date: 2.weeks.ago.utc.to_date, due_date: 1.week.ago.utc.to_date)}
+
+        it 'sets state to upcoming' do
+          expect(iteration.state).to eq('closed')
+
+          iteration.start_date += 3.weeks
+          iteration.due_date += 3.weeks
+          iteration.save!
+
+          expect(iteration.state).to eq('upcoming')
+        end
+
+        context 'and today is between iteration start and due dates' do
+          it 'sets state to started' do
+            expect(iteration.state).to eq('closed')
+
+            iteration.start_date += 2.weeks
+            iteration.due_date += 2.weeks
+            iteration.save!
+
+            expect(iteration.state).to eq('started')
+          end
+        end
+      end
+    end
+  end
+
   it_behaves_like 'a timebox', :iteration do
     let(:timebox_args) { [:skip_project_validation] }
     let(:timebox_table_name) { described_class.table_name.to_sym }

ee/spec/requests/api/iterations_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe API::Iterations do
   let_it_be(:group) { create(:group, :private, parent: parent_group) }
 
   let_it_be(:iteration) { create(:iteration, group: group, title: 'search_title') }
-  let_it_be(:closed_iteration) { create(:iteration, :closed, group: group) }
+  let_it_be(:closed_iteration) { create(:iteration, :closed, group: group, start_date: 2.weeks.ago, due_date: 1.week.ago) }
   let_it_be(:ancestor_iteration) { create(:iteration, group: parent_group) }
 
   before_all do

ee/spec/workers/iterations_update_status_worker_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe IterationsUpdateStatusWorker do
       it 'updates the status of iterations that require it', :aggregate_failures do
         expect(closed_iteration1.state).to eq('closed')
         expect(closed_iteration2.state).to eq('closed')
-        expect(started_iteration.state).to eq('upcoming')
+        expect(started_iteration.state).to eq('started')
         expect(upcoming_iteration.state).to eq('upcoming')
 
         closed_iteration2.update!(state: 'upcoming')