Skip to content

Commit

Permalink
Ensure the Project::LifeCycleStep and Definition type column is reado…
Browse files Browse the repository at this point in the history
…nly and it cannot be changed into an invalid state.

The validation added will ensure that if we initialised a Project::Stage
object, then we cannot change it's type to "Project::Gate". This could
lead to bypassing certain child model validations.
  • Loading branch information
dombesz committed Nov 22, 2024
1 parent 946f5fb commit c485739
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 5 deletions.
2 changes: 2 additions & 0 deletions app/models/project/gate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
class Project::Gate < Project::LifeCycleStep
alias_attribute :date, :start_date

# This ensures the type cannot be changed after initialising the class.
validates :type, inclusion: { in: %w[Project::Gate], message: :must_be_a_gate }
validates :date, presence: true
validate :end_date_not_allowed

Expand Down
2 changes: 1 addition & 1 deletion app/models/project/life_cycle_step.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class Project::LifeCycleStep < ApplicationRecord
class_name: "Project::LifeCycleStepDefinition"
has_many :work_packages, inverse_of: :project_life_cycle_step, dependent: :nullify

attr_readonly :definition_id
attr_readonly :definition_id, :type

validates :type, inclusion: { in: %w[Project::Stage Project::Gate], message: :must_be_a_stage_or_gate }

Expand Down
2 changes: 2 additions & 0 deletions app/models/project/life_cycle_step_definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ class Project::LifeCycleStepDefinition < ApplicationRecord
validates :name, presence: true
validates :type, inclusion: { in: %w[Project::StageDefinition Project::GateDefinition], message: :must_be_a_stage_or_gate }

attr_readonly :type

acts_as_list

def initialize(*args)
Expand Down
2 changes: 2 additions & 0 deletions app/models/project/stage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,7 @@
#++

class Project::Stage < Project::LifeCycleStep
# This ensures the type cannot be changed after initialising the class.
validates :type, inclusion: { in: %w[Project::Stage], message: :must_be_a_stage }
validates :start_date, :end_date, presence: true
end
2 changes: 2 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1118,6 +1118,8 @@ en:
attributes:
type:
must_be_a_stage_or_gate: "must be either Project::Stage or Project::Gate"
must_be_a_stage: "must be a Project::Stage"
must_be_a_gate: "must be a Project::Gate"
project/gate:
attributes:
base:
Expand Down
9 changes: 5 additions & 4 deletions spec/models/project/gate_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,18 @@

describe "validations" do
it { is_expected.to validate_presence_of(:date) }
it { is_expected.to validate_inclusion_of(:type).in_array(["Project::Gate"]).with_message(:must_be_a_gate) }

it "is invalid if `end_date` is present" do
gate_with_end_date = build(:project_gate, end_date: Time.zone.today)
subject.end_date = Time.zone.today

expect(gate_with_end_date).not_to be_valid
expect(gate_with_end_date.errors[:base])
expect(subject).not_to be_valid
expect(subject.errors[:base])
.to include("Cannot assign `end_date` to a Project::Gate")
end

it "is valid if `end_date` is not present" do
valid_gate = build(:project_gate)
valid_gate = build(:project_gate, end_date: nil)
expect(valid_gate).to be_valid
end
end
Expand Down
6 changes: 6 additions & 0 deletions spec/models/project/life_cycle_step_definition_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@
expect { described_class.new }.to raise_error(NotImplementedError)
end

context "with a Project::StageDefinition" do
subject { create :project_stage_definition }

it { is_expected.to have_readonly_attribute(:type) }
end

# For more specs see:
# - spec/support/shared/project_life_cycle_helpers.rb
# - spec/models/project/gate_definition_spec.rb
Expand Down
1 change: 1 addition & 0 deletions spec/models/project/life_cycle_step_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
subject { build :project_gate }

it { is_expected.to have_readonly_attribute(:definition_id) }
it { is_expected.to have_readonly_attribute(:type) }
end

# For more specs see:
Expand Down
1 change: 1 addition & 0 deletions spec/models/project/stage_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
describe "validations" do
it { is_expected.to validate_presence_of(:start_date) }
it { is_expected.to validate_presence_of(:end_date) }
it { is_expected.to validate_inclusion_of(:type).in_array(["Project::Stage"]).with_message(:must_be_a_stage) }

it "is valid when `start_date` and `end_date` are present" do
valid_stage = build(:project_stage)
Expand Down

0 comments on commit c485739

Please sign in to comment.