From a93037a981e0e721f19d61f48e58eaf2b722a260 Mon Sep 17 00:00:00 2001
From: "create-issue-branch[bot]"
 <53036503+create-issue-branch[bot]@users.noreply.github.com>
Date: Fri, 30 Aug 2024 16:22:16 +0100
Subject: [PATCH] Project `components` table audit (#418)

closes #415

As per issue #415, but also:
- Uses db:prepare which will run only run db:seed if the database
doesn't exist.
- Makes the classroom_management seeds idempotent, by not proceeding if
a school exists.

---------

Co-authored-by: create-issue-branch[bot] <53036503+create-issue-branch[bot]@users.noreply.github.com>
Co-authored-by: Dan Halson <dan.halson@raspberrypi.org>
---
 Gemfile                                       |  1 +
 Gemfile.lock                                  |  6 +++
 app/controllers/api_controller.rb             |  2 +
 app/models/component.rb                       |  8 ++++
 app/models/project.rb                         |  8 ++++
 bin/docker-debug-entrypoint.sh                |  4 +-
 bin/docker-entrypoint.sh                      |  4 +-
 db/migrate/20240828112433_create_versions.rb  | 39 +++++++++++++++++++
 ...28112434_add_object_changes_to_versions.rb | 12 ++++++
 ...0828122522_add_meta_columns_to_versions.rb | 10 +++++
 db/schema.rb                                  | 15 ++++++-
 lib/tasks/classroom_management.rake           | 15 +++++++
 lib/tasks/classroom_management_helper.rb      | 16 +++++---
 spec/models/component_spec.rb                 | 20 +++++++++-
 spec/models/project_spec.rb                   | 18 ++++++++-
 spec/rails_helper.rb                          |  2 +
 16 files changed, 170 insertions(+), 10 deletions(-)
 create mode 100644 db/migrate/20240828112433_create_versions.rb
 create mode 100644 db/migrate/20240828112434_add_object_changes_to_versions.rb
 create mode 100644 db/migrate/20240828122522_add_meta_columns_to_versions.rb

diff --git a/Gemfile b/Gemfile
index d4181710..43445264 100644
--- a/Gemfile
+++ b/Gemfile
@@ -27,6 +27,7 @@ gem 'omniauth-rpi',
     github: 'RaspberryPiFoundation/omniauth-rpi',
     tag: 'v1.3.1'
 gem 'open-uri'
+gem 'paper_trail'
 gem 'pg', '~> 1.1'
 gem 'postmark-rails'
 gem 'puma', '~> 6'
diff --git a/Gemfile.lock b/Gemfile.lock
index ff1a8d8f..a1e23f0d 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -290,6 +290,9 @@ GEM
       stringio
       time
       uri
+    paper_trail (15.1.0)
+      activerecord (>= 6.1)
+      request_store (~> 1.4)
     parallel (1.22.1)
     parser (3.2.1.0)
       ast (~> 2.4.1)
@@ -364,6 +367,8 @@ GEM
     regexp_parser (2.7.0)
     reline (0.5.9)
       io-console (~> 0.5)
+    request_store (1.7.0)
+      rack (>= 1.4)
     rexml (3.3.0)
       strscan
     roo (2.10.1)
@@ -537,6 +542,7 @@ DEPENDENCIES
   omniauth-rails_csrf_protection (~> 1.0.1)
   omniauth-rpi!
   open-uri
+  paper_trail
   pg (~> 1.1)
   postmark-rails
   pry-byebug
diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
index 05ce4eaa..595d2fbb 100644
--- a/app/controllers/api_controller.rb
+++ b/app/controllers/api_controller.rb
@@ -10,6 +10,8 @@ class ::ParameterError < StandardError; end
   rescue_from CanCan::AccessDenied, with: -> { denied }
   rescue_from ParameterError, with: -> { unprocessable }
 
+  before_action :set_paper_trail_whodunnit
+
   private
 
   def bad_request
diff --git a/app/models/component.rb b/app/models/component.rb
index 4bbce5ec..21718670 100644
--- a/app/models/component.rb
+++ b/app/models/component.rb
@@ -6,6 +6,14 @@ class Component < ApplicationRecord
   validates :extension, presence: true
   validate :default_component_protected_properties, on: :update
 
+  has_paper_trail(
+    if: ->(c) { c.project&.school_id },
+    meta: {
+      meta_project_id: ->(c) { c.project&.id },
+      meta_school_id: ->(c) { c.project&.school_id }
+    }
+  )
+
   private
 
   def default_component_protected_properties
diff --git a/app/models/project.rb b/app/models/project.rb
index d6a52ae4..fd822153 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -21,6 +21,14 @@ class Project < ApplicationRecord
 
   scope :internal_projects, -> { where(user_id: nil) }
 
+  has_paper_trail(
+    if: ->(p) { p&.school_id },
+    meta: {
+      meta_remixed_from_id: ->(p) { p&.remixed_from_id },
+      meta_school_id: ->(p) { p&.school_id }
+    }
+  )
+
   def self.users(current_user)
     school = School.find_by(id: pluck(:school_id))
     SchoolStudent::List.call(school:, token: current_user.token, student_ids: pluck(:user_id))[:school_students] || []
diff --git a/bin/docker-debug-entrypoint.sh b/bin/docker-debug-entrypoint.sh
index 65d03c03..8494d46f 100755
--- a/bin/docker-debug-entrypoint.sh
+++ b/bin/docker-debug-entrypoint.sh
@@ -1,2 +1,4 @@
-rails db:setup --trace
+#!/bin/bash
+
+rails db:prepare
 rdbg -n -o -c -- bin/rails s -p 3009 -b '0.0.0.0'
diff --git a/bin/docker-entrypoint.sh b/bin/docker-entrypoint.sh
index 23e3eaae..b99a8f55 100755
--- a/bin/docker-entrypoint.sh
+++ b/bin/docker-entrypoint.sh
@@ -1,2 +1,4 @@
-rails db:setup --trace
+#!/bin/bash
+
+rails db:prepare
 rails server --port 3009 --binding 0.0.0.0
diff --git a/db/migrate/20240828112433_create_versions.rb b/db/migrate/20240828112433_create_versions.rb
new file mode 100644
index 00000000..cb9b67cb
--- /dev/null
+++ b/db/migrate/20240828112433_create_versions.rb
@@ -0,0 +1,39 @@
+# This migration creates the `versions` table, the only schema PT requires.
+# All other migrations PT provides are optional.
+class CreateVersions < ActiveRecord::Migration[7.1]
+
+  # 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, id: :uuid do |t|
+      t.string   :item_type, null: false
+      t.string   :item_id,   null: false
+      t.string   :event,     null: false
+      t.string   :whodunnit
+      # We're not using versioning, so exclude the original state by not having an options field (see docs)
+      # t.json     :object
+
+      # Known issue in MySQL: fractional second precision
+      # -------------------------------------------------
+      #
+      # MySQL timestamp columns do not support fractional seconds unless
+      # defined with "fractional seconds precision". MySQL users should manually
+      # add fractional seconds precision to this migration, specifically, to
+      # the `created_at` column.
+      # (https://dev.mysql.com/doc/refman/5.6/en/fractional-seconds.html)
+      #
+      # MySQL users should also upgrade to at least rails 4.2, which is the first
+      # version of ActiveRecord with support for fractional seconds in MySQL.
+      # (https://github.com/rails/rails/pull/14359)
+      #
+      # MySQL users should use the following line for `created_at`
+      # t.datetime :created_at, limit: 6
+      t.datetime :created_at
+    end
+    add_index :versions, %i[item_type item_id]
+  end
+end
diff --git a/db/migrate/20240828112434_add_object_changes_to_versions.rb b/db/migrate/20240828112434_add_object_changes_to_versions.rb
new file mode 100644
index 00000000..a521ad12
--- /dev/null
+++ b/db/migrate/20240828112434_add_object_changes_to_versions.rb
@@ -0,0 +1,12 @@
+# This migration adds the optional `object_changes` column, in which PaperTrail
+# will store the `changes` diff for each update event. See the readme for
+# details.
+class AddObjectChangesToVersions < ActiveRecord::Migration[7.1]
+  # The largest text column available in all supported RDBMS.
+  # See `create_versions.rb` for details.
+  TEXT_BYTES = 1_073_741_823
+
+  def change
+    add_column :versions, :object_changes, :json
+  end
+end
diff --git a/db/migrate/20240828122522_add_meta_columns_to_versions.rb b/db/migrate/20240828122522_add_meta_columns_to_versions.rb
new file mode 100644
index 00000000..8cb03107
--- /dev/null
+++ b/db/migrate/20240828122522_add_meta_columns_to_versions.rb
@@ -0,0 +1,10 @@
+# This migration adds the optional `object_changes` column, in which PaperTrail
+# will store the `changes` diff for each update event. See the readme for
+# details.
+class AddMetaColumnsToVersions < ActiveRecord::Migration[7.1]
+  def change
+    add_column :versions, :meta_project_id, :uuid
+    add_column :versions, :meta_school_id, :uuid
+    add_column :versions, :meta_remixed_from_id, :uuid
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index bd8afb0d..d2314839 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_07_04_104847) do
+ActiveRecord::Schema[7.1].define(version: 2024_08_28_122522) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
   enable_extension "plpgsql"
@@ -257,6 +257,19 @@
     t.index ["school_id"], name: "index_teacher_invitations_on_school_id"
   end
 
+  create_table "versions", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+    t.string "item_type", null: false
+    t.string "item_id", null: false
+    t.string "event", null: false
+    t.string "whodunnit"
+    t.datetime "created_at"
+    t.json "object_changes"
+    t.uuid "meta_project_id"
+    t.uuid "meta_school_id"
+    t.uuid "meta_remixed_from_id"
+    t.index ["item_type", "item_id"], name: "index_versions_on_item_type_and_item_id"
+  end
+
   add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id"
   add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id"
   add_foreign_key "class_members", "school_classes"
diff --git a/lib/tasks/classroom_management.rake b/lib/tasks/classroom_management.rake
index aff5cc88..1b4032f0 100644
--- a/lib/tasks/classroom_management.rake
+++ b/lib/tasks/classroom_management.rake
@@ -50,6 +50,11 @@ namespace :classroom_management do
 
   desc 'Create an unverified school'
   task seed_an_unverified_school: :environment do
+    if School.find_by(code: TEST_SCHOOL)
+      puts "Test school (#{TEST_SCHOOL}) already exists, run the destroy_seed_data task to start over)."
+      return
+    end
+
     ActiveRecord::Base.transaction do
       Rails.logger.info 'Attempting to seed data...'
       creator_id = ENV.fetch('SEEDING_CREATOR_ID', TEST_USERS[:jane_doe])
@@ -64,6 +69,11 @@ namespace :classroom_management do
 
   desc 'Create a verified school'
   task seed_a_verified_school: :environment do
+    if School.find_by(code: TEST_SCHOOL)
+      puts "Test school (#{TEST_SCHOOL}) already exists, run the destroy_seed_data task to start over)."
+      return
+    end
+
     ActiveRecord::Base.transaction do
       Rails.logger.info 'Attempting to seed data...'
       creator_id = ENV.fetch('SEEDING_CREATOR_ID', TEST_USERS[:jane_doe])
@@ -79,6 +89,11 @@ namespace :classroom_management do
 
   desc 'Create a school with lessons and students'
   task seed_a_school_with_lessons_and_students: :environment do
+    if School.find_by(code: TEST_SCHOOL)
+      puts "Test school (#{TEST_SCHOOL}) already exists, run the destroy_seed_data task to start over)."
+      return
+    end
+
     ActiveRecord::Base.transaction do
       Rails.logger.info 'Attempting to seed data...'
       creator_id = ENV.fetch('SEEDING_CREATOR_ID', TEST_USERS[:jane_doe])
diff --git a/lib/tasks/classroom_management_helper.rb b/lib/tasks/classroom_management_helper.rb
index aa914a7e..3cd281fc 100644
--- a/lib/tasks/classroom_management_helper.rb
+++ b/lib/tasks/classroom_management_helper.rb
@@ -23,20 +23,26 @@ def create_school(creator_id, school_id = nil)
       school.creator_id = creator_id
       school.creator_agree_authority = true
       school.creator_agree_terms_and_conditions = true
-      school.id = school_id if school_id
     end
   end
 
   def verify_school(school)
+    if school.verified?
+      Rails.logger.info "School #{school.code} is already verified."
+      return
+    end
+
     Rails.logger.info 'Verifying the school...'
-    school.verify!
+
+    School.transaction do
+      school.verify!
+      Role.owner.create!(user_id: school.creator_id, school:)
+      Role.teacher.create!(user_id: school.creator_id, school:)
+    end
 
     # rubocop:disable Rails/SkipsModelValidations
     school.update_column(:code, SCHOOL_CODE) # The code needs to match the one in the profile
     # rubocop:enable Rails/SkipsModelValidations
-
-    Role.owner.create!(user_id: school.creator_id, school:)
-    Role.teacher.create!(user_id: school.creator_id, school:)
   end
 
   def create_school_class(teacher_id, school)
diff --git a/spec/models/component_spec.rb b/spec/models/component_spec.rb
index 506ab8f7..d13eb44e 100644
--- a/spec/models/component_spec.rb
+++ b/spec/models/component_spec.rb
@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-RSpec.describe Component do
+RSpec.describe Component, versioning: true do
   subject { build(:component) }
 
   it { is_expected.to belong_to(:project) }
@@ -37,5 +37,23 @@
           .to include(I18n.t('errors.project.editing.change_default_extension'))
       end
     end
+
+    describe 'auditing' do
+      let(:school) { create(:school) }
+      let(:teacher) { create(:teacher, school:) }
+      let(:student) { create(:student, school:) }
+
+      it 'enables auditing for a component that belongs to a project with a school_id' do
+        project_with_school = create(:project, user_id: student.id, school_id: school.id)
+        component = create(:component, project: project_with_school)
+        expect(component.versions.length).to(eq(1))
+      end
+
+      it 'does not enable auditing for a component that belongs to a project without a school_id' do
+        project_without_school = create(:project, school_id: nil)
+        component = create(:component, project: project_without_school)
+        expect(component.versions.length).to(eq(0))
+      end
+    end
   end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index c71bf0f1..80dc3c7e 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-RSpec.describe Project do
+RSpec.describe Project, versioning: true do
   let(:school) { create(:school) }
 
   describe 'associations' do
@@ -240,4 +240,20 @@
       expect(project.last_edited_at).to eq(latest_component.updated_at)
     end
   end
+
+  describe 'auditing' do
+    let(:school) { create(:school) }
+    let(:teacher) { create(:teacher, school:) }
+    let(:student) { create(:student, school:) }
+
+    it 'enables auditing for projects with a school_id' do
+      project_with_school = create(:project, user_id: student.id, school_id: school.id)
+      expect(project_with_school.versions.length).to(eq(1))
+    end
+
+    it 'does not enable auditing for projects without a school_id' do
+      project_without_school = create(:project, school_id: nil)
+      expect(project_without_school.versions.length).to(eq(0))
+    end
+  end
 end
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 00c9c3ec..37006a75 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -19,6 +19,8 @@
 
 Dir[Rails.root.join('spec/support/**/*.rb')].each { |f| require f }
 
+require 'paper_trail/frameworks/rspec'
+
 # Requires supporting ruby files with custom matchers and macros, etc, in
 # spec/support/ and its subdirectories. Files matching `spec/**/*_spec.rb` are
 # run as spec files by default. This means that files in spec/support that end