add scoping overrides by active enrollment
Add scoping overrides by active enrollment to prevent unenrolled students with overrides from preventing assignments from saving. fixes CNVS-21655 test plan: - Create an assignment - Assign a due date to a particular student - Remove that user from the course - Edit that assignment. - Assignments override should no longer show. - Save the assignment should now work. Change-Id: If08da7119172c7918b15238195e39e07ae75ea16 Reviewed-on: https://gerrit.instructure.com/71305 Tested-by: Jenkins Reviewed-by: Davis McClellan <dmcclellan@instructure.com> QA-Review: Michael Hargiss <mhargiss@instructure.com> Product-Review: Jason Sparks <jsparks@instructure.com>
This commit is contained in:
parent
6274d438c8
commit
267cad5787
|
@ -422,7 +422,7 @@ class AssignmentsController < ApplicationController
|
||||||
}),
|
}),
|
||||||
:ASSIGNMENT_OVERRIDES =>
|
:ASSIGNMENT_OVERRIDES =>
|
||||||
(assignment_overrides_json(
|
(assignment_overrides_json(
|
||||||
@assignment.overrides_for(@current_user)
|
@assignment.overrides_for(@current_user, ensure_set_not_empty: true)
|
||||||
)),
|
)),
|
||||||
:ASSIGNMENT_INDEX_URL => polymorphic_url([@context, :assignments]),
|
:ASSIGNMENT_INDEX_URL => polymorphic_url([@context, :assignments]),
|
||||||
:DIFFERENTIATED_ASSIGNMENTS_ENABLED => @context.feature_enabled?(:differentiated_assignments),
|
:DIFFERENTIATED_ASSIGNMENTS_ENABLED => @context.feature_enabled?(:differentiated_assignments),
|
||||||
|
|
|
@ -402,7 +402,7 @@ class DiscussionTopicsController < ApplicationController
|
||||||
if @topic.assignment.present?
|
if @topic.assignment.present?
|
||||||
hash[:ATTRIBUTES][:assignment][:assignment_overrides] =
|
hash[:ATTRIBUTES][:assignment][:assignment_overrides] =
|
||||||
(assignment_overrides_json(
|
(assignment_overrides_json(
|
||||||
@topic.assignment.overrides_for(@current_user)
|
@topic.assignment.overrides_for(@current_user, ensure_set_not_empty: true)
|
||||||
))
|
))
|
||||||
hash[:ATTRIBUTES][:assignment][:has_student_submissions] = @topic.assignment.has_student_submissions?
|
hash[:ATTRIBUTES][:assignment][:has_student_submissions] = @topic.assignment.has_student_submissions?
|
||||||
end
|
end
|
||||||
|
|
|
@ -279,7 +279,7 @@ class Quizzes::QuizzesController < ApplicationController
|
||||||
|
|
||||||
hash = {
|
hash = {
|
||||||
:ASSIGNMENT_ID => @assignment.present? ? @assignment.id : nil,
|
:ASSIGNMENT_ID => @assignment.present? ? @assignment.id : nil,
|
||||||
:ASSIGNMENT_OVERRIDES => assignment_overrides_json(@quiz.overrides_for(@current_user)),
|
:ASSIGNMENT_OVERRIDES => assignment_overrides_json(@quiz.overrides_for(@current_user, ensure_set_not_empty: true)),
|
||||||
:DIFFERENTIATED_ASSIGNMENTS_ENABLED => @context.feature_enabled?(:differentiated_assignments),
|
:DIFFERENTIATED_ASSIGNMENTS_ENABLED => @context.feature_enabled?(:differentiated_assignments),
|
||||||
:QUIZ => quiz_json(@quiz, @context, @current_user, session),
|
:QUIZ => quiz_json(@quiz, @context, @current_user, session),
|
||||||
:SECTION_LIST => sections.map { |section|
|
:SECTION_LIST => sections.map { |section|
|
||||||
|
|
|
@ -69,6 +69,12 @@ class AssignmentOverride < ActiveRecord::Base
|
||||||
after_save :update_cached_due_dates
|
after_save :update_cached_due_dates
|
||||||
after_save :touch_assignment, :if => :assignment
|
after_save :touch_assignment, :if => :assignment
|
||||||
|
|
||||||
|
def set_not_empty?
|
||||||
|
overridable = assignment? ? assignment : quiz
|
||||||
|
['CourseSection', 'Group'].include?(self.set_type) ||
|
||||||
|
set.any? && overridable.context.current_enrollments.where(user_id: set).exists?
|
||||||
|
end
|
||||||
|
|
||||||
def update_cached_due_dates
|
def update_cached_due_dates
|
||||||
return unless assignment?
|
return unless assignment?
|
||||||
if due_at_overridden_changed? ||
|
if due_at_overridden_changed? ||
|
||||||
|
|
|
@ -27,8 +27,13 @@ module DatesOverridable
|
||||||
end
|
end
|
||||||
|
|
||||||
# All overrides, not just dates
|
# All overrides, not just dates
|
||||||
def overrides_for(user)
|
def overrides_for(user, opts={})
|
||||||
AssignmentOverrideApplicator.overrides_for_assignment_and_user(self, user)
|
overrides = AssignmentOverrideApplicator.overrides_for_assignment_and_user(self, user)
|
||||||
|
if opts[:ensure_set_not_empty]
|
||||||
|
overrides.select(&:set_not_empty?)
|
||||||
|
else
|
||||||
|
overrides
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def overridden_for?(user)
|
def overridden_for?(user)
|
||||||
|
|
|
@ -59,6 +59,45 @@ shared_examples_for "an object whose dates are overridable" do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "assignment overrides_for" do
|
||||||
|
before do
|
||||||
|
student_in_course(:course => course)
|
||||||
|
end
|
||||||
|
|
||||||
|
context "with adhoc" do
|
||||||
|
before do
|
||||||
|
override.override_due_at(7.days.from_now)
|
||||||
|
override.set_type = "ADHOC"
|
||||||
|
override.save!
|
||||||
|
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns adhoc overrides when active students enrolled in adhoc set" do
|
||||||
|
override_student = override.assignment_override_students.build
|
||||||
|
override_student.user = @student
|
||||||
|
override_student.save!
|
||||||
|
|
||||||
|
expect(overridable.overrides_for(@student, ensure_set_not_empty: true).size).to eq 1
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns nothing when no active students enrolled in adhoc set" do
|
||||||
|
expect(overridable.overrides_for(@student, ensure_set_not_empty: true)).to be_empty
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns nothing when active students enrolled in adhoc set removed" do
|
||||||
|
override_student = override.assignment_override_students.build
|
||||||
|
override_student.user = @student
|
||||||
|
override_student.save!
|
||||||
|
|
||||||
|
expect(overridable.overrides_for(@student, ensure_set_not_empty: true).size).to eq 1
|
||||||
|
|
||||||
|
override_student.user.enrollments.delete_all
|
||||||
|
|
||||||
|
expect(overridable.overrides_for(@student, ensure_set_not_empty: true)).to be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe "has_overrides?" do
|
describe "has_overrides?" do
|
||||||
subject { overridable.has_overrides? }
|
subject { overridable.has_overrides? }
|
||||||
|
|
||||||
|
|
|
@ -662,4 +662,38 @@ describe AssignmentOverride do
|
||||||
expect(@override.applies_to_students).to eq [@student]
|
expect(@override.applies_to_students).to eq [@student]
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "assignment_edits" do
|
||||||
|
before do
|
||||||
|
@override = assignment_override_model
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns false if no students who are active in course for ADHOC" do
|
||||||
|
@override.stubs(:set_type).returns "ADHOC"
|
||||||
|
@override.stubs(:set).returns []
|
||||||
|
|
||||||
|
expect(@override.set_not_empty?).to eq false
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns true if no students who are active in course and CourseSection or Group" do
|
||||||
|
@override.stubs(:set_type).returns "CourseSection"
|
||||||
|
@override.stubs(:set).returns []
|
||||||
|
|
||||||
|
expect(@override.set_not_empty?).to eq true
|
||||||
|
|
||||||
|
@override.stubs(:set_type).returns "Group"
|
||||||
|
|
||||||
|
expect(@override.set_not_empty?).to eq true
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns true if has students who are active in course for ADHOC" do
|
||||||
|
student = student_in_course(course: @override.assignment.context)
|
||||||
|
@override.set_type = "ADHOC"
|
||||||
|
@override_student = @override.assignment_override_students.build
|
||||||
|
@override_student.user = student.user
|
||||||
|
@override_student.save!
|
||||||
|
|
||||||
|
expect(@override.set_not_empty?).to eq true
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue