don't send assignment notifs if course starts in the future
fixes CNVS-26359 - Add Enrollment.not_yet_started. - Update `to` blocks in Assignment's set_broadcast_policy declarations to exclude users for whom their enrollment in the course has not started. test plan: Configurations: - A student belonging to a course section that starts in the future. - A course that is published but that starts in the future. - A course without an explicit start date, but belonging to a term that starts in the future. The following should be performed with all of the above configurations: - Add an assignment to the course. - Observe that the student does not receive a notification. - Make a change to an assignment that is at least 30 minutes old, check the "Notify users that this content has changed" checkbox, and save the assignment. - Observe that the student does not receive a notification. - Make a change to the due dates on an assignment that is more than 3 hours old, check the "Notify users that this content has changed" checkbox, and save the assignment. - Observe that the student does not receive a notification. - Unmute a muted assignment. - Observe that the student does not receive a notification. Also confirm that, given a course that has already started, all assignment-related notifications are sent as expected. Change-Id: I5bff23ff35e72081b2c1a3127dfb42a71d3fa25d Reviewed-on: https://gerrit.instructure.com/71217 Tested-by: Jenkins Reviewed-by: Matt Berns <mberns@instructure.com> Reviewed-by: Sterling Cobb <sterling@instructure.com> Reviewed-by: Matthew Wheeler <mwheeler@instructure.com> QA-Review: Michael Hargiss <mhargiss@instructure.com> Product-Review: Jason Sparks <jsparks@instructure.com>
This commit is contained in:
parent
0fad3e1db6
commit
c09b2bdc79
|
@ -532,11 +532,11 @@ class Assignment < ActiveRecord::Base
|
|||
|
||||
set_broadcast_policy do |p|
|
||||
p.dispatch :assignment_due_date_changed
|
||||
p.to {
|
||||
p.to { |assignment|
|
||||
# everyone who is _not_ covered by an assignment override affecting due_at
|
||||
# (the AssignmentOverride records will take care of notifying those users)
|
||||
excluded_ids = participants_with_overridden_due_at.map(&:id).to_set
|
||||
participants(include_observers: true, excluded_user_ids: excluded_ids)
|
||||
BroadcastPolicies::AssignmentParticipants.new(assignment, excluded_ids).to
|
||||
}
|
||||
p.whenever { |assignment|
|
||||
BroadcastPolicies::AssignmentPolicy.new(assignment).
|
||||
|
@ -544,14 +544,18 @@ class Assignment < ActiveRecord::Base
|
|||
}
|
||||
|
||||
p.dispatch :assignment_changed
|
||||
p.to { participants(include_observers: true) }
|
||||
p.to { |assignment|
|
||||
BroadcastPolicies::AssignmentParticipants.new(assignment).to
|
||||
}
|
||||
p.whenever { |assignment|
|
||||
BroadcastPolicies::AssignmentPolicy.new(assignment).
|
||||
should_dispatch_assignment_changed?
|
||||
}
|
||||
|
||||
p.dispatch :assignment_created
|
||||
p.to { participants(include_observers: true) }
|
||||
p.to { |assignment|
|
||||
BroadcastPolicies::AssignmentParticipants.new(assignment).to
|
||||
}
|
||||
p.whenever { |assignment|
|
||||
BroadcastPolicies::AssignmentPolicy.new(assignment).
|
||||
should_dispatch_assignment_created?
|
||||
|
@ -561,12 +565,13 @@ class Assignment < ActiveRecord::Base
|
|||
}
|
||||
|
||||
p.dispatch :assignment_unmuted
|
||||
p.to { participants(include_observers: true) }
|
||||
p.to { |assignment|
|
||||
BroadcastPolicies::AssignmentParticipants.new(assignment).to
|
||||
}
|
||||
p.whenever { |assignment|
|
||||
BroadcastPolicies::AssignmentPolicy.new(assignment).
|
||||
should_dispatch_assignment_unmuted?
|
||||
}
|
||||
|
||||
end
|
||||
|
||||
def notify_of_update=(val)
|
||||
|
|
|
@ -0,0 +1,36 @@
|
|||
module BroadcastPolicies
|
||||
class AssignmentParticipants
|
||||
def initialize(assignment, excluded_ids=nil)
|
||||
@assignment = assignment
|
||||
@excluded_ids = excluded_ids
|
||||
end
|
||||
attr_reader :assignment, :excluded_ids
|
||||
delegate :context, :participants, to: :assignment
|
||||
|
||||
def to
|
||||
all_participants - excluded_enrollment_users
|
||||
end
|
||||
|
||||
private
|
||||
def all_participant_ids
|
||||
all_participants.map(&:id)
|
||||
end
|
||||
|
||||
def all_participants
|
||||
@_all_participants ||= participants({
|
||||
include_observers: true,
|
||||
excluded_user_ids: excluded_ids
|
||||
})
|
||||
end
|
||||
|
||||
def excluded_enrollment_users
|
||||
excluded_enrollments.map(&:user)
|
||||
end
|
||||
|
||||
def excluded_enrollments
|
||||
Enrollment.where({
|
||||
user_id: all_participant_ids
|
||||
}).not_yet_started(context)
|
||||
end
|
||||
end
|
||||
end
|
|
@ -32,7 +32,7 @@ module BroadcastPolicies
|
|||
end
|
||||
|
||||
def should_dispatch_assignment_unmuted?
|
||||
assignment.context.available? &&
|
||||
context_sendable? &&
|
||||
assignment.recently_unmuted
|
||||
end
|
||||
|
||||
|
|
|
@ -78,6 +78,14 @@ class Enrollment < ActiveRecord::Base
|
|||
scope :concluded, -> { joins(:course).where(QueryBuilder.new(:completed).conditions).readonly(false) }
|
||||
scope :current_and_concluded, -> { joins(:course).where(QueryBuilder.new(:current_and_concluded).conditions).readonly(false) }
|
||||
|
||||
def self.not_yet_started(course)
|
||||
collection = where(course_id: course)
|
||||
Canvas::Builders::EnrollmentDateBuilder.preload(collection)
|
||||
collection.select do |enrollment|
|
||||
enrollment.effective_start_at > Time.zone.now
|
||||
end
|
||||
end
|
||||
|
||||
def ensure_role_id
|
||||
self.role_id ||= self.role.id
|
||||
end
|
||||
|
|
|
@ -0,0 +1,43 @@
|
|||
require File.expand_path('../../spec_helper', File.dirname(__FILE__))
|
||||
|
||||
describe BroadcastPolicies::AssignmentParticipants do
|
||||
before :once do
|
||||
course_with_student(active_all: true)
|
||||
assignment_model course: @course
|
||||
@excluded_ids = nil
|
||||
end
|
||||
|
||||
subject do
|
||||
BroadcastPolicies::AssignmentParticipants.new(@assignment, @excluded_ids)
|
||||
end
|
||||
|
||||
describe '#to' do
|
||||
it 'includes students with access to the assignment' do
|
||||
expect(subject.to).to include(@student)
|
||||
end
|
||||
|
||||
context 'with students whose enrollments have not yet started' do
|
||||
before :once do
|
||||
student_in_course({
|
||||
course: @course,
|
||||
start_at: 1.month.from_now
|
||||
})
|
||||
end
|
||||
|
||||
it 'excludes said students' do
|
||||
expect(subject.to).not_to include(@student)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when provided with excluded_ids' do
|
||||
before :once do
|
||||
student_in_course(active_all: true)
|
||||
@excluded_ids = [@student.id]
|
||||
end
|
||||
|
||||
it 'excludes students with provided ids' do
|
||||
expect(subject.to).not_to include(@student)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -1986,4 +1986,20 @@ describe Enrollment do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe ".not_yet_started" do
|
||||
before :once do
|
||||
course_with_student(active_all: true)
|
||||
end
|
||||
|
||||
it 'includes users for whom the enrollment has not yet started' do
|
||||
Enrollment.any_instance.stubs(:effective_start_at).returns(1.month.from_now)
|
||||
expect(Enrollment.not_yet_started(@course)).to include(@enrollment)
|
||||
end
|
||||
|
||||
it 'excludes users for whom the enrollment has started' do
|
||||
@enrollment.stubs(:effective_start_at).returns(1.month.ago)
|
||||
expect(Enrollment.not_yet_started(@course)).not_to include(@enrollment)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue