Commit da24b007 authored by Mario Celi's avatar Mario Celi

Prevent creation or conversion into manual iteration cadences

We are deprecting manual iteration cadences.
No more manual cadences should be created nor
existing automatic should be converted into manual
parent 6d7f66dc
...@@ -221,7 +221,8 @@ module EE ...@@ -221,7 +221,8 @@ module EE
(due_date - start_date + 1).to_i (due_date - start_date + 1).to_i
end end
# TODO: this method should be removed as part of https://gitlab.com/gitlab-org/gitlab/-/issues/296099 # TODO: this method should be removed when manual iteration management is removed.
# See https://gitlab.com/gitlab-org/gitlab/-/issues/356069 the deprecation issue.
def set_iterations_cadence def set_iterations_cadence
return if iterations_cadence return if iterations_cadence
# For now we support only group iterations # For now we support only group iterations
...@@ -307,19 +308,25 @@ module EE ...@@ -307,19 +308,25 @@ module EE
iterations_cadence.update_iteration_sequences iterations_cadence.update_iteration_sequences
end end
# TODO: this method should be removed as part of https://gitlab.com/gitlab-org/gitlab/-/issues/296099 # TODO: this method should be removed when manual iteration management is removed.
# See https://gitlab.com/gitlab-org/gitlab/-/issues/356069 the deprecation issue.
def find_or_create_default_cadence def find_or_create_default_cadence
default_cadence = ::Iterations::Cadence.order(id: :asc).find_by(group: group, automatic: false)
return default_cadence if default_cadence
cadence_title = "#{group.name} Iterations" cadence_title = "#{group.name} Iterations"
start_date = self.start_date || Date.today start_date = self.start_date || Date.today
::Iterations::Cadence.create_with( # We need to skip validation as manual cadence creation is deprecated and not allowed.
# A manual cadence is created here so the iterations feature is not affected during the deprecation period.
::Iterations::Cadence.new(
group: group,
title: cadence_title, title: cadence_title,
start_date: start_date, start_date: start_date,
automatic: false, automatic: false,
# set to 0, i.e. unspecified when creating default iterations as we do validate for presence. iterations_in_advance: 2,
iterations_in_advance: 0, duration_in_weeks: 2
duration_in_weeks: 0 ).tap { |new_cadence| new_cadence.save!(validate: false) }
).order(id: :asc).safe_find_or_create_by!(group: group)
end end
# TODO: remove this as part of https://gitlab.com/gitlab-org/gitlab/-/issues/296100 # TODO: remove this as part of https://gitlab.com/gitlab-org/gitlab/-/issues/296100
......
...@@ -23,6 +23,8 @@ module Iterations ...@@ -23,6 +23,8 @@ module Iterations
validates :automatic, inclusion: [true, false] validates :automatic, inclusion: [true, false]
validates :description, length: { maximum: 5000 } validates :description, length: { maximum: 5000 }
validate :cadence_is_automatic
after_commit :ensure_iterations_in_advance, on: [:create, :update], if: :changed_iterations_automation_fields? after_commit :ensure_iterations_in_advance, on: [:create, :update], if: :changed_iterations_automation_fields?
scope :with_groups, -> (group_ids) { where(group_id: group_ids) } scope :with_groups, -> (group_ids) { where(group_id: group_ids) }
...@@ -84,5 +86,15 @@ module Iterations ...@@ -84,5 +86,15 @@ module Iterations
WHERE t.id=sprints.id AND (sprints.sequence IS DISTINCT FROM t.row_number) WHERE t.id=sprints.id AND (sprints.sequence IS DISTINCT FROM t.row_number)
SQL SQL
end end
private
def cadence_is_automatic
return unless changes.key?(:automatic)
unless automatic?
errors.add(:base, _('Manual iteration cadences are deprecated. Only automatic iteration cadences are allowed.'))
end
end
end end
end end
...@@ -6,7 +6,7 @@ RSpec.describe 'User creates iteration in a cadence', :js do ...@@ -6,7 +6,7 @@ RSpec.describe 'User creates iteration in a cadence', :js do
let_it_be(:now) { Time.zone.now } let_it_be(:now) { Time.zone.now }
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:user) { create(:group_member, :maintainer, user: create(:user), group: group ).user } let_it_be(:user) { create(:group_member, :maintainer, user: create(:user), group: group ).user }
let_it_be(:cadence) { create(:iterations_cadence, group: group, automatic: false, duration_in_weeks: 0) } let_it_be(:cadence) { build(:iterations_cadence, group: group, automatic: false, duration_in_weeks: 0).tap { |cadence| cadence.save!(validate: false) } }
before do before do
stub_licensed_features(iterations: true) stub_licensed_features(iterations: true)
......
...@@ -14,7 +14,7 @@ RSpec.describe Iterations::CadencesFinder do ...@@ -14,7 +14,7 @@ RSpec.describe Iterations::CadencesFinder do
let_it_be(:automatic_iterations_cadence) { create(:iterations_cadence, group: group, automatic: true, duration_in_weeks: 1, title: 'one week iterations') } let_it_be(:automatic_iterations_cadence) { create(:iterations_cadence, group: group, automatic: true, duration_in_weeks: 1, title: 'one week iterations') }
let_it_be(:active_sub_group_iterations_cadence) { create(:iterations_cadence, group: sub_group, active: true, duration_in_weeks: 1, title: 'one week iterations') } let_it_be(:active_sub_group_iterations_cadence) { create(:iterations_cadence, group: sub_group, active: true, duration_in_weeks: 1, title: 'one week iterations') }
let_it_be(:inactive_sub_group_iterations_cadence) { create(:iterations_cadence, group: sub_group, active: false, duration_in_weeks: 2, title: 'two weeks iterations') } let_it_be(:inactive_sub_group_iterations_cadence) { create(:iterations_cadence, group: sub_group, active: false, duration_in_weeks: 2, title: 'two weeks iterations') }
let_it_be(:non_automatic_sub_group_iterations_cadence) { create(:iterations_cadence, group: sub_group, automatic: false, duration_in_weeks: 1, title: 'one week iterations') } let_it_be(:non_automatic_sub_group_iterations_cadence) { build(:iterations_cadence, group: sub_group, automatic: false, duration_in_weeks: 1, title: 'one week iterations').tap { |cadence| cadence.save!(validate: false) } }
let_it_be(:current_group) { group } let_it_be(:current_group) { group }
subject { described_class.new(user, current_group, params).execute } subject { described_class.new(user, current_group, params).execute }
......
...@@ -32,6 +32,41 @@ RSpec.describe ::Iterations::Cadence do ...@@ -32,6 +32,41 @@ RSpec.describe ::Iterations::Cadence do
it { is_expected.not_to validate_presence_of(:start_date) } it { is_expected.not_to validate_presence_of(:start_date) }
end end
describe 'cadence_is_automatic' do
context 'when creating a new cadence' do
it 'does not allow the creation of manul cadences' do
cadence = build(:iterations_cadence, automatic: false).tap { |cadence| cadence.valid? }
expect(cadence.errors.full_messages).to include(_('Manual iteration cadences are deprecated. Only automatic iteration cadences are allowed.'))
end
end
context 'when cadence already existed as manual' do
let_it_be(:manual_cadence, refind: true) { build(:iterations_cadence).tap { |cadence| cadence.save!(validate: false) } }
context 'when `automatic` is not updated' do
it 'allows to change other attributes' do
manual_cadence.assign_attributes(duration_in_weeks: 2, iterations_in_advance: 4)
expect(manual_cadence).to be_valid
end
end
end
context 'when cadence already existed as automatic' do
let_it_be(:automatic_cadence, refind: true) { create(:iterations_cadence) }
context 'when changing a cadence to manual' do
it 'adds a validation error' do
automatic_cadence.assign_attributes(duration_in_weeks: 2, iterations_in_advance: 4, automatic: false)
expect(automatic_cadence).to be_invalid
expect(automatic_cadence.errors.full_messages).to include(_('Manual iteration cadences are deprecated. Only automatic iteration cadences are allowed.'))
end
end
end
end
end end
describe '#update_iteration_sequences', :aggregate_failures do describe '#update_iteration_sequences', :aggregate_failures do
......
...@@ -15,7 +15,7 @@ RSpec.describe 'Creating an iteration cadence' do ...@@ -15,7 +15,7 @@ RSpec.describe 'Creating an iteration cadence' do
start_date: start_date, start_date: start_date,
duration_in_weeks: 1, duration_in_weeks: 1,
iterations_in_advance: 1, iterations_in_advance: 1,
automatic: false, automatic: true,
active: false, active: false,
roll_over: true, roll_over: true,
description: 'Iteration cadence description' description: 'Iteration cadence description'
...@@ -79,13 +79,10 @@ RSpec.describe 'Creating an iteration cadence' do ...@@ -79,13 +79,10 @@ RSpec.describe 'Creating an iteration cadence' do
end end
context 'when creating a manual iteration cadence' do context 'when creating a manual iteration cadence' do
context 'when start date is not provided' do
let(:attributes) { { title: 'automatic cadence', duration_in_weeks: 1, active: true, automatic: false } } let(:attributes) { { title: 'automatic cadence', duration_in_weeks: 1, active: true, automatic: false } }
it 'creates an iteration cadence' do it_behaves_like 'a mutation that returns errors in the response',
expect { post_graphql_mutation(mutation, current_user: current_user) }.to change(Iterations::Cadence, :count).by(1) errors: [_('Manual iteration cadences are deprecated. Only automatic iteration cadences are allowed.')]
end
end
end end
context 'when iteration_cadences feature flag is disabled' do context 'when iteration_cadences feature flag is disabled' do
......
...@@ -17,7 +17,6 @@ RSpec.describe 'Updating an iteration cadence' do ...@@ -17,7 +17,6 @@ RSpec.describe 'Updating an iteration cadence' do
start_date: start_date, start_date: start_date,
duration_in_weeks: 1, duration_in_weeks: 1,
iterations_in_advance: 1, iterations_in_advance: 1,
automatic: false,
active: false, active: false,
roll_over: true, roll_over: true,
description: description description: description
...@@ -80,7 +79,7 @@ RSpec.describe 'Updating an iteration cadence' do ...@@ -80,7 +79,7 @@ RSpec.describe 'Updating an iteration cadence' do
expect(iteration_cadence_hash['startDate'].to_date).to eq(start_date.to_date) expect(iteration_cadence_hash['startDate'].to_date).to eq(start_date.to_date)
expect(iteration_cadence_hash['durationInWeeks']).to eq(1) expect(iteration_cadence_hash['durationInWeeks']).to eq(1)
expect(iteration_cadence_hash['iterationsInAdvance']).to eq(1) expect(iteration_cadence_hash['iterationsInAdvance']).to eq(1)
expect(iteration_cadence_hash['automatic']).to eq(false) expect(iteration_cadence_hash['automatic']).to eq(true)
expect(iteration_cadence_hash['active']).to eq(false) expect(iteration_cadence_hash['active']).to eq(false)
expect(iteration_cadence_hash['rollOver']).to eq(true) expect(iteration_cadence_hash['rollOver']).to eq(true)
expect(iteration_cadence_hash['description']).to eq(description) expect(iteration_cadence_hash['description']).to eq(description)
...@@ -90,7 +89,7 @@ RSpec.describe 'Updating an iteration cadence' do ...@@ -90,7 +89,7 @@ RSpec.describe 'Updating an iteration cadence' do
expect(iteration_cadence.start_date).to eq(start_date.to_date) expect(iteration_cadence.start_date).to eq(start_date.to_date)
expect(iteration_cadence.duration_in_weeks).to eq(1) expect(iteration_cadence.duration_in_weeks).to eq(1)
expect(iteration_cadence.iterations_in_advance).to eq(1) expect(iteration_cadence.iterations_in_advance).to eq(1)
expect(iteration_cadence.automatic).to eq(false) expect(iteration_cadence.automatic).to eq(true)
expect(iteration_cadence.active).to eq(false) expect(iteration_cadence.active).to eq(false)
expect(iteration_cadence.roll_over).to eq(true) expect(iteration_cadence.roll_over).to eq(true)
expect(iteration_cadence.description).to eq(description) expect(iteration_cadence.description).to eq(description)
......
...@@ -7,7 +7,7 @@ RSpec.describe 'Creating an Iteration' do ...@@ -7,7 +7,7 @@ RSpec.describe 'Creating an Iteration' do
let_it_be(:current_user) { create(:user) } let_it_be(:current_user) { create(:user) }
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:cadence) { create(:iterations_cadence, group: group)} let_it_be(:cadence) { build(:iterations_cadence, group: group, automatic: false).tap { |cadence| cadence.save!(validate: false) } }
let(:start_date) { Time.now.strftime('%F') } let(:start_date) { Time.now.strftime('%F') }
let(:end_date) { 1.day.from_now.strftime('%F') } let(:end_date) { 1.day.from_now.strftime('%F') }
......
...@@ -6,7 +6,7 @@ RSpec.describe Iterations::Cadences::CreateIterationsInAdvanceService do ...@@ -6,7 +6,7 @@ RSpec.describe Iterations::Cadences::CreateIterationsInAdvanceService do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:inactive_cadence) { create(:iterations_cadence, group: group, active: false, automatic: true, start_date: 2.weeks.ago) } let_it_be(:inactive_cadence) { create(:iterations_cadence, group: group, active: false, automatic: true, start_date: 2.weeks.ago) }
let_it_be(:manual_cadence) { create(:iterations_cadence, group: group, active: true, automatic: false, start_date: 2.weeks.ago) } let_it_be(:manual_cadence) { build(:iterations_cadence, group: group, active: true, automatic: false, start_date: 2.weeks.ago).tap { |cadence| cadence.save!(validate: false) } }
let_it_be_with_reload(:automated_cadence) { create(:iterations_cadence, group: group, active: true, automatic: true, start_date: 2.weeks.ago) } let_it_be_with_reload(:automated_cadence) { create(:iterations_cadence, group: group, active: true, automatic: true, start_date: 2.weeks.ago) }
subject { described_class.new(user, cadence).execute } subject { described_class.new(user, cadence).execute }
......
...@@ -50,36 +50,13 @@ RSpec.describe Iterations::Cadences::CreateService do ...@@ -50,36 +50,13 @@ RSpec.describe Iterations::Cadences::CreateService do
end end
context 'create manual cadence' do context 'create manual cadence' do
context 'when duration_in_weeks: nil, start_date: nil and iterations_in_advance: nil' do
before do before do
params.merge!(automatic: false, duration_in_weeks: nil, iterations_in_advance: nil, start_date: nil) params.merge!(automatic: false, duration_in_weeks: nil, iterations_in_advance: nil, start_date: nil)
end end
it 'creates an iteration cadence' do it_behaves_like 'does not create an interation cadence', [
expect(response).to be_success _('Manual iteration cadences are deprecated. Only automatic iteration cadences are allowed.')
expect(iteration_cadence).to be_persisted ]
expect(iteration_cadence.title).to eq('My iteration cadence')
expect(iteration_cadence.duration_in_weeks).to be_nil
expect(iteration_cadence.iterations_in_advance).to be_nil
expect(iteration_cadence.active).to eq(true)
expect(iteration_cadence.automatic).to eq(false)
expect(iteration_cadence.start_date).to be_nil
end
end
context 'with out list of values for duration_in_weeks, iterations_in_advance' do
before do
params.merge!(automatic: false, duration_in_weeks: 15, iterations_in_advance: 15)
end
it 'does not create an iteration cadence but returns errors' do
expect(response.error?).to be_truthy
expect(errors).to match([
"Duration in weeks is not included in the list",
"Iterations in advance is not included in the list"
])
end
end
end end
context 'create automatic cadence' do context 'create automatic cadence' do
......
...@@ -147,8 +147,9 @@ RSpec.describe Iterations::CreateService do ...@@ -147,8 +147,9 @@ RSpec.describe Iterations::CreateService do
context 'when iteration_cadences FF is disabled' do context 'when iteration_cadences FF is disabled' do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:cadences) { create_list(:iterations_cadence, 2, group: group) } let_it_be(:first_legacy_cadence) { build(:iterations_cadence, group: group, automatic: false).tap { |cadence| cadence.save!(validate: false) } }
let_it_be(:other_iteration) { create(:iteration, iterations_cadence: cadences.second) } let_it_be(:automatic_cadence) { create(:iterations_cadence, group: group) }
let_it_be(:other_iteration) { create(:iteration, iterations_cadence: automatic_cadence) }
let_it_be(:parent, refind: true) { group } let_it_be(:parent, refind: true) { group }
let(:params) { base_params } let(:params) { base_params }
...@@ -163,12 +164,12 @@ RSpec.describe Iterations::CreateService do ...@@ -163,12 +164,12 @@ RSpec.describe Iterations::CreateService do
expect(response).to be_success expect(response).to be_success
expect(saved_iteration).to be_persisted expect(saved_iteration).to be_persisted
expect(saved_iteration.title).to eq('v2.1.9') expect(saved_iteration.title).to eq('v2.1.9')
expect(saved_iteration.iterations_cadence_id).to eq(ordered_cadences.first.id) expect(saved_iteration.iterations_cadence_id).to eq(first_legacy_cadence.id)
end end
it 'does not update the iterations from the non-default cadences' do it 'does not update the iterations from the non-default cadences' do
expect(response).to be_success expect(response).to be_success
expect(other_iteration.iterations_cadence_id).to eq(ordered_cadences.second.id) expect(other_iteration.iterations_cadence_id).to eq(automatic_cadence.id)
end end
end end
end end
......
...@@ -20,7 +20,7 @@ RSpec.describe Iterations::RollOverIssuesWorker do ...@@ -20,7 +20,7 @@ RSpec.describe Iterations::RollOverIssuesWorker do
describe '#perform' do describe '#perform' do
context 'when iteration cadence is not automatic' do context 'when iteration cadence is not automatic' do
before do before do
cadence1.update!(automatic: false) cadence1.update_columns(automatic: false)
end end
it 'exits early' do it 'exits early' do
......
...@@ -22694,6 +22694,9 @@ msgstr "" ...@@ -22694,6 +22694,9 @@ msgstr ""
msgid "Manual" msgid "Manual"
msgstr "" msgstr ""
msgid "Manual iteration cadences are deprecated. Only automatic iteration cadences are allowed."
msgstr ""
msgid "ManualOrdering|Couldn't save the order of the issues" msgid "ManualOrdering|Couldn't save the order of the issues"
msgstr "" msgstr ""
......
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