Skip to content

Commit

Permalink
don't enable double blind grader anonymity by default
Browse files Browse the repository at this point in the history 
fixes GRADE-1205

test plan:
* BEFORE checking out this patchset, make sure you have some assignments
  in your database
* check out this patchset and run migrations
* check your database, all assignment rows should be updated so that
  grader_comments_visible_to_graders and grader_names_visible_to_final_grader
  are true

Change-Id: I2750375838b9cadba490accc190e9abcbd7d9b25
Reviewed-on: https://gerrit.instructure.com/152296
Tested-by: Jenkins
Reviewed-by: Adrian Packel <[email protected]>
Reviewed-by: Spencer Olson <[email protected]>
QA-Review: Adrian Packel <[email protected]>
Product-Review: Keith T. Garner <[email protected]>
  • Loading branch information
@neilgupta
neilgupta committed Jun 6, 2018
1 parent 0bcd57a commit adf95bc
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 148 deletions.
10 changes: 5 additions & 5 deletions app/models/assignment.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,10 @@ class Assignment < ActiveRecord::Base
self.muted = true if moderated_grading? && anonymous_moderated_marking?
end

validates :graders_anonymous_to_graders, absence: true, unless: :anonymous_grading?

with_options unless: :moderated_grading? do
validates :grader_comments_visible_to_graders, absence: true
validates :graders_anonymous_to_graders, absence: true
validates :grader_section, absence: true
validates :final_grader, absence: true
validates :grader_names_visible_to_final_grader, absence: true
end

with_options if: -> { moderated_grading? && anonymous_moderated_marking? } do
Expand Down Expand Up @@ -2817,7 +2814,7 @@ def can_view_other_grader_identities?(user)
return true if context.account_membership_allows(user, :select_final_grade)
return false unless grader_comments_visible_to_graders?

!(anonymous_grading? && graders_anonymous_to_graders?)
!graders_anonymous_to_graders?
end

def can_view_other_grader_comments?(user)
Expand Down Expand Up @@ -2923,6 +2920,9 @@ def anonymous_moderated_marking?
def clear_moderated_grading_attributes(assignment)
assignment.final_grader_id = nil
assignment.grader_count = nil
assignment.grader_names_visible_to_final_grader = true
assignment.grader_comments_visible_to_graders = true
assignment.graders_anonymous_to_graders = false
end

def create_moderation_grader_if_needed(grader:)
Expand Down
30 changes: 30 additions & 0 deletions db/migrate/20180601142716_fix_grader_visibility_data.rb
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#
# Copyright (C) 2018 - 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 FixGraderVisibilityData < ActiveRecord::Migration[5.0]
tag :postdeploy

def self.up
DataFixup::FixGraderVisibilityData.send_later_if_production_enqueue_args(
:run,
{
priority: Delayed::LOW_PRIORITY,
n_strand: ["DataFixup::FixGraderVisibilityData", Shard.current.database_server.id]
}
)
end
end
30 changes: 30 additions & 0 deletions db/migrate/20180601153016_change_anonymous_grading_defaults.rb
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#
# Copyright (C) 2018 - 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 ChangeAnonymousGradingDefaults < ActiveRecord::Migration[5.0]
tag :predeploy

def self.up
change_column_default :assignments, :grader_comments_visible_to_graders, true
change_column_default :assignments, :grader_names_visible_to_final_grader, true
end

def self.down
change_column_default :assignments, :grader_comments_visible_to_graders, false
change_column_default :assignments, :grader_names_visible_to_final_grader, false
end
end
27 changes: 27 additions & 0 deletions lib/data_fixup/fix_grader_visibility_data.rb
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#
# Copyright (C) 2018 - 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::FixGraderVisibilityData
def self.run
Assignment.where("grader_comments_visible_to_graders IS NOT TRUE AND grader_names_visible_to_final_grader IS NOT TRUE").
in_batches.update_all(
grader_comments_visible_to_graders: true,
grader_names_visible_to_final_grader: true
)
end
end
152 changes: 9 additions & 143 deletions spec/models/assignment_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,130 +208,6 @@
end
end

describe "default values for boolean attributes" do
before(:once) do
@assignment = @course.assignments.create!
end

let(:values) do
Assignment.where(id: @assignment).pluck(
:could_be_locked,
:grade_group_students_individually,
:anonymous_peer_reviews,
:turnitin_enabled,
:vericite_enabled,
:anonymous_grading,
:moderated_grading,
:omit_from_final_grade,
:freeze_on_copy,
:copied,
:only_visible_to_overrides,
:post_to_sis,
:peer_reviews_assigned,
:peer_reviews,
:automatic_peer_reviews,
:muted,
:intra_group_peer_reviews
).first
end

it "saves boolean attributes as false if they are set to nil" do
@assignment.update!(
could_be_locked: nil,
grade_group_students_individually: nil,
anonymous_peer_reviews: nil,
turnitin_enabled: nil,
vericite_enabled: nil,
moderated_grading: nil,
omit_from_final_grade: nil,
freeze_on_copy: nil,
copied: nil,
only_visible_to_overrides: nil,
post_to_sis: nil,
peer_reviews_assigned: nil,
peer_reviews: nil,
automatic_peer_reviews: nil,
muted: nil,
intra_group_peer_reviews: nil,
anonymous_grading: nil,
graders_anonymous_to_graders: nil,
)

expect(values).to eq([false] * values.length)
end

it "saves boolean attributes as false if they are set to false" do
@assignment.update!(
could_be_locked: false,
grade_group_students_individually: false,
anonymous_peer_reviews: false,
turnitin_enabled: false,
vericite_enabled: false,
moderated_grading: false,
omit_from_final_grade: false,
freeze_on_copy: false,
copied: false,
only_visible_to_overrides: false,
post_to_sis: false,
peer_reviews_assigned: false,
peer_reviews: false,
automatic_peer_reviews: false,
muted: false,
intra_group_peer_reviews: false,
anonymous_grading: false,
graders_anonymous_to_graders: false,
)

expect(values).to eq([false] * values.length)
end

it "saves boolean attributes as true if they are set to true" do
# exluding the moderated_grading attribute because it cannot be
# true when peer_reviews is true
@assignment.update!(
could_be_locked: true,
grade_group_students_individually: true,
anonymous_peer_reviews: true,
turnitin_enabled: true,
vericite_enabled: true,
omit_from_final_grade: true,
freeze_on_copy: true,
copied: true,
only_visible_to_overrides: true,
post_to_sis: true,
peer_reviews_assigned: true,
peer_reviews: true,
automatic_peer_reviews: true,
muted: true,
intra_group_peer_reviews: true,
anonymous_grading: true,
graders_anonymous_to_graders: true,
)

values = Assignment.where(id: @assignment).pluck(
:could_be_locked,
:grade_group_students_individually,
:anonymous_peer_reviews,
:turnitin_enabled,
:vericite_enabled,
:omit_from_final_grade,
:freeze_on_copy,
:copied,
:only_visible_to_overrides,
:post_to_sis,
:peer_reviews_assigned,
:peer_reviews,
:automatic_peer_reviews,
:muted,
:intra_group_peer_reviews,
:anonymous_grading,
:graders_anonymous_to_graders
).first

expect(values).to eq([true] * values.length)
end
end

describe "scope: expects_submissions" do
it 'includes assignments expecting online submissions' do
assignment_model(submission_types: "online_text_entry,online_url,online_upload", course: @course)
Expand Down Expand Up @@ -662,7 +538,7 @@
@course.enroll_ta(ta, enrollment_state: 'active')
ta
end
let_once(:assignment) { @course.assignments.create!(final_grader: @teacher, grader_count: 2, moderated_grading: true) }
let_once(:assignment) { @course.assignments.create!(final_grader: @teacher, grader_count: 2, moderated_grading: true, anonymous_grading: true) }

shared_examples "grader comment hiding does not apply" do
it 'returns true when the user has permission to manage grades' do
Expand Down Expand Up @@ -697,6 +573,14 @@
it_behaves_like "grader comment hiding does not apply"
end

context 'when the assignment is not anonymously graded' do
before :once do
assignment.update!(anonymous_grading: false)
end

it_behaves_like "grader comment hiding does not apply"
end

context 'when grader comments are visible to other graders' do
before :once do
assignment.update!(
Expand Down Expand Up @@ -6368,28 +6252,12 @@ def assignment(group_category = nil)
assignment_model(course: @course)
end

describe 'Anonymous Grading validation' do
context 'when anonymous_grading is not enabled' do
subject { @course.assignments.build }

it { is_expected.to validate_absence_of(:graders_anonymous_to_graders) }
end

context 'when anonymous_grading is enabled' do
subject { @course.assignments.build(anonymous_grading: true) }

it { is_expected.not_to validate_absence_of(:graders_anonymous_to_graders) }
end
end

describe 'Moderated Grading validation' do
context 'when moderated_grading is not enabled' do
subject(:assignment) { @course.assignments.build }

it { is_expected.to validate_absence_of(:grader_comments_visible_to_graders) }
it { is_expected.to validate_absence_of(:grader_section) }
it { is_expected.to validate_absence_of(:final_grader) }
it { is_expected.to validate_absence_of(:grader_names_visible_to_final_grader) }

it 'before validation, sets final_grader_id to nil if it is present' do
teacher = User.create!
Expand Down Expand Up @@ -6427,8 +6295,6 @@ def assignment(group_category = nil)
subject { @course.assignments.create(moderated_grading: true, grader_count: 1, final_grader: @section1_ta) }

it { is_expected.to be_muted }
it { is_expected.not_to validate_absence_of(:grader_comments_visible_to_graders) }
it { is_expected.not_to validate_absence_of(:grader_names_visible_to_final_grader) }
it { is_expected.to validate_numericality_of(:grader_count).is_greater_than(0) }
end

Expand Down

0 comments on commit adf95bc

Please sign in to comment.