From f5c9d24dff13220a57e4141100781c89b8747d73 Mon Sep 17 00:00:00 2001 From: Brad Price Date: Wed, 30 Oct 2019 18:33:10 -0500 Subject: [PATCH] Remove default_scope for reordering records Using a default scope to reorder records is not always the best idea, since it's easy to introduce bugs when using `.first` and `.last` methods. This change fixes an issue when trying to fetch the latest ping for a website, using `.last`, for the retry logic, we were actually fetching the first ping. --- app/models/ping.rb | 2 -- app/models/website.rb | 2 +- app/services/notifiers/ping_notifier.rb | 4 ++-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/models/ping.rb b/app/models/ping.rb index a920f5b6..a5c134d8 100644 --- a/app/models/ping.rb +++ b/app/models/ping.rb @@ -10,8 +10,6 @@ class Ping < ApplicationRecord validates(:status, presence: true) validates(:response_time, presence: true) - default_scope { order(created_at: :desc) } - after_create(:send_to_ping_notifier, unless: :skip_callbacks) def self.without_outliers diff --git a/app/models/website.rb b/app/models/website.rb index 0905341a..46e95cb9 100644 --- a/app/models/website.rb +++ b/app/models/website.rb @@ -7,7 +7,7 @@ class Website < ApplicationRecord VALID_URL_REGEX = /\A#{URI::regexp(['http', 'https'])}\z/ has_many :pings, dependent: :delete_all - has_many :recent_pings, -> { limit(5) }, class_name: 'Ping' + has_many :recent_pings, -> { order(created_at: :desc).limit(5) }, class_name: 'Ping' has_many :daily_pings, -> { limit(1_440) }, class_name: 'Ping' # Only query the max number from pagination. diff --git a/app/services/notifiers/ping_notifier.rb b/app/services/notifiers/ping_notifier.rb index 019ea5c1..e79871a8 100644 --- a/app/services/notifiers/ping_notifier.rb +++ b/app/services/notifiers/ping_notifier.rb @@ -35,11 +35,11 @@ def can_notify? end def first_ping - @first_ping ||= previous_ping.nil? + previous_ping.nil? end def previous_ping - website.pings.second + @previous_ping ||= website.pings.order(created_at: :desc).second end def message