Remove i18n code #2167
Merged
Remove i18n code #2167
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using this script was failing if/when the ERB question templates contained interpolated values. Using the `MethodMissingHelper` module allows us to avoid these interpolation errors.
This script has now served its purpose. All flows have been converted from using i18n templates to ERB templates.
I've had to introduce `Flow#use_i18n_templates_for_questions` temporarily, so that I can keep all the tests passing.
It's now the default to use ERB templates for questions so we no longer need to specify it explicitly. I've reversed the assertions in the "change default renderer for questions" test in test/unit/flow_test.rb. It now asserts that the first question uses ERB and that the second has been switched to use I18n. I've switched to using `Flow.build` instead of `Flow.new` and `Flow#define` in flow_registration_presenter_with_erb_renderer_test and graph_presenter_with_erb_renderer_test. This is because it's no longer necessary to call `#use_erb_templates_for_questions` between instantiation and the call to `#define`.
It's now the default to use ERB templates for questions so we no longer need to specify it explicitly. I've also updated the checksum data for each flow now that they've changed.
It's now the default to use ERB templates for questions so we don't need this option any more.
I've removed the flow_registration_presenter_test file because the equivalent tests still exist flow_registration_presenter_with_erb_renderer_test, but for when the flow is using ERB question templates. I've removed the two i18n yaml files that were only being used by this test.
The name no longer needs to include "with_erb_renderer" as this is now the only unit test for the FlowRegistrationPresenter.
And associated graph.yml locale file.
|
I'm currently running all regression tests in this branch to ensure that I haven't inadvertently broken something. |
chrisroos
force-pushed
the
remove-i18n-code
branch
from
e344013 to
ae28aff
Compare
|
All regression tests passed in this branch on my local machine. |
The name no longer needs to contain "with_erb_renderer" as this is now the only unit test for the GraphPresenter.
And associated question-presenter-sample.yml file. This test was exercising the i18n functionality of the QuestionPresenter, which I'm in the process of removing.
The name no longer needs to include "with_erb_renderer" as this is now the only unit test for the QuestionPresenter.
This removes the ability to disable ERB templates for questions.
Additionally stop passing the `use_erb_templates_for_questions` option in `Flow#add_question`.
This is a small step toward removing it completely.
The `#use_erb_template?` method always returns true so this `else` branch is never used.
The `use_erb_template?` method always returns true so this `else` branch is never used.
The `use_erb_template?` method always returns true so this `else` branch is never used.
The `use_erb_template?` method always returns true so this `else` branch is never used.
The `use_erb_template?` method always returns true so this `else` branch is never used.
The `use_erb_template?` method always returns true so this `else` branch is never used.
The `use_erb_template?` method always returns true so this `else` branch is never used.
The `use_erb_template?` method always returns true so this `else` branch is never used.
The `use_erb_template?` method always returns true so this `else` branch is never used.
This is no longer used anywhere.
These delegated methods are all redundant now that we only render ERB templates for questions.
This is no longer being used anywhere.
Note that `StartNodePresenter#initialize` calls `super` and passes the temporary 'unused-i18n-prefix' string. I'll remove this when I remove `i18n_prefix` from `NodePresenter#initialize`.
I've had to add the conditional code to `SmartAnswerPresenter#presenter_for` so that it instantiates the `OutcomePresenter` correctly. This will go away once we're no longer passing `i18n_prefix` to any of the `NodePresenter`s. Note that `OutcomePresenter#initialize` calls `super` and passes the temporary 'unused-i18n-prefix' string. I'll remove this when I remove `i18n_prefix` from `NodePresenter#initialize`.
I've had to amend the conditional code in `SmartAnswerPresenter#presenter_for` so that it instantiates the `OutcomePresenter` and `QuestionPresenter` correctly. This will go away once we're no longer passing `i18n_prefix` to any of the `NodePresenter`s. Note that `QuestionPresenter#initialize` calls `super` and passes the temporary 'unused-i18n-prefix' string. I'll remove this when I remove `i18n_prefix` from `NodePresenter#initialize`.
I've removed the conditional code from `SmartAnswerPresenter#presenter_for` now that all presenter `#initialize` methods have the same signature (i.e. none of them take an `i18n_prefix`) any more.
Setting the local `i18n_prefix` in the flow had/has no effect. I've also updated the checksum data for legalisation-document-checker.
These are no longer being used anywhere.
We're no longer using i18n locale files to store content for Smart Answers.
The `QuestionPresenter` now always renders ERB templates so we don't need to worry about rescuing `I18n` exceptions.
None of the i18n helper methods are being used now that we've completely switched to ERB templates for questions.
I'm not sure whether these were ever used as we no longer have (never had?) a flow named 'holiday_pay'.
And use this instead of looking it up in I18n locale file using the `flow.defaults.error_message` key.
We haven't been using PhraseLists for a while and so these test names don't make sense.
This is no longer being used anywhere.
These are no longer used anywhere.
chrisroos
force-pushed
the
remove-i18n-code
branch
from
ae28aff to
8590043
Compare
|
I've tweaked some of the commit messages and force pushed in preparation for merging. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I also tried to remove the "rails-i18n" Gem as part of this pull request. Removing it caused a number of regression tests to fail because, for example,
number_to_currencystarted returning$instead of£.