Skip to content

Commit

Permalink
Project components table audit (#418)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
create-issue-branch[bot] and danhalson authored Aug 30, 2024
1 parent c0fd6b3 commit a93037a
Show file tree
Hide file tree
Showing 16 changed files with 170 additions and 10 deletions.
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
6 changes: 6 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/api_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions app/models/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions app/models/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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] || []
Expand Down
4 changes: 3 additions & 1 deletion bin/docker-debug-entrypoint.sh
Original file line number Diff line number Diff line change
@@ -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'
4 changes: 3 additions & 1 deletion bin/docker-entrypoint.sh
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
rails db:setup --trace
#!/bin/bash

rails db:prepare
rails server --port 3009 --binding 0.0.0.0
39 changes: 39 additions & 0 deletions db/migrate/20240828112433_create_versions.rb
Original file line number Diff line number Diff line change
@@ -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
12 changes: 12 additions & 0 deletions db/migrate/20240828112434_add_object_changes_to_versions.rb
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions db/migrate/20240828122522_add_meta_columns_to_versions.rb
Original file line number Diff line number Diff line change
@@ -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
15 changes: 14 additions & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions lib/tasks/classroom_management.rake
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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])
Expand All @@ -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])
Expand Down
16 changes: 11 additions & 5 deletions lib/tasks/classroom_management_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 19 additions & 1 deletion spec/models/component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand Down Expand Up @@ -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
18 changes: 17 additions & 1 deletion spec/models/project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

require 'rails_helper'

RSpec.describe Project do
RSpec.describe Project, versioning: true do
let(:school) { create(:school) }

describe 'associations' do
Expand Down Expand Up @@ -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
2 changes: 2 additions & 0 deletions spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit a93037a

Please sign in to comment.