fix/ensure that latepolicy has unique course_id

Closes CNVS-36864

Runs a fixup to clean up any LatePolicies that have the same course
id, keeping only the newest one. Adds a unique constraint on the db
and activerecord.

Test Plan
 - Before pulling code, create some LatePolicies in the console all
 with the same course.
 - Verify that you have multiple LatePolicies with the same course_id
 - Pull this code and run migrations
 - Check in the console to see if you still have LatePolicies that
 share a course_id with another LatePolicy
 - Attempt to create a LatePolicy that shares a course_id with
 another LatePolicy

Change-Id: I78fd49789dc7f235a399d261a6b6a1a30c42bb8e
Reviewed-on: https://gerrit.instructure.com/112152
Tested-by: Jenkins
Reviewed-by: Neil Gupta <ngupta@instructure.com>
Reviewed-by: Shahbaz Javeed <sjaveed@instructure.com>
QA-Review: Spencer Olson <solson@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
This commit is contained in:
Gary Mei 2017-05-17 16:01:20 -05:00
parent 72de5c3c38
commit 5d4e8ada04
4 changed files with 70 additions and 1 deletions

View File

@ -20,7 +20,8 @@ class LatePolicy < ActiveRecord::Base
belongs_to :course, inverse_of: :late_policy
validates :course_id,
presence: true
presence: true,
uniqueness: true
validates :late_submission_minimum_percent, :missing_submission_deduction, :late_submission_deduction,
presence: true,
numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: 100 }

View File

@ -0,0 +1,35 @@
#
# Copyright (C) 2017 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas 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.
#
# Canvas 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 <http://www.gnu.org/licenses/>.
class MakeLatePolicyUnique < ActiveRecord::Migration[4.2]
# running fixup as a predeploy and add_index without concern of lockup
# because we're certain the tables are small. Only one endpoint has been
# exposed, in beta, and that endpoint is behind a feature flag. The only way a
# duplicate LatePolicy can be created at this point is through a rails console
tag :predeploy
def change
reversible do |dir|
dir.up do
DataFixup::MakeLatePolicyUnique.run
end
end
remove_index :late_policies, column: :course_id
add_index :late_policies, :course_id, unique: true
end
end

View File

@ -0,0 +1,25 @@
#
# Copyright (C) 2017 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas 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.
#
# Canvas 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 <http://www.gnu.org/licenses/>.
module DataFixup
module MakeLatePolicyUnique
def self.run
duplicates = LatePolicy.group(:course_id).having("count(id) > 1").pluck("course_id, max(id)").transpose
LatePolicy.where(course_id: duplicates[0]).where.not(id: duplicates[1]).delete_all
end
end
end

View File

@ -52,6 +52,14 @@ describe LatePolicy do
# Inclusion
it { is_expected.to validate_inclusion_of(:late_submission_interval).in_array(['day', 'hour']) }
# Uniqueness
describe 'uniqueness' do
subject { course.create_late_policy }
let(:course) { Course.create! }
it { is_expected.to validate_uniqueness_of(:course_id) }
end
end
describe 'default values' do