Skip to content
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

Split marriage-abroad outcome for Brazil #2598

Merged
merged 7 commits into from
Jul 6, 2016

Conversation

chrisroos
Copy link
Contributor

@chrisroos chrisroos commented Jun 27, 2016

The current outcome for ceremonies in Brazil (outcome_marriage_in_brazil_when_residing_in_brazil_or_third_country) contains two logical outcomes:

  • One for residents of Brazil
  • One for residents of a third country (i.e. not resident in the UK or Brazill)

The outcome body contains a single if/else statement that decides which logical outcome to display. Splitting these logical outcomes and moving the "routing logic" to the flow is more consistent with the majority of this Smart Answer.

@floehopper floehopper self-assigned this Jun 27, 2016
@floehopper
Copy link
Contributor

As for #2597, I think it would be helpful to explain a little more about your motivation for making this change.

@floehopper
Copy link
Contributor

This looks good to me 👍

The current outcome for ceremonies in Brazil
(outcome_marriage_in_brazil_when_residing_in_brazil_or_third_country) contains
two logical outcomes:

* One for residents of Brazil
* One for residents of a third country (i.e. not resident in the UK or Brazill)

The outcome body contains a single `if/else` statement that decides which
logical outcome to display. Splitting these logical outcomes and moving the
"routing logic" to the flow is more consistent with the majority of this Smart
Answer.

This change will make it easier to split the outcome in a subsequent commit.
The new template is a duplicate of
outcome_marriage_in_brazil_when_residing_in_brazil_or_third_country.

I'll remove the resident-of-Brazil logic and content from this new template in
a subsequent commit.
I'll remove the resident-of-third-country logic and content from this template
in a subsequent commit.
I've removed all the logic and content for residents of a third country.

The diff is probably best viewed using the `--ignore-all-space` option.
I've removed all the logic and content for residents of Brazil.

The diff is probably best viewed using the `--ignore-all-space` option.
This outcome is only ever shown for residents of a third country.
Updating because the marriage-abroad regression tests are passing.
@chrisroos chrisroos force-pushed the split-marriage-abroad-outcome-for-brazil branch from 6c11ff7 to 6aed605 Compare July 6, 2016 10:11
@chrisroos
Copy link
Contributor Author

As for #2597, I think it would be helpful to explain a little more about your motivation for making this change.

I've updated the PR description to better explain why I'm making this change. I've also rewritten some of the commit notes to add more information.

@chrisroos
Copy link
Contributor Author

I've rebased this on master and force pushed in preparation for merging.

@chrisroos chrisroos merged commit 7c3d6b4 into master Jul 6, 2016
@chrisroos chrisroos deleted the split-marriage-abroad-outcome-for-brazil branch July 6, 2016 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants