Skip to content

Commit

Permalink
Merge pull request #650 from alphagov/notification-banner-refactorings
Browse files Browse the repository at this point in the history 
Notification banner refactorings and test clean-up
  • Loading branch information
@boffbowsh
boffbowsh committed Sep 16, 2015
2 parents 457cb7d + f1338fd commit 940f697
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 93 deletions.
2 changes: 1 addition & 1 deletion app/helpers/notification_helper.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module NotificationHelper
include ActionView::Helpers::TagHelper

def banner_notification
if node = NotificationFileLookup.instance.banner
if node = Static.banner
content_tag(:section, "<div>#{node[:file]}</div>",
{:id => "banner-notification", :class => node[:colour]}, false)
else
Expand Down
4 changes: 4 additions & 0 deletions config/initializers/notification_banner.rb
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
require 'static'
require 'notification_file_lookup'

Static.banner = NotificationFileLookup.new.banner
25 changes: 12 additions & 13 deletions lib/notification_file_lookup.rb
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
class NotificationFileLookup
include Singleton

cattr_accessor :banner_file
def initialize(banner_file_location = "#{Rails.root}/app/views/notifications")
@banner_file_location = banner_file_location
end

def banner
@banner_file ||= identify_banner_file
@banner_file[:file].blank? ? nil : @banner_file
end

def banner=(file)
Expand All @@ -15,16 +14,16 @@ def banner=(file)
private

def identify_banner_file
red = File.read("#{Rails.root}/app/views/notifications/banner_red.erb").strip
unless red.blank?
return { :file => red, :colour => :red }
end
red = File.read(File.join(@banner_file_location, "banner_red.erb")).strip
green = File.read(File.join(@banner_file_location, "banner_green.erb")).strip

green = File.read("#{Rails.root}/app/views/notifications/banner_green.erb").strip
unless green.blank?
return { :file => green, :colour => :green }
case
when red.present?
{ file: red, colour: :red }
when green.present?
{ file: green, colour: :green }
else
nil
end

{ :file => nil, :colour => nil }
end
end
3 changes: 3 additions & 0 deletions lib/static.rb
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module Static
mattr_accessor :banner
end
65 changes: 34 additions & 31 deletions test/integration/notifications_test.rb
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
require 'integration_test_helper'

class NotificationsTest < ActionDispatch::IntegrationTest
setup do
@original_banner = Static.banner
end

teardown do
Static.banner = @original_banner
end

context "banner files" do
should "have a green file" do
assert File.exist? "#{Rails.root}/app/views/notifications/banner_green.erb"
Expand All @@ -12,16 +20,16 @@ class NotificationsTest < ActionDispatch::IntegrationTest
end

context "banner notifications" do
setup do
NotificationFileLookup.instance.banner = nil
teardown do
clean_up_test_files
end

context "given view files are empty" do
setup do
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_green.erb")
.returns('')
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_red.erb")
.returns('')
create_test_file(filename: "banner_green.erb", content: '')
create_test_file(filename: "banner_red.erb", content: '')

Static.banner = NotificationFileLookup.new(location_of_test_files).banner
end

should "not show a banner notification on the page" do
Expand All @@ -32,36 +40,31 @@ class NotificationsTest < ActionDispatch::IntegrationTest

context "given view files are present for a green notification" do
setup do
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_green.erb")
.returns('<p>Everything is fine</p>')
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_red.erb")
.returns('')
end
create_test_file(filename: "banner_green.erb", content: '<p>Everything is fine</p>')
create_test_file(filename: "banner_red.erb", content: '')

context "given view files are present for a green notification" do
setup do
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_green.erb")
.returns('<p>Everything is fine</p>')
end
Static.banner = NotificationFileLookup.new(location_of_test_files).banner
end

should "show a banner notification on the page" do
visit "/templates/wrapper.html.erb"
assert page.has_selector? "#banner-notification.green"
assert_match '<p>Everything is fine</p>', page.body
end
should "show a banner notification on the page" do
visit "/templates/wrapper.html.erb"
assert page.has_selector? "#banner-notification.green"
assert_match '<p>Everything is fine</p>', page.body
end
end

context "given view files are present for a red notification" do
setup do
create_test_file(filename: "banner_green.erb", content: '')
create_test_file(filename: "banner_red.erb", content: '<p>Everything is not fine</p>')

context "given view files are present for a red notification" do
setup do
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_red.erb")
.returns('<p>Everything is fine</p>')
end
Static.banner = NotificationFileLookup.new(location_of_test_files).banner
end

should "show a banner notification on the page" do
visit "/templates/wrapper.html.erb"
assert page.has_selector? "#banner-notification.red"
assert_match '<p>Everything is fine</p>', page.body
end
should "show a banner notification on the page" do
visit "/templates/wrapper.html.erb"
assert page.has_selector? "#banner-notification.red"
assert_match '<p>Everything is not fine</p>', page.body
end
end
end
Expand Down
16 changes: 16 additions & 0 deletions test/test_helper.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,19 @@
require 'test/unit'
require 'rails/test_help'
require 'mocha/mini_test'

class ActiveSupport::TestCase
def create_test_file(filename:, content:)
FileUtils.mkdir_p location_of_test_files
full_path = File.join(location_of_test_files, filename)
File.open(full_path, "w") {|f| f << content }
end

def location_of_test_files
Rails.root.join("tmp", "test")
end

def clean_up_test_files
FileUtils.rm_r location_of_test_files
end
end
82 changes: 41 additions & 41 deletions test/unit/notification_file_lookup_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,57 +2,57 @@
require_relative '../../lib/notification_file_lookup'

describe NotificationFileLookup do
describe "banner" do
before do
NotificationFileLookup.instance.banner = nil
end

it "returns nil if both banner content files are empty" do
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_green.erb")
.returns('')
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_red.erb")
.returns('')

assert_nil NotificationFileLookup.instance.banner
end

it "returns the red banner content if present" do
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_red.erb")
.returns('<p>Keep calm and carry on.</p>')
describe "banner" do
it "returns nil if both banner content files are empty" do
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_green.erb")
.returns('')
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_red.erb")
.returns('')

assert_nil NotificationFileLookup.new.banner
end

it "returns the red banner content if present" do
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_red.erb")
.returns('<p>Keep calm and carry on.</p>')
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_green.erb")
.returns('')

expected = {:file => "<p>Keep calm and carry on.</p>", :colour => :red}
assert_equal expected, NotificationFileLookup.instance.banner
end
assert_equal expected, NotificationFileLookup.new.banner
end

it "opens banner content file only once" do
File.expects(:read).with("#{Rails.root}/app/views/notifications/banner_green.erb")
.returns('Test')
File.expects(:read).with("#{Rails.root}/app/views/notifications/banner_red.erb")
.returns('')
it "opens banner content file only once" do
lookup = NotificationFileLookup.new

File.expects(:read).with("#{Rails.root}/app/views/notifications/banner_green.erb")
.returns('Test')
File.expects(:read).with("#{Rails.root}/app/views/notifications/banner_red.erb")
.returns('')

expected = {:file => "Test", :colour => :green}
3.times do
assert_equal expected, NotificationFileLookup.instance.banner
end
end
3.times do
assert_equal expected, lookup.banner
end
end

it "returns nil if the banner content only contains whitespace" do
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_green.erb")
.returns("\n\n\r\n\r\n\n\n")
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_red.erb")
.returns('')
it "returns nil if the banner content only contains whitespace" do
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_green.erb")
.returns("\n\n\r\n\r\n\n\n")
File.stubs(:read).with("#{Rails.root}/app/views/notifications/banner_red.erb")
.returns('')

assert_nil NotificationFileLookup.instance.banner
end
assert_nil NotificationFileLookup.new.banner
end

it "falls back to green if the red file is empty" do
it "falls back to green if the red file is empty" do
File.expects(:read).with("#{Rails.root}/app/views/notifications/banner_green.erb")
.returns('<p>Nothing to see here.</p>')
.returns('<p>Nothing to see here.</p>')
File.expects(:read).with("#{Rails.root}/app/views/notifications/banner_red.erb")
.returns('')
.returns('')

expected = {:file => "<p>Nothing to see here.</p>", :colour => :green}
assert_equal expected, NotificationFileLookup.instance.banner
end
end
assert_equal expected, NotificationFileLookup.new.banner
end
end
end
19 changes: 12 additions & 7 deletions test/unit/notification_helper_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,34 @@
include NotificationHelper

describe "banner_notification" do
before do
@original_banner = Static.banner
end

after do
Static.banner = @original_banner
end

it "should return an empty string if no banner is present" do
NotificationFileLookup.any_instance.stubs(:banner).returns(nil)
Static.banner = nil
assert_equal "", banner_notification
end

describe "when a banner is present" do
it "should return a banner notification" do
banner = {:file => "<p>You've got notifications!</p>", :colour => :green}
NotificationFileLookup.any_instance.stubs(:banner).returns(banner)
Static.banner = {:file => "<p>You've got notifications!</p>", :colour => :green}
assert_match "<p>You've got notifications!</p>", banner_notification
end

it "should have a section wrapper with the banner colour for a green notification" do
banner = {:file => "<p>You've got notifications!</p>", :colour => :green}
NotificationFileLookup.any_instance.stubs(:banner).returns(banner)
Static.banner = {:file => "<p>You've got notifications!</p>", :colour => :green}

assert_equal "<section class=\"green\" id=\"banner-notification\"><div><p>You've got notifications!</p></div></section>",
banner_notification
end

it "should have a section wrapper with the banner colour for a red notification" do
banner = {:file => "<p>You've got notifications!</p>", :colour => :red}
NotificationFileLookup.any_instance.stubs(:banner).returns(banner)
Static.banner = {:file => "<p>You've got notifications!</p>", :colour => :red}

assert_equal "<section class=\"red\" id=\"banner-notification\"><div><p>You've got notifications!</p></div></section>",
banner_notification
Expand Down

0 comments on commit 940f697

Please sign in to comment.