Commit 0009b596 authored by Mario de la Ossa's avatar Mario de la Ossa

Validate Iteration dates across group tree

Iterations should validate that their dates do not conflict with parent
group's iterations as well as their own group's/project's
parent 0e74f73d
...@@ -94,13 +94,25 @@ class Iteration < ApplicationRecord ...@@ -94,13 +94,25 @@ class Iteration < ApplicationRecord
private private
def parent_group
group || project.group
end
def start_or_due_dates_changed? def start_or_due_dates_changed?
start_date_changed? || due_date_changed? start_date_changed? || due_date_changed?
end 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 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")) errors.add(:base, s_("Iteration|Dates cannot overlap with other existing Iterations"))
end end
......
---
title: Fix iteration validation not checking parent groups
merge_request: 43234
author:
type: fixed
...@@ -10,8 +10,8 @@ RSpec.describe IterationsFinder do ...@@ -10,8 +10,8 @@ RSpec.describe IterationsFinder do
let_it_be(:user) { create(:user) } 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!(: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!(: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_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: 4.days.from_now, due_date: 5.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] } let(:project_ids) { [project_1.id, project_2.id] }
subject { described_class.new(user, params).execute } subject { described_class.new(user, params).execute }
...@@ -130,8 +130,8 @@ RSpec.describe IterationsFinder do ...@@ -130,8 +130,8 @@ RSpec.describe IterationsFinder do
end end
it 'returns iterations which end after the timeframe' do 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) 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: 6.days.from_now, end_date: 7.days.from_now) params.merge!(start_date: 9.days.from_now, end_date: 10.days.from_now)
expect(subject).to match_array([iteration]) expect(subject).to match_array([iteration])
end end
......
...@@ -119,7 +119,7 @@ RSpec.describe Iteration do ...@@ -119,7 +119,7 @@ RSpec.describe Iteration do
let(:start_date) { 5.days.from_now } let(:start_date) { 5.days.from_now }
let(:due_date) { 6.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 context 'when start_date is in range' do
let(:start_date) { 5.days.from_now } let(:start_date) { 5.days.from_now }
let(:due_date) { 3.weeks.from_now } let(:due_date) { 3.weeks.from_now }
...@@ -129,11 +129,13 @@ RSpec.describe Iteration do ...@@ -129,11 +129,13 @@ RSpec.describe Iteration do
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')
end end
unless skip_constraint_test
it 'is not valid even if forced' do it 'is not valid even if forced' do
subject.validate # to generate iid/etc subject.validate # to generate iid/etc
expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/)
end end
end end
end
context 'when end_date is in range' do context 'when end_date is in range' do
let(:start_date) { Time.current } let(:start_date) { Time.current }
...@@ -144,11 +146,13 @@ RSpec.describe Iteration do ...@@ -144,11 +146,13 @@ RSpec.describe Iteration do
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')
end end
unless skip_constraint_test
it 'is not valid even if forced' do it 'is not valid even if forced' do
subject.validate # to generate iid/etc subject.validate # to generate iid/etc
expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/)
end end
end end
end
context 'when both overlap' do context 'when both overlap' do
it 'is not valid' do it 'is not valid' do
...@@ -156,12 +160,14 @@ RSpec.describe Iteration do ...@@ -156,12 +160,14 @@ RSpec.describe Iteration do
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')
end end
unless skip_constraint_test
it 'is not valid even if forced' do it 'is not valid even if forced' do
subject.validate # to generate iid/etc subject.validate # to generate iid/etc
expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/)
end end
end end
end end
end
context 'group' do context 'group' do
it_behaves_like 'overlapping dates' do it_behaves_like 'overlapping dates' do
...@@ -177,6 +183,14 @@ RSpec.describe Iteration do ...@@ -177,6 +183,14 @@ RSpec.describe Iteration do
expect { subject.save! }.not_to raise_exception expect { subject.save! }.not_to raise_exception
end end
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 end
context 'project' do context 'project' do
...@@ -210,6 +224,17 @@ RSpec.describe Iteration do ...@@ -210,6 +224,17 @@ RSpec.describe Iteration do
end end
end 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
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment