Skip to content

Commit

Permalink
Fix I18n::MissingTranslationData exception for checkbox 'none'
Browse files Browse the repository at this point in the history
This has been a problem since #2005, but it only came to light after a
production deployment.

Checkbox questions have an implicit 'none' response if the user doesn't select
any of the checkboxes. `CheckboxQuestionPresenter#response_labels` translates
the options for display in the 'Previous answers' section at the bottom of the
page.

When the translation fallbacks were removed in #2005, a
`I18n::MissingTranslationData` was being raised when the user did not select
any checkboxes.

Unfortunately, this commit [1] added a translation for the 'none' option even
though the `CheckboxSampleFlow` [2] didn't have an explicit `:none` option.
Note also that the `assert_page_has_content` was changed to check for 'None'
instead of 'none'.

In this commit, I've removed the translation for the 'none' option which gave
me a failing test which replicated the problem we'd seen in production.

I've fixed the failing test by adding an `if/else` statement to
`CheckboxQuestionPresenter#response_labels` which effectively reintroduces
the translation fallback in this very specific case.

I'm not totally happy with this solution, but hopefully it'll do for now.

[1]: f50ec82
[2]: https://github.com/alphagov/smart-answers/blob/master/test/fixtures/smart_answer_flows/checkbox-sample.rb
  • Loading branch information
floehopper committed Oct 15, 2015
1 parent 49905a5 commit 805ee6c
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 3 deletions.
6 changes: 5 additions & 1 deletion app/presenters/checkbox_question_presenter.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
class CheckboxQuestionPresenter < QuestionPresenter
def response_labels(values)
values.split(',').map do |value|
translate_option(value)
if value == SmartAnswer::Question::Checkbox::NONE_OPTION
value.to_s
else
translate_option(value)
end
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,3 @@ en-GB:
peppers: "Peppers"
ice_cream: "Ice Cream!!!"
pepperoni: "Pepperoni"
none: "None"
2 changes: 1 addition & 1 deletion test/integration/engine/checkbox_questions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class CheckboxQuestionsTest < EngineIntegrationTest
within 'td.previous-question-title' do
assert_page_has_content "What do you want on your pizza?"
end
within('td.previous-question-body') { assert_page_has_content "None" }
within('td.previous-question-body') { assert_page_has_content "none" }
within('.link-right') { assert page.has_link?("Change", href: "/checkbox-sample/y?previous_response=none") }
end
end
Expand Down

0 comments on commit 805ee6c

Please sign in to comment.