Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Changed concatenate operator to keep the "localized_label" string unmodified. #805

Merged
merged 2 commits into from

2 participants

@flexoid

Required for localizer caching.
Using << instead of + causes "required_string" merge with string in cache every request and we have labels like "Title******************" after several requests.

@justinfrench

Weird! Either operator should be safe. Have you got any idea where the root cause is? Happy to merge this in, but would also love to plug the real cause and add spec coverage against a regression.

@flexoid

The problem is because << changes the object on its left hand side. And string from the localized_label is the same object that stored in the cache. So, using << we change value in the cache every time we execute
((localized_label || humanized_method_name) << requirement_text).html_safe, what means every request to the page with this label.
+ solves this problem because it leaves localized_label and, respectively, value in cache, unchanged.

@justinfrench justinfrench merged commit 4463c41 into justinfrench:master
@justinfrench

Merged in, thanks.

@justinfrench

@flexoid Looks like this failed the build under 1.8.7 and ree. Can you take a look for any 1.9-specific code, check out the fail and submit a patch please? http://travis-ci.org/#!/justinfrench/formtastic/builds/730924

@justinfrench

Looks like some 1.9-only hash syntax.

@flexoid

Sorry, just forgot about compatibility. Thanks for fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 18, 2012
  1. @flexoid

    Changed operator to keep the "localized_label" string unmodified.

    flexoid authored
    Required for localizer caching.
Commits on Feb 23, 2012
  1. @flexoid
This page is out of date. Refresh to see the latest.
View
2  lib/formtastic/inputs/base/labelling.rb
@@ -20,7 +20,7 @@ def label_html_options
end
def label_text
- ((localized_label || humanized_method_name) << requirement_text).html_safe
+ ((localized_label || humanized_method_name) + requirement_text).html_safe
end
# TODO: why does this need to be memoized in order to make the inputs_spec tests pass?
View
22 spec/inputs/label_spec.rb
@@ -10,6 +10,28 @@
mock_everything
end
+ it 'should add "required string" only once with caching enabled' do
+ with_config :i18n_cache_lookups, true do
+ ::I18n.backend.store_translations :en, { formtastic: { labels: { post: { title: "I18n title" } } } }
+ required_string = "[req_string]"
+ default_required_str = Formtastic::FormBuilder.required_string
+ Formtastic::FormBuilder.required_string = required_string
+
+ concat(semantic_form_for(@new_post) do |builder|
+ builder.input(:title, required: true, label: true)
+ end)
+ output_buffer.clear
+ concat(semantic_form_for(@new_post) do |builder|
+ builder.input(:title, required: true, label: true)
+ end)
+
+ ::I18n.backend.store_translations :en, { formtastic: { labels: { post: { title: nil } } } }
+ Formtastic::FormBuilder.required_string = default_required_str
+
+ output_buffer.scan(required_string).count.should == 1
+ end
+ end
+
it 'should humanize the given attribute' do
concat(semantic_form_for(@new_post) do |builder|
builder.input(:title)
Something went wrong with that request. Please try again.