Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Message cleanup #41

Merged
merged 5 commits into from

2 participants

James Alexander Rosen Luke van der Hoeven
James Alexander Rosen
Owner
  1. Add validation errors to the arturo.features.flash.error_updating message
  2. Show flash messages in the template, using a new helper method
James A. Rosen added some commits
James A. Rosen FeaturesController#update_all: pass model errors to translation 146a3ca
James A. Rosen FeaturesHelper: add arturo_flash_messages helper 4a4440d
James A. Rosen Views: use arturo_flash_messages helper be72d1d
James A. Rosen FeaturesHelper: require 'action_view'
https://travis-ci.org/jamesarosen/arturo/jobs/5747404 is failing when
trying to automatically load TagHelper. I can't replicate this locally,
even on the same Ruby, RubyGems, and RVM. My first hypothesis is that
loading action_view will cause the proper dependencies to be loaded.
06d04fe
James A. Rosen Gemfile: use rubygems.org directly as source 5757c81
James Alexander Rosen jamesarosen merged commit 01a64fb into from
James Alexander Rosen
Owner

@plukevdh could you give this a once-over? There's nothing too outlandish going on. I accidentally clicked merge already, but I'd still like a sanity check.

James Alexander Rosen jamesarosen deleted the branch
Luke van der Hoeven plukevdh commented on the diff
app/helpers/arturo/features_helper.rb
@@ -5,6 +6,22 @@ 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)
Luke van der Hoeven Collaborator

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.

James Alexander Rosen Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
James Alexander Rosen jamesarosen was assigned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 23, 2013
  1. FeaturesHelper: add arturo_flash_messages helper

    James A. Rosen authored
  2. Views: use arturo_flash_messages helper

    James A. Rosen authored
  3. FeaturesHelper: require 'action_view'

    James A. Rosen authored
    https://travis-ci.org/jamesarosen/arturo/jobs/5747404 is failing when
    trying to automatically load TagHelper. I can't replicate this locally,
    even on the same Ruby, RubyGems, and RVM. My first hypothesis is that
    loading action_view will cause the proper dependencies to be loaded.
  4. Gemfile: use rubygems.org directly as source

    James A. Rosen authored
This page is out of date. Refresh to see the latest.
2  Gemfile
View
@@ -1,3 +1,3 @@
-source 'https://bundler-api.herokuapp.com'
+source 'https://rubygems.org'
gemspec
2  app/controllers/arturo/features_controller.rb
View
@@ -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?
17 app/helpers/arturo/features_helper.rb
View
@@ -1,3 +1,4 @@
+require 'action_view'
require 'action_view/helpers/tag_helper'
require 'action_view/helpers/form_tag_helper'
@@ -5,6 +6,22 @@ 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)
Luke van der Hoeven Collaborator

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.

James Alexander Rosen Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ 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 = {
3  app/views/arturo/features/edit.html.erb
View
@@ -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  app/views/arturo/features/forbidden.html.erb
View
@@ -1,2 +1,5 @@
<h2><%= t('.title') %></h2>
+
+<%= arturo_flash_messages %>
+
<p><%= t('.text') %></p>
3  app/views/arturo/features/index.html.erb
View
@@ -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>
3  app/views/arturo/features/new.html.erb
View
@@ -1,2 +1,5 @@
<h2><%= t('.title') %></h2>
+
+<%= arturo_flash_messages %>
+
<%= render :partial => 'form', :locals => { :feature => @feature, :legend => t('.legend') } %>
3  app/views/arturo/features/show.html.erb
View
@@ -1,2 +1,5 @@
<h2><%= t('.title', :name => @feature.name) %></h2>
+
+<%= arturo_flash_messages %>
+
<p>Deployment percentage: <%= @feature.deployment_percentage %></p>
2  config/locales/en.yml
View
@@ -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  gemfiles/rails3_0.gemfile
View
@@ -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"
2  gemfiles/rails3_1.gemfile
View
@@ -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"
2  gemfiles/rails3_2.gemfile
View
@@ -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"
2  gemfiles/rails4_0.gemfile
View
@@ -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"
23 test/dummy_app/test/unit/features_helper_test.rb
View
@@ -4,6 +4,7 @@
class ArturoFeaturesHelperTest < ActiveSupport::TestCase
include ActionView::Helpers::TagHelper
+ include ActionDispatch::Assertions::SelectorAssertions
include Arturo::FeaturesHelper
attr_accessor :output_buffer
@@ -15,6 +16,16 @@ 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)
@@ -22,4 +33,16 @@ def test_error_messages_for
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
Something went wrong with that request. Please try again.