Merge branch 'disallow-project-level-iterations-for-now' into 'master'

Disallow project-level iteration creation

See merge request gitlab-org/gitlab!38637

app/models/iteration.rb

@@ -4,6 +4,7 @@ class Iteration < ApplicationRecord
   self.table_name = 'sprints'
 
   attr_accessor :skip_future_date_validation
+  attr_accessor :skip_project_validation
 
   STATE_ENUM_MAP = {
       upcoming: 1,
@@ -24,6 +25,7 @@ class Iteration < ApplicationRecord
 
   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
 
   scope :upcoming, -> { with_state(:upcoming) }
   scope :started, -> { with_state(:started) }
@@ -113,6 +115,12 @@ class Iteration < ApplicationRecord
       errors.add(:due_date, s_("Iteration|cannot be more than 500 years in the future")) if due_date > 500.years.from_now
     end
   end
+
+  def no_project
+    return unless project_id.present?
+
+    errors.add(:project_id, s_("is not allowed. We do not currently support project-level iterations"))
+  end
 end
 
 Iteration.prepend_if_ee('EE::Iteration')

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, project: project_1, start_date: 2.days.from_now, due_date: 3.days.from_now) }
-  let!(:iteration_from_project_2) { create(:started_iteration, 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: 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(:project_ids) { [project_1.id, project_2.id] }
 
   subject { described_class.new(user, params).execute }
@@ -123,14 +123,14 @@ RSpec.describe IterationsFinder do
         end
 
         it 'returns iterations which start before the timeframe' do
-          iteration = create(:iteration, :skip_future_date_validation, project: project_2, start_date: now - 5.days, due_date: now - 3.days)
+          iteration = create(:iteration, :skip_project_validation, :skip_future_date_validation, project: project_2, start_date: now - 5.days, due_date: now - 3.days)
           params.merge!(start_date: now - 3.days, end_date: now - 2.days)
 
           expect(subject).to match_array([iteration])
         end
 
         it 'returns iterations which end after the timeframe' do
-          iteration = create(:iteration, project: project_2, start_date: 6.days.from_now, due_date: 2.weeks.from_now)
+          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)
 
           expect(subject).to match_array([iteration])

ee/spec/graphql/mutations/issues/set_iteration_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe Mutations::Issues::SetIteration do
   subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
 
   describe '#resolve' do
-    let(:iteration) { create(:iteration, project: issue.project) }
+    let(:iteration) { create(:iteration, :skip_project_validation, project: issue.project) }
     let(:mutated_issue) { subject[:issue] }
 
     subject { mutation.resolve(project_path: issue.project.full_path, iid: issue.iid, iteration: iteration) }
@@ -40,7 +40,7 @@ RSpec.describe Mutations::Issues::SetIteration do
         let(:iteration) { nil }
 
         it 'removes the iteration' do
-          issue.update!(iteration: create(:iteration, project: issue.project))
+          issue.update!(iteration: create(:iteration, :skip_project_validation, project: issue.project))
 
           expect(mutated_issue.iteration).to eq(nil)
         end

ee/spec/lib/banzai/reference_parser/iteration_parser_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe Banzai::ReferenceParser::IterationParser do
 
   let(:project) { create(:project, :public) }
   let(:user) { create(:user) }
-  let(:iteration) { create(:iteration, project: project) }
+  let(:iteration) { create(:iteration, :skip_project_validation, project: project) }
   subject { described_class.new(Banzai::RenderContext.new(project, user)) }
 
   let(:link) { empty_html_link }

ee/spec/models/ee/iteration_spec.rb

@@ -7,6 +7,7 @@ RSpec.describe Iteration do
   let_it_be(:group) { create(:group) }
 
   it_behaves_like 'a timebox', :iteration do
+    let(:timebox_args) { [:skip_project_validation] }
     let(:timebox_table_name) { described_class.table_name.to_sym }
   end
 end

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

@@ -88,6 +88,10 @@ RSpec.describe 'Creating an Iteration' do
         end
 
         it 'creates the iteration for a project' do
+          allow_next_instance_of(Iteration) do |iteration|
+            allow(iteration).to receive(:skip_project_validation).and_return(true)
+          end
+
           post_graphql_mutation(mutation, current_user: current_user)
 
           iteration_hash = mutation_response['iteration']

ee/spec/services/ee/issues/update_service_spec.rb

@@ -131,7 +131,7 @@ RSpec.describe Issues::UpdateService do
 
       context 'project iterations' do
         it 'creates a system note' do
-          project_iteration = create(:iteration, project: project)
+          project_iteration = create(:iteration, :skip_project_validation, project: project)
 
           expect do
             update_issue(iteration: project_iteration)

ee/spec/services/ee/notification_service_spec.rb

@@ -439,7 +439,7 @@ RSpec.describe EE::NotificationService, :mailer do
         let(:guest) { create(:user) }
         let(:admin) { create(:admin) }
         let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignees: [assignee]) }
-        let(:iteration) { create(:iteration, project: project, issues: [confidential_issue]) }
+        let(:iteration) { create(:iteration, :skip_project_validation, project: project, issues: [confidential_issue]) }
 
         it "emails subscribers of the issue's iteration that can read the issue" do
           project.add_developer(member)
@@ -470,7 +470,7 @@ RSpec.describe EE::NotificationService, :mailer do
       let(:mailer_method) { :changed_iteration_issue_email }
 
       context do
-        let(:new_iteration) { create(:iteration, project: project, issues: [issue]) }
+        let(:new_iteration) { create(:iteration, :skip_project_validation, project: project, issues: [issue]) }
         let!(:subscriber_to_new_iteration) { create(:user) { |u| issue.toggle_subscription(u, project) } }
 
         it_behaves_like 'altered iteration notification on issue' do
@@ -493,7 +493,7 @@ RSpec.describe EE::NotificationService, :mailer do
         let(:guest) { create(:user) }
         let(:admin) { create(:admin) }
         let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignees: [assignee]) }
-        let(:new_iteration) { create(:iteration, project: project, issues: [confidential_issue]) }
+        let(:new_iteration) { create(:iteration, :skip_project_validation, project: project, issues: [confidential_issue]) }
 
         it "emails subscribers of the issue's iteration that can read the issue" do
           project.add_developer(member)

ee/spec/services/ee/system_notes/issuables_service_spec.rb

@@ -123,7 +123,7 @@ RSpec.describe ::SystemNotes::IssuablesService do
     subject { service.change_iteration(iteration) }
 
     context 'for a project iteration' do
-      let(:iteration) { create(:iteration, project: project) }
+      let(:iteration) { create(:iteration, :skip_project_validation, project: project) }
 
       it_behaves_like 'a system note' do
         let(:action) { 'iteration' }

ee/spec/services/iterations/create_service_spec.rb

@@ -31,6 +31,10 @@ RSpec.describe Iterations::CreateService do
 
         context 'valid params' do
           it 'creates an iteration' do
+            allow_next_instance_of(Iteration) do |iteration|
+              allow(iteration).to receive(:skip_project_validation).and_return(true)
+            end
+
             expect(response.success?).to be_truthy
             expect(iteration).to be_persisted
             expect(iteration.title).to eq('v2.1.9')
@@ -45,6 +49,10 @@ RSpec.describe Iterations::CreateService do
           end
 
           it 'does not create an iteration but returns errors' do
+            allow_next_instance_of(Iteration) do |iteration|
+              allow(iteration).to receive(:skip_project_validation).and_return(true)
+            end
+
             expect(response.error?).to be_truthy
             expect(errors.messages).to match({ title: ["can't be blank"], due_date: ["can't be blank"], start_date: ["can't be blank"] })
           end

locale/gitlab.pot

@@ -28818,6 +28818,9 @@ msgstr ""
 msgid "is not allowed. Try again with a different email address, or contact your GitLab admin."
 msgstr ""
 
+msgid "is not allowed. We do not currently support project-level iterations"
+msgstr ""
+
 msgid "is not an email you own"
 msgstr ""
 

spec/factories/iterations.rb

@@ -36,6 +36,12 @@ FactoryBot.define do
       end
     end
 
+    trait(:skip_project_validation) do
+      after(:stub, :build) do |iteration|
+        iteration.skip_project_validation = true
+      end
+    end
+
     after(:build, :stub) do |iteration, evaluator|
       if evaluator.group
         iteration.group = evaluator.group
@@ -49,7 +55,7 @@ FactoryBot.define do
         id = evaluator.resource_parent.id
         evaluator.resource_parent.is_a?(Group) ? evaluator.group_id = id : evaluator.project_id = id
       else
-        iteration.project = create(:project)
+        iteration.group = create(:group)
       end
     end
 

spec/models/iteration_spec.rb

@@ -8,11 +8,11 @@ RSpec.describe Iteration do
 
   describe "#iid" do
     it "is properly scoped on project and group" do
-      iteration1 = create(:iteration, project: project)
-      iteration2 = create(:iteration, project: project)
+      iteration1 = create(:iteration, :skip_project_validation, project: project)
+      iteration2 = create(:iteration, :skip_project_validation, project: project)
       iteration3 = create(:iteration, group: group)
       iteration4 = create(:iteration, group: group)
-      iteration5 = create(:iteration, project: project)
+      iteration5 = create(:iteration, :skip_project_validation, project: project)
 
       want = {
           iteration1: 1,
@@ -35,6 +35,15 @@ 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
+      subject { build(:iteration, project: project, start_date: Time.current, due_date: 1.day.from_now) }
+
+      it 'is invalid' do
+        expect(subject).not_to be_valid
+        expect(subject.errors[:project_id]).to include('is not allowed. We do not currently support project-level iterations')
+      end
+    end
+
     describe '#dates_do_not_overlap' do
       let_it_be(:existing_iteration) { create(:iteration, group: group, start_date: 4.days.from_now, due_date: 1.week.from_now) }
 
@@ -115,24 +124,12 @@ RSpec.describe Iteration do
               expect { subject.save! }.not_to raise_exception
             end
           end
-
-          context 'in a project' do
-            let(:project) { create(:project) }
-
-            subject { build(:iteration, project: project, start_date: start_date, due_date: due_date) }
-
-            it { is_expected.to be_valid }
-
-            it 'does not trigger exclusion constraints' do
-              expect { subject.save! }.not_to raise_exception
-            end
-          end
         end
 
         context 'project' do
-          let_it_be(:existing_iteration) { create(:iteration, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) }
+          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, 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' }
@@ -215,9 +212,9 @@ RSpec.describe Iteration do
 
   context 'time scopes' do
     let_it_be(:project) { create(:project, :empty_repo) }
-    let_it_be(:iteration_1) { create(:iteration, :skip_future_date_validation, project: project, start_date: 3.days.ago, due_date: 1.day.from_now) }
-    let_it_be(:iteration_2) { create(:iteration, :skip_future_date_validation, project: project, start_date: 10.days.ago, due_date: 4.days.ago) }
-    let_it_be(:iteration_3) { create(:iteration, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) }
+    let_it_be(:iteration_1) { create(:iteration, :skip_future_date_validation, :skip_project_validation, project: project, start_date: 3.days.ago, due_date: 1.day.from_now) }
+    let_it_be(:iteration_2) { create(:iteration, :skip_future_date_validation, :skip_project_validation, project: project, start_date: 10.days.ago, due_date: 4.days.ago) }
+    let_it_be(:iteration_3) { create(:iteration, :skip_project_validation, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) }
 
     describe 'start_date_passed' do
       it 'returns iterations where start_date is in the past but due_date is in the future' do
@@ -235,9 +232,9 @@ RSpec.describe Iteration do
   describe '.within_timeframe' do
     let_it_be(:now) { Time.current }
     let_it_be(:project) { create(:project, :empty_repo) }
-    let_it_be(:iteration_1) { create(:iteration, project: project, start_date: now, due_date: 1.day.from_now) }
-    let_it_be(:iteration_2) { create(:iteration, project: project, start_date: 2.days.from_now, due_date: 3.days.from_now) }
-    let_it_be(:iteration_3) { create(:iteration, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) }
+    let_it_be(:iteration_1) { create(:iteration, :skip_project_validation, project: project, start_date: now, due_date: 1.day.from_now) }
+    let_it_be(:iteration_2) { create(:iteration, :skip_project_validation, project: project, start_date: 2.days.from_now, due_date: 3.days.from_now) }
+    let_it_be(:iteration_3) { create(:iteration, :skip_project_validation, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) }
 
     it 'returns iterations with start_date and/or end_date between timeframe' do
       iterations = described_class.within_timeframe(2.days.from_now, 3.days.from_now)

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

@@ -3,7 +3,8 @@
 RSpec.shared_examples 'a timebox' do |timebox_type|
   let(:project) { create(:project, :public) }
   let(:group) { create(:group) }
-  let(:timebox) { create(timebox_type, project: project) }
+  let(:timebox_args) { [] }
+  let(:timebox) { create(timebox_type, *timebox_args, project: project) }
   let(:issue) { create(:issue, project: project) }
   let(:user) { create(:user) }
   let(:timebox_table_name) { timebox_type.to_s.pluralize.to_sym }
@@ -12,7 +13,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, project: build(:project), group: nil) }
+        let(:instance) { build(timebox_type, *timebox_args, project: build(:project), group: nil) }
         let(:scope) { :project }
         let(:scope_attrs) { { project: instance.project } }
         let(:usage) { timebox_table_name }
@@ -22,7 +23,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, project: nil, group: build(:group)) }
+        let(:instance) { build(timebox_type, *timebox_args, project: nil, group: build(:group)) }
         let(:scope) { :group }
         let(:scope_attrs) { { namespace: instance.group } }
         let(:usage) { timebox_table_name }
@@ -37,14 +38,14 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
 
     describe 'start_date' do
       it 'adds an error when start_date is greater then due_date' do
-        timebox = build(timebox_type, start_date: Date.tomorrow, due_date: Date.yesterday)
+        timebox = build(timebox_type, *timebox_args, start_date: Date.tomorrow, due_date: Date.yesterday)
 
         expect(timebox).not_to be_valid
         expect(timebox.errors[:due_date]).to include("must be greater than start date")
       end
 
       it 'adds an error when start_date is greater than 9999-12-31' do
-        timebox = build(timebox_type, start_date: Date.new(10000, 1, 1))
+        timebox = build(timebox_type, *timebox_args, start_date: Date.new(10000, 1, 1))
 
         expect(timebox).not_to be_valid
         expect(timebox.errors[:start_date]).to include("date must not be after 9999-12-31")
@@ -53,7 +54,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
 
     describe 'due_date' do
       it 'adds an error when due_date is greater than 9999-12-31' do
-        timebox = build(timebox_type, due_date: Date.new(10000, 1, 1))
+        timebox = build(timebox_type, *timebox_args, due_date: Date.new(10000, 1, 1))
 
         expect(timebox).not_to be_valid
         expect(timebox.errors[:due_date]).to include("date must not be after 9999-12-31")
@@ -64,7 +65,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
       it { is_expected.to validate_presence_of(:title) }
 
       it 'is invalid if title would be empty after sanitation' do
-        timebox = build(timebox_type, project: project, title: '<img src=x onerror=prompt(1)>')
+        timebox = build(timebox_type, *timebox_args, project: project, title: '<img src=x onerror=prompt(1)>')
 
         expect(timebox).not_to be_valid
         expect(timebox.errors[:title]).to include("can't be blank")
@@ -73,7 +74,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
 
     describe '#timebox_type_check' do
       it 'is invalid if it has both project_id and group_id' do
-        timebox = build(timebox_type, group: group)
+        timebox = build(timebox_type, *timebox_args, group: group)
         timebox.project = project
 
         expect(timebox).not_to be_valid
@@ -98,7 +99,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
       end
 
       context "per group" do
-        let(:timebox) { create(timebox_type, group: group) }
+        let(:timebox) { create(timebox_type, *timebox_args, group: group) }
 
         before do
           project.update(group: group)
@@ -111,7 +112,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
         end
 
         it "does not accept the same title of a child project timebox" do
-          create(timebox_type, project: group.projects.first)
+          create(timebox_type, *timebox_args, project: group.projects.first)
 
           new_timebox = described_class.new(group: group, title: timebox.title)
 
@@ -143,7 +144,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
     end
 
     context 'when project_id is not present' do
-      let(:timebox) { build(timebox_type, group: group) }
+      let(:timebox) { build(timebox_type, *timebox_args, group: group) }
 
       it 'returns false' do
         expect(timebox.project_timebox?).to be_falsey
@@ -153,7 +154,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
 
   describe '#group_timebox?' do
     context 'when group_id is present' do
-      let(:timebox) { build(timebox_type, group: group) }
+      let(:timebox) { build(timebox_type, *timebox_args, group: group) }
 
       it 'returns true' do
         expect(timebox.group_timebox?).to be_truthy
@@ -168,7 +169,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
   end
 
   describe '#safe_title' do
-    let(:timebox) { create(timebox_type, title: "<b>foo & bar -> 2.2</b>") }
+    let(:timebox) { create(timebox_type, *timebox_args, title: "<b>foo & bar -> 2.2</b>") }
 
     it 'normalizes the title for use as a slug' do
       expect(timebox.safe_title).to eq('foo-bar-22')
@@ -177,7 +178,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
 
   describe '#resource_parent' do
     context 'when group is present' do
-      let(:timebox) { build(timebox_type, group: group) }
+      let(:timebox) { build(timebox_type, *timebox_args, group: group) }
 
       it 'returns the group' do
         expect(timebox.resource_parent).to eq(group)
@@ -192,7 +193,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
   end
 
   describe "#title" do
-    let(:timebox) { create(timebox_type, title: "<b>foo & bar -> 2.2</b>") }
+    let(:timebox) { create(timebox_type, *timebox_args, title: "<b>foo & bar -> 2.2</b>") }
 
     it "sanitizes title" do
       expect(timebox.title).to eq("foo & bar -> 2.2")
@@ -203,28 +204,28 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
     context "per project" do
       it "is true for projects with MRs enabled" do
         project = create(:project, :merge_requests_enabled)
-        timebox = create(timebox_type, project: project)
+        timebox = create(timebox_type, *timebox_args, project: project)
 
         expect(timebox.merge_requests_enabled?).to be_truthy
       end
 
       it "is false for projects with MRs disabled" do
         project = create(:project, :repository_enabled, :merge_requests_disabled)
-        timebox = create(timebox_type, project: project)
+        timebox = create(timebox_type, *timebox_args, project: project)
 
         expect(timebox.merge_requests_enabled?).to be_falsey
       end
 
       it "is false for projects with repository disabled" do
         project = create(:project, :repository_disabled)
-        timebox = create(timebox_type, project: project)
+        timebox = create(timebox_type, *timebox_args, project: project)
 
         expect(timebox.merge_requests_enabled?).to be_falsey
       end
     end
 
     context "per group" do
-      let(:timebox) { create(timebox_type, group: group) }
+      let(:timebox) { create(timebox_type, *timebox_args, group: group) }
 
       it "is always true for groups, for performance reasons" do
         expect(timebox.merge_requests_enabled?).to be_truthy
@@ -234,7 +235,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
 
   describe '#to_ability_name' do
     it 'returns timebox' do
-      timebox = build(timebox_type)
+      timebox = build(timebox_type, *timebox_args)
 
       expect(timebox.to_ability_name).to eq(timebox_type.to_s)
     end