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

Content changes for opposite and same sex outcomes where user lives in UK or SA #2467

Merged
merged 3 commits into from
Apr 26, 2016

Conversation

ikennaokpala
Copy link
Contributor

@ikennaokpala ikennaokpala commented Apr 15, 2016

Trello Story: https://trello.com/c/SSdBWMvO/107-marriage-abroad-south-africa

Factcheck

  • Opposite sex outcomes where user is planning on getting married in South Africa and is resident in the UK:

Expected changes

  • Content changes for opposite sex outcomes where user is resident in the UK
    • Changes to Legalisation and translation

Before

screen shot 2016-04-18 at 11 53 59

After

screen shot 2016-04-18 at 11 53 51

Factcheck

  • Same sex outcomes where user is planning on getting married in South Africa and is resident in the UK:

Expected changes

  • Content changes for opposite sex outcomes where user is resident in the UK
    • Changes to Legalisation and translation

Before

screen shot 2016-04-18 at 11 57 13

After

screen shot 2016-04-18 at 11 57 40

Factcheck

  • Opposite sex outcomes where user is planning on getting married in South Africa and is resident in the South Africa:

Expected changes

  • Content changes for opposite sex outcomes where user is resident in the South Africa
    • Changes to Legalisation and translation

Before

screen shot 2016-04-18 at 12 00 33

After

screen shot 2016-04-18 at 12 00 20

Factcheck

  • Same sex outcomes where user is planning on getting married in South Africa and is resident in the South Africa:

Expected changes

  • Content changes for opposite sex outcomes where user is resident in the South Africa
    • Changes to Legalisation and translation

Before

screen shot 2016-04-18 at 12 02 29

After

screen shot 2016-04-18 at 12 02 52

@ikennaokpala ikennaokpala force-pushed the marriage-outcomes-uk-sa branch from 67beb52 to 8b7de14 Compare April 15, 2016 16:00
@ikennaokpala ikennaokpala force-pushed the marriage-outcomes-uk-sa branch 8 times, most recently from cd2d8b8 to 7c41882 Compare April 22, 2016 14:51
@ikennaokpala ikennaokpala force-pushed the marriage-outcomes-uk-sa branch from 7c41882 to 67e7d4c Compare April 25, 2016 09:08
@leenagupte leenagupte self-assigned this Apr 26, 2016
calculator.resident_of_uk? ||
calculator.resident_of_ceremony_country? ||
calculator.resident_of_third_country?
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this check needed? A user must always be either a resident of the UK, a resident of South Africa or a resident of another country, so this check will always be truthy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leenagupte this check initially started with like with the require for user either a uk resident or resident of ceremony_country

elsif calculator.ceremony_country == 'south-africa' &&
      (
         calculator.resident_of_uk? ||
         calculator.resident_of_ceremony_country? 
      )

After factcheck FCO made another requirement for third_country

I have decided to keep it this way for explicit reasons, make it a bit more obvious the 3 potential areas affected by this node.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @leenagupte about removing this check. I think it makes the code more complex for no real benefit.

@leenagupte
Copy link
Contributor

Apart from my comments, LGTM.

@ikennaokpala ikennaokpala force-pushed the marriage-outcomes-uk-sa branch 3 times, most recently from 79229df to 580a1c7 Compare April 26, 2016 10:17
Ikenna Okpala added 3 commits April 26, 2016 11:58
These changes affect All opposite and same sex outcomes where user lives in the
United Kingdom or in South Africa.

These changes were requested by the content team.

Affecting only these URLs/pages:

UK citizen living in the UK

/marriage-abroad/y/south-africa/uk/partner_british/opposite_sex
/marriage-abroad/y/south-africa/uk/partner_british/same_sex
/marriage-abroad/y/south-africa/uk/partner_local/opposite_sex
/marriage-abroad/y/south-africa/uk/partner_local/same_sex
/marriage-abroad/y/south-africa/uk/partner_other/opposite_sex
/marriage-abroad/y/south-africa/uk/partner_other/same_sex

UK citizen in ceremony  country

/marriage-abroad/y/south-africa/ceremony_country/partner_british/opposite_sex
/marriage-abroad/y/south-africa/ceremony_country/partner_british/same_sex
/marriage-abroad/y/south-africa/ceremony_country/partner_local/opposite_sex
/marriage-abroad/y/south-africa/ceremony_country/partner_local/same_sex
/marriage-abroad/y/south-africa/ceremony_country/partner_other/opposite_sex
/marriage-abroad/y/south-africa/ceremony_country/partner_other/same_sex

UK citizen in third country

/marriage-abroad/y/south-africa/third_country/partner_british/opposite_sex
/marriage-abroad/y/south-africa/third_country/partner_british/same_sex
/marriage-abroad/y/south-africa/third_country/partner_local/opposite_sex
/marriage-abroad/y/south-africa/third_country/partner_local/same_sex
/marriage-abroad/y/south-africa/third_country/partner_other/opposite_sex
/marriage-abroad/y/south-africa/third_country/partner_other/same_sex

Update marriage abroad test expectations.

Add tests for marriage outcomes South Africa
@ikennaokpala ikennaokpala force-pushed the marriage-outcomes-uk-sa branch from 580a1c7 to a728525 Compare April 26, 2016 10:58
@ikennaokpala ikennaokpala merged commit c6034a9 into master Apr 26, 2016
@ikennaokpala ikennaokpala deleted the marriage-outcomes-uk-sa branch April 26, 2016 11:27
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.

3 participants