Merge pull request #1959 from codidact/0valt/1250/pinned-links-time-filter

Allow pinned links (the tool) to be filtered by period

Author: Oaphi

app/controllers/pinned_links_controller.rb

@@ -4,21 +4,36 @@ class PinnedLinksController < ApplicationController
   before_action :verify_mod_on_current_community, only: [:edit, :update]
 
   def index
-    links = if current_user.at_least_global_moderator? && params[:global] == '2'
-              PinnedLink.unscoped
-            elsif current_user.at_least_global_moderator? && params[:global] == '1'
-              PinnedLink.where(community: nil)
-            else
-              PinnedLink.where(community: @community)
-            end
+    @period = params[:period].presence || 'current'
+
+    @links = if current_user.at_least_global_moderator? && params[:global] == '2'
+               PinnedLink.unscoped
+             elsif current_user.at_least_global_moderator? && params[:global] == '1'
+               PinnedLink.where(community: nil)
+             else
+               PinnedLink.where(community: @community)
+             end
+
+    @links = case @period
+             when 'past'
+               @links.past
+             when 'current'
+               @links.current
+             when 'future'
+               @links.future
+             else
+               @links
+             end
+
     @links = case params[:filter]
              when 'all'
-               links.all
+               @links.all
              when 'inactive'
-               links.where(active: false).all
+               @links.where(active: false).all
              else
-               links.where(active: true).all
+               @links.where(active: true).all
              end
+
     render layout: 'without_sidebar'
   end
 

app/models/pinned_link.rb

@@ -7,14 +7,66 @@ class PinnedLink < ApplicationRecord
     includes(:post, post: [:community])
   }
 
+  scope :past, lambda {
+    timed.where('shown_before < ?', DateTime.now)
+  }
+
+  scope :current, lambda {
+    where(shown_after: nil, shown_before: nil).or(
+      timed.where('shown_after is null or shown_after <= ?', DateTime.now)
+           .where.not('shown_before < ?', DateTime.now)
+    )
+  }
+
+  scope :future, lambda {
+    timed.where('shown_after > ?', DateTime.now)
+         .where('shown_before is null or shown_before >= ?', DateTime.now)
+  }
+
+  scope :timed, lambda {
+    after = where.not(shown_after: nil)
+    before = where.not(shown_before: nil)
+    before.or(after)
+  }
+
+  validate :check_period
   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?
     shown_before.present? || shown_after.present?
   end
 
+  def check_period
+    return unless shown_before.present? && shown_after.present?
+
+    if shown_before < shown_after
+      errors.add(:base, 'end date cannot be earlier than the start date')
+    end
+  end
+
   def check_post_or_url
     unless post_id.present? || link.present?
       errors.add(:base, 'either a post or a URL must be set')

app/views/pinned_links/index.html.erb

@@ -35,6 +35,22 @@
                         class: "button is-muted is-outlined #{(params[:filter] == 'all') ? 'is-active' : ''}" %>
         </div>
     </div>
+    <div class="grid--cell">
+        <div class="button-list is-gutterless">
+            <%= link_to 'past',
+                        query_url(period: 'past'),
+                        class: "button is-muted is-outlined #{'is-active' if @period == 'past'}" %>
+            <%= link_to 'current',
+                        query_url(period: 'current'),
+                        class: "button is-muted is-outlined #{'is-active' if @period == 'current'}" %>
+            <%= link_to 'future',
+                        query_url(period: 'future'),
+                        class: "button is-muted is-outlined #{'is-active' if @period == 'future'}" %>
+            <%= link_to 'any',
+                        query_url(period: 'any'),
+                        class: "button is-muted is-outlined #{'is-active' if @period == 'any'}" %>
+        </div>
+    </div>
     <div class="grid--cell is-flexible">
     </div>
     <div class="grid--cell">
@@ -61,7 +77,7 @@
         <% pl_link = pl.post.nil? ? pl.link : ('Post #' + pl.post.id.to_s) %>
         <% pl_label = pl.post.nil? ? pl.label : (pl.post.parent.nil? ? pl.post.title : pl.post.parent.title) %>
         <tr>
-            <td>
+            <td class="nowrap">
                 <% if pl.shown_before.nil? %>
                     <% if pl.post.nil? %>
                         link

test/controllers/pinned_links_controller_test.rb

@@ -3,6 +3,103 @@
 class PinnedLinksControllerTest < ActionController::TestCase
   include Devise::Test::ControllerHelpers
 
+  test ':index should correctly filter by community' do
+    sign_in users(:global_moderator)
+
+    get :index, params: { global: '1' }
+    @links = assigns(:links)
+
+    assert_response(:success)
+    assert @links.any?
+    assert @links.none?(&:community_id?)
+
+    get :index, params: { global: '2' }
+    @links = assigns(:links)
+
+    assert_response(:success)
+    assert @links.any?
+
+    links_ids = pinned_links.map(&:id)
+
+    assert(@links.all? { |link| links_ids.include?(link.id) })
+  end
+
+  test ':index should correctly filter by activity status' do
+    sign_in users(:moderator)
+
+    get :index, params: { filter: 'all' }
+    assert_response(:success)
+    assert assigns(:links).any?
+
+    get :index, params: { filter: 'inactive' }
+    @links = assigns(:links)
+
+    assert_response(:success)
+    assert @links.any?
+    assert @links.none?(&:active?)
+  end
+
+  test ':index should correctly filter by period' do
+    sign_in users(:moderator)
+
+    now = DateTime.now
+
+    get :index
+    @links = assigns(:links)
+    assert_response(:success)
+    assert @links.any?
+
+    links_ids = pinned_links.map(&:id)
+    assert(@links.all? { |link| links_ids.include?(link.id) })
+
+    get :index, params: { period: 'current' }
+    @links = assigns(:links)
+
+    assert_response(:success)
+    assert @links.any?
+    @links.each do |link|
+      assert link.current?(now)
+    end
+
+    get :index, params: { period: 'past' }
+    @links = assigns(:links)
+
+    assert_response(:success)
+    assert @links.any?
+    @links.each do |link|
+      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.future?(now)
+    end
+  end
+
+  test ':index should treat invalid period filter as no filter' do
+    sign_in users(:moderator)
+
+    get :index, params: { period: 'invalid' }
+    @links = assigns(:links)
+    assert_response(:success)
+
+    links_ids = pinned_links.map(&:id)
+    assert(@links.all? { |link| links_ids.include?(link.id) })
+  end
+
+  test 'only mods or higher should be able to see pinned links' do
+    users.each do |user|
+      sign_in user
+      get :index
+
+      assert_response(user.at_least_moderator? ? :success : :not_found)
+    end
+  end
+
   test 'only mods or higher should be able to create pinned links' do
     post = posts(:question_one)
 
@@ -61,7 +158,7 @@ class PinnedLinksControllerTest < ActionController::TestCase
     assert_equal 'updated label', assigns(:link).label
   end
 
-  test 'update should correctly handle invlid pinned links' do
+  test 'update should correctly handle invalid pinned links' do
     sign_in users(:moderator)
     try_update_pinned_link(pinned_links(:active_with_label), link: nil)
     assert_response(:bad_request)

test/fixtures/pinned_links.yml

@@ -14,3 +14,32 @@ inactive:
   label: test link
   link: https://example.com
   active: false
+
+timed_past:
+  community: sample
+  label: link in the past
+  link: https://example.com
+  active: true
+  shown_before: 2019-01-01T00:00:00.000000Z
+
+timed_present:
+  community: sample
+  label: link in the present
+  link: https://example.com
+  active: true
+  shown_after: 2019-01-01T00:00:00.000000Z
+  shown_before: <%= DateTime.now + 1 %>
+
+timed_future:
+  community: sample
+  label: link in the future
+  link: https://example.com
+  active: true
+  shown_after: <%= DateTime.now + 1 %>
+  shown_before: <%= DateTime.now + 1 %>
+
+global_active_with_label:
+  community: ~
+  label: test global link
+  link: https://example.com
+  active: true

test/models/pinned_link_test.rb

@@ -1,7 +1,57 @@
 require 'test_helper'
 
 class PinnedLinkTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  test ':timed scope should only return time-constrained links' do
+    timed = PinnedLink.timed
+
+    assert timed.any?
+    timed.each do |link|
+      assert link.timed?, "link \"#{link.label}\" is not timed"
+    end
+  end
+
+  test ':current scope should return non-timed or current links' do
+    now = DateTime.now
+    current = PinnedLink.current
+
+    assert current.any?
+    current.each do |link|
+      assert link.current?(now)
+    end
+  end
+
+  test ':past scope should only return past links' do
+    now = DateTime.now
+    past = PinnedLink.past
+
+    assert past.any?
+    past.each do |link|
+      assert link.past?(now)
+    end
+  end
+
+  test ':future scope should only return future links' do
+    now = DateTime.now
+    future = PinnedLink.future
+
+    assert future.any?
+    future.each do |link|
+      assert link.future?(now)
+    end
+  end
+
+  test 'pinned links should be correctly validated' do
+    valid_with_link = PinnedLink.new(link: 'https://example.com')
+    valid_with_post = PinnedLink.new(post: posts(:question_one))
+    period_mismatch = PinnedLink.new(shown_before: DateTime.now - 1, shown_after: DateTime.now)
+
+    [
+      [valid_with_link, true],
+      [valid_with_post, true],
+      [period_mismatch, false]
+    ].each do |test|
+      link, status = test
+      assert_equal status, link.valid?
+    end
+  end
 end