ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

ensure graded assignments aren't gradable after removal from overrides 

closes GRADE-124

test plan:
* Create a differentiated assignment
* Submit the assignment from two students
* Assign grades to all but one student
* Unassign the assignment from a graded student and the ungraded
  student
* Go back to gradebook
* Verify that the cell for the graded student is grayed out and not
  editable
* Verify that the cell for the ungraded student is grayed out and
  not editable

Change-Id: I93c9a2f4dec0888d56ba39682bc2184196f25bba
Reviewed-on: https://gerrit.instructure.com/122731
Reviewed-by: Spencer Olson <solson@instructure.com>
Reviewed-by: Matt Taylor <mtaylor@instructure.com>
QA-Review: KC Naegle <knaegle@instructure.com>
Tested-by: Jenkins
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Matt Goodwin <mattg@instructure.com>
This commit is contained in:
Shahbaz Javeed 2017-08-14 18:35:38 -04:00
parent 12f57bbdfb
commit e966615880
19 changed files with 370 additions and 161 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/coffeescripts/gradezilla/Gradebook.coffee
View File

@ -928,7 +928,7 @@ define [
@submissionsForStudent(student), @submissionsForStudent(student),
@assignmentGroups, @assignmentGroups,
@options.group_weighting_scheme, @options.group_weighting_scheme,
@gradingPeriodSet if hasGradingPeriods, (@gradingPeriodSet if hasGradingPeriods),
EffectiveDueDates.scopeToUser(@effectiveDueDates, student.id) if hasGradingPeriods EffectiveDueDates.scopeToUser(@effectiveDueDates, student.id) if hasGradingPeriods
) )

9
app/controllers/submissions_api_controller.rb
View File

@ -642,13 +642,12 @@ class SubmissionsApiController < ApplicationController
@assignment = @context.assignments.active.find(params[:assignment_id]) @assignment = @context.assignments.active.find(params[:assignment_id])
@user = get_user_considering_section(params[:user_id]) @user = get_user_considering_section(params[:user_id])
authorized = false @submission = @assignment.all_submissions.find_or_create_by!(user: @user)
@submission = @assignment.submissions.find_or_create_by!(user: @user)
if params[:submission] || params[:rubric_assessment] authorized = if params[:submission] || params[:rubric_assessment]
authorized = authorized_action(@submission, @current_user, :grade) authorized_action(@submission, @current_user, :grade)
else else
authorized = authorized_action(@submission, @current_user, :comment) authorized_action(@submission, @current_user, :comment)
end end
if authorized if authorized

2
app/models/assignment_override.rb
View File

@ -124,7 +124,7 @@ class AssignmentOverride < ActiveRecord::Base
if due_at_overridden_changed? || set_id_changed? || if due_at_overridden_changed? || set_id_changed? ||
(set_type_changed? && set_type != 'ADHOC') || (set_type_changed? && set_type != 'ADHOC') ||
(due_at_overridden && due_at_changed?) || (due_at_overridden && due_at_changed?) ||
(due_at_overridden && workflow_state_changed?) workflow_state_changed?
DueDateCacher.recompute(assignment) DueDateCacher.recompute(assignment)
end end
end end

10
app/models/submission.rb
View File

@ -46,6 +46,10 @@ class Submission < ActiveRecord::Base
assignment_in_closed_grading_period: { assignment_in_closed_grading_period: {
status: false, status: false,
message: I18n.t('This assignment is in a closed grading period for this student') message: I18n.t('This assignment is in a closed grading period for this student')
}.freeze,
not_applicable: {
status: false,
message: I18n.t('This assignment is not applicable to this student')
}.freeze }.freeze
}.freeze }.freeze
@ -1368,11 +1372,10 @@ class Submission < ActiveRecord::Base
private :can_autograde? private :can_autograde?
def can_autograde_symbolic_status def can_autograde_symbolic_status
return :not_applicable if deleted?
return :unpublished unless assignment.published? return :unpublished unless assignment.published?
return :not_autograded if grader_id >= 0 return :not_autograded if grader_id >= 0
student_id = user_id || self.user.try(:id)
if grading_period&.closed? if grading_period&.closed?
:assignment_in_closed_grading_period :assignment_in_closed_grading_period
else else
@ -1395,12 +1398,11 @@ class Submission < ActiveRecord::Base
def can_grade_symbolic_status(user = nil) def can_grade_symbolic_status(user = nil)
user ||= grader user ||= grader
return :not_applicable if deleted?
return :unpublished unless assignment.published? return :unpublished unless assignment.published?
return :cant_manage_grades unless context.grants_right?(user, nil, :manage_grades) return :cant_manage_grades unless context.grants_right?(user, nil, :manage_grades)
return :account_admin if context.account_membership_allows(user) return :account_admin if context.account_membership_allows(user)
student_id = self.user_id || self.user.try(:id)
if grading_period&.closed? if grading_period&.closed?
:assignment_in_closed_grading_period :assignment_in_closed_grading_period
else else

241
db/migrate/20170815103759_ignore_deleted_submissions_for_assignment_visibility.rb Normal file
View File

@ -0,0 +1,241 @@
#
# 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 IgnoreDeletedSubmissionsForAssignmentVisibility < ActiveRecord::Migration[5.0]
tag :postdeploy
def up
self.connection.execute "DROP VIEW #{connection.quote_table_name('assignment_student_visibilities')}"
self.connection.execute "DROP VIEW #{connection.quote_table_name('quiz_student_visibilities')}"
# Update the view so submission scores aren't as important as the submission's workflow_state
self.connection.execute %(CREATE VIEW #{connection.quote_table_name('assignment_student_visibilities')} AS
SELECT DISTINCT a.id as assignment_id,
e.user_id as user_id,
c.id as course_id
FROM #{Assignment.quoted_table_name} a
JOIN #{Course.quoted_table_name} c
ON a.context_id = c.id
AND a.context_type = 'Course'
JOIN #{Enrollment.quoted_table_name} e
ON e.course_id = c.id
AND e.type IN ('StudentEnrollment', 'StudentViewEnrollment')
AND e.workflow_state != 'deleted'
JOIN #{CourseSection.quoted_table_name} cs
ON cs.course_id = c.id
AND e.course_section_id = cs.id
LEFT JOIN #{GroupMembership.quoted_table_name} gm
ON gm.user_id = e.user_id
AND gm.workflow_state = 'accepted'
LEFT JOIN #{Group.quoted_table_name} g
ON g.context_type = 'Course'
AND g.context_id = c.id
AND g.workflow_state = 'available'
AND gm.group_id = g.id
LEFT JOIN #{AssignmentOverrideStudent.quoted_table_name} aos
ON aos.assignment_id = a.id
AND aos.user_id = e.user_id
LEFT JOIN #{AssignmentOverride.quoted_table_name} ao
ON ao.assignment_id = a.id
AND ao.workflow_state = 'active'
AND (
(ao.set_type = 'CourseSection' AND ao.set_id = cs.id)
OR (ao.set_type = 'ADHOC' AND ao.set_id IS NULL AND ao.id = aos.assignment_override_id)
OR (ao.set_type = 'Group' AND ao.set_id = g.id)
)
LEFT JOIN #{Submission.quoted_table_name} s
ON s.user_id = e.user_id
AND s.assignment_id = a.id
AND s.workflow_state != 'deleted'
WHERE a.workflow_state NOT IN ('deleted','unpublished')
AND(
( a.only_visible_to_overrides = 'true' AND (ao.id IS NOT NULL OR s.id IS NOT NULL))
OR (COALESCE(a.only_visible_to_overrides, 'false') = 'false')
)
)
self.connection.execute %(CREATE VIEW #{connection.quote_table_name('quiz_student_visibilities')} AS
SELECT DISTINCT q.id as quiz_id,
e.user_id as user_id,
c.id as course_id
FROM #{Quizzes::Quiz.quoted_table_name} q
JOIN #{Course.quoted_table_name} c
ON q.context_id = c.id
AND q.context_type = 'Course'
JOIN #{Enrollment.quoted_table_name} e
ON e.course_id = c.id
AND e.type IN ('StudentEnrollment', 'StudentViewEnrollment')
AND e.workflow_state != 'deleted'
JOIN #{CourseSection.quoted_table_name} cs
ON cs.course_id = c.id
AND e.course_section_id = cs.id
LEFT JOIN #{AssignmentOverrideStudent.quoted_table_name} aos
ON aos.quiz_id = q.id
AND aos.user_id = e.user_id
LEFT JOIN #{AssignmentOverride.quoted_table_name} ao
ON ao.quiz_id = q.id
AND ao.workflow_state = 'active'
AND (
(ao.set_type = 'CourseSection' AND ao.set_id = cs.id)
OR (ao.set_type = 'ADHOC' AND ao.set_id IS NULL AND ao.id = aos.assignment_override_id)
)
LEFT JOIN #{Assignment.quoted_table_name} a
ON a.context_id = q.context_id
AND a.submission_types LIKE 'online_quiz'
AND a.id = q.assignment_id
LEFT JOIN #{Submission.quoted_table_name} s
ON s.user_id = e.user_id
AND s.assignment_id = a.id
AND s.workflow_state != 'deleted'
WHERE q.workflow_state NOT IN ('deleted','unpublished')
AND(
( q.only_visible_to_overrides = 'true' AND (ao.id IS NOT NULL OR s.id IS NOT NULL))
OR (COALESCE(q.only_visible_to_overrides, 'false') = 'false')
)
)
end
def down
self.connection.execute "DROP VIEW #{connection.quote_table_name('assignment_student_visibilities')}"
self.connection.execute "DROP VIEW #{connection.quote_table_name('quiz_student_visibilities')}"
# Recreate the old view where submission scores are significant
self.connection.execute %(CREATE VIEW #{connection.quote_table_name('assignment_student_visibilities')} AS
SELECT DISTINCT a.id as assignment_id,
e.user_id as user_id,
c.id as course_id
FROM #{Assignment.quoted_table_name} a
JOIN #{Course.quoted_table_name} c
ON a.context_id = c.id
AND a.context_type = 'Course'
JOIN #{Enrollment.quoted_table_name} e
ON e.course_id = c.id
AND e.type IN ('StudentEnrollment', 'StudentViewEnrollment')
AND e.workflow_state != 'deleted'
JOIN #{CourseSection.quoted_table_name} cs
ON cs.course_id = c.id
AND e.course_section_id = cs.id
LEFT JOIN #{GroupMembership.quoted_table_name} gm
ON gm.user_id = e.user_id
AND gm.workflow_state = 'accepted'
LEFT JOIN #{Group.quoted_table_name} g
ON g.context_type = 'Course'
AND g.context_id = c.id
AND g.workflow_state = 'available'
AND gm.group_id = g.id
LEFT JOIN #{AssignmentOverrideStudent.quoted_table_name} aos
ON aos.assignment_id = a.id
AND aos.user_id = e.user_id
LEFT JOIN #{AssignmentOverride.quoted_table_name} ao
ON ao.assignment_id = a.id
AND ao.workflow_state = 'active'
AND (
(ao.set_type = 'CourseSection' AND ao.set_id = cs.id)
OR (ao.set_type = 'ADHOC' AND ao.set_id IS NULL AND ao.id = aos.assignment_override_id)
OR (ao.set_type = 'Group' AND ao.set_id = g.id)
)
LEFT JOIN #{Submission.quoted_table_name} s
ON s.user_id = e.user_id
AND s.assignment_id = a.id
AND s.score IS NOT NULL
WHERE a.workflow_state NOT IN ('deleted','unpublished')
AND(
( a.only_visible_to_overrides = 'true' AND (ao.id IS NOT NULL OR s.id IS NOT NULL))
OR (COALESCE(a.only_visible_to_overrides, 'false') = 'false')
)
)
self.connection.execute %(CREATE VIEW #{connection.quote_table_name('quiz_student_visibilities')} AS
SELECT DISTINCT q.id as quiz_id,
e.user_id as user_id,
c.id as course_id
FROM #{Quizzes::Quiz.quoted_table_name} q
JOIN #{Course.quoted_table_name} c
ON q.context_id = c.id
AND q.context_type = 'Course'
JOIN #{Enrollment.quoted_table_name} e
ON e.course_id = c.id
AND e.type IN ('StudentEnrollment', 'StudentViewEnrollment')
AND e.workflow_state != 'deleted'
JOIN #{CourseSection.quoted_table_name} cs
ON cs.course_id = c.id
AND e.course_section_id = cs.id
LEFT JOIN #{AssignmentOverrideStudent.quoted_table_name} aos
ON aos.quiz_id = q.id
AND aos.user_id = e.user_id
LEFT JOIN #{AssignmentOverride.quoted_table_name} ao
ON ao.quiz_id = q.id
AND ao.workflow_state = 'active'
AND (
(ao.set_type = 'CourseSection' AND ao.set_id = cs.id)
OR (ao.set_type = 'ADHOC' AND ao.set_id IS NULL AND ao.id = aos.assignment_override_id)
)
LEFT JOIN #{Assignment.quoted_table_name} a
ON a.context_id = q.context_id
AND a.submission_types LIKE 'online_quiz'
AND a.id = q.assignment_id
LEFT JOIN #{Submission.quoted_table_name} s
ON s.user_id = e.user_id
AND s.assignment_id = a.id
AND s.score IS NOT NULL
WHERE q.workflow_state NOT IN ('deleted','unpublished')
AND(
( q.only_visible_to_overrides = 'true' AND (ao.id IS NOT NULL OR s.id IS NOT NULL))
OR (COALESCE(q.only_visible_to_overrides, 'false') = 'false')
)
)
end
end

25
lib/effective_due_dates.rb
View File

@ -286,29 +286,6 @@ class EffectiveDueDates
#{filter_students_sql('e')} #{filter_students_sql('e')}
), ),
-- fetch all students who have graded submissions
-- because if the student received a grade, they
-- shouldn't lose visibility to the assignment
override_submissions_students AS (
SELECT
s.user_id AS student_id,
s.assignment_id,
NULL::integer AS override_id,
NULL::timestamp AS due_at,
'Submission'::varchar AS override_type,
FALSE AS due_at_overridden,
3 AS priority
FROM
models a
INNER JOIN #{Submission.quoted_table_name} s ON s.assignment_id = a.id
INNER JOIN #{Enrollment.quoted_table_name} e ON e.course_id = a.context_id AND e.user_id = s.user_id
WHERE
(s.grade IS NOT NULL OR s.excused) AND
e.workflow_state NOT IN ('rejected', 'deleted') AND
e.type IN ('StudentEnrollment', 'StudentViewEnrollment')
#{filter_students_sql('s')}
),
-- join all these students together into a single table -- join all these students together into a single table
override_all_students AS ( override_all_students AS (
SELECT * FROM override_adhoc_students SELECT * FROM override_adhoc_students
@ -318,8 +295,6 @@ class EffectiveDueDates
SELECT * FROM override_sections_students SELECT * FROM override_sections_students
UNION ALL UNION ALL
SELECT * FROM override_everyonelse_students SELECT * FROM override_everyonelse_students
UNION ALL
SELECT * FROM override_submissions_students
), ),
-- and pick the latest override date as the effective due date -- and pick the latest override date as the effective due date

39
spec/apis/v1/submissions_api_spec.rb
View File

@ -1690,18 +1690,6 @@ describe 'Submissions API', type: :request do
expect(json.size).to eq 0 expect(json.size).to eq 0
end end
it "returns the graded submisson even if the student is not in the overriden section" do
@assignment.grade_student(@student, grade: 5, grader: @teacher)
Score.where(enrollment_id: @student.enrollments).delete_all
@student.enrollments.each(&:destroy_permanently!)
student_in_section(@section2, user: @student)
json = call_to_for_students(as_student: false)
expect(json.size).to eq 1
json.each { |submission| expect(submission['user_id']).to eq @student.id }
end
end end
end end
end end
@ -2208,6 +2196,33 @@ describe 'Submissions API', type: :request do
expect(json['score']).to eq 12.9 expect(json['score']).to eq 12.9
end end
it "doesn't allow grading a deleted submission" do
submission = @assignment.submissions.find_by(user: @student)
submission.destroy
api_call(
:put,
"/api/v1/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{@student.id}.json",
{
controller: 'submissions_api',
action: 'update',
format: 'json',
course_id: @course.id.to_s,
assignment_id: @assignment.id.to_s,
user_id: @student.id.to_s
}, {
submission: {
posted_grade: 'B'
},
}, {
expected_status: :unauthorized
}
)
expect(submission.score).to be_nil
expect(submission.grade).to be_nil
end
it "can excuse assignments" do it "can excuse assignments" do
json = api_call( json = api_call(
:put, :put,

48
spec/lib/effective_due_dates_spec.rb
View File

@ -656,23 +656,12 @@ describe Course do
expect(edd.to_hash).to eq({}) expect(edd.to_hash).to eq({})
end end
it 'includes not-assigned students with existing graded submissions' do it 'ignores not-assigned students with existing graded submissions' do
@assignment1.grade_student(@student1, grade: 5, grader: @teacher) @assignment1.grade_student(@student1, grade: 5, grader: @teacher)
edd = EffectiveDueDates.for_course(@test_course, @assignment1) edd = EffectiveDueDates.for_course(@test_course, @assignment1)
result = edd.to_hash result = edd.to_hash
expected = { expect(result).to be_empty
@assignment1.id => {
@student1.id => {
due_at: nil,
grading_period_id: nil,
in_closed_grading_period: false,
override_id: nil,
override_source: 'Submission'
}
}
}
expect(result).to eq expected
end end
it 'uses assigned date instead of submission date even if submission was late' do it 'uses assigned date instead of submission date even if submission was late' do
@ -1440,39 +1429,6 @@ describe Course do
} }
expect(result).to eq expected expect(result).to eq expected
end end
it 'is false if the due date is only due to a graded submission even if the last grading period is closed' do
Factories::GradingPeriodHelper.new.create_for_group(@gp_group, {
start_date: 50.days.ago(@now),
end_date: 35.days.ago(@now),
close_date: 30.days.from_now(@now)
})
Factories::GradingPeriodHelper.new.create_for_group(@gp_group, {
start_date: 20.days.ago(@now),
end_date: 15.days.ago(@now),
close_date: 10.days.ago(@now)
})
@assignment1.grade_student(@student1, grade: 5, grader: @teacher)
@assignment1.submissions.find_by!(user: @student1).update!(
submitted_at: 1.week.from_now(@now),
submission_type: 'online_text_entry'
)
edd = EffectiveDueDates.for_course(@test_course, @assignment1)
result = edd.to_hash
expected = {
@assignment1.id => {
@student1.id => {
due_at: nil,
grading_period_id: nil,
in_closed_grading_period: false,
override_id: nil,
override_source: 'Submission'
}
}
}
expect(result).to eq expected
end
end end
end end
end end

33
spec/lib/grade_calculator_spec.rb
View File

@ -1174,27 +1174,32 @@ describe GradeCalculator do
set_up_course_for_differentiated_assignments set_up_course_for_differentiated_assignments
end end
it "should calculate scores based on visible assignments only" do it "should calculate scores based on visible assignments only" do
# 5 + 15 + 10 + 20 + 10 # Non-overridden assignments are not visible to this student at all even though she's been graded on them
expect(final_grade_info(@user, @course)[:total]).to eq 60 # because the assignment is only visible to overrides. Therefore only the (first three) overridden assignments
expect(final_grade_info(@user, @course)[:possible]).to eq 100 # ever count towards her final grade in all these specs
# 5 + 15 + 10
expect(final_grade_info(@user, @course)[:total]).to eq 30
expect(final_grade_info(@user, @course)[:possible]).to eq 60
end end
it "should drop the lowest visible when that rule is in place" do it "should drop the lowest visible when that rule is in place" do
@group.update_attribute(:rules, 'drop_lowest:1') @group.update_attribute(:rules, 'drop_lowest:1') # rubocop:disable Rails/SkipsModelValidations
# 5 + 15 + 10 + 20 + 10 - 5 # 5 + 15 + 10 - 5
expect(final_grade_info(@user, @course)[:total]).to eq 55 expect(final_grade_info(@user, @course)[:total]).to eq 25
expect(final_grade_info(@user, @course)[:possible]).to eq 80 expect(final_grade_info(@user, @course)[:possible]).to eq 40
end end
it "should drop the highest visible when that rule is in place" do it "should drop the highest visible when that rule is in place" do
@group.update_attribute(:rules, 'drop_highest:1') @group.update_attribute(:rules, 'drop_highest:1') # rubocop:disable Rails/SkipsModelValidations
# 5 + 15 + 10 + 20 + 10 - 20 # 5 + 15 + 10 - 15
expect(final_grade_info(@user, @course)[:total]).to eq 40 expect(final_grade_info(@user, @course)[:total]).to eq 15
expect(final_grade_info(@user, @course)[:possible]).to eq 80 expect(final_grade_info(@user, @course)[:possible]).to eq 40
end end
it "should not count an invisible assignment with never drop on" do it "should not count an invisible assignment with never drop on" do
# rubocop:disable Rails/SkipsModelValidations
@group.update_attribute(:rules, "drop_lowest:2\nnever_drop:#{@overridden_lowest.id}") @group.update_attribute(:rules, "drop_lowest:2\nnever_drop:#{@overridden_lowest.id}")
# 5 + 15 + 10 + 20 + 10 - 10 - 10 # rubocop:enable Rails/SkipsModelValidations
expect(final_grade_info(@user, @course)[:total]).to eq 40 # 5 + 15 + 10 - 10
expect(final_grade_info(@user, @course)[:possible]).to eq 60 expect(final_grade_info(@user, @course)[:total]).to eq 20
expect(final_grade_info(@user, @course)[:possible]).to eq 40
end end
end end
end end

12
spec/lib/gradebook_user_ids_spec.rb
View File

@ -462,9 +462,8 @@ describe GradebookUserIds do
it "places students that are not assigned at the end, but before fake students" do it "places students that are not assigned at the end, but before fake students" do
@assignment.update!(only_visible_to_overrides: true) @assignment.update!(only_visible_to_overrides: true)
create_adhoc_override_for_assignment(@assignment, [@student1, @student3, @student2], due_at: nil) create_adhoc_override_for_assignment(@assignment, [@student1, @student3, @student2], due_at: nil)
student5 = student_in_course(course: @course, active_all: true).user
expect(gradebook_user_ids.user_ids).to eq( expect(gradebook_user_ids.user_ids).to eq(
[@student2.id, @student1.id, @student4.id, @student3.id, student5.id, @fake_student.id] [@student2.id, @student1.id, @student3.id, @student4.id, @fake_student.id]
) )
end end
end end
@ -495,9 +494,8 @@ describe GradebookUserIds do
it "places students that are not assigned at the end, but before fake students" do it "places students that are not assigned at the end, but before fake students" do
@assignment.update!(only_visible_to_overrides: true) @assignment.update!(only_visible_to_overrides: true)
create_adhoc_override_for_assignment(@assignment, [@student1, @student3, @student2], due_at: nil) create_adhoc_override_for_assignment(@assignment, [@student1, @student3, @student2], due_at: nil)
student5 = student_in_course(course: @course, active_all: true).user
expect(gradebook_user_ids.user_ids).to eq( expect(gradebook_user_ids.user_ids).to eq(
[@student2.id, @student1.id, @student3.id, @student4.id, student5.id, @fake_student.id] [@student2.id, @student1.id, @student3.id, @student4.id, @fake_student.id]
) )
end end
end end
@ -541,9 +539,8 @@ describe GradebookUserIds do
it "places students that are not assigned at the end, but before fake students" do it "places students that are not assigned at the end, but before fake students" do
@assignment.update!(only_visible_to_overrides: true) @assignment.update!(only_visible_to_overrides: true)
create_adhoc_override_for_assignment(@assignment, [@student1, @student3, @student2], due_at: nil) create_adhoc_override_for_assignment(@assignment, [@student1, @student3, @student2], due_at: nil)
student5 = student_in_course(course: @course, active_all: true).user
expect(gradebook_user_ids.user_ids).to eq( expect(gradebook_user_ids.user_ids).to eq(
[@student3.id, @student4.id, @student1.id, @student2.id, student5.id, @fake_student.id] [@student3.id, @student1.id, @student2.id, @student4.id, @fake_student.id]
) )
end end
end end
@ -576,9 +573,8 @@ describe GradebookUserIds do
it "places students that are not assigned at the end, but before fake students" do it "places students that are not assigned at the end, but before fake students" do
@assignment.update!(only_visible_to_overrides: true) @assignment.update!(only_visible_to_overrides: true)
create_adhoc_override_for_assignment(@assignment, [@student1, @student3, @student2], due_at: nil) create_adhoc_override_for_assignment(@assignment, [@student1, @student3, @student2], due_at: nil)
student5 = student_in_course(course: @course, active_all: true).user
expect(gradebook_user_ids.user_ids).to eq( expect(gradebook_user_ids.user_ids).to eq(
[@student4.id, @student3.id, @student1.id, @student2.id, student5.id, @fake_student.id] [@student3.id, @student1.id, @student2.id, @student4.id, @fake_student.id]
) )
end end
end end

12
spec/models/assignment_override_spec.rb
View File

@ -727,24 +727,24 @@ describe AssignmentOverride do
@override.save @override.save
end end
it "does not trigger when non-applicable override is created" do it "triggers when override without a due_date is created" do
expect(DueDateCacher).to receive(:recompute).never expect(DueDateCacher).to receive(:recompute)
@assignment.assignment_overrides.create @assignment.assignment_overrides.create
end end
it "does not trigger when non-applicable override deleted" do it "triggers when override without a due_date deleted" do
@override.clear_due_at_override @override.clear_due_at_override
@override.save @override.save
expect(DueDateCacher).to receive(:recompute).never expect(DueDateCacher).to receive(:recompute)
@override.destroy @override.destroy
end end
it "does not trigger when non-applicable override undeleted" do it "triggers when override without a due_date undeleted" do
@override.clear_due_at_override @override.clear_due_at_override
@override.destroy @override.destroy
expect(DueDateCacher).to receive(:recompute).never expect(DueDateCacher).to receive(:recompute)
@override.workflow_state = 'active' @override.workflow_state = 'active'
@override.save @override.save
end end

6
spec/models/assignment_override_student_spec.rb
View File

@ -83,7 +83,11 @@ describe AssignmentOverrideStudent do
it 'on destroy, recalculates cached due dates on the assignment' do it 'on destroy, recalculates cached due dates on the assignment' do
override_student = @assignment_override.assignment_override_students.create!(user: @student) override_student = @assignment_override.assignment_override_students.create!(user: @student)
expect(DueDateCacher).to receive(:recompute).with(@assignment)
# Expect DueDateCacher to be called once from AssignmentOverrideStudent after it's destroyed and another time
# after it realizes that its corresponding AssignmentOverride can also be destroyed because it now has an empty
# set of students. Hence the specific nature of this expectation.
expect(DueDateCacher).to receive(:recompute).with(@assignment).twice
override_student.destroy override_student.destroy
end end
end end

28
spec/models/assignment_spec.rb
View File

@ -1840,30 +1840,28 @@ describe Assignment do
context "differentiated_assignments" do context "differentiated_assignments" do
before :once do before :once do
setup_differentiated_assignments setup_differentiated_assignments
@submissions = [] @assignment.submit_homework(@student1, submission_type: 'online_url', url: 'http://www.google.com')
[@student1, @student2].each do |u| @submissions = @assignment.submissions
@submissions << @assignment.submit_homework(u, :submission_type => "online_url", :url => "http://www.google.com")
end
end end
context "feature on" do context "feature on" do
it "should assign peer reviews only to students with visibility" do it "should assign peer reviews only to students with visibility" do
@assignment.peer_review_count = 1 @assignment.peer_review_count = 1
res = @assignment.assign_peer_reviews res = @assignment.assign_peer_reviews
expect(res.length).to eql(0) expect(res.length).to be 0
@submissions.each do |s| @submissions.reload.each do |s|
expect(res.map{|a| a.asset}).not_to be_include(s) expect(res.map(&:asset)).not_to include(s)
expect(res.map{|a| a.assessor_asset}).not_to be_include(s) expect(res.map(&:assessor_asset)).not_to include(s)
end end
# once graded the student will have visibility # let's add this student to the section the assignment is assigned to
# and will therefore show up in the peer reviews student_in_section(@section1, user: @student2)
@assignment.grade_student(@student2, :grader => @teacher, :grade => '100') @assignment.submit_homework(@student2, submission_type: 'online_url', url: 'http://www.google.com')
res = @assignment.assign_peer_reviews res = @assignment.assign_peer_reviews
expect(res.length).to eql(@submissions.length) expect(res.length).to be 2
@submissions.each do |s| @submissions.reload.each do |s|
expect(res.map{|a| a.asset}).to be_include(s) expect(res.map(&:asset)).to include(s)
expect(res.map{|a| a.assessor_asset}).to be_include(s) expect(res.map(&:assessor_asset)).to include(s)
end end
end end

18
spec/models/assignment_student_visibility_spec.rb
View File

@ -226,11 +226,11 @@ describe "differentiated_assignments" do
teacher_in_course(course: @course) teacher_in_course(course: @course)
enroll_user_in_group(@group_foo, {user: @student}) enroll_user_in_group(@group_foo, {user: @student})
end end
it "should keep the assignment visible if there is a grade" do it "should not keep the assignment visible even if there is a grade" do
@assignment.grade_student(@student, grade: 10, grader: @teacher) @assignment.grade_student(@student, grade: 10, grader: @teacher)
@student.group_memberships.each(&:destroy!) @student.group_memberships.each(&:destroy!)
enroll_user_in_group(@group_bar, {user: @student}) enroll_user_in_group(@group_bar, {user: @student})
ensure_user_sees_assignment ensure_user_does_not_see_assignment
end end
it "should not keep the assignment visible if there is no grade" do it "should not keep the assignment visible if there is no grade" do
@ -240,11 +240,11 @@ describe "differentiated_assignments" do
ensure_user_does_not_see_assignment ensure_user_does_not_see_assignment
end end
it "should keep the assignment visible if the grade is zero" do it "should not keep the assignment visible even if the grade is zero" do
@assignment.grade_student(@student, grade: 0, grader: @teacher) @assignment.grade_student(@student, grade: 0, grader: @teacher)
@student.group_memberships.each(&:destroy!) @student.group_memberships.each(&:destroy!)
enroll_user_in_group(@group_bar, {user: @student}) enroll_user_in_group(@group_bar, {user: @student})
ensure_user_sees_assignment ensure_user_does_not_see_assignment
end end
end end
@ -297,12 +297,12 @@ describe "differentiated_assignments" do
enroller_user_in_section(@section_foo) enroller_user_in_section(@section_foo)
end end
it "should keep the assignment visible if there is a grade" do it "should not keep the assignment visible even if there is a grade" do
@assignment.grade_student(@user, grade: 10, grader: @teacher) @assignment.grade_student(@user, grade: 10, grader: @teacher)
Score.where(enrollment_id: @user.enrollments).delete_all Score.where(enrollment_id: @user.enrollments).delete_all
@user.enrollments.each(&:destroy_permanently!) @user.enrollments.each(&:destroy_permanently!)
enroller_user_in_section(@section_bar, {user: @user}) enroller_user_in_section(@section_bar, {user: @user})
ensure_user_sees_assignment ensure_user_does_not_see_assignment
end end
it "should not keep the assignment visible if there is no grade" do it "should not keep the assignment visible if there is no grade" do
@ -313,12 +313,12 @@ describe "differentiated_assignments" do
ensure_user_does_not_see_assignment ensure_user_does_not_see_assignment
end end
it "should keep the assignment visible if the grade is zero" do it "should not keep the assignment visible even if the grade is zero" do
@assignment.grade_student(@user, grade: 0, grader: @teacher) @assignment.grade_student(@user, grade: 0, grader: @teacher)
Score.where(enrollment_id: @user.enrollments).delete_all Score.where(enrollment_id: @user.enrollments).delete_all
@user.enrollments.each(&:destroy_permanently!) @user.enrollments.each(&:destroy_permanently!)
enroller_user_in_section(@section_bar, {user: @user}) enroller_user_in_section(@section_bar, {user: @user})
ensure_user_sees_assignment ensure_user_does_not_see_assignment
end end
end end
@ -345,7 +345,7 @@ describe "differentiated_assignments" do
end end
it "should update when the override is deleted" do it "should update when the override is deleted" do
ensure_user_sees_assignment ensure_user_sees_assignment
@assignment.assignment_overrides.each(&:destroy_permanently!) @assignment.assignment_overrides.each(&:destroy!)
ensure_user_does_not_see_assignment ensure_user_does_not_see_assignment
end end
it "should not return duplicate visibilities with multiple visible sections" do it "should not return duplicate visibilities with multiple visible sections" do

10
spec/models/quiz_student_visibility_spec.rb
View File

@ -187,12 +187,12 @@ describe "differentiated_assignments" do
@student = @user @student = @user
teacher_in_course(course: @course) teacher_in_course(course: @course)
end end
it "should keep the quiz visible if there is a grade" do it "should not keep the quiz visible even if there is a grade" do
@quiz.assignment.grade_student(@student, grade: 10, grader: @teacher) @quiz.assignment.grade_student(@student, grade: 10, grader: @teacher)
Score.where(enrollment_id: @student.enrollments).delete_all Score.where(enrollment_id: @student.enrollments).delete_all
@student.enrollments.each(&:destroy_permanently!) @student.enrollments.each(&:destroy_permanently!)
enroller_user_in_section(@section_bar, {user: @student}) enroller_user_in_section(@section_bar, {user: @student})
ensure_user_sees_quiz ensure_user_does_not_see_quiz
end end
it "should not keep the quiz visible if there is no score, even if it has a grade" do it "should not keep the quiz visible if there is no score, even if it has a grade" do
@ -205,12 +205,12 @@ describe "differentiated_assignments" do
ensure_user_does_not_see_quiz ensure_user_does_not_see_quiz
end end
it "should keep the quiz visible if the grade is zero" do it "should not keep the quiz visible even if the grade is zero" do
@quiz.assignment.grade_student(@student, grade: 0, grader: @teacher) @quiz.assignment.grade_student(@student, grade: 0, grader: @teacher)
Score.where(enrollment_id: @student.enrollments).delete_all Score.where(enrollment_id: @student.enrollments).delete_all
@student.enrollments.each(&:destroy_permanently!) @student.enrollments.each(&:destroy_permanently!)
enroller_user_in_section(@section_bar, {user: @student}) enroller_user_in_section(@section_bar, {user: @student})
ensure_user_sees_quiz ensure_user_does_not_see_quiz
end end
end end
@ -233,7 +233,7 @@ describe "differentiated_assignments" do
end end
it "should update when the override is deleted" do it "should update when the override is deleted" do
ensure_user_sees_quiz ensure_user_sees_quiz
@quiz.assignment_overrides.to_a.each(&:destroy_permanently!) @quiz.assignment_overrides.to_a.each(&:destroy!)
ensure_user_does_not_see_quiz ensure_user_does_not_see_quiz
end end
end end

27
spec/models/submission_spec.rb
View File

@ -74,6 +74,29 @@ describe Submission do
end end
describe "grade" do describe "grade" do
context "the submission is deleted" do
before(:once) do
@assignment.due_at = in_open_grading_period
@assignment.save!
submission_spec_model
@submission.update(workflow_state: 'deleted')
end
it "does not have grade permissions if the user is a root account admin" do
expect(@submission.grants_right?(@admin, :grade)).to eq(false)
end
it "does not have grade permissions if the user is non-root account admin with manage_grades permissions" do
expect(@submission.grants_right?(@teacher, :grade)).to eq(false)
end
it "doesn't have grade permissions if the user is non-root account admin without manage_grades permissions" do
@student = user_factory(active_all: true)
@context.enroll_student(@student)
expect(@submission.grants_right?(@student, :grade)).to eq(false)
end
end
context "the submission is due in an open grading period" do context "the submission is due in an open grading period" do
before(:once) do before(:once) do
@assignment.due_at = in_open_grading_period @assignment.due_at = in_open_grading_period
@ -89,7 +112,7 @@ describe Submission do
expect(@submission.grants_right?(@teacher, :grade)).to eq(true) expect(@submission.grants_right?(@teacher, :grade)).to eq(true)
end end
it "has not have grade permissions if the user is non-root account admin without manage_grades permissions" do it "doesn't have grade permissions if the user is non-root account admin without manage_grades permissions" do
@student = user_factory(active_all: true) @student = user_factory(active_all: true)
@context.enroll_student(@student) @context.enroll_student(@student)
expect(@submission.grants_right?(@student, :grade)).to eq(false) expect(@submission.grants_right?(@student, :grade)).to eq(false)
@ -111,7 +134,7 @@ describe Submission do
expect(@submission.grants_right?(@teacher, :grade)).to eq(true) expect(@submission.grants_right?(@teacher, :grade)).to eq(true)
end end
it "has not have grade permissions if the user is non-root account admin without manage_grades permissions" do it "doesn't have grade permissions if the user is non-root account admin without manage_grades permissions" do
@student = user_factory(active_all: true) @student = user_factory(active_all: true)
@context.enroll_student(@student) @context.enroll_student(@student)
expect(@submission.grants_right?(@student, :grade)).to eq(false) expect(@submission.grants_right?(@student, :grade)).to eq(false)

3
spec/selenium/differentiated_assignments/da_quizzes_spec.rb
View File

@ -82,7 +82,6 @@ describe "interaction with differentiated quizzes" do
submit_quiz(@da_quiz) submit_quiz(@da_quiz)
# destroy the override and automatically generated grade providing visibility to the current student # destroy the override and automatically generated grade providing visibility to the current student
AssignmentOverride.find(@da_quiz.assignment_overrides.first!.id).destroy AssignmentOverride.find(@da_quiz.assignment_overrides.first!.id).destroy
@da_quiz.assignment.grade_student(@user, grade: nil, grader: @teacher)
create_section_override_for_assignment(@da_quiz, course_section: @section1) create_section_override_for_assignment(@da_quiz, course_section: @section1)
# assure we get the no longer counted banner on the quiz page # assure we get the no longer counted banner on the quiz page
get "/courses/#{@course.id}/quizzes/#{@da_quiz.id}" get "/courses/#{@course.id}/quizzes/#{@da_quiz.id}"
@ -97,7 +96,6 @@ describe "interaction with differentiated quizzes" do
create_section_override_for_assignment(@da_quiz) create_section_override_for_assignment(@da_quiz)
submit_quiz(@da_quiz) submit_quiz(@da_quiz)
AssignmentOverride.find(@da_quiz.assignment_overrides.first!.id).destroy AssignmentOverride.find(@da_quiz.assignment_overrides.first!.id).destroy
@da_quiz.assignment.grade_student(@user, grade: nil, grader: @teacher)
create_section_override_for_assignment(@da_quiz, course_section: @section1) create_section_override_for_assignment(@da_quiz, course_section: @section1)
get "/courses/#{@course.id}/quizzes/#{@da_quiz.id}" get "/courses/#{@course.id}/quizzes/#{@da_quiz.id}"
expect(f("#content")).not_to contain_css(".take_quiz_link") expect(f("#content")).not_to contain_css(".take_quiz_link")
@ -183,7 +181,6 @@ describe "interaction with differentiated quizzes" do
submit_quiz(@da_quiz) submit_quiz(@da_quiz)
# destroy the override and automatically generated grade providing visibility to the current student # destroy the override and automatically generated grade providing visibility to the current student
AssignmentOverride.find(@da_quiz.assignment_overrides.first!.id).destroy AssignmentOverride.find(@da_quiz.assignment_overrides.first!.id).destroy
@da_quiz.assignment.grade_student(@user, grade: nil, grader: @teacher)
create_section_override_for_assignment(@da_quiz, course_section: @section1) create_section_override_for_assignment(@da_quiz, course_section: @section1)
# assure we get the no longer counted banner on the quiz page # assure we get the no longer counted banner on the quiz page
get "/courses/#{@course.id}/quizzes/#{@da_quiz.id}" get "/courses/#{@course.id}/quizzes/#{@da_quiz.id}"

3
spec/selenium/grades/gradebook/gradebook_differentiated_assignments_spec.rb
View File

@ -52,12 +52,11 @@ describe "gradebook" do
expect(cell.find_element(:css, '.gradebook-cell')).not_to have_class('grayed-out') expect(cell.find_element(:css, '.gradebook-cell')).not_to have_class('grayed-out')
end end
it "should gray out cells after removing a score which removes visibility" do it "should gray out cells after removing an override which removes visibility" do
selector = '#gradebook_grid .container_1 .slick-row:nth-child(1) .l5' selector = '#gradebook_grid .container_1 .slick-row:nth-child(1) .l5'
@da_assignment.grade_student(@student_1, grade: 42, grader: @teacher) @da_assignment.grade_student(@student_1, grade: 42, grader: @teacher)
@override.destroy @override.destroy
get "/courses/#{@course.id}/gradebook" get "/courses/#{@course.id}/gradebook"
edit_grade(selector, '')
cell = f(selector) cell = f(selector)
expect(cell.find_element(:css, '.gradebook-cell')).to have_class('grayed-out') expect(cell.find_element(:css, '.gradebook-cell')).to have_class('grayed-out')
end end

3
spec/selenium/grades/gradezilla/gradebook_differentiated_assignments_spec.rb
View File

@ -53,12 +53,11 @@ describe "Gradezilla" do
expect(cell.find_element(:css, '.gradebook-cell')).not_to have_class('grayed-out') expect(cell.find_element(:css, '.gradebook-cell')).not_to have_class('grayed-out')
end end
it "should gray out cells after removing a score which removes visibility" do it "should gray out cells after removing an override which removes visibility" do
selector = '#gradebook_grid .container_1 .slick-row:nth-child(1) .l4' selector = '#gradebook_grid .container_1 .slick-row:nth-child(1) .l4'
@da_assignment.grade_student(@student_1, grade: 42, grader: @teacher) @da_assignment.grade_student(@student_1, grade: 42, grader: @teacher)
@override.destroy @override.destroy
Gradezilla.visit(@course) Gradezilla.visit(@course)
edit_grade(selector, '')
cell = f(selector) cell = f(selector)
expect(cell.find_element(:css, '.gradebook-cell')).to have_class('grayed-out') expect(cell.find_element(:css, '.gradebook-cell')).to have_class('grayed-out')
end end