Use audited to track investment changes

Our manual implementation had a few issues. In particular, it didn't
track changes related to associations, which became more of an issue
when we made investments translatable.

Using audited gives us more functionality while at the same time
simplifies our code. However, it adds one more external dependency to
our project.

The reason for choosing audited over paper trail is audited seems to
make it easier to handle associations.

.rubocop.yml

@@ -208,6 +208,7 @@ Rails/CreateTableWithTimestamps:
   Enabled: true
   Exclude:
     - "db/migrate/201[5-8]*"
+    - "db/migrate/*install_audited.rb"
 
 Rails/Date:
   Enabled: true

Gemfile

@@ -6,6 +6,7 @@ gem "acts-as-taggable-on", "~> 5.0.0"
 gem "acts_as_votable", "~> 0.11.1"
 gem "ahoy_matey", "~> 1.6.0"
 gem "ancestry", "~> 3.0.7"
+gem "audited", "~> 4.9.0"
 gem "autoprefixer-rails", "~> 8.2.0"
 gem "axlsx", "~> 3.0.0.pre"
 gem "axlsx_rails", "~> 0.5.2"

Gemfile.lock

@@ -72,6 +72,8 @@ GEM
       activerecord (>= 3.2.0)
     arel (7.1.4)
     ast (2.4.0)
+    audited (4.9.0)
+      activerecord (>= 4.2, < 6.1)
     autoprefixer-rails (8.2.0)
       execjs
     axlsx (3.0.0.pre)
@@ -588,6 +590,7 @@ DEPENDENCIES
   acts_as_votable (~> 0.11.1)
   ahoy_matey (~> 1.6.0)
   ancestry (~> 3.0.7)
+  audited (~> 4.9.0)
   autoprefixer-rails (~> 8.2.0)
   axlsx (~> 3.0.0.pre)
   axlsx_rails (~> 0.5.2)

app/assets/stylesheets/admin.scss

@@ -595,17 +595,6 @@ code {
   }
 }
 
-.log-value {
-  max-height: rem-calc(65);
-  overflow: hidden;
-  max-width: rem-calc(200);
-
-  &:hover {
-    max-height: rem-calc(1000);
-    transition: max-height 0.9s;
-  }
-}
-
 // 04. Stats
 // ---------
 

app/controllers/admin/budget_investments_controller.rb

@@ -9,12 +9,11 @@ class Admin::BudgetInvestmentsController < Admin::BaseController
   has_orders %w[oldest], only: [:show, :edit]
   has_filters %w[all], only: [:index, :toggle_selection]
 
-  before_action :load_budget, except: :show_investment_log
+  before_action :load_budget, except: :audit
   before_action :load_investment, only: [:show, :edit, :update, :toggle_selection]
   before_action :load_ballot, only: [:show, :index]
   before_action :parse_valuation_filters
   before_action :load_investments, only: [:index, :toggle_selection]
-  before_action :load_change_log, only: [:show]
 
   def index
     load_tags
@@ -61,9 +60,9 @@ class Admin::BudgetInvestmentsController < Admin::BaseController
     load_investments
   end
 
-  def show_investment_log
+  def audit
-    @log = Budget::Investment::ChangeLog.find_by(id: params[:id])
+    @audit = Audited::Audit.find(params[:id])
-    render "admin/change_logs/show"
+    render "admin/audits/show"
   end
 
   private
@@ -132,8 +131,4 @@ class Admin::BudgetInvestmentsController < Admin::BaseController
         end
       end
     end
-
-    def load_change_log
-      @logs = Budget::Investment::ChangeLog.by_investment(@investment.id)
-    end
 end

app/controllers/application_controller.rb

@@ -15,7 +15,6 @@ class ApplicationController < ActionController::Base
   before_action :set_locale
   before_action :track_email_campaign
   before_action :set_return_url
-  before_action :set_current_user
 
   check_authorization unless: :devise_controller?
   self.responder = ApplicationResponder
@@ -121,8 +120,4 @@ class ApplicationController < ActionController::Base
     def current_budget
       Budget.current
     end
-
-    def set_current_user
-      User.current_user = current_user
-    end
 end

app/models/budget/investment.rb

@@ -3,7 +3,6 @@ class Budget
   class Investment < ApplicationRecord
     SORTING_OPTIONS = { id: "id", supports: "cached_votes_up" }.freeze
 
-    include ActiveModel::Dirty
     include Rails.application.routes.url_helpers
     include Measurable
     include Sanitizable
@@ -33,6 +32,8 @@ class Budget
     translates :description, touch: true
     include Globalizable
 
+    audited on: [:update, :destroy]
+
     belongs_to :author, -> { with_hidden }, class_name: "User", inverse_of: :budget_investments
     belongs_to :heading
     belongs_to :group
@@ -115,7 +116,6 @@ class Budget
     after_save :recalculate_heading_winners
     before_validation :set_responsible_name
     before_validation :set_denormalized_ids
-    after_update :change_log
 
     def comments_count
       comments.count
@@ -404,20 +404,6 @@ class Budget
         self.original_heading_id = heading_id
       end
 
-      def change_log
-        self.changed.each do |field|
-          unless field == "updated_at"
-            log = Budget::Investment::ChangeLog.new
-            log.field = field
-            log.author_id = User.current_user.id unless User.current_user.nil?
-            log.investment_id = self.id
-            log.new_value = self.send field
-            log.old_value = self.send "#{field}_was"
-            log.save!
-          end
-        end
-      end
-
       def searchable_translations_definitions
         { title       => "A",
           description => "D" }

app/models/budget/investment/change_log.rb

@@ -1,11 +0,0 @@
-class Budget::Investment::ChangeLog < ApplicationRecord
-  belongs_to :author, -> { with_hidden },
-    class_name:  "User",
-    foreign_key: "author_id",
-    inverse_of:  :budget_investment_change_logs,
-    required:    false
-
-  validates :field, presence: true
-
-  scope :by_investment, ->(investment_id) { where(investment_id: investment_id) }
-end

app/models/user.rb

@@ -27,10 +27,6 @@ class User < ApplicationRecord
     class_name:  "Budget::Investment",
     foreign_key: :author_id,
     inverse_of:  :author
-  has_many :budget_investment_change_logs,
-    foreign_key: :author_id,
-    inverse_of:  :author,
-    class_name:  "Budget::Investment::ChangeLog"
   has_many :comments, -> { with_hidden }, inverse_of: :user
   has_many :failed_census_calls
   has_many :notifications
@@ -400,14 +396,6 @@ class User < ApplicationRecord
     followables.compact.map { |followable| followable.tags.map(&:name) }.flatten.compact.uniq
   end
 
-  def self.current_user
-    Thread.current[:user]
-  end
-
-  def self.current_user=(user)
-    Thread.current[:user] = user
-  end
-
   def send_devise_notification(notification, *args)
     devise_mailer.send(notification, self, *args).deliver_later
   end

app/views/admin/audits/_audits.html.erb

@@ -0,0 +1,46 @@
+<h2><%= t("admin.audits.title") %></h2>
+
+<% if resource.audits.empty? %>
+  <p><%= t("admin.audits.empty") %></p>
+<% else %>
+  <table>
+    <thead>
+      <tr>
+        <th><%= t("admin.audits.id") %></th>
+        <th><%= t("admin.audits.field") %></th>
+        <th><%= t("admin.audits.old_value") %></th>
+        <th><%= t("admin.audits.new_value") %></th>
+        <th><%= t("admin.audits.edited_at") %></th>
+        <th><%= t("admin.audits.edited_by") %></th>
+        <th><%= t("admin.audits.actions") %></th>
+      </tr>
+    </thead>
+    <tbody>
+      <% resource.audits.order(:created_at).each do |audit| %>
+        <% audit.audited_changes.each do |field, (old_value, new_value)| %>
+          <tr>
+            <td class="text-center"><%= audit.id %></td>
+            <td class="small"><%= field.capitalize %></td>
+            <td class="small">
+              <div class="audit-value"><%= old_value %></div>
+            </td>
+            <td class="small">
+              <div class="audit-value"><%= new_value %></div>
+            </td>
+            <td class="small">
+              <%= audit.created_at.to_date %>
+            </td>
+            <td class="small">
+              <%= audit.user&.name %>
+            </td>
+            <td>
+              <%= link_to t("shared.show"),
+                admin_audit_path(audit),
+                class: "button hollow primary" %>
+            </td>
+          </tr>
+        <% end %>
+      <% end %>
+    </tbody>
+  </table>
+<% end %>

app/views/admin/audits/show.html.erb

@@ -0,0 +1,19 @@
+<h2 class="inline-block"><%= t("admin.audits.title") %></h2>
+
+<label><strong><%= t("admin.audits.id") %></strong></label>
+<p><%= @audit.id %></p>
+
+<% @audit.audited_changes.each do |field, (old_value, new_value)| %>
+  <label><strong><%= t("admin.audits.field") %></strong></label>
+  <p><%= field %></p>
+  <label><strong><%= t("admin.audits.old_value") %></strong></label>
+  <p><%= old_value %></p>
+  <label><strong><%= t("admin.audits.new_value") %></strong></label>
+  <p><%= new_value %></p>
+<% end %>
+
+<label><strong><%= t("admin.audits.edited_at") %></strong></label>
+<p><%= @audit.created_at.to_date %></p>
+<strong><%= t("admin.audits.edited_by") %></strong>
+<label><strong><%= t("admin.audits.edited_by") %></strong></label>
+<p><%= @audit.user&.name %></p>

app/views/admin/budget_investments/show.html.erb

@@ -66,6 +66,6 @@
 
 <%= render "valuation/budget_investments/valuation_comments" %>
 
-<%= render "admin/change_logs/change_log", logs: @logs %>
+<%= render "admin/audits/audits", resource: @investment %>
 
 <%= render "admin/milestones/milestones", milestoneable: @investment %>

app/views/admin/change_logs/_change_log.html.erb

@@ -1,44 +0,0 @@
-<h2 class="inline-block"><%= t("admin.change_log.title") %></h2>
-
-<% if logs.empty? %>
-  <label><%= t("admin.change_log.empty") %></label>
-<% else %>
-  <table>
-    <thead>
-    <tr>
-      <th><%= t("admin.change_log.id") %></th>
-      <th><%= t("admin.change_log.field") %></th>
-      <th><%= t("admin.change_log.old_value") %></th>
-      <th><%= t("admin.change_log.new_value") %></th>
-      <th><%= t("admin.change_log.edited_at") %></th>
-      <th><%= t("admin.change_log.edited_by") %></th>
-      <th><%= t("admin.change_log.actions") %></th>
-    </tr>
-    </thead>
-    <tbody>
-    <% logs.each do |log| %>
-      <tr id="log_<%= log.id %>">
-        <td class="text-center"><%= log.id %></td>
-        <td class="small"><%= log.field.capitalize %></td>
-        <td class="small">
-          <div class="log-value"><%= log.old_value %></div>
-        </td>
-        <td class="small">
-          <div class="log-value"><%= log.new_value %></div>
-        </td>
-        <td class="small">
-          <%= log.created_at.to_date %>
-        </td>
-        <td class="small">
-          <%= log.author.name unless log.author.nil? %>
-        </td>
-        <td>
-          <%= link_to admin_change_log_path(id: log) do %>
-            <button class="button hollow primary"><%= t("shared.show") %></button>
-          <% end %>
-        </td>
-      </tr>
-    <% end %>
-    </tbody>
-  </table>
-<% end %>

app/views/admin/change_logs/show.html.erb

@@ -1,12 +0,0 @@
-<h2 class="inline-block"><%= t("admin.change_log.title") %></h2>
-
-<label><strong><%= t("admin.change_log.id") %></strong></label>
-<p><%= @log.id %></p>
-<label><strong><%= t("admin.change_log.old_value") %></strong></label>
-<p><%= @log.old_value %></p>
-<label><strong><%= t("admin.change_log.new_value") %></strong></label>
-<p><%= @log.new_value %></p>
-<label><strong><%= t("admin.change_log.edited_at") %></strong></label>
-<p><%= @log.created_at.to_date %></p>
-<label><strong><%= t("admin.change_log.edited_by") %></strong></label>
-<p><%= @log.author.name unless @log.author.nil? %></p>

config/locales/en/admin.yml

@@ -1583,7 +1583,7 @@ en:
         submit_header: Save header
         card_title: Edit card
         submit_card: Save card
-    change_log:
+    audits:
       title: "Change Log"
       id: "ID"
       field: "Field"

config/locales/es/admin.yml

@@ -1582,7 +1582,7 @@ es:
         submit_header: Guardar encabezado
         card_title: Editar tarjeta
         submit_card: Guardar tarjeta
-    change_log:
+    audits:
       title: "Historial"
       id: "ID"
       field: "Campo"

config/routes/admin.rb

@@ -250,7 +250,7 @@ namespace :admin do
   get "download_settings/:resource", to: "download_settings#edit", as: "edit_download_settings"
   put "download_settings/:resource", to: "download_settings#update", as: "update_download_settings"
 
-  get "/change_log/:id", to: "budget_investments#show_investment_log", as: "change_log"
+  get "/admin/audits/:id", to: "budget_investments#audit", as: "audit"
 
   resources :local_census_records
   namespace :local_census_records do

db/migrate/20191102002206_install_audited.rb

@@ -0,0 +1,26 @@
+class InstallAudited < ActiveRecord::Migration[5.0]
+  def change
+    create_table :audits, force: true do |t|
+      t.column :auditable_id, :integer
+      t.column :auditable_type, :string
+      t.column :associated_id, :integer
+      t.column :associated_type, :string
+      t.column :user_id, :integer
+      t.column :user_type, :string
+      t.column :username, :string
+      t.column :action, :string
+      t.column :audited_changes, :jsonb
+      t.column :version, :integer, default: 0
+      t.column :comment, :string
+      t.column :remote_address, :string
+      t.column :request_uuid, :string
+      t.column :created_at, :datetime
+    end
+
+    add_index :audits, [:auditable_type, :auditable_id, :version], name: "auditable_index"
+    add_index :audits, [:associated_type, :associated_id], name: "associated_index"
+    add_index :audits, [:user_id, :user_type], name: "user_index"
+    add_index :audits, :request_uuid
+    add_index :audits, :created_at
+  end
+end

db/migrate/20191102002238_drop_budget_investment_change_logs.rb

@@ -0,0 +1,13 @@
+class DropBudgetInvestmentChangeLogs < ActiveRecord::Migration[5.0]
+  def change
+    drop_table :budget_investment_change_logs do |t|
+      t.integer :investment_id
+      t.integer :author_id
+      t.string  :field
+      t.string  :new_value
+      t.string  :old_value
+
+      t.timestamps null: false
+    end
+  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.define(version: 20191101183155) do
+ActiveRecord::Schema.define(version: 20191102002238) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -84,6 +84,28 @@ ActiveRecord::Schema.define(version: 20191101183155) do
     t.index ["visit_id"], name: "index_ahoy_events_on_visit_id", using: :btree
   end
 
+  create_table "audits", force: :cascade do |t|
+    t.integer  "auditable_id"
+    t.string   "auditable_type"
+    t.integer  "associated_id"
+    t.string   "associated_type"
+    t.integer  "user_id"
+    t.string   "user_type"
+    t.string   "username"
+    t.string   "action"
+    t.jsonb    "audited_changes"
+    t.integer  "version",         default: 0
+    t.string   "comment"
+    t.string   "remote_address"
+    t.string   "request_uuid"
+    t.datetime "created_at"
+    t.index ["associated_type", "associated_id"], name: "associated_index", using: :btree
+    t.index ["auditable_type", "auditable_id", "version"], name: "auditable_index", using: :btree
+    t.index ["created_at"], name: "index_audits_on_created_at", using: :btree
+    t.index ["request_uuid"], name: "index_audits_on_request_uuid", using: :btree
+    t.index ["user_id", "user_type"], name: "user_index", using: :btree
+  end
+
   create_table "banner_sections", force: :cascade do |t|
     t.integer  "banner_id"
     t.integer  "web_section_id"
@@ -203,16 +225,6 @@ ActiveRecord::Schema.define(version: 20191101183155) do
     t.index ["group_id"], name: "index_budget_headings_on_group_id", using: :btree
   end
 
-  create_table "budget_investment_change_logs", force: :cascade do |t|
-    t.integer  "investment_id"
-    t.integer  "author_id"
-    t.string   "field"
-    t.string   "new_value"
-    t.string   "old_value"
-    t.datetime "created_at",    null: false
-    t.datetime "updated_at",    null: false
-  end
-
   create_table "budget_investment_milestone_translations", force: :cascade do |t|
     t.integer  "budget_investment_milestone_id", null: false
     t.string   "locale",                         null: false