From 864f750d92832bf3fb78637ecc361b498ba84f87 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Fri, 8 Nov 2019 21:00:09 +0100
Subject: [PATCH] Remove duplication in poll permissions

We were checking for `expired?` and `results_enabled?` in views and
helpers, when we've already defined a rule for accessing stats and
results for a poll.

This way we also fix a bug when stats were enabled but the poll wasn't
finished. In this scenario, the link pointed to the stats page, but when
clicking it we'd get a "you don't have permission" message.

Now the link doesn't point to the stats page anymore.
---
 app/helpers/polls_helper.rb           | 8 ++------
 app/views/polls/_poll_subnav.html.erb | 6 +++---
 spec/features/polls/polls_spec.rb     | 8 ++++----
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/app/helpers/polls_helper.rb b/app/helpers/polls_helper.rb
index f4f57ec5c..a3de04ccf 100644
--- a/app/helpers/polls_helper.rb
+++ b/app/helpers/polls_helper.rb
@@ -49,19 +49,15 @@ module PollsHelper
   end
 
   def link_to_poll(text, poll)
-    if poll.results_enabled?
+    if can?(:results, poll)
       link_to text, results_poll_path(id: poll.slug || poll.id)
-    elsif poll.stats_enabled?
+    elsif can?(:stats, poll)
       link_to text, stats_poll_path(id: poll.slug || poll.id)
     else
       link_to text, poll_path(id: poll.slug || poll.id)
     end
   end
 
-  def show_stats_or_results?
-    @poll.expired? && (@poll.results_enabled? || @poll.stats_enabled?)
-  end
-
   def results_menu?
     controller_name == "polls" && action_name == "results"
   end
diff --git a/app/views/polls/_poll_subnav.html.erb b/app/views/polls/_poll_subnav.html.erb
index e50d64e0a..1fdc2d71d 100644
--- a/app/views/polls/_poll_subnav.html.erb
+++ b/app/views/polls/_poll_subnav.html.erb
@@ -1,8 +1,8 @@
-<% if show_stats_or_results? %>
+<% if can?(:stats, @poll) || can?(:results, @poll) %>
   <div class="row margin-top">
     <div class="small-12 column">
       <ul class="menu simple clear">
-        <% if @poll.results_enabled? %>
+        <% if can?(:results, @poll) %>
           <% if results_menu? %>
             <li class="is-active">
               <h2><%= t("polls.show.results_menu") %></h2>
@@ -12,7 +12,7 @@
           <% end %>
         <% end %>
 
-        <% if @poll.stats_enabled? %>
+        <% if can?(:stats, @poll) %>
           <% if stats_menu? %>
             <li class="is-active">
               <h2><%= t("polls.show.stats_menu") %></h2>
diff --git a/spec/features/polls/polls_spec.rb b/spec/features/polls/polls_spec.rb
index 968479094..6479285f4 100644
--- a/spec/features/polls/polls_spec.rb
+++ b/spec/features/polls/polls_spec.rb
@@ -110,17 +110,17 @@ describe "Polls" do
     end
 
     scenario "Poll title link to stats if enabled" do
-      poll = create(:poll, name: "Poll with stats", stats_enabled: true)
+      poll = create(:poll, :expired, name: "Poll with stats", stats_enabled: true)
 
-      visit polls_path
+      visit polls_path(filter: "expired")
 
       expect(page).to have_link("Poll with stats", href: stats_poll_path(poll.slug))
     end
 
     scenario "Poll title link to results if enabled" do
-      poll = create(:poll, name: "Poll with results", stats_enabled: true, results_enabled: true)
+      poll = create(:poll, :expired, name: "Poll with results", stats_enabled: true, results_enabled: true)
 
-      visit polls_path
+      visit polls_path(filter: "expired")
 
       expect(page).to have_link("Poll with results", href: results_poll_path(poll.slug))
     end