reworked pinned links period scoped to fix some edge cases + added respective predicates

Author: Oaphi

app/models/pinned_link.rb

@@ -7,23 +7,20 @@ class PinnedLink < ApplicationRecord
     includes(:post, post: [:community])
   }
 
-  # a past link is one that ended in the past
   scope :past, lambda {
     timed.where('shown_before < ?', DateTime.now)
   }
 
-  # a current link is not timed or started in the past and ends in the future
   scope :current, lambda {
     where(shown_after: nil, shown_before: nil).or(
-      timed.where('shown_after <= ?', DateTime.now)
-           .where('shown_before > ?', DateTime.now)
+      timed.where('shown_after is null or shown_after <= ?', DateTime.now)
+           .where.not('shown_before < ?', DateTime.now)
     )
   }
 
-  # a future link is one that both starts and ends in the future
   scope :future, lambda {
     timed.where('shown_after > ?', DateTime.now)
-         .where('shown_before > ?', DateTime.now)
+         .where('shown_before is null or shown_before >= ?', DateTime.now)
   }
 
   scope :timed, lambda {
@@ -34,6 +31,27 @@ class PinnedLink < ApplicationRecord
 
   validate :check_post_or_url
 
+  # Is the link not timed or started in the past & hasn't ended yet?
+  # @param now [DateTime, nil] timestamp to compare to
+  # @return [Boolean] check result
+  def current?(now = DateTime.now)
+    !timed? || !(future?(now) || past?(now))
+  end
+
+  # Does the link start in the future?
+  # @param now [DateTime, nil] timestamp to compare to
+  # @return [Boolean] check result
+  def future?(now = DateTime.now)
+    shown_after.present? && shown_after > now && (shown_before.nil? || shown_before >= now)
+  end
+
+  # Has the link ended in the past?
+  # @param now [DateTime, nil] timestamp to compare to
+  # @return [Boolean] check result
+  def past?(now = DateTime.now)
+    shown_before.present? && shown_before < now
+  end
+
   # Is the link time-constrained?
   # @return [Boolean] check result
   def timed?

test/controllers/pinned_links_controller_test.rb

@@ -52,31 +52,31 @@ class PinnedLinksControllerTest < ActionController::TestCase
     links_ids = pinned_links.map(&:id)
     assert(@links.all? { |link| links_ids.include?(link.id) })
 
-    get :index, params: { period: 'past' }
+    get :index, params: { period: 'current' }
     @links = assigns(:links)
+
     assert_response(:success)
     assert @links.any?
-
     @links.each do |link|
-      assert link.timed? && link.shown_before < now
+      assert link.current?(now)
     end
 
-    get :index, params: { period: 'current' }
+    get :index, params: { period: 'past' }
     @links = assigns(:links)
+
     assert_response(:success)
     assert @links.any?
-
     @links.each do |link|
-      assert !link.timed? || (link.shown_before > now && link.shown_after <= now)
+      assert link.past?(now)
     end
 
     get :index, params: { period: 'future' }
     @links = assigns(:links)
+
     assert_response(:success)
     assert @links.any?
-
     @links.each do |link|
-      assert link.timed? && link.shown_before > now && link.shown_after > now
+      assert link.future?(now)
     end
   end
 

test/models/pinned_link_test.rb

@@ -5,7 +5,6 @@ class PinnedLinkTest < ActiveSupport::TestCase
     timed = PinnedLink.timed
 
     assert timed.any?
-
     timed.each do |link|
       assert link.timed?, "link \"#{link.label}\" is not timed"
     end
@@ -16,9 +15,8 @@ class PinnedLinkTest < ActiveSupport::TestCase
     current = PinnedLink.current
 
     assert current.any?
-
     current.each do |link|
-      assert !link.timed? || (link.shown_before > now && link.shown_after <= now)
+      assert link.current?(now)
     end
   end
 
@@ -27,9 +25,8 @@ class PinnedLinkTest < ActiveSupport::TestCase
     past = PinnedLink.past
 
     assert past.any?
-
     past.each do |link|
-      assert link.timed? && link.shown_before < now
+      assert link.past?(now)
     end
   end
 
@@ -38,9 +35,8 @@ class PinnedLinkTest < ActiveSupport::TestCase
     future = PinnedLink.future
 
     assert future.any?
-
     future.each do |link|
-      assert link.timed? && link.shown_before > now && link.shown_after > now
+      assert link.future?(now)
     end
   end
 end