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
Fix select helper with block returning non-string #51743
base: main
Are you sure you want to change the base?
Conversation
As suggested by the docs for [FormOptionsHelper#select][1], the helper can be called with a block to customize option tag attributes: select(report, :campaign_ids) do available_campaigns.each do |c| tag.option(c.name, value: c.id, data: { tags: c.tags.to_json }) end end Prior to this commit, a NoMethodError error would be raised when available_campaigns in the above example was the empty array. This happened because the block passed to select would return a non-string value (the empty array) causing the call to capture in Helpers::Tags::Select#initialize to return nil: def initialize(object_name, method_name, template_object, choices, options, html_options) @Choices = block_given? ? template_object.capture { yield || "" } : choices This commit fixes this by moving the `|| ""` outside the block passed to capture. [1]: https://api.rubyonrails.org/classes/ActionView/Helpers/FormOptionsHelper.html#method-i-select
As requested by RuboCop.
@post = Post.new | ||
|
||
output_buffer = fields_for :post, @post do |f| | ||
concat(f.select(:category) { [] }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the expectation here, that it'd return an array of choices, rather than option tags? That seems new behavior, can we test that it works like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great question!
No, the expectation remains the same: the block either outputs several option tags that get captured into the output buffer or returns a string that includes the option tags.
The problem is that when the helper is used as suggested in the docs to output several option tags within a call to each
, and available_campaigns
is empty, no option tags will be output, and the block will effectively return []
which will cause @choices
in ActionView::Helpers::Tags::Select#initialize
to be set to nil
.
select(report, :campaign_ids) do
available_campaigns.each do |c|
tag.option(c.name, value: c.id, data: { tags: c.tags.to_json })
end
end
This will in turn cause the following error in options_for_select
:
NoMethodError: undefined method `map' for nil
lib/action_view/helpers/form_options_helper.rb:365:in `options_for_select'
lib/action_view/helpers/tags/select.rb:28:in `render'
lib/action_view/helpers/form_options_helper.rb:160:in `select'
lib/action_view/helpers/form_options_helper.rb:849:in `select'
test/template/form_options_helper_test.rb:722:in `block in test_select_under_fields_for_with_block_returning_a_non_string_value'
lib/action_view/helpers/capture_helper.rb:50:in `block in capture'
lib/action_view/buffers.rb:75:in `capture'
lib/action_view/helpers/capture_helper.rb:50:in `capture'
lib/action_view/helpers/form_helper.rb:1088:in `fields'
lib/action_view/helpers/form_helper.rb:1031:in `fields_for'
test/template/form_options_helper_test.rb:721:in `test_select_under_fields_for_with_block_returning_a_non_string_value'
Would changing this line to something like this explain the situation better?
concat(f.select(:category) { [] }) | |
concat(f.select(:category) { [].each { |title| content_tag(:option, title) }) |
As suggested by the docs for FormOptionsHelper#select, the helper can be called with a block to customize option tag attributes:
Prior to this commit, a NoMethodError error would be raised when available_campaigns in the above example was the empty array. This happened because the block passed to select would return a non-string value (the empty array) causing the call to capture in Helpers::Tags::Select#initialize to return nil:
This commit fixes this by moving the
|| ""
outside the block passed to capture.