diff --git a/app/models/card/engageable.rb b/app/models/card/engageable.rb index 9db8e6a64..ca0914087 100644 --- a/app/models/card/engageable.rb +++ b/app/models/card/engageable.rb @@ -44,7 +44,7 @@ module Card::Engageable transaction do reopen engagement&.destroy - touch(:last_active_at) + touch_last_active_at end end end diff --git a/app/models/card/eventable.rb b/app/models/card/eventable.rb index b9815fd93..520fd95d4 100644 --- a/app/models/card/eventable.rb +++ b/app/models/card/eventable.rb @@ -12,10 +12,15 @@ module Card::Eventable def event_was_created(event) transaction do create_system_comment_for(event) - touch(:last_active_at) + touch_last_active_at end end + def touch_last_active_at + # Not using touch so that we can detect attribute change on callbacks + update!(last_active_at: Time.current) + end + private def should_track_event? published? diff --git a/app/models/card/stallable.rb b/app/models/card/stallable.rb index 6a50c09b5..30bed9a99 100644 --- a/app/models/card/stallable.rb +++ b/app/models/card/stallable.rb @@ -9,7 +9,8 @@ module Card::Stallable scope :with_activity_spikes, -> { joins(:activity_spike) } scope :stalled, -> { open.with_activity_spikes.where("card_activity_spikes.updated_at": ..STALLED_AFTER_LAST_SPIKE_PERIOD.ago) } - after_update_commit :detect_activity_spikes_later, if: :saved_change_to_last_active_at? + before_update :remember_to_detect_activity_spikes + after_update_commit :detect_activity_spikes_later, if: :should_detect_activity_spikes? end def stalled? @@ -25,6 +26,14 @@ module Card::Stallable end private + def remember_to_detect_activity_spikes + @should_detect_activity_spikes = published? && last_active_at_changed? + end + + def should_detect_activity_spikes? + @should_detect_activity_spikes + end + def detect_activity_spikes_later Card::ActivitySpike::DetectionJob.perform_later(self) end diff --git a/app/models/comment/eventable.rb b/app/models/comment/eventable.rb index a3bb541b1..0b30c0504 100644 --- a/app/models/comment/eventable.rb +++ b/app/models/comment/eventable.rb @@ -8,7 +8,7 @@ module Comment::Eventable end def event_was_created(event) - card.touch(:last_active_at) + card.touch_last_active_at end private diff --git a/test/fixtures/cards.yml b/test/fixtures/cards.yml index 2ca8fbf8f..7c418cf55 100644 --- a/test/fixtures/cards.yml +++ b/test/fixtures/cards.yml @@ -6,7 +6,7 @@ logo: due_on: <%= 3.days.from_now %> created_at: <%= 1.week.ago %> status: published - last_active_at: <%= Time.current %> + last_active_at: <%= 1.week.ago %> layout: collection: writebook @@ -15,16 +15,15 @@ layout: title: Layout is broken created_at: <%= 1.week.ago %> status: published - last_active_at: <%= Time.current %> + last_active_at: <%= 1.week.ago %> text: collection: writebook creator: kevin - stage: qa_triage title: The text is too small created_at: <%= 1.week.ago %> status: published - last_active_at: <%= Time.current %> + last_active_at: <%= 1.week.ago %> stage: qa_in_progress shipping: @@ -34,4 +33,4 @@ shipping: title: We need to ship the app created_at: <%= 1.week.ago %> status: published - last_active_at: <%= Time.current %> + last_active_at: <%= 1.week.ago %> diff --git a/test/models/card/stallable_test.rb b/test/models/card/stallable_test.rb index ce3259ca4..eaf8d492c 100644 --- a/test/models/card/stallable_test.rb +++ b/test/models/card/stallable_test.rb @@ -35,4 +35,16 @@ class Card::StallableTest < ActiveSupport::TestCase assert cards(:logo).reload.stalled? assert_includes Card.stalled, cards(:logo) end + + test "don't detect activity spikes when updating attributes other than last_active_at" do + assert_no_enqueued_jobs only: Card::ActivitySpike::DetectionJob do + cards(:logo).update! created_at: 1.day.ago + end + end + + test "don't detect activity spikes when creating new cards" do + assert_no_enqueued_jobs only: Card::ActivitySpike::DetectionJob do + collections(:writebook).cards.create! title: "A new card", creator: users(:kevin) + end + end end