Commit 96c74d44 authored by Mark Florian's avatar Mark Florian

Do not raise if selected item is not found

As noticed in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/80681#note_862019211
and
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/81370#note_854661919,
the result of raising if the selected item cannot be found is a 500
error. Considering this can be triggered by user behaviour (i.e.,
changing the URL) the only sensible way to handle this is to select the
first item, as was already the case for `selected: nil`.

Addresses https://gitlab.com/gitlab-org/gitlab/-/issues/354424.
parent 49d63fea
...@@ -16,8 +16,10 @@ module ListboxHelper ...@@ -16,8 +16,10 @@ module ListboxHelper
# the sort key), `text` is the user-facing string for the item, and `href` is # the sort key), `text` is the user-facing string for the item, and `href` is
# the path to redirect to when that item is selected. # the path to redirect to when that item is selected.
# #
# The `selected` parameter is the currently selected `value`, and must # The `selected` parameter is the currently selected `value`, and should
# correspond to one of the `items`, or be `nil`. When `selected.nil?`, the first item is selected. # correspond to one of the `items`, or be `nil`. When `selected.nil?` or
# a value which does not correspond to one of the items, the first item is
# selected.
# #
# The final parameter `html_options` applies arbitrary attributes to the # The final parameter `html_options` applies arbitrary attributes to the
# returned tag. Some of these are passed to the underlying Vue component as # returned tag. Some of these are passed to the underlying Vue component as
...@@ -37,9 +39,12 @@ module ListboxHelper ...@@ -37,9 +39,12 @@ module ListboxHelper
webpack_bundle_tag 'redirect_listbox' webpack_bundle_tag 'redirect_listbox'
end end
selected ||= items.first[:value]
selected_option = items.find { |opt| opt[:value] == selected } selected_option = items.find { |opt| opt[:value] == selected }
raise ArgumentError, "cannot find #{selected} in #{items}" unless selected_option
unless selected_option
selected_option = items.first
selected = selected_option[:value]
end
button = button_tag(type: :button, class: DROPDOWN_BUTTON_CLASSES) do button = button_tag(type: :button, class: DROPDOWN_BUTTON_CLASSES) do
content_tag(:span, selected_option[:text], class: DROPDOWN_INNER_CLASS) + content_tag(:span, selected_option[:text], class: DROPDOWN_INNER_CLASS) +
......
...@@ -65,10 +65,13 @@ RSpec.describe ListboxHelper do ...@@ -65,10 +65,13 @@ RSpec.describe ListboxHelper do
end end
context 'when selected does not match any item' do context 'when selected does not match any item' do
let(:selected) { 'qux' } where(selected: [nil, 'qux'])
it 'raises an error' do with_them do
expect { subject }.to raise_error(ArgumentError, /cannot find qux/) it 'selects first item' do
expect(subject.at_css('button').content).to eq('Foo')
expect(subject.attributes['data-selected'].value).to eq('foo')
end
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment