Browse files

Refactor announcement logic into a presenter.

This makes the view and controller much simpler, and also makes it
easier to extract shared logic between the today and last 7 days
collections.
  • Loading branch information...
1 parent f979952 commit f30eab8b13cf4d6c35c2b9e47d9a5ed2f159ecbd @lazyatom-and-floehopper lazyatom-and-floehopper committed Feb 3, 2012
View
25 app/controllers/announcements_controller.rb
@@ -2,35 +2,12 @@ class AnnouncementsController < PublicFacingController
def index
@featured_news_articles = featured_news_articles
- @announced_in_last_7_days = announced_in_last_7_days
- @announced_today = announced_today
- @announced_today_featured = @announced_today.select { |a| a.respond_to?(:image) && a.image.present? }.take(3)
- @announced_today_not_featured = @announced_today - @announced_today_featured
+ @announced = AnnouncementPresenter.new
end
private
def featured_news_articles
NewsArticle.published.featured.by_first_published_at.limit(3).includes(:document_identity, :document_relations, :policy_areas)
end
-
- def announced_today
- today = 1.day.ago
- news_today = NewsArticle.published.first_published_since(today).not_featured.
- by_first_published_at.includes(:document_identity, :document_relations, :policy_areas)
- speeches_today = Speech.published.first_published_since(today).
- by_first_published_at.includes(:document_identity, role_appointment: [:person, :role])
-
- Announcement.by_first_published_at(news_today + speeches_today)
- end
-
- def announced_in_last_7_days
- this_week = 1.week.ago..1.day.ago
- news_this_week = NewsArticle.published.first_published_during(this_week).not_featured.
- by_first_published_at.includes(:document_identity, :document_relations, :policy_areas)
- speeches_this_week = Speech.published.first_published_during(this_week).
- by_first_published_at.includes(:document_identity, role_appointment: [:person, :role])
-
- Announcement.by_first_published_at(news_this_week + speeches_this_week)
- end
end
View
51 app/models/announcement_presenter.rb
@@ -0,0 +1,51 @@
+require "delegate"
+
+class AnnouncementPresenter
+ class Set < SimpleDelegator
+ attr_reader :all
+
+ def initialize(announcements, number_to_feature=3)
+ @all = announcements.sort_by(&:first_published_at).reverse
+ super(@all)
+ @number_to_feature = number_to_feature
+ end
+
+ def featured
+ @all.select { |a| a.respond_to?(:image) && a.image.present? }.take(@number_to_feature)
+ end
+
+ def unfeatured
+ @all - featured
+ end
+ end
+
+ def initialize(number_to_feature=3)
+ @number_to_feature = number_to_feature
+ end
+
+ def today
+ return @today if @today
+ date = 1.day.ago
+ news_today = candidate_news.first_published_since(date)
+ speeches_today = candidate_speeches.first_published_since(date)
+ @today = Set.new(news_today + speeches_today, @number_to_feature)
+ end
+
+ def in_last_7_days
+ return @in_last_7_days if @in_last_7_days
+ this_week = 1.week.ago..1.day.ago
+ news_this_week = candidate_news.first_published_during(this_week)
+ speeches_this_week = candidate_speeches.first_published_during(this_week)
+ @in_last_7_days = Set.new(news_this_week + speeches_this_week, @number_to_feature)
+ end
+
+ private
+
+ def candidate_news
+ NewsArticle.published.not_featured.by_first_published_at.includes(:document_identity, :document_relations, :policy_areas)
+ end
+
+ def candidate_speeches
+ Speech.published.by_first_published_at.includes(:document_identity, role_appointment: [:person, :role])
+ end
+end
View
16 app/views/announcements/index.html.erb
@@ -34,27 +34,27 @@
</section>
<% end %>
- <% if @announced_today.any? %>
+ <% if @announced.today.any? %>
<section class="announcements group" id="last_24_hours">
<h1>In the last 24 hours</h1>
<div class="expanded group">
- <%= announcement_group(@announced_today_featured, groups_of: 3, partial: "expanded") %>
+ <%= announcement_group(@announced.today.featured, groups_of: 3, partial: "expanded") %>
</div>
- <%= announcement_group(@announced_today_not_featured, groups_of: 3, class: "group", partial: "default") %>
+ <%= announcement_group(@announced.today.unfeatured, groups_of: 3, class: "group", partial: "default") %>
</section>
<% end %>
- <% if @announced_in_last_7_days.any? %>
+ <% if @announced.in_last_7_days.any? %>
<section class="announcements group" id="last_7_days">
<h1>In the last 7 days</h1>
- <% if @announced_today.any? %>
- <%= announcement_group(@announced_in_last_7_days, groups_of: 3, class: "group", partial: "default") %>
+ <% if @announced.today.any? %>
+ <%= announcement_group(@announced.in_last_7_days, groups_of: 3, class: "group", partial: "default") %>
<% else %>
<div class="expanded group">
- <%= announcement_group(@announced_in_last_7_days.take(3), groups_of: 3, partial: "expanded") %>
+ <%= announcement_group(@announced.in_last_7_days.featured, groups_of: 3, partial: "expanded") %>
</div>
<div class="articles">
- <%= announcement_group(@announced_in_last_7_days.from(3), groups_of: 3, class: "group", partial: "default") %>
+ <%= announcement_group(@announced.in_last_7_days.unfeatured, groups_of: 3, class: "group", partial: "default") %>
</div>
<% end %>
</section>
View
171 test/functional/announcements_controller_test.rb
@@ -58,88 +58,25 @@ class AnnouncementsControllerTest < ActionController::TestCase
end
test "index shows news and speeches from the last 24 hours" do
- older_announcements = [create(:published_news_article, published_at: 25.hours.ago), create(:published_speech, published_at: 26.hours.ago)]
- announced_today = [create(:published_news_article, published_at: Time.zone.now), create(:published_speech, published_at: (23.hours.ago + 59.minutes))]
+ announced_today = [create(:published_news_article), create(:published_speech)]
+ AnnouncementPresenter.any_instance.stubs(:today).returns(AnnouncementPresenter::Set.new(announced_today))
get :index
- assert_equal announced_today, assigns[:announced_today]
-
assert_select '#last_24_hours' do
announced_today.each do |announcement|
assert_select_object announcement
end
end
end
- test "index shows news and speeches from the last 24 hours in order of first publication" do
- announced_today = [create(:published_news_article, published_at: Time.zone.now), create(:published_speech, published_at: (23.hours.ago + 59.minutes))]
-
- editor = create(:departmental_editor)
- announced_today.push(updated_announcement = announced_today.pop.create_draft(editor))
- updated_announcement.change_note = "change-note"
- updated_announcement.publish_as(editor, force: true)
-
- get :index
-
- assert_equal announced_today, assigns[:announced_today]
- end
-
- test "index does not show recently-updated old news and speeches as happening in the last 24 hours" do
- older_announcements = [create(:published_news_article, published_at: 25.hours.ago), create(:published_speech, published_at: 26.hours.ago)]
- announced_today = [create(:published_news_article, published_at: Time.zone.now), create(:published_speech, published_at: (23.hours.ago + 59.minutes))]
-
- editor = create(:departmental_editor)
- updated_older_announcements = older_announcements.map do |older_announcement|
- older_announcement.create_draft(editor).tap do |updated_older_announcement|
- updated_older_announcement.publish_as(editor, force: true)
- end
- end
-
- get :index
-
- assert_equal announced_today, assigns[:announced_today]
- assert_select '#last_24_hours' do
- updated_older_announcements.each do |updated_older_announcement|
- refute_select_object updated_older_announcement
- end
- end
- end
-
- test "featured stories should not appear in the last 24 hours or last 7 days lists of announcements" do
- featured = [
- create(:featured_news_article, published_at: Time.zone.now),
- create(:featured_news_article, published_at: 24.hours.ago),
- create(:featured_news_article, published_at: 2.days.ago)
- ]
-
- announced_today = [create(:published_news_article, published_at: Time.zone.now), create(:published_speech, published_at: (23.hours.ago + 59.minutes))]
-
- announced_in_last_7_days = [
- create(:published_news_article, published_at: 24.hours.ago),
- create(:published_speech, published_at: 6.days.ago),
- ]
-
- get :index
-
- assert_equal featured, assigns[:featured_news_articles]
- assert_equal announced_today, assigns[:announced_today]
- assert_equal announced_in_last_7_days, assigns[:announced_in_last_7_days]
- end
-
test "index highlights first three announcements with images that have been published in the last 24 hours" do
- image = fixture_file_upload('portas-review.jpg')
- latest_news_with_image = create(:published_news_article, image: image, published_at: Time.zone.now)
- latest_news_without_image = create(:published_news_article, published_at: Time.zone.now)
- earlier_speech_without_image = create(:published_speech, published_at: 1.hour.ago)
- even_earlier_speech_without_image = create(:published_speech, published_at: 2.hours.ago)
- early_news_with_image = create(:published_news_article, image: image, published_at: 3.hours.ago)
- early_news_without_image = create(:published_news_article, published_at: 3.hours.ago)
- earliest_news_with_image = create(:published_news_article, image: image, published_at: 5.hours.ago)
-
- featured = [latest_news_with_image, early_news_with_image, earliest_news_with_image]
- non_featured = [latest_news_without_image, early_news_without_image,
- even_earlier_speech_without_image, early_news_without_image]
+ featured = [create(:published_news_article), create(:published_speech), create(:published_news_article)]
+ unfeatured = [create(:published_news_article), create(:published_speech), create(:published_news_article),
+ create(:published_news_article)]
+
+ today = stub("today", featured: featured, unfeatured: unfeatured, any?: true)
+ AnnouncementPresenter.any_instance.stubs(:today).returns(today)
get :index
@@ -158,7 +95,7 @@ class AnnouncementsControllerTest < ActionController::TestCase
end
end
- non_featured.each do |announcement|
+ unfeatured.each do |announcement|
assert_select_object announcement do
refute_select "img"
assert_select_announcement_title announcement
@@ -174,103 +111,71 @@ class AnnouncementsControllerTest < ActionController::TestCase
refute_select '#last_24_hours'
end
- test "index shows news and speeches from the last 7 days excluding those within the last 24 hours" do
- older_announcements = [create(:published_news_article, published_at: 8.days.ago), create(:published_speech, published_at: 8.days.ago)]
- announced_today = [create(:published_news_article, published_at: Time.zone.now), create(:published_speech, published_at: (23.hours.ago + 59.minutes))]
-
- announced_in_last_7_days = [
- create(:published_news_article, published_at: 24.hours.ago),
- create(:published_speech, published_at: 6.days.ago),
- ]
-
- get :index
-
- assert_equal announced_in_last_7_days.to_set, assigns[:announced_in_last_7_days].to_set
- end
-
- test "index shows news and speeches from the last 7 days in order of first publication" do
+ test "should display list of correctly formatted announcements for the last 7 days" do
announced_in_last_7_days = [
- create(:published_news_article, published_at: 24.hours.ago),
- create(:published_speech, published_at: 6.days.ago),
+ create(:published_news_article),
+ create(:published_speech),
+ create(:published_news_article)
]
-
- editor = create(:departmental_editor)
- announced_in_last_7_days.push(updated_announcement = announced_in_last_7_days.pop.create_draft(editor))
- updated_announcement.change_note = "change-note"
- updated_announcement.publish_as(editor, force: true)
+ AnnouncementPresenter.any_instance.stubs(:in_last_7_days).returns(AnnouncementPresenter::Set.new(announced_in_last_7_days))
get :index
- assert_equal announced_in_last_7_days, assigns[:announced_in_last_7_days]
- end
-
- test "index does not show old news and speeches updated in the last 7 days as happening in the last 7 days" do
- older_announcements = [create(:published_news_article, published_at: 8.days.ago), create(:published_speech, published_at: 8.days.ago)]
- announced_today = [create(:published_news_article, published_at: Time.zone.now), create(:published_speech, published_at: (23.hours.ago + 59.minutes))]
-
- announced_in_last_7_days = [
- create(:published_news_article, published_at: 24.hours.ago),
- create(:published_speech, published_at: 6.days.ago),
- ]
+ assert_select '#last_7_days' do
+ assert_select 'article', count: 3
- Timecop.travel(3.days.ago) do
- editor = create(:departmental_editor)
- older_announcements.each do |older_announcement|
- older_announcement.create_draft(editor).publish_as(editor, force: true)
+ announced_in_last_7_days.each do |announcement|
+ assert_select_object announcement do
+ refute_select "img"
+ assert_select_announcement_title announcement
+ assert_select_announcement_summary announcement
+ assert_select_announcement_metadata announcement
+ end
end
end
-
- get :index
-
- assert_equal announced_in_last_7_days.to_set, assigns[:announced_in_last_7_days].to_set
end
- test "should display list of correctly formatted announcements for the last 7 days" do
+ test "should not show images for any announcements in last 7 days if some exist in the last 24 hours" do
announced_today = [create(:published_news_article, published_at: Time.zone.now), create(:published_speech, published_at: (23.hours.ago + 59.minutes))]
announced_in_last_7_days = [
- create(:published_news_article, published_at: 1.day.ago),
- create(:published_speech, published_at: 2.days.ago),
- create(:published_news_article, published_at: 3.days.ago),
- create(:published_news_article, published_at: 4.days.ago),
- create(:published_news_article, published_at: 5.days.ago),
- create(:published_speech, published_at: 5.days.ago),
- create(:published_speech, published_at: 6.days.ago),
+ create(:published_news_article),
+ create(:published_speech),
+ create(:published_news_article)
]
+ AnnouncementPresenter.any_instance.stubs(:today).returns(AnnouncementPresenter::Set.new(announced_today))
+ AnnouncementPresenter.any_instance.stubs(:in_last_7_days).returns(AnnouncementPresenter::Set.new(announced_in_last_7_days))
get :index
assert_select '#last_7_days' do
- assert_select 'article', count: 7
assert_select '.expanded', count: 0
announced_in_last_7_days.each do |announcement|
assert_select_object announcement do
refute_select "img"
- assert_select_announcement_title announcement
- assert_select_announcement_summary announcement
- assert_select_announcement_metadata announcement
end
end
end
end
test "announcements in the last 7 days should show expanded view if there are no stories in the last 24 hours" do
- announced_in_last_7_days = [
- create(:published_news_article, published_at: 1.day.ago),
- create(:published_speech, published_at: 2.days.ago),
- create(:published_news_article, published_at: 3.days.ago),
- create(:published_news_article, published_at: 4.days.ago)
- ]
+ unfeatured = [create(:published_news_article), create(:published_speech)]
+ featured = [create(:published_news_article)]
+
+ announced_in_last_7_days = stub("last_7_days", featured: featured, unfeatured: unfeatured, any?: true)
+
+ AnnouncementPresenter.any_instance.stubs(:today).returns(AnnouncementPresenter::Set.new([]))
+ AnnouncementPresenter.any_instance.stubs(:in_last_7_days).returns(announced_in_last_7_days)
get :index
refute_select '#last_24_hours'
assert_select '#last_7_days' do
- assert_select 'article', count: 4
+ assert_select 'article', count: 3
assert_select '.expanded' do
- assert_select 'article', count: 3
+ assert_select 'article', count: 1
end
end
end
View
121 test/unit/announcement_presenter_test.rb
@@ -0,0 +1,121 @@
+require "test_helper"
+
+class AnnouncementPresenterTest < ActiveSupport::TestCase
+ include ActionDispatch::TestProcess
+
+ test "#today returns news and speeches from the last 24 hours" do
+ older_announcements = [create(:published_news_article, published_at: 25.hours.ago), create(:published_speech, published_at: 26.hours.ago)]
+ announced_today = [create(:published_news_article, published_at: Time.zone.now), create(:published_speech, published_at: (23.hours.ago + 59.minutes))]
+
+ assert_equal announced_today, AnnouncementPresenter.new.today
+ end
+
+ test "#today returns news and speeches from the last 24 hours in order of first publication" do
+ announced_today = [create(:published_news_article, published_at: Time.zone.now), create(:published_speech, published_at: (23.hours.ago + 59.minutes))]
+
+ editor = create(:departmental_editor)
+ announced_today.push(updated_announcement = announced_today.pop.create_draft(editor))
+ updated_announcement.change_note = "change-note"
+ updated_announcement.publish_as(editor, force: true)
+
+ assert_equal announced_today, AnnouncementPresenter.new.today
+ end
+
+ test "#today does not return recently-updated old news and speeches as happening in the last 24 hours" do
+ older_announcements = [create(:published_news_article, published_at: 25.hours.ago), create(:published_speech, published_at: 26.hours.ago)]
+ announced_today = [create(:published_news_article, published_at: Time.zone.now), create(:published_speech, published_at: (23.hours.ago + 59.minutes))]
+
+ editor = create(:departmental_editor)
+ updated_older_announcements = older_announcements.map do |older_announcement|
+ older_announcement.create_draft(editor).tap do |updated_older_announcement|
+ updated_older_announcement.publish_as(editor, force: true)
+ end
+ end
+
+ assert_equal announced_today, AnnouncementPresenter.new.today
+ end
+
+ test "#today does not return any featured stories" do
+ featured_news = create(:featured_news_article, published_at: Time.zone.now)
+
+ refute AnnouncementPresenter.new.today.include?(featured_news)
+ end
+
+ test "#in_last_7_days does not return any featured stories" do
+ featured_news = create(:featured_news_article, published_at: 3.days.ago)
+
+ refute AnnouncementPresenter.new.in_last_7_days.include?(featured_news)
+ end
+
+ test "#featured only returns documents with images" do
+ image = fixture_file_upload('portas-review.jpg')
+ latest_news_with_image = create(:published_news_article, image: image, image_alt_text: "blah", published_at: Time.zone.now)
+ latest_news_without_image = create(:published_news_article, published_at: Time.zone.now)
+
+ assert_equal [latest_news_with_image], AnnouncementPresenter.new.today.featured
+ end
+
+ test "#featured only returns 3 documents by default" do
+ image = fixture_file_upload('portas-review.jpg')
+ a = create(:published_news_article, image: image, image_alt_text: "blah", published_at: Time.zone.now)
+ b = create(:published_news_article, image: image, image_alt_text: "blah", published_at: 1.minute.ago)
+ c = create(:published_news_article, image: image, image_alt_text: "blah", published_at: 2.minutes.ago)
+ d = create(:published_news_article, image: image, image_alt_text: "blah", published_at: 3.minutes.ago)
+
+ assert_equal [a, b, c], AnnouncementPresenter.new.today.featured
+ end
+
+ test "#unfeatured returns any documents not present in featured" do
+ image = fixture_file_upload('portas-review.jpg')
+ latest_news_with_image = create(:published_news_article, image: image, image_alt_text: "blah", published_at: Time.zone.now)
+ latest_news_without_image = create(:published_news_article, published_at: Time.zone.now)
+ speech = create(:published_speech, published_at: 2.minutes.ago)
+
+ assert_equal [latest_news_without_image, speech], AnnouncementPresenter.new.today.unfeatured
+ end
+
+ test "#in_last_7_days doesn't include anything from within the last 24 hours" do
+ older_announcements = [create(:published_news_article, published_at: 8.days.ago), create(:published_speech, published_at: 8.days.ago)]
+ announced_today = [create(:published_news_article, published_at: Time.zone.now), create(:published_speech, published_at: (23.hours.ago + 59.minutes))]
+
+ announced_in_last_7_days = [
+ create(:published_news_article, published_at: 24.hours.ago),
+ create(:published_speech, published_at: 6.days.ago),
+ ]
+
+ assert_equal announced_in_last_7_days.to_set, AnnouncementPresenter.new.in_last_7_days.to_set
+ end
+
+ test "#in_last_7_days returns documents in order of first publication" do
+ announced_in_last_7_days = [
+ create(:published_news_article, published_at: 24.hours.ago),
+ create(:published_speech, published_at: 6.days.ago),
+ ]
+
+ editor = create(:departmental_editor)
+ announced_in_last_7_days.push(updated_announcement = announced_in_last_7_days.pop.create_draft(editor))
+ updated_announcement.change_note = "change-note"
+ updated_announcement.publish_as(editor, force: true)
+
+ assert_equal announced_in_last_7_days, AnnouncementPresenter.new.in_last_7_days
+ end
+
+ test "#in_last_7_days does not return old news and speeches updated in the last 7 days as happening in the last 7 days" do
+ older_announcements = [create(:published_news_article, published_at: 8.days.ago), create(:published_speech, published_at: 8.days.ago)]
+ announced_today = [create(:published_news_article, published_at: Time.zone.now), create(:published_speech, published_at: (23.hours.ago + 59.minutes))]
+
+ announced_in_last_7_days = [
+ create(:published_news_article, published_at: 24.hours.ago),
+ create(:published_speech, published_at: 6.days.ago),
+ ]
+
+ Timecop.travel(3.days.ago) do
+ editor = create(:departmental_editor)
+ older_announcements.each do |older_announcement|
+ older_announcement.create_draft(editor).publish_as(editor, force: true)
+ end
+ end
+
+ assert_equal announced_in_last_7_days, AnnouncementPresenter.new.in_last_7_days
+ end
+end

0 comments on commit f30eab8

Please sign in to comment.