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 <[email protected]>

Author: create-issue-branch[bot]

Diff for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: db/schema.rb

@@ -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 for: lib/tasks/classroom_management.rake

@@ -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 for: lib/tasks/classroom_management_helper.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 for: spec/models/component_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 for: spec/models/project_spec.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