From 3e704bcabcb9e8428520a8a13eac381d25aae26e Mon Sep 17 00:00:00 2001 From: Mooktakim Ahmed Date: Wed, 23 Oct 2024 16:34:48 +0100 Subject: [PATCH] [CPDLP-3631] paper_trail update to use JSON fields, Application and User timestamps replaced with ECF --- app/models/application.rb | 4 +++- app/models/user.rb | 4 ++++ .../migration/migrators/application.rb | 9 ++++---- app/services/migration/migrators/user.rb | 3 +++ ...change_object_type_to_json_for_versions.rb | 6 +++++ .../20241023113236_add_note_to_versions.rb | 5 +++++ db/schema.rb | 9 ++++++-- spec/models/application_spec.rb | 22 +++++++++++++++++++ spec/models/user_spec.rb | 22 +++++++++++++++++++ .../migration/migrators/application_spec.rb | 21 ++++++++++++++++++ .../services/migration/migrators/user_spec.rb | 21 ++++++++++++++++++ 11 files changed, 118 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20241023112339_change_object_type_to_json_for_versions.rb create mode 100644 db/migrate/20241023113236_add_note_to_versions.rb diff --git a/app/models/application.rb b/app/models/application.rb index 12762056d5..e9de9b1e96 100644 --- a/app/models/application.rb +++ b/app/models/application.rb @@ -11,7 +11,7 @@ class Application < ApplicationRecord establishment-ineligible ].freeze - has_paper_trail only: %i[lead_provider_approval_status participant_outcome_state] + has_paper_trail meta: { note: :version_note } belongs_to :user belongs_to :course @@ -34,6 +34,8 @@ class Application < ApplicationRecord scope :eligible_for_funding, -> { where(eligible_for_funding: true) } scope :with_targeted_delivery_funding_eligibility, -> { where(targeted_delivery_funding_eligibility: true) } + attr_accessor :version_note + validate :schedule_cohort_matches # TODO: add constraints into the DB after separation validates :ecf_id, presence: true, if: -> { Feature.ecf_api_disabled? } diff --git a/app/models/user.rb b/app/models/user.rb index e01532ffdf..979944ffa4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,8 @@ class User < ApplicationRecord devise :omniauthable, omniauth_providers: [:tra_openid_connect] + has_paper_trail meta: { note: :version_note } + has_many :applications, dependent: :destroy has_many :declarations, through: :applications has_many :ecf_sync_request_logs, as: :syncable, dependent: :destroy @@ -38,6 +40,8 @@ class User < ApplicationRecord enum email_updates_status: EMAIL_UPDATES_ALL_STATES, _suffix: true + attr_accessor :version_note + def latest_participant_outcome(lead_provider, course_identifier) declarations.eligible_for_outcomes(lead_provider, course_identifier) .first diff --git a/app/services/migration/migrators/application.rb b/app/services/migration/migrators/application.rb index b8ec146afe..e66532e459 100644 --- a/app/services/migration/migrators/application.rb +++ b/app/services/migration/migrators/application.rb @@ -25,6 +25,8 @@ class Application < Base teacher_catchment_country notes funded_place + created_at + updated_at ].freeze class << self @@ -59,11 +61,7 @@ def call migrate(self.class.ecf_npq_applications) do |ecf_npq_application| application = applications_by_ecf_id[ecf_npq_application.id] - application ||= ::Application.new( - ecf_id: ecf_npq_application.id, - created_at: ecf_npq_application.created_at, - updated_at: ecf_npq_application.updated_at, - ) + application ||= ::Application.new(ecf_id: ecf_npq_application.id) application.cohort_id = find_cohort_id!(ecf_id: ecf_npq_application.cohort_id) application.itt_provider_id = find_itt_provider_id!(itt_provider: ecf_npq_application.itt_provider) if ecf_npq_application.itt_provider @@ -94,6 +92,7 @@ def call end application.user_id = find_user_id!(ecf_id: ecf_npq_application.user.id) + application.version_note = "Changes migrated from ECF to NPQ" application.update!(ecf_npq_application.attributes.slice(*ATTRIBUTES)) end end diff --git a/app/services/migration/migrators/user.rb b/app/services/migration/migrators/user.rb index 06d410622c..ab9c0aff4e 100644 --- a/app/services/migration/migrators/user.rb +++ b/app/services/migration/migrators/user.rb @@ -52,6 +52,9 @@ def call national_insurance_number: npq_application.nino || user.national_insurance_number, active_alert: npq_application.active_alert || user.active_alert, trn_verified: ecf_verified_trn.present? || (ecf_unverified_trn.blank? && user&.trn_verified), + created_at: ecf_user.created_at, + updated_at: ecf_user.updated_at, + version_note: "Changes migrated from ECF to NPQ", ) end end diff --git a/db/migrate/20241023112339_change_object_type_to_json_for_versions.rb b/db/migrate/20241023112339_change_object_type_to_json_for_versions.rb new file mode 100644 index 0000000000..be5d277e3c --- /dev/null +++ b/db/migrate/20241023112339_change_object_type_to_json_for_versions.rb @@ -0,0 +1,6 @@ +class ChangeObjectTypeToJsonForVersions < ActiveRecord::Migration[7.1] + def change + change_column :versions, :object, :json, using: "object::text::json" + add_column :versions, :object_changes, :json + end +end diff --git a/db/migrate/20241023113236_add_note_to_versions.rb b/db/migrate/20241023113236_add_note_to_versions.rb new file mode 100644 index 0000000000..d860e7cf93 --- /dev/null +++ b/db/migrate/20241023113236_add_note_to_versions.rb @@ -0,0 +1,5 @@ +class AddNoteToVersions < ActiveRecord::Migration[7.1] + def change + add_column :versions, :note, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 6d10e4fc24..908621a394 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_10_16_122750) do +ActiveRecord::Schema[7.1].define(version: 2024_10_23_113236) do # These are extensions that must be enabled in order to support this database enable_extension "btree_gin" enable_extension "citext" @@ -408,6 +408,7 @@ t.bigint "lead_provider_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "page" t.index ["lead_provider_id"], name: "index_response_comparisons_on_lead_provider_id" end @@ -531,6 +532,8 @@ t.boolean "notify_user_for_future_reg", default: false t.integer "email_updates_status", default: 0 t.string "email_updates_unsubscribe_key" + t.string "archived_email" + t.datetime "archived_at" t.index ["ecf_id"], name: "index_users_on_ecf_id" t.index ["email"], name: "index_users_on_email", unique: true t.index ["provider"], name: "index_users_on_provider" @@ -542,8 +545,10 @@ t.bigint "item_id", null: false t.string "event", null: false t.string "whodunnit" - t.text "object" + t.json "object" t.datetime "created_at", precision: nil + t.json "object_changes" + t.string "note" t.index ["item_type", "item_id"], name: "index_versions_on_item_type_and_item_id" end diff --git a/spec/models/application_spec.rb b/spec/models/application_spec.rb index 12ade7d5c7..aa8023cf94 100644 --- a/spec/models/application_spec.rb +++ b/spec/models/application_spec.rb @@ -18,6 +18,28 @@ it { is_expected.to have_many(:declarations) } end + describe "paper_trail" do + subject { create(:application, lead_provider_approval_status: "pending") } + + it "enables paper trail" do + expect(subject).to be_versioned + end + + it "creates a version with a note" do + with_versioning do + expect(PaperTrail).to be_enabled + + subject.update!( + lead_provider_approval_status: "rejected", + version_note: "This is a test", + ) + version = subject.versions.last + expect(version.note).to eq("This is a test") + expect(version.object_changes["lead_provider_approval_status"]).to eq(%w[pending rejected]) + end + end + end + describe "validations" do context "when the schedule cohort does not match the application cohort" do subject do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 68f5f91af2..abbf970256 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -10,6 +10,28 @@ it { is_expected.to have_many(:declarations).through(:applications) } end + describe "paper_trail" do + subject { create(:user, full_name: "Joe") } + + it "enables paper trail" do + expect(subject).to be_versioned + end + + it "creates a version with a note" do + with_versioning do + expect(PaperTrail).to be_enabled + + subject.update!( + full_name: "Changed Name", + version_note: "This is a test", + ) + version = subject.versions.last + expect(version.note).to eq("This is a test") + expect(version.object_changes["full_name"]).to eq(["Joe", "Changed Name"]) + end + end + end + describe "validations" do it { is_expected.to validate_presence_of(:full_name).with_message("Enter a full name") } it { is_expected.to validate_presence_of(:email).on(:npq_separation).with_message("Enter an email address") } diff --git a/spec/services/migration/migrators/application_spec.rb b/spec/services/migration/migrators/application_spec.rb index 547683da66..9f8763ca90 100644 --- a/spec/services/migration/migrators/application_spec.rb +++ b/spec/services/migration/migrators/application_spec.rb @@ -37,6 +37,27 @@ def setup_failure_state expect(application.accepted_at).to eq(ecf_resource1.profile.created_at) end + it "sets the timestamps from the ECF NPQApplication on the NPQ application" do + created_at = 10.days.ago + updated_at = 3.days.ago + ecf_resource1.update!(created_at:, updated_at:) + + with_versioning do + expect(PaperTrail).to be_enabled + + instance.call + + application = ::Application.find_by_ecf_id!(ecf_resource1.id) + expect(application.created_at.to_s).to eq(created_at.to_s) + expect(application.updated_at.to_s).to eq(updated_at.to_s) + + version = application.versions.last + expect(version.note).to eq("Changes migrated from ECF to NPQ") + expect(version.object_changes["created_at"].last).to eq(created_at.iso8601(3)) + expect(version.object_changes["updated_at"].last).to eq(updated_at.iso8601(3)) + end + end + it "sets the schedule from the ECF NPQApplication on the NPQ application" do instance.call diff --git a/spec/services/migration/migrators/user_spec.rb b/spec/services/migration/migrators/user_spec.rb index 30d05b0503..a151c04faf 100644 --- a/spec/services/migration/migrators/user_spec.rb +++ b/spec/services/migration/migrators/user_spec.rb @@ -51,6 +51,27 @@ def setup_failure_state ) end + it "sets the timestamps from the ECF User" do + created_at = 10.days.ago + updated_at = 3.days.ago + ecf_resource1.update!(created_at:, updated_at:) + + with_versioning do + expect(PaperTrail).to be_enabled + + instance.call + + user = ::User.find_by_ecf_id!(ecf_resource1.id) + expect(user.created_at.to_s).to eq(created_at.to_s) + expect(user.updated_at.to_s).to eq(updated_at.to_s) + + version = user.versions.last + expect(version.note).to eq("Changes migrated from ECF to NPQ") + expect(version.object_changes["created_at"].last).to eq(created_at.iso8601(3)) + expect(version.object_changes["updated_at"].last).to eq(updated_at.iso8601(3)) + end + end + describe "TRN prioritisation" do it "records a failure when there are multiple, different/verified TRNs for the user's NPQApplications in ECF and no teacher profile TRN" do ecf_user = create(:ecf_migration_user, :npq).tap { |u| u.teacher_profile.update!(trn: nil) }