use AR to check changes and previous values in callbacks

fixes CNVS-10345

test plan:
- basic regression test of assignments, assignment groups, and submissions,
  specifically around behavior that should happen after they are changed, like
  grades being updated

Change-Id: I8e2f21376bb1b993f30f03de3606158eab893849
Reviewed-on: https://gerrit.instructure.com/28246
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Amber Taniuchi <amber@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>

app/models/assignment.rb

@@ -155,23 +155,22 @@ class Assignment < ActiveRecord::Base
     write_attribute(:allowed_extensions, new_value)
   end
 
-  before_save   :set_old_assignment_group_id,
-                :infer_grading_type,
-                :process_if_quiz,
-                :default_values,
-                :update_submissions_if_details_changed,
-                :maintain_group_category_attribute
+  before_save :infer_grading_type,
+              :process_if_quiz,
+              :default_values,
+              :update_submissions_if_details_changed,
+              :maintain_group_category_attribute
 
-  after_save    :update_grades_if_details_changed,
-                :touch_assignment_group,
-                :touch_context,
-                :update_grading_standard,
-                :update_quiz_or_discussion_topic,
-                :update_submissions_later,
-                :schedule_do_auto_peer_review_job_if_automatic_peer_review,
-                :delete_empty_abandoned_children,
-                :validate_assignment_overrides,
-                :update_cached_due_dates
+  after_save  :update_grades_if_details_changed,
+              :touch_assignment_group,
+              :touch_context,
+              :update_grading_standard,
+              :update_quiz_or_discussion_topic,
+              :update_submissions_later,
+              :schedule_do_auto_peer_review_job_if_automatic_peer_review,
+              :delete_empty_abandoned_children,
+              :validate_assignment_overrides,
+              :update_cached_due_dates
 
   has_a_broadcast_policy
 
@@ -236,10 +235,7 @@ class Assignment < ActiveRecord::Base
   end
 
   def update_grades_if_details_changed
-    if @points_possible_was != self.points_possible ||
-        @grades_affected ||
-        @muted_was != self.muted ||
-        workflow_state_changed?
+    if points_possible_changed? || muted_changed? || workflow_state_changed?
       connection.after_transaction_commit { self.context.recompute_student_scores }
     end
     true
@@ -309,11 +305,6 @@ class Assignment < ActiveRecord::Base
     end
     self.submission_types ||= "none"
     self.peer_reviews_assign_at = [self.due_at, self.peer_reviews_assign_at].compact.max
-    @workflow_state_was = self.workflow_state_was
-    @points_possible_was = self.points_possible_was
-    @muted_was = self.muted_was
-    @submission_types_was = self.submission_types_was
-    @due_at_was = self.due_at_was
     self.points_possible = nil if self.submission_types == 'not_graded'
   end
   protected :default_values
@@ -336,7 +327,7 @@ class Assignment < ActiveRecord::Base
   end
 
   def delete_empty_abandoned_children
-    if @submission_types_was != self.submission_types
+    if submission_types_changed?
       unless self.submission_types == 'discussion_topic'
         self.discussion_topic.unlink_from(:assignment) if self.discussion_topic
       end
@@ -347,11 +338,11 @@ class Assignment < ActiveRecord::Base
   end
 
   def update_submissions_later
-    if @old_assignment_group_id != self.assignment_group_id
-      AssignmentGroup.find_by_id(@old_assignment_group_id).try(:touch) if @old_assignment_group_id.present?
+    if assignment_group_id_changed? && assignment_group_id_was.present?
+      AssignmentGroup.find_by_id(assignment_group_id_was).try(:touch)
     end
     self.assignment_group.touch if self.assignment_group
-    if @points_possible_was != self.points_possible
+    if points_possible_changed?
       send_later_if_production(:update_submissions)
     end
   end
@@ -508,7 +499,6 @@ class Assignment < ActiveRecord::Base
   def destroy
     self.workflow_state = 'deleted'
     ContentTag.delete_for(self)
-    @grades_affected = true
     self.save
 
     self.discussion_topic.destroy if self.discussion_topic && !self.discussion_topic.deleted?
@@ -521,7 +511,6 @@ class Assignment < ActiveRecord::Base
 
   def restore(from=nil)
     self.workflow_state = self.context.feature_enabled?(:draft_state) ? 'unpublished' : 'published'
-    @grades_affected = true
     self.save
     self.discussion_topic.restore if self.discussion_topic && from != :discussion_topic
     self.quiz.restore if self.quiz && from != :quiz
@@ -672,11 +661,6 @@ class Assignment < ActiveRecord::Base
     score
   end
 
-  def set_old_assignment_group_id
-    @old_assignment_group_id = self.assignment_group_id_was
-  end
-  protected :set_old_assignment_group_id
-
   def infer_times
     # set the time to 11:59 pm in the creator's time zone, if none given
     self.due_at = CanvasTime.fancy_midnight(self.due_at)

app/models/assignment_group.rb

@@ -37,26 +37,24 @@ class AssignmentGroup < ActiveRecord::Base
 
   before_save :set_context_code
   before_save :generate_default_values
-  before_save :group_weight_changed
   after_save :course_grading_change
   after_save :touch_context
   after_save :update_student_grades
 
   def generate_default_values
-    if self.name == "" || self.name.nil?
+    if self.name.blank?
       self.name = t 'default_title', "Assignments"
     end
     if !self.group_weight
       self.group_weight = 0
     end
-    @grades_changed = self.rules_changed? || self.group_weight_changed?
     self.default_assignment_name = self.name
     self.default_assignment_name = self.default_assignment_name.singularize if I18n.locale == :en
   end
   protected :generate_default_values
 
   def update_student_grades
-    if @grades_changed
+    if self.rules_changed? || self.group_weight_changed?
       connection.after_transaction_commit { self.context.recompute_student_scores }
     end
   end
@@ -146,13 +144,8 @@ class AssignmentGroup < ActiveRecord::Base
   scope :for_context_codes, lambda { |codes| active.where(:context_code => codes).order(:position) }
   scope :for_course, lambda { |course| where(:context_id => course, :context_type => 'Course') }
 
-  def group_weight_changed
-    @group_weight_changed = self.group_weight_changed?
-    true
-  end
-
   def course_grading_change
-    self.context.grade_weight_changed! if @group_weight_changed && self.context && self.context.group_weighting_scheme == 'percent'
+    self.context.grade_weight_changed! if group_weight_changed? && self.context && self.context.group_weighting_scheme == 'percent'
     true
   end
 

app/models/submission.rb

@@ -214,7 +214,7 @@ class Submission < ActiveRecord::Base
   end
 
   def update_final_score
-    if @score_changed
+    if score_changed?
       connection.after_transaction_commit { Enrollment.send_later_if_production(:recompute_final_score, self.user_id, self.context.id) }
       self.assignment.send_later_if_production(:multiple_module_actions, [self.user_id], :scored, self.score) if self.assignment
     end
@@ -504,7 +504,6 @@ class Submission < ActiveRecord::Base
     end
     @just_submitted = self.submitted? && self.submission_type && (self.new_record? || self.workflow_state_changed?)
     if score_changed?
-      @score_changed = true
       self.grade = assignment ?
         assignment.score_to_grade(score, grade) :
         score.to_s