Skip to content

Commit

Permalink
assignment created notifications only on publish
Browse files Browse the repository at this point in the history
fixes CNVS-9521

test plan:
 * with draft state enabled
 - as a teacher, create an assignment
 - view your notifications (stream/emails) and there should not be a
   notification about the new assignment
 - as a student, view your notifications and there should not
   be a notification about the new assignment
 - as a teacher again, publish the assignment
 - there should be a notification about the assignment being created
 - as a student, there should be a notification
   about the assignment being created
 - API: follow the above steps using the API as well
 - also, 'assignment changed' notifications should not be sent
   when the assignment has been published

 * without draft state
 - follow the same steps as above
 - a notification in the stream and an email should only be sent
   out when the assignment is created

Change-Id: Ied600393ece287ea3b573413eec38ee16a48cd30
Reviewed-on: https://gerrit.instructure.com/26771
Tested-by: Jenkins <[email protected]>
QA-Review: Caleb Guanzon <[email protected]>
Reviewed-by: Simon Williams <[email protected]>
Product-Review: Cameron Sutter <[email protected]>
  • Loading branch information
cameronsutter committed Dec 11, 2013
1 parent 94da804 commit dfbaa8b
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 9 deletions.
14 changes: 7 additions & 7 deletions app/models/assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -458,18 +458,18 @@ def do_notifications!(prior_version=nil, notify=false)

p.dispatch :assignment_created
p.to { participants }
p.whenever { |record|
record.context.available? &&
record.just_created
p.whenever { |assignment|
policy = BroadcastPolicies::AssignmentPolicy.new( assignment )
policy.should_dispatch_assignment_created?
}
p.filter_asset_by_recipient { |record, user|
record.overridden_for(user)
p.filter_asset_by_recipient { |assignment, user|
assignment.overridden_for(user)
}

p.dispatch :assignment_unmuted
p.to { participants }
p.whenever { |record|
record.recently_unmuted
p.whenever { |assignment|
assignment.recently_unmuted
}

end
Expand Down
19 changes: 19 additions & 0 deletions app/models/broadcast_policies/assignment_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ def initialize(assignment)
def should_dispatch_assignment_due_date_changed?
accepting_messages? &&
assignment.changed_in_state(:published, :fields => :due_at) &&
!just_published? &&
!AssignmentPolicy.due_dates_equal?(assignment.due_at, prior_version.due_at) &&
created_before(3.hours.ago)
end
Expand All @@ -19,9 +20,19 @@ def should_dispatch_assignment_changed?
assignment.published? &&
!assignment.muted? &&
created_before(30.minutes.ago) &&
!just_published? &&
(assignment.points_possible != prior_version.points_possible || assignment.assignment_changed)
end

def should_dispatch_assignment_created?
return false unless assignment.context.available?
if assignment.context.feature_enabled?(:draft_state)
published_on_create? || just_published?
else
assignment.just_created
end
end

private
def accepting_messages?
assignment.context.available? &&
Expand All @@ -35,5 +46,13 @@ def prior_version
def created_before(time)
assignment.created_at < time
end

def published_on_create?
assignment.just_created && assignment.published?
end

def just_published?
assignment.workflow_state_changed? && assignment.published?
end
end
end
1 change: 1 addition & 0 deletions spec/apis/v1/assignments_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1429,6 +1429,7 @@ def greater_than_255
@course.account.update_attributes! turnitin_account_id: 1234,
turnitin_shared_secret: 'foo',
turnitin_host: 'example.com'
@assignment.reload
}

it "contains a turnitin_enabled key" do
Expand Down
59 changes: 57 additions & 2 deletions spec/models/broadcast_policies/assignment_policy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,69 @@

module BroadcastPolicies
describe AssignmentPolicy do
let(:context) { stub(:available? => true) }
let(:context) {
ctx = mock()
ctx.stubs(:available?).returns(true)
ctx.stubs(:feature_enabled?).with(:draft_state).returns(false)
ctx
}
let(:prior_version) { stub(:due_at => 7.days.ago, :points_possible => 50) }
let(:assignment) do
stub(:context => context, :prior_version => prior_version,
:published? => true, :muted? => false, :created_at => 4.hours.ago,
:changed_in_state => true, :due_at => Time.now,
:points_possible => 100, :assignment_changed => false)
:points_possible => 100, :assignment_changed => false,
:just_created => true)
end

let(:policy) { AssignmentPolicy.new(assignment) }

context 'draft state' do

before do
context.stubs(:feature_enabled?).with(:draft_state).returns(true)
assignment.stubs(:workflow_state_changed?).returns true
end

describe "#should_dispatch_assignment_created?" do
it 'is true when an assignment is published' do
policy.should_dispatch_assignment_created?.should be_true
end

def wont_send_when
yield
policy.should_dispatch_assignment_created?.should be_false
end

specify {
wont_send_when {
assignment.stubs(:just_created).returns false
assignment.stubs(:workflow_state_changed?).returns false
}
}
specify { wont_send_when { assignment.stubs(:published?).returns false}}
end
end

describe "#should_dispatch_assignment_created?" do
it 'is true when an assignment is created' do
policy.should_dispatch_assignment_created?.should be_true
end

def wont_send_when
yield
policy.should_dispatch_assignment_created?.should be_false
end

specify { wont_send_when { context.stubs(:available?).returns false } }
specify { wont_send_when { assignment.stubs(:just_created).returns false } }
end

describe '#should_dispatch_assignment_due_date_changed?' do
before do
assignment.stubs(:workflow_state_changed?).returns false
end

it 'is true when the dependent inputs are true' do
policy.should_dispatch_assignment_due_date_changed?.should be_true
end
Expand All @@ -31,6 +82,10 @@ def wont_send_when
end

describe '#should_dispatch_assignment_changed?' do
before do
assignment.stubs(:workflow_state_changed?).returns false
end

it 'is true when the dependent inputs are true' do
policy.should_dispatch_assignment_changed?.should be_true
end
Expand Down

0 comments on commit dfbaa8b

Please sign in to comment.