rails 5.1: fix a bunch of after_save deprecation warnings (part 2)

Change-Id: Ic6d1c46a6fb9616f1f07373974667978a2e44898
Reviewed-on: https://gerrit.instructure.com/144401
Reviewed-by: Rob Orton <rob@instructure.com>
Tested-by: Jenkins
Product-Review: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>

app/models/account_user.rb

@@ -55,7 +55,7 @@ class AccountUser < ActiveRecord::Base
   end
 
   def update_account_associations_if_changed
-    being_deleted = self.workflow_state == 'deleted' && self.workflow_state_was != 'deleted'
+    being_deleted = self.workflow_state == 'deleted' && self.workflow_state_before_last_save != 'deleted'
     if (self.saved_change_to_account_id? || self.saved_change_to_user_id?) || being_deleted
       if self.new_record?
         return if %w{creation_pending deleted}.include?(self.user.workflow_state)

app/models/appointment_group.rb

@@ -265,11 +265,11 @@ class AppointmentGroup < ActiveRecord::Base
   set_broadcast_policy do
     dispatch :appointment_group_published
     to       { possible_users }
-    whenever { contexts.any?(&:available?) && active? && workflow_state_changed? }
+    whenever { contexts.any?(&:available?) && active? && saved_change_to_workflow_state? }
 
     dispatch :appointment_group_updated
     to       { possible_users }
-    whenever { contexts.any?(&:available?) && active? && new_appointments && !workflow_state_changed? }
+    whenever { contexts.any?(&:available?) && active? && new_appointments && !saved_change_to_workflow_state? }
 
     dispatch :appointment_group_deleted
     to       { possible_users }
@@ -399,7 +399,7 @@ class AppointmentGroup < ActiveRecord::Base
 
   def update_appointments
     changed = Hash[
-      EVENT_ATTRIBUTES.select{ |attr| send("#{attr}_changed?") }.
+      EVENT_ATTRIBUTES.select{ |attr| saved_change_to_attribute?(attr) }.
       map{ |attr| [attr, attr == :description ? description_html : send(attr)] }
     ]
 
@@ -421,8 +421,8 @@ class AppointmentGroup < ActiveRecord::Base
     end
 
     if desc
-      appointments.where(:description => description_was).update_all(:description => desc)
+      appointments.where(:description => description_before_last_save).update_all(:description => desc)
-      CalendarEvent.joins(:parent_event).where(workflow_state: ['active', 'locked'], parent_events_calendar_events: { context_id: self, context_type: 'AppointmentGroup' }, description: description_was).update_all(:description => desc)
+      CalendarEvent.joins(:parent_event).where(workflow_state: ['active', 'locked'], parent_events_calendar_events: { context_id: self, context_type: 'AppointmentGroup' }, description: description_before_last_save).update_all(:description => desc)
     end
 
     @new_appointments.each(&:reload) if @new_appointments.present?

app/models/assessment_question.rb

@@ -183,8 +183,6 @@ class AssessmentQuestion < ActiveRecord::Base
       # we may be modifying this data (translate_links), and only want to work on a copy
       data = data.try(:dup)
     end
-    # force AR to think this attribute has changed
-    self.question_data_will_change!
     write_attribute(:question_data, data.to_hash.with_indifferent_access)
   end
 

app/models/assignment.rb

@@ -390,7 +390,7 @@ class Assignment < ActiveRecord::Base
   after_save :remove_assignment_updated_flag # this needs to be after has_a_broadcast_policy for the message to be sent
 
   def validate_assignment_overrides(opts={})
-    if opts[:force_override_destroy] || group_category_id_changed?
+    if opts[:force_override_destroy] || saved_change_to_group_category_id?
       # needs to be .each(&:destroy) instead of .update_all(:workflow_state =>
       # 'deleted') so that the override gets versioned properly
       active_assignment_overrides.

app/models/assignment_override.rb

@@ -89,12 +89,12 @@ class AssignmentOverride < ActiveRecord::Base
   end
 
   def update_grading_period_grades
-    return true unless due_at_overridden && due_at_changed? && !id_changed?
+    return true unless due_at_overridden && saved_change_to_due_at? && !saved_change_to_id?
 
     course = assignment&.context || quiz&.context || quiz&.assignment&.context
     return true unless course&.grading_periods?
 
-    grading_period_was = GradingPeriod.for_date_in_course(date: due_at_was, course: course)
+    grading_period_was = GradingPeriod.for_date_in_course(date: due_at_before_last_save, course: course)
     grading_period = GradingPeriod.for_date_in_course(date: due_at, course: course)
     return true if grading_period_was&.id == grading_period&.id
 
@@ -337,9 +337,9 @@ class AssignmentOverride < ActiveRecord::Base
     self.assignment.context.available? &&
     self.assignment.published? &&
     self.assignment.created_at < 3.hours.ago &&
-    (workflow_state_changed? ||
+    (saved_change_to_workflow_state? ||
-      due_at_overridden_changed? ||
+      saved_change_to_due_at_overridden? ||
-      due_at_overridden && !Assignment.due_dates_equal?(due_at, due_at_was))
+      due_at_overridden && !Assignment.due_dates_equal?(due_at, due_at_before_last_save))
   end
 
   def set_title_if_needed

app/models/broadcast_policies/assignment_policy.rb

@@ -28,7 +28,7 @@ module BroadcastPolicies
       accepting_messages? &&
       assignment.changed_in_state(:published, :fields => :due_at) &&
       !just_published? &&
-      !AssignmentPolicy.due_dates_equal?(assignment.due_at, assignment.due_at_was) &&
+      !AssignmentPolicy.due_dates_equal?(assignment.due_at, assignment.due_at_before_last_save) &&
       created_before(3.hours.ago)
     end
 
@@ -38,14 +38,14 @@ module BroadcastPolicies
       !assignment.muted? &&
       created_before(30.minutes.ago) &&
       !just_published? &&
-      (assignment.points_possible_changed? || assignment.assignment_changed)
+      (assignment.saved_change_to_points_possible? || assignment.assignment_changed)
     end
 
     def should_dispatch_assignment_created?
       return false unless context_sendable?
 
       published_on_create? || just_published? ||
-        (assignment.workflow_state_changed? && assignment.published?)
+        (assignment.saved_change_to_workflow_state? && assignment.published?)
     end
 
     def should_dispatch_assignment_unmuted?
@@ -74,7 +74,7 @@ module BroadcastPolicies
     end
 
     def just_published?
-      assignment.workflow_state_changed? && assignment.published?
+      assignment.saved_change_to_workflow_state? && assignment.published?
     end
   end
 end

app/models/broadcast_policies/submission_policy.rb

@@ -95,12 +95,12 @@ module BroadcastPolicies
     end
 
     def just_submitted_late?
-      (just_submitted? || submission.submitted_at_changed?)
+      (just_submitted? || submission.saved_change_to_submitted_at?)
     end
 
     def is_a_resubmission?
-      submission.submitted_at_was &&
+      submission.submitted_at_before_last_save &&
-      submission.submitted_at_changed?
+      submission.saved_change_to_submitted_at?
     end
 
     def grade_updated?

app/models/calendar_event.rb

@@ -276,8 +276,8 @@ class CalendarEvent < ActiveRecord::Base
   ]
 
   def sync_child_events
-    locked_changes = LOCKED_ATTRIBUTES.select { |attr| send("#{attr}_changed?") }
+    locked_changes = LOCKED_ATTRIBUTES.select { |attr| saved_change_to_attribute?(attr) }
-    cascaded_changes = CASCADED_ATTRIBUTES.select { |attr| send("#{attr}_changed?") }
+    cascaded_changes = CASCADED_ATTRIBUTES.select { |attr| saved_change_to_attribute?(attr) }
     child_events.are_locked.update_all Hash[locked_changes.map{ |attr| [attr, send(attr)] }] if locked_changes.present?
     child_events.are_unlocked.update_all Hash[cascaded_changes.map{ |attr| [attr, send(attr)] }] if cascaded_changes.present?
   end
@@ -286,7 +286,7 @@ class CalendarEvent < ActiveRecord::Base
   def sync_parent_event
     return unless parent_event
     return if appointment_group
-    return unless start_at_changed? || end_at_changed? || workflow_state_changed?
+    return unless saved_change_to_start_at? || saved_change_to_end_at? || saved_change_to_workflow_state?
     return if @skip_sync_parent_event
     parent_event.cache_child_event_ranges! unless workflow_state == 'deleted'
   end
@@ -375,7 +375,7 @@ class CalendarEvent < ActiveRecord::Base
     whenever {
       appointment_group && parent_event &&
       deleted? &&
-      workflow_state_changed? &&
+      saved_change_to_workflow_state? &&
       @updating_user &&
       context == appointment_group.participant_for(@updating_user)
     }
@@ -397,7 +397,7 @@ class CalendarEvent < ActiveRecord::Base
     whenever {
       appointment_group && parent_event &&
       deleted? &&
-      workflow_state_changed?
+      saved_change_to_workflow_state?
     }
     data { {
       :updating_user_name => @updating_user.name,

app/models/content_participation.rb

@@ -50,7 +50,7 @@ class ContentParticipation < ActiveRecord::Base
   end
 
   def update_participation_count
-    return unless workflow_state_changed?
+    return unless saved_change_to_workflow_state?
     ContentParticipationCount.create_or_update({
       :context => content.context,
       :user => user,

app/models/context_module.rb

@@ -48,21 +48,21 @@ class ContextModule < ActiveRecord::Base
     return if self.new_record?
 
     if self.context.available? && self.active?
-      if self.workflow_state_changed? && self.workflow_state_was == "unpublished"
+      if self.saved_change_to_workflow_state? && self.workflow_state_before_last_save == "unpublished"
         # should trigger when publishing a prerequisite for an already active module
         @relock_warning = true if self.context.context_modules.active.any?{|mod| self.is_prerequisite_for?(mod)}
       end
-      if self.completion_requirements_changed?
+      if self.saved_change_to_completion_requirements?
         # removing a requirement shouldn't trigger
-        @relock_warning = true if (self.completion_requirements.to_a - self.completion_requirements_was.to_a).present?
+        @relock_warning = true if (self.completion_requirements.to_a - self.completion_requirements_before_last_save.to_a).present?
       end
-      if self.prerequisites_changed?
+      if self.saved_change_to_prerequisites?
         # ditto with removing a prerequisite
-        @relock_warning = true if (self.prerequisites.to_a - self.prerequisites_was.to_a).present?
+        @relock_warning = true if (self.prerequisites.to_a - self.prerequisites_before_last_save.to_a).present?
       end
-      if self.unlock_at_changed?
+      if self.saved_change_to_unlock_at?
         # adding a unlock_at date should trigger
-        @relock_warning = true if self.unlock_at.present? && self.unlock_at_was.blank?
+        @relock_warning = true if self.unlock_at.present? && self.unlock_at_before_last_save.blank?
       end
     end
   end

app/models/discussion_entry.rb

@@ -181,7 +181,7 @@ class DiscussionEntry < ActiveRecord::Base
   end
 
   def update_discussion
-    if %w(workflow_state message attachment_id editor_id).any? { |a| self.changed.include?(a) }
+    if %w(workflow_state message attachment_id editor_id).any? { |a| self.saved_change_to_attribute?(a) }
       dt = self.discussion_topic
       loop do
         dt.touch

app/models/discussion_topic.rb

@@ -202,7 +202,7 @@ class DiscussionTopic < ActiveRecord::Base
   end
 
   def update_materialized_view_if_changed
-    if self.sort_by_rating_changed?
+    if self.saved_change_to_sort_by_rating?
       update_materialized_view
     end
   end
@@ -220,7 +220,7 @@ class DiscussionTopic < ActiveRecord::Base
   end
 
   def update_subtopics
-    if !self.deleted? && (self.has_group_category? || !!self.group_category_id_was)
+    if !self.deleted? && (self.has_group_category? || !!self.group_category_id_before_last_save)
       send_later_if_production :refresh_subtopics
     end
   end
@@ -277,7 +277,7 @@ class DiscussionTopic < ActiveRecord::Base
       self.sync_assignment
       self.assignment.workflow_state = "published" if is_announcement && deleted_assignment
       self.assignment.description = self.message
-      if group_category_id_changed?
+      if saved_change_to_group_category_id?
         self.assignment.validate_assignment_overrides(force_override_destroy: true)
       end
       self.assignment.save
@@ -287,7 +287,7 @@ class DiscussionTopic < ActiveRecord::Base
     # ungraded to graded, or from one assignment to another; we ignore the
     # transition from graded to ungraded) we acknowledge that the users that
     # have posted have contributed to the topic
-    if self.assignment_id && self.assignment_id_changed?
+    if self.assignment_id && self.saved_change_to_assignment_id?
       recalculate_context_module_actions!
     end
   end
@@ -871,7 +871,7 @@ class DiscussionTopic < ActiveRecord::Base
 
   on_update_send_to_streams do
     check_state = !is_announcement ? 'unpublished' : 'post_delayed'
-    became_active = workflow_state_was == check_state && workflow_state == 'active'
+    became_active = workflow_state_before_last_save == check_state && workflow_state == 'active'
     if should_send_to_stream && (@content_changed || became_active)
       self.active_participants_with_visibility
     end

app/models/late_policy.rb

@@ -73,7 +73,7 @@ class LatePolicy < ActiveRecord::Base
         'late_submission_minimum_percent_enabled',
         'late_submission_minimum_percent',
         'missing_submission_deduction_enabled'
-      ] & changed
+      ] & saved_changes.keys
     ).present?
   end
 end

app/models/learning_outcome.rb

@@ -342,10 +342,10 @@ class LearningOutcome < ActiveRecord::Base
 
   def propagate_changes_to_rubrics
     # exclude new outcomes
-    return if self.id_changed?
+    return if self.saved_change_to_id?
-    return if !self.data_changed? &&
+    return if !self.saved_change_to_data? &&
-      !self.short_description_changed? &&
+      !self.saved_change_to_short_description? &&
-      !self.description_changed?
+      !self.saved_change_to_description?
 
     self.send_later_if_production(:update_associated_rubrics)
   end

app/models/rubric.rb

@@ -146,7 +146,7 @@ class Rubric < ActiveRecord::Base
   end
 
   def alignments_need_update?
-    data_changed? || workflow_state_changed?
+    saved_change_to_data? || saved_change_to_workflow_state?
   end
 
   def data_outcome_ids

app/models/submission.rb

@@ -1684,7 +1684,7 @@ class Submission < ActiveRecord::Base
   end
 
   def assignment_graded_in_the_last_hour?
-    graded_at_was && graded_at_was > 1.hour.ago
+    graded_at_before_last_save && graded_at_before_last_save > 1.hour.ago
   end
 
   def teacher
@@ -2210,7 +2210,7 @@ class Submission < ActiveRecord::Base
 
     return unless self.context.grants_right?(self.user, :participate_as_student)
 
-    if score_changed? || grade_changed? || excused_changed?
+    if saved_change_to_score? || saved_change_to_grade? || saved_change_to_excused?
       ContentParticipation.create_or_update({
         :content => self,
         :user => self.user,
@@ -2221,7 +2221,7 @@ class Submission < ActiveRecord::Base
 
   def update_line_item_result
     return if lti_result.nil?
-    lti_result.update(result_score: score) if score_changed?
+    lti_result.update(result_score: score) if saved_change_to_score?
   end
 
   def point_data?

app/models/submission_comment.rb

@@ -263,7 +263,7 @@ class SubmissionComment < ActiveRecord::Base
 
   def update_participation
     # id_changed? because new_record? is false in after_save callbacks
-    if id_changed? || (hidden_changed? && !hidden?)
+    if saved_change_to_id? || (saved_change_to_hidden? && !hidden?)
       return if submission.user_id == author_id
       return if submission.assignment.deleted? || submission.assignment.muted?
       return if provisional_grade_id.present?

app/models/wiki_page.rb

@@ -468,7 +468,7 @@ class WikiPage < ActiveRecord::Base
   end
 
   def post_to_pandapub_when_revised
-    if revised_at_changed?
+    if saved_change_to_revised_at?
       CanvasPandaPub.post_update(
         "/private/wiki_page/#{self.global_id}/update", {
           revised_at: self.revised_at

gems/canvas_sanitize/lib/canvas_sanitize/canvas_sanitize.rb

@@ -399,7 +399,7 @@ module CanvasSanitize #:nodoc:
     def fully_sanitize_fields
       fields_hash = self.class.fully_sanitize_fields_config || {}
       fields_hash.each do |field, config|
-        next unless self.changes[field].present?
+        next unless self.attribute_changed?(field)
         config ||= Sanitize::Config::RESTRICTED
         config = Sanitize::Config::RESTRICTED if config.empty?
         # Doesn't try to sanitize nil

lib/has_content_tags.rb

@@ -61,4 +61,4 @@ module HasContentTags
       mod.relock_progressions(relocked_modules, student_ids)
     end
   end
-end
+end

spec/models/broadcast_policies/assignment_policy_spec.rb

@@ -32,9 +32,9 @@ module BroadcastPolicies
            :changed_in_state => true, :due_at => Time.zone.now,
            :points_possible => 100, :assignment_changed => false,
            :just_created => false, :workflow_state => 'published',
-           :due_at_was => 7.days.ago, :points_possible_changed? => true,
+           :due_at_before_last_save => 7.days.ago, :saved_change_to_points_possible? => true,
-           :workflow_state_changed? => false,
+           :saved_change_to_workflow_state? => false,
-           :workflow_state_was => 'published')
+           :workflow_state_before_last_save => 'published')
     end
 
     let(:policy) { AssignmentPolicy.new(assignment) }
@@ -50,8 +50,8 @@ module BroadcastPolicies
 
       it 'is true when the prior version was unpublished' do
         allow(assignment).to receive(:just_created).and_return false
-        allow(assignment).to receive(:workflow_state_was).and_return 'unpublished'
+        allow(assignment).to receive(:workflow_state_before_last_save).and_return 'unpublished'
-        allow(assignment).to receive(:workflow_state_changed?).and_return true
+        allow(assignment).to receive(:saved_change_to_workflow_state?).and_return true
         expect(policy.should_dispatch_assignment_created?).to be_truthy
       end
 
@@ -63,8 +63,8 @@ module BroadcastPolicies
       specify {
         wont_send_when {
           allow(assignment).to receive(:just_created).and_return false
-          allow(assignment).to receive(:workflow_state_was).and_return 'published'
+          allow(assignment).to receive(:workflow_state_before_last_save).and_return 'published'
-          allow(assignment).to receive(:workflow_state_changed?).and_return false
+          allow(assignment).to receive(:saved_change_to_workflow_state?).and_return false
         }
       }
       specify { wont_send_when { allow(assignment).to receive(:published?).and_return false}}
@@ -73,7 +73,7 @@ module BroadcastPolicies
 
     describe '#should_dispatch_assignment_due_date_changed?' do
       before do
-        allow(assignment).to receive(:workflow_state_changed?).and_return false
+        allow(assignment).to receive(:saved_change_to_workflow_state?).and_return false
       end
 
       it 'is true when the dependent inputs are true' do
@@ -89,13 +89,13 @@ module BroadcastPolicies
       specify { wont_send_when { allow(context).to receive(:concluded?).and_return true } }
       specify { wont_send_when { allow(assignment).to receive(:just_created).and_return true } }
       specify { wont_send_when { allow(assignment).to receive(:changed_in_state).and_return false } }
-      specify { wont_send_when { allow(assignment).to receive(:due_at).and_return assignment.due_at_was } }
+      specify { wont_send_when { allow(assignment).to receive(:due_at).and_return assignment.due_at_before_last_save } }
       specify { wont_send_when { allow(assignment).to receive(:created_at).and_return 2.hours.ago } }
     end
 
     describe '#should_dispatch_assignment_changed?' do
       before do
-        allow(assignment).to receive(:workflow_state_changed?).and_return false
+        allow(assignment).to receive(:saved_change_to_workflow_state?).and_return false
       end
 
       it 'is true when the dependent inputs are true' do
@@ -113,7 +113,7 @@ module BroadcastPolicies
       specify { wont_send_when { allow(assignment).to receive(:published?).and_return false } }
       specify { wont_send_when { allow(assignment).to receive(:muted?).and_return true } }
       specify { wont_send_when { allow(assignment).to receive(:created_at).and_return 20.minutes.ago } }
-      specify { wont_send_when { allow(assignment).to receive(:points_possible_changed?).and_return false } }
+      specify { wont_send_when { allow(assignment).to receive(:saved_change_to_points_possible?).and_return false } }
     end
 
     describe '#should_dispatch_assignment_unmuted?' do

spec/models/broadcast_policies/submission_policy_spec.rb

@@ -61,8 +61,8 @@ module BroadcastPolicies
         allow(s).to receive(:quiz_submission).and_return(nil)
         allow(s).to receive(:user).and_return(user)
         allow(s).to receive(:context).and_return(course)
-        allow(s).to receive(:submitted_at_was).and_return(nil)
+        allow(s).to receive(:submitted_at_before_last_save).and_return(nil)
-        allow(s).to receive(:submitted_at_changed?).and_return(false)
+        allow(s).to receive(:saved_change_to_submitted?).and_return(false)
         allow(s).to receive(:changed_state_to).with(:submitted).and_return true
       end
     end
@@ -112,8 +112,8 @@ module BroadcastPolicies
 
     describe '#should_dispatch_assignment_resubmitted' do
       before do
-        allow(submission).to receive(:submitted_at_was).and_return(1.day.ago)
+        allow(submission).to receive(:submitted_at_before_last_save).and_return(1.day.ago)
-        allow(submission).to receive(:submitted_at_changed?).and_return(true)
+        allow(submission).to receive(:saved_change_to_submitted_at?).and_return(true)
       end
 
       def wont_send_when