From e975e1e9dcb678ffd88d54dad86cc146f3dd8140 Mon Sep 17 00:00:00 2001 From: Dombi Attila <83396+dombesz@users.noreply.github.com> Date: Mon, 9 Dec 2024 18:44:23 +0200 Subject: [PATCH] Add life cycle step increasing dates validation to the ProjectLifeCycleStep::BaseContract --- .../project_life_cycle_steps/base_contract.rb | 36 +++++++ config/locales/en.yml | 5 +- .../base_contract_spec.rb | 98 +++++++++++++++++-- 3 files changed, 131 insertions(+), 8 deletions(-) diff --git a/app/contracts/project_life_cycle_steps/base_contract.rb b/app/contracts/project_life_cycle_steps/base_contract.rb index dfd78adcf195..bb0e43a0227f 100644 --- a/app/contracts/project_life_cycle_steps/base_contract.rb +++ b/app/contracts/project_life_cycle_steps/base_contract.rb @@ -29,11 +29,47 @@ module ProjectLifeCycleSteps class BaseContract < ::ModelContract validate :select_custom_fields_permission + validate :consecutive_steps_have_increasing_dates def select_custom_fields_permission return if user.allowed_in_project?(:edit_project_stages_and_gates, model) errors.add :base, :error_unauthorized end + + def consecutive_steps_have_increasing_dates + # Filter out steps with missing dates before proceeding with comparison + filtered_steps = model.available_life_cycle_steps.select(&:start_date) + + # Only proceed with comparisons if there are at least 2 valid steps + return if filtered_steps.size < 2 + + # Compare consecutive steps in pairs + filtered_steps.each_cons(2) do |previous_step, current_step| + if start_date_for(current_step) <= end_date_for(previous_step) + step = previous_step.is_a?(Project::Stage) ? "Stage" : "Gate" + field = current_step.is_a?(Project::Stage) ? :date_range : :date + model.errors.import( + current_step.errors.add(field, :non_continuous_dates, step:), + attribute: :"available_life_cycle_steps.#{field}" + ) + end + end + end + + private + + def start_date_for(step) + step.start_date + end + + def end_date_for(step) + case step + when Project::Gate + step.date + when Project::Stage + step.end_date || step.start_date # Use the start_date as fallback for single date stages + end + end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index b336532cfd62..4db0886655c4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1198,10 +1198,13 @@ en: end_date_not_allowed: "Cannot assign end date to a Project::Gate" type: type_and_class_name_mismatch: "must be a Project::Gate" + date: + non_continuous_dates: "can’t be earlier than the previous %{step}'s end date." project/stage: attributes: date_range: - start_date_must_be_before_end_date: "Start date must be before the end date" + start_date_must_be_before_end_date: "start date must be before the end date." + non_continuous_dates: "can’t be earlier than the previous %{step}'s end date." type: type_and_class_name_mismatch: "must be a Project::Stage" query: diff --git a/spec/contracts/project_life_cycle_steps/base_contract_spec.rb b/spec/contracts/project_life_cycle_steps/base_contract_spec.rb index 87ee661c0edc..63b69e43c336 100644 --- a/spec/contracts/project_life_cycle_steps/base_contract_spec.rb +++ b/spec/contracts/project_life_cycle_steps/base_contract_spec.rb @@ -34,27 +34,111 @@ RSpec.describe ProjectLifeCycleSteps::BaseContract do include_context "ModelContract shared context" - let(:contract) { described_class.new(project_life_cycle_step, user) } - let(:user) { build_stubbed(:admin) } - let(:project_life_cycle_step) { build_stubbed(:project_gate) } + let(:contract) { described_class.new(project, user) } + let(:project) { build_stubbed(:project) } context "with authorised user" do let(:user) { build_stubbed(:user) } + let(:project) { build_stubbed(:project, available_life_cycle_steps: steps) } + let(:steps) { [] } before do mock_permissions_for(user) do |mock| - mock.allow_in_project(:edit_project_stages_and_gates, project: project_life_cycle_step.project) + mock.allow_in_project(:edit_project_stages_and_gates, project:) end end it_behaves_like "contract is valid" + include_examples "contract reuses the model errors" + + describe "validations" do + describe "#consecutive_steps_have_increasing_dates" do + let(:step1) { build_stubbed(:project_gate, start_date: Date.new(2024, 1, 1)) } + let(:step2) { build_stubbed(:project_stage, start_date: Date.new(2024, 2, 1), end_date: Date.new(2024, 2, 28)) } + let(:step3) { build_stubbed(:project_gate, start_date: Date.new(2024, 3, 1), end_date: Date.new(2024, 3, 15)) } + let(:steps) { [step1, step2, step3] } + + context "when no steps are present" do + let(:steps) { [] } + + it_behaves_like "contract is valid" + end + + context "when only one step is present" do + let(:steps) { [step1] } + + it_behaves_like "contract is valid" + end + + context "when steps have valid and increasing dates" do + let(:steps) { [step1, step2] } + + it_behaves_like "contract is valid" + end + + context "when steps have decreasing dates" do + context "and the erroneous step is a Gate" do + let(:steps) { [step3, step1] } + + it_behaves_like "contract is invalid", + "available_life_cycle_steps.date": :non_continuous_dates + + it "adds an error to the decreasing step" do + contract.validate + expect(step1.errors.symbols_for(:date)).to include(:non_continuous_dates) + end + end + + context "and the erroneous step is a Stage" do + let(:steps) { [step3, step2] } + + it_behaves_like "contract is invalid", + "available_life_cycle_steps.date_range": :non_continuous_dates + + it "adds an error to the decreasing step" do + contract.validate + expect(step2.errors.symbols_for(:date_range)).to include(:non_continuous_dates) + end + end + end + + context "when steps with identical dates" do + let(:step4) { build_stubbed(:project_gate, start_date: Date.new(2024, 1, 1)) } + let(:steps) { [step1, step4] } + + it_behaves_like "contract is invalid", + "available_life_cycle_steps.date": :non_continuous_dates + end + + context "when a step has missing start dates" do + let(:step_missing_dates) { build_stubbed(:project_stage, start_date: nil, end_date: nil) } + + context "and the other steps have increasing dates" do + let(:steps) { [step1, step_missing_dates, step2] } + + it_behaves_like "contract is invalid", + "available_life_cycle_steps.date_range": :blank + end + + context "and the other steps have decreasing dates" do + let(:steps) { [step2, step_missing_dates, step1] } + + it_behaves_like "contract is invalid", + "available_life_cycle_steps.date": :non_continuous_dates + + it "adds an error to the decreasing step" do + contract.validate + expect(step1.errors.symbols_for(:date)).to include(:non_continuous_dates) + end + end + end + end + end end context "with unauthorised user" do let(:user) { build_stubbed(:user) } - it_behaves_like "contract is invalid", base: :error_unauthorized + it_behaves_like "contract user is unauthorized" end - - include_examples "contract reuses the model errors" end