Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Message cleanup #41

Merged
merged 5 commits into from Mar 23, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
@@ -1,3 +1,3 @@
source 'https://bundler-api.herokuapp.com'
source 'https://rubygems.org'

gemspec
2 changes: 1 addition & 1 deletion app/controllers/arturo/features_controller.rb
Expand Up @@ -40,7 +40,7 @@ def update_all
elsif feature.update_attributes(attributes)
updated_count += 1
else
errors << t('arturo.features.flash.error_updating', :id => id)
errors << t('arturo.features.flash.error_updating', :id => id, :errors => feature.errors.full_messages.to_sentence)
end
end
if errors.any?
Expand Down
17 changes: 17 additions & 0 deletions app/helpers/arturo/features_helper.rb
@@ -1,10 +1,27 @@
require 'action_view'
require 'action_view/helpers/tag_helper'
require 'action_view/helpers/form_tag_helper'

module Arturo
module FeaturesHelper
include ActionView::Helpers::TagHelper

def arturo_flash_messages(flash = self.flash)
[ :success, :notice, :error ].inject(''.html_safe) do |output, status|
[* flash[status] ].each do |messages|
output += arturo_flash_message(status, messages)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this maybe be better as

[* flash[status] ].map do |messages|
  arturo_flash_message(status, messages)
end.join(" ") 

I dunno how the format looks, if it would all be mashed together or if the current format exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That won't work because the result of arturo_flash_message is an HTML-safe string, but " " isn't, and Array#join doesn't take HTML safety into account.

end
output
end
end

def arturo_flash_message(status, message)
content_tag(:div, :class => "alert alert-#{status} alert-arturo") do
close = content_tag(:a, '&times;'.html_safe, :href => '#', :class => 'close', 'data-dismiss' => 'alert')
content_tag(:span, message) + close
end
end

def deployment_percentage_range_and_output_tags(name, value, options = {})
id = sanitize_to_id(name)
options = {
Expand Down
3 changes: 3 additions & 0 deletions app/views/arturo/features/edit.html.erb
@@ -1,2 +1,5 @@
<h2><%= t('.title', :name => @feature.name) %></h2>

<%= arturo_flash_messages %>

<%= render :partial => 'form', :locals => { :feature => @feature, :legend => t('.legend', :name => @feature.name) } %>
3 changes: 3 additions & 0 deletions app/views/arturo/features/forbidden.html.erb
@@ -1,2 +1,5 @@
<h2><%= t('.title') %></h2>

<%= arturo_flash_messages %>

<p><%= t('.text') %></p>
3 changes: 3 additions & 0 deletions app/views/arturo/features/index.html.erb
@@ -1,4 +1,7 @@
<h2><%= t('.title') %></h2>

<%= arturo_flash_messages %>

<%= form_tag(arturo_engine.features_path, :method => 'put', 'data-update-path' => arturo_engine.feature_path(:id => ':id'), :remote => true) do %>
<fieldset>
<legend><%= t('.title') %></legend>
Expand Down
3 changes: 3 additions & 0 deletions app/views/arturo/features/new.html.erb
@@ -1,2 +1,5 @@
<h2><%= t('.title') %></h2>

<%= arturo_flash_messages %>

<%= render :partial => 'form', :locals => { :feature => @feature, :legend => t('.legend') } %>
3 changes: 3 additions & 0 deletions app/views/arturo/features/show.html.erb
@@ -1,2 +1,5 @@
<h2><%= t('.title', :name => @feature.name) %></h2>

<%= arturo_flash_messages %>

<p>Deployment percentage: <%= @feature.deployment_percentage %></p>
2 changes: 1 addition & 1 deletion config/locales/en.yml
Expand Up @@ -35,6 +35,6 @@ en:
created: "Created %{name}"
error_creating: "Sorry, there was an error creating the feature."
updated: "Updated %{name}"
error_updating: "Sorry, there was an error updating %{name}"
error_updating: "Could not update feature #%{id}: %{errors}."
removed: "Removed %{name}"
error_removing: "Sorry, there was an error removing %{name}"
2 changes: 1 addition & 1 deletion gemfiles/rails3_0.gemfile
@@ -1,6 +1,6 @@
# This file was generated by Appraisal

source "https://bundler-api.herokuapp.com"
source "https://rubygems.org"

gem "rails", "~> 3.0.20"

Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rails3_1.gemfile
@@ -1,6 +1,6 @@
# This file was generated by Appraisal

source "https://bundler-api.herokuapp.com"
source "https://rubygems.org"

gem "rails", "~> 3.1.11"

Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rails3_2.gemfile
@@ -1,6 +1,6 @@
# This file was generated by Appraisal

source "https://bundler-api.herokuapp.com"
source "https://rubygems.org"

gem "rails", "~> 3.2.8"

Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rails4_0.gemfile
@@ -1,6 +1,6 @@
# This file was generated by Appraisal

source "https://bundler-api.herokuapp.com"
source "https://rubygems.org"

gem "rails", "4.0.0.beta1"

Expand Down
23 changes: 23 additions & 0 deletions test/dummy_app/test/unit/features_helper_test.rb
Expand Up @@ -4,6 +4,7 @@
class ArturoFeaturesHelperTest < ActiveSupport::TestCase

include ActionView::Helpers::TagHelper
include ActionDispatch::Assertions::SelectorAssertions
include Arturo::FeaturesHelper

attr_accessor :output_buffer
Expand All @@ -15,11 +16,33 @@ def bad_feature
end
end

def assert_select_in(html, selector, equality=nil, message=nil, &block)
assert_select(HTML::Document.new(html).root, selector, equality, &block)
rescue ArgumentError => e
if e.message =~ /assertion message must be String or Proc/
raise Test::Unit::AssertionFailedError.new("Expected #{selector.inspect} to match, but it didn't")
else
raise
end
end

def test_error_messages_for
expected = "<ul class=\"errors\"><li class=\"error\">must be less than or equal to 100</li></ul>"
actual = error_messages_for(bad_feature, :deployment_percentage)

assert_equal expected, actual
assert actual.html_safe?
end

def test_flash_messages
html = arturo_flash_messages({
:notice => 'foo',
:error => [ 'bar', 'baz' ]
})
assert_select_in html, '.alert.alert-arturo .close[data-dismiss="alert"]', :count => 3
assert_select_in html, '.alert-notice', /^foo/
assert_select_in html, '.alert-error', /^bar/
assert_select_in html, '.alert-error', /^baz/
end

end