[CPDLP-3631] paper_trail update to use JSON fields, Application and U…

…ser timestamps replaced with ECF

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? }

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

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

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

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

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

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
 

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

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") }

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
 

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) }