Split marriage-abroad outcome_opposite_sex_other_countries #2504
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
I'm planning to split the outcome_opposite_sex_marriage_in_other_countries template. The countries handled by that template are: * Burma * Iran * North Korea * Saudi Arabia * Somalia * Syria * Yemen All but Somalia and Syria already exist in the marriage-abroad regression test data. I'm adding those two countries so that I can confirm the changes in this branch don't affect their rendered outcomes.
The new outcome is a duplicate of
outcome_opposite_sex_marriage_in_other_countries:
$ cp lib/smart_answer_flows/marriage-abroad/outcomes/outcome_opposite_sex_marriage_in_{other_countries,burma}.govspeak.erb
The outcome_opposite_sex_marriage_in_other_countries outcome used a case
statement to display 5 different outcomes. It feels more natural that
these should be split into separate outcomes.
The next steps will be to remove the Burma specific code from
outcome_opposite_sex_marriage_in_other_countries and to remove all but
the Burma specific code from outcome_opposite_sex_marriage_in_burma.
I've also updated the marriage_abroad_test and responses-and-expected-results
regression test data as part of this commit.
Services for countries other than Burma are handled by the outcome_opposite_sex_marriage_in_other_countries. Use `git show --ignore-all-space` for a better view of this diff.
Services in Burma are now handled by outcome_opposite_sex_marriage_in_burma.
This is no longer required now that we've introduced the Burma specific outcome: outcome_opposite_sex_marriage_in_burma.
The new outcome is a duplicate of
outcome_opposite_sex_marriage_in_other_countries:
$ cp lib/smart_answer_flows/marriage-abroad/outcomes/outcome_opposite_sex_marriage_in_{other_countries,north_korea}.govspeak.erb
The outcome_opposite_sex_marriage_in_other_countries outcome used a case
statement to display 5 different outcomes. It feels more natural that
these should be split into separate outcomes.
The next steps will be to remove the North Korea specific code from
outcome_opposite_sex_marriage_in_other_countries and to remove all but
the North Korea specific code from
outcome_opposite_sex_marriage_in_north_korea.
I've also updated the marriage_abroad_test and responses-and-expected-results
regression test data as part of this commit.
Services for countries other than North Korea are handled by the outcome_opposite_sex_marriage_in_other_countries. Use `git show --ignore-all-space` for a better view of this diff.
Services in North Korea are now handled by outcome_opposite_sex_marriage_in_north_korea.
This is no longer required now that we've introduced the North Korea specific outcome: outcome_opposite_sex_marriage_in_north_korea.
The new outcome is a duplicate of
outcome_opposite_sex_marriage_in_other_countries:
$ cp lib/smart_answer_flows/marriage-abroad/outcomes/outcome_opposite_sex_marriage_in_{other_countries,yemen}.govspeak.erb
The outcome_opposite_sex_marriage_in_other_countries outcome used a case
statement to display 5 different outcomes. It feels more natural that
these should be split into separate outcomes.
The next steps will be to remove the Yemen specific code from
outcome_opposite_sex_marriage_in_other_countries and to remove all but
the Yemen specific code from outcome_opposite_sex_marriage_in_yemen.
I've also updated the marriage_abroad_test and responses-and-expected-results
regression test data as part of this commit.
Services for countries other than Yemen are handled by the outcome_opposite_sex_marriage_in_other_countries. Use `git show --ignore-all-space` for a better view of this diff.
Services in Yemen are now handled by outcome_opposite_sex_marriage_in_yemen.
This is no longer required now that we've introduced the Yemen specific outcome: outcome_opposite_sex_marriage_in_yemen.
The new outcome is a duplicate of
outcome_opposite_sex_marriage_in_other_countries:
$ cp lib/smart_answer_flows/marriage-abroad/outcomes/outcome_opposite_sex_marriage_in_{other_countries,saudi_arabia}.govspeak.erb
The outcome_opposite_sex_marriage_in_other_countries outcome used a case
statement to display 5 different outcomes. It feels more natural that
these should be split into separate outcomes.
The next steps will be to remove the Saudi Arabia specific code from
outcome_opposite_sex_marriage_in_other_countries and to remove all but
the Saudi Arabia specific code from
outcome_opposite_sex_marriage_in_saudi_arabia.
I've also updated the marriage_abroad_test and responses-and-expected-results
regression test data as part of this commit.
…abia Services for countries other than Saudi Arabia are handled by the outcome_opposite_sex_marriage_in_other_countries. Use `git show --ignore-all-space` for a better view of this diff.
…ries Services in Saudi Arabia are now handled by outcome_opposite_sex_marriage_in_saudi_arabia.
This is no longer required now that we've introduced the Saudi Arabia specific outcome: outcome_opposite_sex_marriage_in_saudi_arabia.
This outcome is only shown if ceremony country is one of Iran, Somalia or Syria which means that this `else` block will never be reached.
…ntries We can only reach this outcome if ceremony country is one of Iran, Somalia or Syria (i.e. it exists in the `OS_OTHER_COUNTRIES` array) which means this `case` statement is unnecessary.
I'm not convinced that this constant and the associated methods were adding any value. It's possible that there's some other connection between Iran, Somalia and Syria (e.g. countries with no British consular facilities) but I'm not confident enough about that to rename the constant and methods.
The marriage-abroad regression tests pass so I'm happy to update this checksum data.
chrisroos
deleted the
split-marriage-abroad-outcome-opposite-sex-other-countries
branch
floehopper
added a commit
that referenced
this pull request
May 24, 2016
This (mostly) reverts commit 50ab9b8. Note, however, that I have *not* reverted the introduction of `IndexableContentTest`, because it's still a useful test to make sure we don't introduce any more code which results in raising exceptions. This commit was originally introduced so that we could generate `indexable_content` for the `outcome_os_other_countries` outcome in `marriage-abroad`. We had to add this special case to handle the potential exception raised by the outcome. This later pull request [1] updates the `outcome_os_other_countries` outcome such that it no longer raises an exception, which means that we can simplify the code by reverting this commit. [1]: #2504
floehopper
added a commit
that referenced
this pull request
May 24, 2016
This (mostly) reverts commit 50ab9b8. Note, however, that I have *not* reverted the introduction of `IndexableContentTest`, because it's still a useful test to make sure we don't introduce any more code which results in raising exceptions. This commit was originally introduced so that we could generate `indexable_content` for the `outcome_os_other_countries` outcome in `marriage-abroad`. We had to add this special case to handle the potential exception raised by the outcome. This later pull request [1] updates the `outcome_os_other_countries` outcome such that it no longer raises an exception, which means that we can simplify the code by reverting this commit. [1]: #2504
floehopper
added a commit
that referenced
this pull request
Jun 1, 2016
This (mostly) reverts commit 50ab9b8. Note, however, that I have *not* reverted the introduction of `IndexableContentTest`, because it's still a useful test to make sure we don't introduce any more code which results in raising exceptions. This commit was originally introduced so that we could generate `indexable_content` for the `outcome_os_other_countries` outcome in `marriage-abroad`. We had to add this special case to handle the potential exception raised by the outcome. This later pull request [1] updates the `outcome_os_other_countries` outcome such that it no longer raises an exception, which means that we can simplify the code by reverting this commit. [1]: #2504
floehopper
added a commit
that referenced
this pull request
Jun 2, 2016
This (mostly) reverts commit 50ab9b8. Note, however, that I have *not* reverted the introduction of `IndexableContentTest`, because it's still a useful test to make sure we don't introduce any more code which results in raising exceptions. This commit was originally introduced so that we could generate `indexable_content` for the `outcome_os_other_countries` outcome in `marriage-abroad`. We had to add this special case to handle the potential exception raised by the outcome. This later pull request [1] updates the `outcome_os_other_countries` outcome such that it no longer raises an exception, which means that we can simplify the code by reverting this commit. [1]: #2504
floehopper
added a commit
that referenced
this pull request
Jun 3, 2016
This (mostly) reverts commit 50ab9b8. Note, however, that I have *not* reverted the introduction of `IndexableContentTest`, because it's still a useful test to make sure we don't introduce any more code which results in raising exceptions. This commit was originally introduced so that we could generate `indexable_content` for the `outcome_os_other_countries` outcome in `marriage-abroad`. We had to add this special case to handle the potential exception raised by the outcome. This later pull request [1] updates the `outcome_os_other_countries` outcome such that it no longer raises an exception, which means that we can simplify the code by reverting this commit. [1]: #2504
ikennaokpala
pushed a commit
that referenced
this pull request
Jun 9, 2016
This (mostly) reverts commit 50ab9b8. Note, however, that I have *not* reverted the introduction of `IndexableContentTest`, because it's still a useful test to make sure we don't introduce any more code which results in raising exceptions. This commit was originally introduced so that we could generate `indexable_content` for the `outcome_os_other_countries` outcome in `marriage-abroad`. We had to add this special case to handle the potential exception raised by the outcome. This later pull request [1] updates the `outcome_os_other_countries` outcome such that it no longer raises an exception, which means that we can simplify the code by reverting this commit. [1]: #2504
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.
This supersedes PR #2503. I'm creating it so that I can merge the branch. It doesn't need to be reviewed.
The outcome_opposite_sex_other_countries template used a
casestatement to render one of what were essentially five different outcomes. I've split the template so that we have five actual outcomes.This removes logic from the template and reduces (although doesn't complete remove) the use of the rather vague "other countries" terminology.