fix assignment changed notifications with overrides

fixes CNVS-4480

test plan:
- set one of the student's notification policies for "Course Content" to
  immediate
- find an old assignment with overrides (created at least 30 min ago)
- edit it and change something trivial (like the assignment group)
- check the 'notify of update' box
- hit save
- the student should get an 'assignment changed' notification

Change-Id: Ied7c008d1e1f18d1e3f5716968c46593b6d6574f
Reviewed-on: https://gerrit.instructure.com/18419
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Stanley Stuart <stanley@instructure.com>
QA-Review: Myller de Araujo <myller@instructure.com>

app/models/assignment.rb

@@ -430,7 +430,7 @@ class Assignment < ActiveRecord::Base
 
   # call this to perform notifications on an Assignment that is not being saved
   # (useful when a batch of overrides associated with a new assignment have been saved)
-  def do_notifications!(prior_version=nil)
+  def do_notifications!(prior_version=nil, notify=false)
     self.prior_version = prior_version
     @broadcasted = false
     # TODO: this will blow up if the group_category string is set on the
@@ -438,7 +438,9 @@ class Assignment < ActiveRecord::Base
     # and the association.  one more reason to drop the db column
     self.prior_version ||= self.versions.previous(self.current_version.number).try(:model)
     self.just_created = self.prior_version.nil?
+    self.notify_of_update = notify || false
     broadcast_notifications
+    remove_assignment_updated_flag
   end
 
   set_broadcast_policy do |p|
@@ -460,7 +462,7 @@ class Assignment < ActiveRecord::Base
     p.whenever { |record|
       !self.suppress_broadcast and
       !record.muted? and
-      record.created_at < Time.now - 30.minutes and
+      record.created_at < 30.minutes.ago and
       record.context.state == :available and [:available, :published].include?(record.state) and
       record.prior_version and (record.points_possible != record.prior_version.points_possible || @assignment_changed)
     }
@@ -504,7 +506,7 @@ class Assignment < ActiveRecord::Base
   end
 
   def notify_of_update=(val)
-    @assignment_changed = (val == '1' || val == true)
+    @assignment_changed = Canvas::Plugin.value_to_boolean(val)
   end
 
   def notify_of_update

lib/api/v1/assignment.rb

@@ -181,6 +181,7 @@ module Api::V1::Assignment
     turnitin_settings
     grading_standard_id
     freeze_on_copy
+    notify_of_update
   )
 
   API_ALLOWED_TURNITIN_SETTINGS = %w(
@@ -207,13 +208,10 @@ module Api::V1::Assignment
 
     if overrides
       assignment.transaction do
-        # TODO: We need to handle notifications better here. We want to send
-        # notifications if the "notify_of_update" field is passed, but *after*
-        # the assignment overrides have been created.
         assignment.save_without_broadcasting!
         batch_update_assignment_overrides(assignment, overrides)
       end
-      assignment.do_notifications!(old_assignment)
+      assignment.do_notifications!(old_assignment, assignment_params[:notify_of_update])
     else
       assignment.save!
     end

spec/apis/v1/assignments_api_spec.rb

@@ -513,6 +513,51 @@ describe AssignmentsApiController, :type => :integration do
       end
     end
 
+    context "broadcasting while updating overrides" do
+      before :all do
+        @notification = Notification.create! :name => "Assignment Changed"
+        course_with_teacher(:active_all => true)
+        student_in_course(:course => @course, :active_all => true)
+        @student.communication_channels.create(:path => "student@instructure.com").confirm!
+        @student.email_channel.notification_policies.
+          find_or_create_by_notification_id(@notification.id).
+          update_attribute(:frequency, 'immediately')
+        @assignment = @course.assignments.create!
+        Assignment.update_all({:created_at => Time.zone.now - 1.day}, {:id => @assignment.id})
+        @adhoc_due_at = 5.days.from_now
+        @section_due_at = 7.days.from_now
+        @user = @teacher
+        @params = {
+          'name' => 'Assignment With Overrides',
+          'assignment_overrides' => {
+            '0' => {
+              'student_ids' => [@student.id],
+              'title' => 'adhoc override',
+              'due_at' => @adhoc_due_at.iso8601
+            },
+            '1' => {
+              'course_section_id' => @course.default_section.id,
+              'due_at' => @section_due_at.iso8601
+            }
+          }
+        }
+      end
+
+      after(:all) do
+        truncate_all_tables
+      end
+
+      it "should not send assignment_changed if notify_of_update is not set" do
+        api_update_assignment_call(@course,@assignment,@params)
+        @student.messages.detect{|m| m.notification_id == @notification.id}.should be_nil
+      end
+
+      it "should send assignment_changed if notify_of_update is set" do
+        api_update_assignment_call(@course,@assignment,@params.merge({:notify_of_update => true}))
+        @student.messages.detect{|m| m.notification_id == @notification.id}.should be_present
+      end
+    end
+
     context "when turnitin is enabled on the context" do
       before do
         course_with_teacher(:active_all => true)