Show up to three featured policy areas.

We've ignored the design for now as Jase is currently working on consistently displaying featuring "things" across the site. The FeaturedPolicyChooser would never display a featured policy that didn't have any published policies. We've moved this behaviour into the `feature` action of the Consultations controller.
leereilly · Jan 30, 2012 · 33a3b4c · 33a3b4c
1 parent 79a5da1
commit 33a3b4c
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 90 deletions.
diff --git a/app/controllers/admin/policy_areas_controller.rb b/app/controllers/admin/policy_areas_controller.rb
@@ -34,8 +34,12 @@ def update
 
   def feature
     @policy_area = PolicyArea.find(params[:id])
-    @policy_area.feature
-    redirect_to admin_policy_areas_path, notice: "The policy area #{@policy_area.name} is now featured"
+    if @policy_area.published_policies.any?
+      @policy_area.feature
+      redirect_to admin_policy_areas_path, notice: "The policy area #{@policy_area.name} is now featured"
+    else
+      redirect_to admin_policy_areas_path, alert: "The policy area #{@policy_area.name} cannot be featured because it has no published policies"
+    end
   end
 
   def unfeature

diff --git a/app/controllers/policy_areas_controller.rb b/app/controllers/policy_areas_controller.rb
@@ -1,7 +1,7 @@
 class PolicyAreasController < PublicFacingController
   def index
     @policy_areas = PolicyArea.with_published_policies
-    @featured_policy_area = FeaturedPolicyAreaChooser.choose_policy_area
+    @featured_policy_areas = PolicyArea.featured.order("updated_at DESC").limit(3)
   end
 
   def show
@@ -12,22 +12,6 @@ def show
     @featured_policies = FeaturedPolicyPresenter.new(@policy_area)
   end
 
-  class FeaturedPolicyAreaChooser
-    class << self
-      def choose_policy_area
-        choose_random_featured_policy_area || choose_random_policy_area
-      end
-
-      def choose_random_featured_policy_area
-        PolicyArea.unscoped.featured.randomized.first
-      end
-
-      def choose_random_policy_area
-        PolicyArea.unscoped.with_published_policies.randomized.first
-      end
-    end
-  end
-
   class FeaturedPolicyPresenter < Whitehall::Presenters::Collection
     def initialize(policy_area)
       super(policy_area.featured_policies)

diff --git a/app/views/policy_areas/index.html.erb b/app/views/policy_areas/index.html.erb
@@ -7,18 +7,20 @@
 </div>
 
 <div class="g3">
-  <% if @featured_policy_area %>
+  <% if @featured_policy_areas.any? %>
     <section class="featured">
-      <div class="g2 first">
-        <%= content_tag_for :article, @featured_policy_area do %>
-          <h1 class="name"><%= link_to @featured_policy_area.name, policy_area_path(@featured_policy_area) %></h1>
-          <p class="description"><%= @featured_policy_area.description %></p>
-        <% end %>
-      </div>
+      <% @featured_policy_areas.each do |featured_policy_area| %>
+        <div class="g2 first">
+          <%= content_tag_for :article, featured_policy_area do %>
+            <h1 class="name"><%= link_to featured_policy_area.name, policy_area_path(featured_policy_area) %></h1>
+            <p class="description"><%= featured_policy_area.description %></p>
+          <% end %>
+        </div>
 
-      <div class="g1 related">
-        <%= render partial: "documents/policies", object: @featured_policy_area.policies.published.limit(2) %>
-      </div>
+        <div class="g1 related">
+          <%= render partial: "documents/policies", object: featured_policy_area.policies.published.limit(2) %>
+        </div>
+      <% end %>
     </section>
   <% end %>
 

diff --git a/test/functional/admin/policy_areas_controller_test.rb b/test/functional/admin/policy_areas_controller_test.rb
@@ -239,18 +239,26 @@ class Admin::PolicyAreasControllerTest < ActionController::TestCase
   end
 
   test "featuring sets policy area featured flag" do
-    policy_area = create(:policy_area, featured: false)
+    policy_area = create(:policy_area, featured: false, policies: [build(:published_policy)])
     post :feature, id: policy_area
     assert policy_area.reload.featured?
   end
 
   test "featuring redirects to index and informs user the policy area is now featured" do
-    policy_area = create(:policy_area, featured: false)
+    policy_area = create(:policy_area, featured: false, policies: [build(:published_policy)])
     post :feature, id: policy_area
     assert_redirected_to admin_policy_areas_path
     assert_equal flash[:notice], "The policy area #{policy_area.name} is now featured"
   end
 
+  test "featuring is prohibited when a policy area has no published policies" do
+    policy_area = create(:policy_area, featured: false, policies: [])
+    post :feature, id: policy_area
+    assert_redirected_to admin_policy_areas_path
+    assert_equal "The policy area #{policy_area.name} cannot be featured because it has no published policies", flash[:alert]
+    refute policy_area.reload.featured?
+  end
+
   test "unfeaturing unsets policy area featured flag" do
     policy_area = create(:policy_area, featured: true)
     post :unfeature, id: policy_area

diff --git a/test/functional/policy_areas_controller_test.rb b/test/functional/policy_areas_controller_test.rb
@@ -165,7 +165,6 @@ class PolicyAreasControllerTest < ActionController::TestCase
   test "should show list of policy areas with published documents" do
     policy_area_1, policy_area_2 = create(:policy_area), create(:policy_area)
     PolicyArea.stubs(:with_published_policies).returns([policy_area_1, policy_area_2])
-    PolicyAreasController::FeaturedPolicyAreaChooser.stubs(:choose_policy_area)
 
     get :index
 
@@ -175,16 +174,14 @@ class PolicyAreasControllerTest < ActionController::TestCase
 
   test "should not display an empty list of policy areas" do
     PolicyArea.stubs(:with_published_policies).returns([])
-    PolicyAreasController::FeaturedPolicyAreaChooser.stubs(:choose_policy_area)
 
     get :index
 
     refute_select ".policy_areas"
   end
 
   test "shows a featured policy area if one exists" do
-    policy_area = create(:policy_area)
-    PolicyAreasController::FeaturedPolicyAreaChooser.stubs(:choose_policy_area).returns(policy_area)
+    policy_area = create(:featured_policy_area)
 
     get :index
 
@@ -193,71 +190,29 @@ class PolicyAreasControllerTest < ActionController::TestCase
     end
   end
 
-  test "shows featured policy area policies" do
-    policy = create(:published_policy)
-    policy_area = create(:policy_area, policies: [policy])
-    PolicyAreasController::FeaturedPolicyAreaChooser.stubs(:choose_policy_area).returns(policy_area)
+  test "shows maximum of three featured policy areas by most recently updated" do
+    older = create(:featured_policy_area, updated_at: 3.day.ago)
+    newest = create(:featured_policy_area, updated_at: 1.day.ago)
+    oldest = create(:featured_policy_area, updated_at: 4.day.ago)
+    newer = create(:featured_policy_area, updated_at: 2.day.ago)
 
     get :index
 
-    assert_select_object policy
+    assert_select ".featured .policy_area", count: 3
+    assert_select ".featured" do
+      assert_select_object newest
+      assert_select_object newer
+      assert_select_object older
+      refute_select_object oldest
+    end
   end
 
   test "shows a maximum of 2 featured policy area policies" do
     policies = [create(:published_policy), create(:published_policy), create(:published_policy)]
-    policy_area = create(:policy_area, policies: policies)
-    PolicyAreasController::FeaturedPolicyAreaChooser.stubs(:choose_policy_area).returns(policy_area)
+    policy_area = create(:featured_policy_area, policies: policies)
 
     get :index
 
     assert_select ".featured .policy", count: 2
   end
-
-  class FeaturedPolicyAreaChooserTest < ActiveSupport::TestCase
-    test "chooses random featured policy area if one exists" do
-      PolicyAreasController::FeaturedPolicyAreaChooser.stubs(:choose_random_featured_policy_area).returns(:random_featured_policy_area)
-      PolicyAreasController::FeaturedPolicyAreaChooser.expects(:choose_random_policy_area).never
-      assert_equal :random_featured_policy_area, PolicyAreasController::FeaturedPolicyAreaChooser.choose_policy_area
-    end
-
-    test "chooses random policy area if no featured policy areas found" do
-      PolicyAreasController::FeaturedPolicyAreaChooser.stubs(:choose_random_featured_policy_area).returns(nil)
-      PolicyAreasController::FeaturedPolicyAreaChooser.expects(:choose_random_policy_area).returns(:random_policy_area)
-      assert_equal :random_policy_area, PolicyAreasController::FeaturedPolicyAreaChooser.choose_policy_area
-    end
-
-    test "chooses a featured policy area at random" do
-      available_featured_policy_areas = Array.new(2) { create(:featured_policy_area) }
-      repetitions_to_reduce_the_chance_of_getting_the_same_policy_area_each_time = 10
-      randomly_chosen_featured_policy_areas = (0..repetitions_to_reduce_the_chance_of_getting_the_same_policy_area_each_time).collect do
-        PolicyAreasController::FeaturedPolicyAreaChooser.choose_random_featured_policy_area
-      end
-      assert_equal available_featured_policy_areas.uniq.sort, randomly_chosen_featured_policy_areas.uniq.sort
-    end
-
-    test "never chooses a non-featured policy area" do
-      non_featured_policy_area = create(:policy_area)
-      repetitions_to_reduce_the_chance_of_getting_the_same_policy_area_each_time = 10
-      (0..repetitions_to_reduce_the_chance_of_getting_the_same_policy_area_each_time).collect do
-        assert_nil PolicyAreasController::FeaturedPolicyAreaChooser.choose_random_featured_policy_area
-      end
-    end
-
-    test "chooses a policy area with published policies at random" do
-      available_policy_areas = Array.new(2) { create(:policy_area, policies: [create(:published_policy)]) }
-      repetitions_to_reduce_the_chance_of_getting_the_same_policy_area_each_time = 10
-      randomly_chosen_policy_areas = (0..repetitions_to_reduce_the_chance_of_getting_the_same_policy_area_each_time).collect do
-        PolicyAreasController::FeaturedPolicyAreaChooser.choose_random_policy_area
-      end
-      assert_equal available_policy_areas.uniq.sort, randomly_chosen_policy_areas.uniq.sort
-    end
-
-    test "never chooses a policy area without published documents" do
-      policy_area_without_published_document = create(:policy_area)
-      repetitions_to_reduce_the_chance_of_getting_the_same_policy_area_each_time = 10
-      (0..repetitions_to_reduce_the_chance_of_getting_the_same_policy_area_each_time).collect do
-        assert_nil PolicyAreasController::FeaturedPolicyAreaChooser.choose_random_policy_area
-      end
-    end
-  end
 end