Replace Discard with Paper Trail

Gemfile

@@ -23,8 +23,6 @@ gem "govuk_design_system_formbuilder"
 gem "notifications-ruby-client"
 # Turbo and Stimulus
 gem "hotwire-rails"
-# Soft delete ActiveRecords objects
-gem "discard"
 # Administration framework
 gem "activeadmin", git: "https://github.com/tagliala/activeadmin.git", branch: "feature/railties-7"
 # Admin charts
@@ -47,6 +45,8 @@ gem "postcodes_io"
 gem "view_component"
 # Use the AWS S3 SDK as storage mechanism
 gem "aws-sdk-s3"
+# Track changes to models for auditing or versioning.
+gem "paper_trail"
 
 group :development, :test do
   # Call 'byebug' anywhere in the code to stop execution and get a debugger console

Gemfile.lock

@@ -156,8 +156,6 @@ GEM
     deep_merge (1.2.2)
     diff-lcs (1.5.0)
     digest (3.1.0)
-    discard (1.2.1)
-      activerecord (>= 4.2, < 8)
     docile (1.4.0)
     dotenv (2.7.6)
     dotenv-rails (2.7.6)
@@ -266,6 +264,9 @@ GEM
       childprocess (>= 0.6.3, < 5)
       iniparse (~> 1.4)
       rexml (~> 3.2)
+    paper_trail (12.2.0)
+      activerecord (>= 5.2)
+      request_store (~> 1.1)
     parallel (1.21.0)
     parser (3.1.0.0)
       ast (~> 2.4.1)
@@ -326,6 +327,8 @@ GEM
     rb-inotify (0.10.1)
       ffi (~> 1.0)
     regexp_parser (2.2.0)
+    request_store (1.5.1)
+      rack (>= 1.4)
     responders (3.0.1)
       actionpack (>= 5.0)
       railties (>= 5.0)
@@ -456,7 +459,6 @@ DEPENDENCIES
   capybara-lockstep
   chartkick
   devise!
-  discard
   dotenv-rails
   factory_bot_rails
   govuk-components
@@ -466,6 +468,7 @@ DEPENDENCIES
   listen (~> 3.3)
   notifications-ruby-client
   overcommit (>= 0.37.0)
+  paper_trail
   pg (~> 1.1)
   postcodes_io
   pry-byebug

app/controllers/case_logs_controller.rb

@@ -60,7 +60,7 @@ class CaseLogsController < ApplicationController
 
   def destroy
     if @case_log
-      if @case_log.discard
+      if @case_log.delete
         head :no_content
       else
         render json: { errors: @case_log.errors.messages }, status: :unprocessable_entity

app/models/admin_user.rb

@@ -6,6 +6,8 @@ class AdminUser < ApplicationRecord
 
   has_one_time_password(encrypted: true)
 
+  has_paper_trail
+
   validates :phone, presence: true, numericality: true
 
   MFA_SMS_TEMPLATE_ID = "bf309d93-804e-4f95-b1f4-bd513c48ecb0".freeze

app/models/case_log.rb

@@ -30,11 +30,11 @@ private
 end
 
 class CaseLog < ApplicationRecord
-  include Discard::Model
   include Validations::SoftValidations
   include Constants::CaseLog
   include Constants::IncomeRanges
-  default_scope -> { kept }
+
+  has_paper_trail
 
   validates_with CaseLogValidator
   before_validation :process_postcode_changes!, if: :property_postcode_changed?

app/models/organisation.rb

@@ -3,7 +3,10 @@ class Organisation < ApplicationRecord
   has_many :owned_case_logs, class_name: "CaseLog", foreign_key: "owning_organisation_id"
   has_many :managed_case_logs, class_name: "CaseLog", foreign_key: "managing_organisation_id"
 
+  has_paper_trail
+
   include Constants::Organisation
+
   enum provider_type: PROVIDER_TYPE
 
   def case_logs

app/models/user.rb

@@ -10,6 +10,8 @@ class User < ApplicationRecord
   has_many :owned_case_logs, through: :organisation
   has_many :managed_case_logs, through: :organisation
 
+  has_paper_trail
+
   enum role: ROLES
 
   def case_logs

db/migrate/20220207151239_create_versions.rb

@@ -0,0 +1,22 @@
+# This migration creates the `versions` table, the only schema PT requires.
+# All other migrations PT provides are optional.
+class CreateVersions < ActiveRecord::Migration[7.0]
+  # The largest text column available in all supported RDBMS is
+  # 1024^3 - 1 bytes, roughly one gibibyte.  We specify a size
+  # so that MySQL will use `longtext` instead of `text`.  Otherwise,
+  # when serializing very large objects, `text` might not be big enough.
+  TEXT_BYTES = 1_073_741_823
+
+  def change
+    create_table :versions do |t|
+      t.string   :item_type, null: false, limit: 191
+      t.bigint   :item_id,   null: false
+      t.string   :event,     null: false
+      t.string   :whodunnit
+      t.text     :object, limit: TEXT_BYTES
+
+      t.datetime :created_at
+    end
+    add_index :versions, %i[item_type item_id]
+  end
+end

db/migrate/20220207151312_remove_discarded_at_from_case_logs.rb

@@ -0,0 +1,11 @@
+class RemoveDiscardedAtFromCaseLogs < ActiveRecord::Migration[7.0]
+  def up
+    remove_index :case_logs, :discarded_at
+    remove_column :case_logs, :discarded_at
+  end
+
+  def down
+    add_column :case_logs, :discarded_at, :datetime
+    add_index :case_logs, :discarded_at
+  end
+end

db/schema.rb

@@ -93,6 +93,7 @@ ActiveRecord::Schema.define(version: 202202071123100) do
     t.integer "beds"
     t.integer "offered"
     t.integer "wchair"
+    t.integer "earnings"
     t.integer "incfreq"
     t.integer "benefits"
     t.integer "period"
@@ -127,7 +128,6 @@ ActiveRecord::Schema.define(version: 202202071123100) do
     t.integer "rp_medwel"
     t.integer "rp_hardship"
     t.integer "rp_dontknow"
-    t.datetime "discarded_at"
     t.string "tenancyother"
     t.integer "override_net_income_validation"
     t.string "property_owner_organisation"
@@ -182,7 +182,6 @@ ActiveRecord::Schema.define(version: 202202071123100) do
     t.integer "is_carehome"
     t.integer "letting_in_sheltered_accomodation"
     t.integer "household_charge"
-    t.integer "earnings"
     t.integer "referral"
     t.decimal "brent", precision: 10, scale: 2
     t.decimal "scharge", precision: 10, scale: 2
@@ -192,7 +191,6 @@ ActiveRecord::Schema.define(version: 202202071123100) do
     t.decimal "tshortfall", precision: 10, scale: 2
     t.decimal "chcharge", precision: 10, scale: 2
     t.integer "declaration"
-    t.index ["discarded_at"], name: "index_case_logs_on_discarded_at"
     t.index ["managing_organisation_id"], name: "index_case_logs_on_managing_organisation_id"
     t.index ["owning_organisation_id"], name: "index_case_logs_on_owning_organisation_id"
   end
@@ -252,4 +250,14 @@ ActiveRecord::Schema.define(version: 202202071123100) do
     t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
   end
 
+  create_table "versions", force: :cascade do |t|
+    t.string "item_type", limit: 191, null: false
+    t.bigint "item_id", null: false
+    t.string "event", null: false
+    t.string "whodunnit"
+    t.text "object"
+    t.datetime "created_at", precision: 6
+    t.index ["item_type", "item_id"], name: "index_versions_on_item_type_and_item_id"
+  end
+
 end

spec/factories/case_log.rb

@@ -108,7 +108,6 @@ FactoryBot.define do
       rp_medwel { "No" }
       rp_hardship { "No" }
       rp_dontknow { "No" }
-      discarded_at { nil }
       tenancyother { nil }
       override_net_income_validation { nil }
       net_income_known { "Yes" }

spec/models/admin_user_spec.rb

@@ -20,33 +20,46 @@ RSpec.describe AdminUser, type: :model do
         )
       }.to raise_error(ActiveRecord::RecordInvalid)
     end
-  end
 
-  it "requires an email" do
-    expect {
-      described_class.create!(
-        password: "password123",
-        phone: "075752137",
-      )
-    }.to raise_error(ActiveRecord::RecordInvalid)
-  end
+    it "requires an email" do
+      expect {
+        described_class.create!(
+          password: "password123",
+          phone: "075752137",
+        )
+      }.to raise_error(ActiveRecord::RecordInvalid)
+    end
 
-  it "requires a password" do
-    expect {
-      described_class.create!(
-        email: "admin_test@example.com",
-        phone: "075752137",
-      )
-    }.to raise_error(ActiveRecord::RecordInvalid)
+    it "requires a password" do
+      expect {
+        described_class.create!(
+          email: "admin_test@example.com",
+          phone: "075752137",
+        )
+      }.to raise_error(ActiveRecord::RecordInvalid)
+    end
+
+    it "can be created" do
+      expect {
+        described_class.create!(
+          email: "admin_test@example.com",
+          password: "password123",
+          phone: "075752137",
+        )
+      }.to change(described_class, :count).by(1)
+    end
   end
 
-  it "can be created" do
-    expect {
-      described_class.create!(
-        email: "admin_test@example.com",
-        password: "password123",
-        phone: "075752137",
-      )
-    }.to change(described_class, :count).by(1)
+  describe "paper trail" do
+    let(:admin_user) { FactoryBot.create(:admin_user) }
+
+    it "creates a record of changes to a log" do
+      expect { admin_user.update!(phone: "09673867853") }.to change(admin_user.versions, :count).by(1)
+    end
+
+    it "allows case logs to be restored to a previous version" do
+      admin_user.update!(phone: "09673867853")
+      expect(admin_user.paper_trail.previous_version.phone).to eq("07563867654")
+    end
   end
 end

spec/models/case_log_spec.rb

@@ -1172,4 +1172,17 @@ RSpec.describe CaseLog do
       end
     end
   end
+
+  describe "paper trail" do
+    let(:case_log) { FactoryBot.create(:case_log, :in_progress) }
+
+    it "creates a record of changes to a log" do
+      expect { case_log.update!(age1: 64) }.to change(case_log.versions, :count).by(1)
+    end
+
+    it "allows case logs to be restored to a previous version" do
+      case_log.update!(age1: 63)
+      expect(case_log.paper_trail.previous_version.age1).to eq(17)
+    end
+  end
 end

spec/models/organisation_spec.rb

@@ -54,4 +54,17 @@ RSpec.describe Organisation, type: :model do
       end
     end
   end
+
+  describe "paper trail" do
+    let(:organisation) { FactoryBot.create(:organisation) }
+
+    it "creates a record of changes to a log" do
+      expect { organisation.update!(name: "new test name") }.to change(organisation.versions, :count).by(1)
+    end
+
+    it "allows case logs to be restored to a previous version" do
+      organisation.update!(name: "new test name")
+      expect(organisation.paper_trail.previous_version.name).to eq("DLUHC")
+    end
+  end
 end

spec/models/user_spec.rb

@@ -52,4 +52,17 @@ RSpec.describe User, type: :model do
       expect(user.data_coordinator?).to be false
     end
   end
+
+  describe "paper trail" do
+    let(:user) { FactoryBot.create(:user) }
+
+    it "creates a record of changes to a log" do
+      expect { user.update!(name: "new test name") }.to change(user.versions, :count).by(1)
+    end
+
+    it "allows case logs to be restored to a previous version" do
+      user.update!(name: "new test name")
+      expect(user.paper_trail.previous_version.name).to eq("Danny Rojas")
+    end
+  end
 end

spec/requests/case_logs_controller_spec.rb

@@ -401,9 +401,8 @@ RSpec.describe CaseLogsController, type: :request do
         expect(response).to have_http_status(:success)
       end
 
-      it "soft deletes the case log" do
+      it "deletes the case log" do
         expect { CaseLog.find(id) }.to raise_error(ActiveRecord::RecordNotFound)
-        expect(CaseLog.with_discarded.find(id)).to be_a(CaseLog)
       end
 
       context "with an invalid case log id" do
@@ -428,7 +427,7 @@ RSpec.describe CaseLogsController, type: :request do
     context "when a case log deletion fails" do
       before do
         allow(CaseLog).to receive(:find_by).and_return(case_log)
-        allow(case_log).to receive(:discard).and_return(false)
+        allow(case_log).to receive(:delete).and_return(false)
         delete "/logs/#{id}", headers: headers
       end