diff --git a/app/components/timeline_entry/component.rb b/app/components/timeline_entry/component.rb index f4677f26d8..a33d562087 100644 --- a/app/components/timeline_entry/component.rb +++ b/app/components/timeline_entry/component.rb @@ -46,14 +46,16 @@ def status_changed_vars old_status: render( StatusTag::Component.new( - status: timeline_event.old_state, + status: + timeline_event.old_state.presence || timeline_event.old_value, class_context: "timeline-event", ), ).strip, new_status: render( StatusTag::Component.new( - status: timeline_event.new_state, + status: + timeline_event.new_state.presence || timeline_event.new_value, class_context: "timeline-event", ), ).strip, diff --git a/app/lib/application_form_status_updater.rb b/app/lib/application_form_status_updater.rb index 46c3fae6e6..010e38731f 100644 --- a/app/lib/application_form_status_updater.rb +++ b/app/lib/application_form_status_updater.rb @@ -29,8 +29,8 @@ def call application_form.update!(status:) create_timeline_event( event_type: "status_changed", - old_state: old_status, - new_state: status, + old_value: old_status, + new_value: status, ) end diff --git a/app/models/timeline_event.rb b/app/models/timeline_event.rb index 7543a0654b..03cac4cbab 100644 --- a/app/models/timeline_event.rb +++ b/app/models/timeline_event.rb @@ -90,15 +90,6 @@ class TimelineEvent < ApplicationRecord absence: true, unless: -> { assessor_assigned? || reviewer_assigned? } - validates :old_state, - :new_state, - presence: true, - if: -> { status_changed? || assessment_section_recorded? } - validates :old_state, - :new_state, - absence: true, - unless: -> { status_changed? || assessment_section_recorded? } - validates :assessment_section, presence: true, if: :assessment_section_recorded? @@ -154,15 +145,15 @@ class TimelineEvent < ApplicationRecord :new_value, presence: true, if: -> do - action_required_by_changed? || information_changed? || - stage_changed? + action_required_by_changed? || assessment_section_recorded? || + information_changed? || stage_changed? || status_changed? end validates :old_value, :new_value, absence: true, unless: -> do - action_required_by_changed? || information_changed? || - stage_changed? + action_required_by_changed? || assessment_section_recorded? || + information_changed? || stage_changed? || status_changed? end validates :column_name, presence: true, if: :information_changed? diff --git a/app/services/update_assessment_section.rb b/app/services/update_assessment_section.rb index 736817eb6d..270ed5ad76 100644 --- a/app/services/update_assessment_section.rb +++ b/app/services/update_assessment_section.rb @@ -11,7 +11,7 @@ def initialize(assessment_section:, user:, params:) end def call - old_state = assessment_section.status + old_status = assessment_section.status ActiveRecord::Base.transaction do selected_keys = selected_failure_reasons.keys @@ -36,7 +36,7 @@ def call next false unless assessment_section.update(params) update_application_form_assessor - create_timeline_event(old_state:) + create_timeline_event(old_status:) update_assessment_started_at update_application_form_state @@ -61,17 +61,17 @@ def update_application_form_assessor end end - def create_timeline_event(old_state:) - new_state = assessment_section.status - return if old_state == new_state + def create_timeline_event(old_status:) + new_status = assessment_section.status + return if old_status == new_status TimelineEvent.create!( creator: user, event_type: :assessment_section_recorded, assessment_section:, application_form:, - old_state:, - new_state:, + old_value: old_status, + new_value: new_status, ) end diff --git a/lib/tasks/timeline_events.rake b/lib/tasks/timeline_events.rake index 13bceac257..4f3d15437c 100644 --- a/lib/tasks/timeline_events.rake +++ b/lib/tasks/timeline_events.rake @@ -1,8 +1,15 @@ namespace :timeline_events do - desc "Migrate state_changed events" - task migrate_state_changed: :environment do - TimelineEvent.where(event_type: "state_changed").update_all( - event_type: "status_changed", - ) + desc "Migrate old_state/new_state" + task migrate_old_new_state: :environment do + TimelineEvent + .where(event_type: %w[status_changed assessment_section_recorded]) + .each do |timeline_event| + timeline_event.update!( + old_value: timeline_event.old_state, + new_value: timeline_event.new_state, + old_state: "", + new_state: "", + ) + end end end diff --git a/spec/components/timeline_entry_spec.rb b/spec/components/timeline_entry_spec.rb index 2ff368c7bb..71d421deb0 100644 --- a/spec/components/timeline_entry_spec.rb +++ b/spec/components/timeline_entry_spec.rb @@ -56,20 +56,20 @@ create( :timeline_event, :status_changed, - old_state: "submitted", - new_state: "awarded", + old_value: "submitted", + new_value: "awarded", ) end - let(:old_state) do - I18n.t("components.status_tag.#{timeline_event.old_state}") + let(:old_status_tag) do + I18n.t("components.status_tag.#{timeline_event.old_value}") end - let(:new_state) do - I18n.t("components.status_tag.#{timeline_event.new_state}") + let(:new_status_tag) do + I18n.t("components.status_tag.#{timeline_event.new_value}") end it "describes the event" do expect(component.text.squish).to include( - "Status changed from #{old_state} to #{new_state}", + "Status changed from #{old_status_tag} to #{new_status_tag}", ) end @@ -87,7 +87,7 @@ create( :timeline_event, :assessment_section_recorded, - new_state: "completed", + new_value: "completed", assessment_section:, ) end diff --git a/spec/factories/application_forms.rb b/spec/factories/application_forms.rb index 27019a5f30..e75ce35027 100644 --- a/spec/factories/application_forms.rb +++ b/spec/factories/application_forms.rb @@ -190,8 +190,8 @@ :status_changed, application_form:, creator: application_form.teacher, - old_state: "draft", - new_state: "submitted", + old_value: "draft", + new_value: "submitted", ) end end diff --git a/spec/factories/timeline_events.rb b/spec/factories/timeline_events.rb index 1dccde3e2e..1829d1d557 100644 --- a/spec/factories/timeline_events.rb +++ b/spec/factories/timeline_events.rb @@ -68,8 +68,8 @@ trait :status_changed do event_type { "status_changed" } - old_state { ApplicationForm.statuses.keys.sample } - new_state { ApplicationForm.statuses.keys.sample } + old_value { ApplicationForm.statuses.keys.sample } + new_value { ApplicationForm.statuses.keys.sample } end trait :assessment_section_recorded do @@ -82,8 +82,8 @@ assessment: build(:assessment, application_form:), ) end - old_state { %i[not_started invalid completed].sample } - new_state { %i[not_started invalid completed].sample } + old_value { %i[not_started invalid completed].sample } + new_value { %i[not_started invalid completed].sample } end trait :note_created do diff --git a/spec/lib/application_form_status_updater_spec.rb b/spec/lib/application_form_status_updater_spec.rb index 9b874c0761..45b1a6fc34 100644 --- a/spec/lib/application_form_status_updater_spec.rb +++ b/spec/lib/application_form_status_updater_spec.rb @@ -72,8 +72,8 @@ :status_changed, creator: user, application_form:, - old_state: "draft", - new_state: new_status, + old_value: "draft", + new_value: new_status, ) end end diff --git a/spec/models/timeline_event_spec.rb b/spec/models/timeline_event_spec.rb index 8b978dec4e..a0f20cde2c 100644 --- a/spec/models/timeline_event_spec.rb +++ b/spec/models/timeline_event_spec.rb @@ -100,8 +100,6 @@ before { timeline_event.event_type = :assessor_assigned } it { is_expected.to_not validate_presence_of(:assignee) } - it { is_expected.to validate_absence_of(:old_state) } - it { is_expected.to validate_absence_of(:new_state) } it { is_expected.to validate_absence_of(:assessment_section) } it { is_expected.to validate_absence_of(:note) } it { is_expected.to validate_absence_of(:mailer_class_name) } @@ -123,8 +121,6 @@ before { timeline_event.event_type = :reviewer_assigned } it { is_expected.to_not validate_presence_of(:assignee) } - it { is_expected.to validate_absence_of(:old_state) } - it { is_expected.to validate_absence_of(:new_state) } it { is_expected.to validate_absence_of(:assessment_section) } it { is_expected.to validate_absence_of(:note) } it { is_expected.to validate_absence_of(:mailer_class_name) } @@ -146,8 +142,6 @@ before { timeline_event.event_type = :status_changed } it { is_expected.to validate_absence_of(:assignee) } - it { is_expected.to validate_presence_of(:old_state) } - it { is_expected.to validate_presence_of(:new_state) } it { is_expected.to validate_absence_of(:assessment_section) } it { is_expected.to validate_absence_of(:note) } it { is_expected.to validate_absence_of(:mailer_class_name) } @@ -161,16 +155,14 @@ it { is_expected.to validate_absence_of(:requestable_type) } it { is_expected.to validate_absence_of(:work_history_id) } it { is_expected.to validate_absence_of(:column_name) } - it { is_expected.to validate_absence_of(:old_value) } - it { is_expected.to validate_absence_of(:new_value) } + it { is_expected.to validate_presence_of(:old_value) } + it { is_expected.to validate_presence_of(:new_value) } end context "with an assessment section recorded event type" do before { timeline_event.event_type = :assessment_section_recorded } it { is_expected.to validate_absence_of(:assignee) } - it { is_expected.to validate_presence_of(:old_state) } - it { is_expected.to validate_presence_of(:new_state) } it { is_expected.to validate_presence_of(:assessment_section) } it { is_expected.to validate_absence_of(:note) } it { is_expected.to validate_absence_of(:mailer_class_name) } @@ -184,16 +176,14 @@ it { is_expected.to validate_absence_of(:requestable_type) } it { is_expected.to validate_absence_of(:work_history_id) } it { is_expected.to validate_absence_of(:column_name) } - it { is_expected.to validate_absence_of(:old_value) } - it { is_expected.to validate_absence_of(:new_value) } + it { is_expected.to validate_presence_of(:old_value) } + it { is_expected.to validate_presence_of(:new_value) } end context "with a note created event type" do before { timeline_event.event_type = :note_created } it { is_expected.to validate_absence_of(:assignee) } - it { is_expected.to validate_absence_of(:old_state) } - it { is_expected.to validate_absence_of(:new_state) } it { is_expected.to validate_absence_of(:assessment_section) } it { is_expected.to validate_presence_of(:note) } it { is_expected.to validate_absence_of(:mailer_class_name) } @@ -215,8 +205,6 @@ before { timeline_event.event_type = :email_sent } it { is_expected.to validate_absence_of(:assignee) } - it { is_expected.to validate_absence_of(:old_state) } - it { is_expected.to validate_absence_of(:new_state) } it { is_expected.to validate_absence_of(:assessment_section) } it { is_expected.to validate_absence_of(:note) } it { is_expected.to validate_presence_of(:mailer_class_name) } @@ -238,8 +226,6 @@ before { timeline_event.event_type = :age_range_subjects_verified } it { is_expected.to validate_absence_of(:assignee) } - it { is_expected.to validate_absence_of(:old_state) } - it { is_expected.to validate_absence_of(:new_state) } it { is_expected.to validate_absence_of(:assessment_section) } it { is_expected.to validate_absence_of(:note) } it { is_expected.to validate_absence_of(:mailer_class_name) } @@ -261,8 +247,6 @@ before { timeline_event.event_type = :requestable_received } it { is_expected.to validate_absence_of(:assignee) } - it { is_expected.to validate_absence_of(:old_state) } - it { is_expected.to validate_absence_of(:new_state) } it { is_expected.to validate_absence_of(:assessment_section) } it { is_expected.to validate_absence_of(:note) } it { is_expected.to validate_absence_of(:mailer_class_name) } @@ -294,8 +278,6 @@ before { timeline_event.event_type = :requestable_received } it { is_expected.to validate_absence_of(:assignee) } - it { is_expected.to validate_absence_of(:old_state) } - it { is_expected.to validate_absence_of(:new_state) } it { is_expected.to validate_absence_of(:assessment_section) } it { is_expected.to validate_absence_of(:note) } it { is_expected.to validate_absence_of(:mailer_class_name) } @@ -327,8 +309,6 @@ before { timeline_event.event_type = :requestable_expired } it { is_expected.to validate_absence_of(:assignee) } - it { is_expected.to validate_absence_of(:old_state) } - it { is_expected.to validate_absence_of(:new_state) } it { is_expected.to validate_absence_of(:assessment_section) } it { is_expected.to validate_absence_of(:note) } it { is_expected.to validate_absence_of(:mailer_class_name) } @@ -360,8 +340,6 @@ before { timeline_event.event_type = :requestable_assessed } it { is_expected.to validate_absence_of(:assignee) } - it { is_expected.to validate_absence_of(:old_state) } - it { is_expected.to validate_absence_of(:new_state) } it { is_expected.to validate_absence_of(:assessment_section) } it { is_expected.to validate_absence_of(:note) } it { is_expected.to validate_absence_of(:mailer_class_name) } @@ -393,8 +371,6 @@ before { timeline_event.event_type = :action_required_by_changed } it { is_expected.to validate_absence_of(:assignee) } - it { is_expected.to validate_absence_of(:old_state) } - it { is_expected.to validate_absence_of(:new_state) } it { is_expected.to validate_absence_of(:assessment_section) } it { is_expected.to validate_absence_of(:note) } it { is_expected.to validate_absence_of(:mailer_class_name) } @@ -416,8 +392,6 @@ before { timeline_event.event_type = :stage_changed } it { is_expected.to validate_absence_of(:assignee) } - it { is_expected.to validate_absence_of(:old_state) } - it { is_expected.to validate_absence_of(:new_state) } it { is_expected.to validate_absence_of(:assessment_section) } it { is_expected.to validate_absence_of(:note) } it { is_expected.to validate_absence_of(:mailer_class_name) } diff --git a/spec/services/submit_application_form_spec.rb b/spec/services/submit_application_form_spec.rb index 1e609aa1e0..c075181e9b 100644 --- a/spec/services/submit_application_form_spec.rb +++ b/spec/services/submit_application_form_spec.rb @@ -64,8 +64,8 @@ :status_changed, creator: user, application_form:, - old_state: "draft", - new_state: "submitted", + old_value: "draft", + new_value: "submitted", ) end