From c93e13b9594405fd0d1f2b70f77d8a4fbc67adf6 Mon Sep 17 00:00:00 2001 From: "Keith T. Garner" Date: Mon, 18 Nov 2019 10:08:22 -0600 Subject: [PATCH] add omit_from_final_grade to course config refs TALLY-301 test plan: - migration runs Change-Id: Ic8ba3cb0db3c722bd1e804dd28d2b3e6957b383f Reviewed-on: https://gerrit.instructure.com/c/rollcall-attendance/+/217597 Tested-by: Service Cloud Jenkins Reviewed-by: Simon Williams QA-Review: Spencer Olson Product-Review: Keith Garner --- config/application.rb | 2 + ..._omit_from_final_grade_to_course_config.rb | 44 +++++++++++++++++++ db/schema.rb | 3 +- lib/data_fixup/backfill_nulls.rb | 23 ++++++++++ spec/factories/factories.rb | 7 +++ spec/lib/data_fixup/backfill_nulls_spec.rb | 22 ++++++++++ 6 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20191118151647_add_omit_from_final_grade_to_course_config.rb create mode 100644 lib/data_fixup/backfill_nulls.rb create mode 100644 spec/lib/data_fixup/backfill_nulls_spec.rb diff --git a/config/application.rb b/config/application.rb index 8c42111..0e50990 100644 --- a/config/application.rb +++ b/config/application.rb @@ -48,6 +48,8 @@ class Application < Rails::Application config.action_dispatch.default_headers = { 'X-Frame-Options' => 'ALLOWALL' } + config.autoload_paths += %w[lib] + # Our deploy tooling exports a DATABASE_URL like: # mysql://user:pass@db:port/database, so handle that module MysqlProtocolResolver diff --git a/db/migrate/20191118151647_add_omit_from_final_grade_to_course_config.rb b/db/migrate/20191118151647_add_omit_from_final_grade_to_course_config.rb new file mode 100644 index 0000000..bf4881c --- /dev/null +++ b/db/migrate/20191118151647_add_omit_from_final_grade_to_course_config.rb @@ -0,0 +1,44 @@ +# +# Copyright (C) 2019 - present Instructure, Inc. +# +# This file is part of Rollcall. +# +# Rollcall is free software: you can redistribute it and/or modify it under +# the terms of the GNU Affero General Public License as published by the Free +# Software Foundation, version 3 of the License. +# +# Rollcall is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along +# with this program. If not, see . + +class AddOmitFromFinalGradeToCourseConfig < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def up + if ActiveRecord::ConnectionAdapters.const_defined?("Mysql2Adapter") && ActiveRecord::Base.connection.instance_of?(ActiveRecord::ConnectionAdapters::Mysql2Adapter) + # Look at https://dev.mysql.com/doc/refman/5.6/en/innodb-online-ddl-operations.html#online-ddl-column-operations + # for some additional information. We're trying to do the database changes in place without needing a copy table, + # the MySQL default. We're on a new enough MySQL that we should be able to the add the column, set the default, + # and set it NOT NULL all without a copy nor a lock. + execute <<~SQL + ALTER TABLE course_configs + ADD COLUMN omit_from_final_grade BOOL DEFAULT false NOT NULL, + ALGORITHM=INPLACE, + LOCK=NONE; + SQL + else + add_column :course_configs, :omit_from_final_grade, :boolean + change_column_default :course_configs, :omit_from_final_grade, false + DataFixup::BackfillNulls.run(CourseConfig, :omit_from_final_grade, new_value: false, batch_size: 1000) + change_column_null(:course_configs, :omit_from_final_grade, false) + end + end + + def down + remove_column :course_configs, :omit_from_final_grade + end +end diff --git a/db/schema.rb b/db/schema.rb index 8ec805a..63264a8 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.define(version: 2015_08_13_210811) do +ActiveRecord::Schema.define(version: 2019_11_18_151647) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -70,6 +70,7 @@ t.datetime "updated_at" t.string "view_preference" t.string "tool_consumer_instance_guid" + t.boolean "omit_from_final_grade", default: false, null: false t.index ["course_id", "tool_consumer_instance_guid"], name: "index_course_configs, uniquely", unique: true end diff --git a/lib/data_fixup/backfill_nulls.rb b/lib/data_fixup/backfill_nulls.rb new file mode 100644 index 0000000..aed215c --- /dev/null +++ b/lib/data_fixup/backfill_nulls.rb @@ -0,0 +1,23 @@ +# +# Copyright (C) 2019 - present Instructure, Inc. +# +# This file is part of Rollcall. +# +# Rollcall is free software: you can redistribute it and/or modify it under +# the terms of the GNU Affero General Public License as published by the Free +# Software Foundation, version 3 of the License. +# +# Rollcall is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along +# with this program. If not, see . + +module DataFixup::BackfillNulls + def self.run(klass, field, new_value:, batch_size: 1000) + objects_to_backfill = klass.where(field => nil) + objects_to_backfill.in_batches(of: batch_size).update_all(field => new_value, updated_at: Time.zone.now) + end +end diff --git a/spec/factories/factories.rb b/spec/factories/factories.rb index 136d942..9f1c4d6 100644 --- a/spec/factories/factories.rb +++ b/spec/factories/factories.rb @@ -60,4 +60,11 @@ cached_account {} descendant {} end + + factory :course_config do + course_id 1 + tardy_weight 0.8 + omit_from_final_grade false + tool_consumer_instance_guid "abc123" + end end diff --git a/spec/lib/data_fixup/backfill_nulls_spec.rb b/spec/lib/data_fixup/backfill_nulls_spec.rb new file mode 100644 index 0000000..62bd3b8 --- /dev/null +++ b/spec/lib/data_fixup/backfill_nulls_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe DataFixup::BackfillNulls do + context '.run' do + before do + @course1 = create(:course_config, course_id: 1234, tardy_weight: nil) + @course2 = create(:course_config, course_id: 1235, tardy_weight: 0.8) + end + + it 'replaces null values of "field" with the new value' do + expect { + DataFixup::BackfillNulls.run(CourseConfig, :tardy_weight, new_value: 0.5) + }.to change { @course1.reload.tardy_weight }.from(nil).to(0.5) + end + + it 'does not replace not-null values of "field"' do + expect { + DataFixup::BackfillNulls.run(CourseConfig, :tardy_weight, new_value: 0.5) + }.not_to change { @course2.reload.tardy_weight }.from(0.8) + end + end +end