Remove marriage-abroad outcome_os_consular_cni precalculate blocks #2394
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 can only imagine this was an oversight when the array/string was first added in ff781c5.
Move this array of countries from the flow to the `MarriageAbroadDataQuery`.
Move this array of countries from the flow to the `MarriageAbroadDataQuery`.
Move this array of countries from the flow to the `MarriageAbroadDataQuery`.
Move this array of countries from the flow to the `MarriageAbroadDataQuery`.
Move this array of countries from the flow to the `MarriageAbroadDataQuery`.
Move this array of countries from the flow to the `MarriageAbroadDataQuery`.
The outcome_os_consular_cni template doesn't use the string that was being returned: it's only interested in whether this value is truthy or falsey.
The outcome_os_consular_cni template doesn't use the string that was being returned: it's only interested in whether this value is truthy or falsey.
It's only used in the `:birth_cert_inclusion` precalculate block.
Use it instead of the `:three_day_residency_requirement_applies` precalculate block. Part of refactoring marriage-abroad. See doc/refactoring.md for more.
Note. We can ignore Italy in this country list as opposite sex ceremonies in Italy are handled by the outcome_os_italy outcome.. There are a few places in the outcome_os_consular_cni template that deal with ceremonies in Croatia and Russia. Inlining this condition makes it clearer that this is related to those other conditions in this template. I think this'll make it easier to remove this duplication in future.
Use it instead of the `:cni_posted_after_14_days_countries` precalculate block. Part of refactoring marriage-abroad. See doc/refactoring.md for more.
There are a few places in the outcome_os_consular_cni template that deal with ceremonies in Croatia. Inlining this condition makes it clearer that this is related to those other conditions in this template. I think this'll make it easier to remove this duplication in future.
This is confusing as the NO_BIRTH_CERT_REQUIREMENT array contains a list of countries that don't require a birth certificate as a supporting document. We then check that the ceremony country isn't in that list in order to determine whether a birth certificate is required. I'm sure this can be improved but I think it's OK for now.
Note. We can ignore Japan in this list as opposite sex ceremonies in Japan are handled in the outcome_os_japan outcome. I've got a note to remove this separately. I _think_ Macedonia is in this list because it's handled elsewhere in the outcome_os_consular_cni template, rather than because it's a `:notary_public_inclusion` country. There are already other special cases dealing with Macedonia in the template and I'm hoping this'll make it easy to remove more duplication further down the line.
In preparation for splitting this onto multiple lines. This'll make it easier to see other changes I plan to make to these conditions.
To make it easier to see the changes I want to make to it.
This will make it easier to remove duplication between the use of `cni_notary_public_countries` and `notary_public_inclusion`.
Remove duplication by using `notary_public_inclusion` instead of `cni_notary_public_countries.include?(calculator.ceremony_country`.
It's only used in the `:notary_public_inclusion` precalculate block.
Use it instead of the `:notary_public_inclusion` precalculate block. Part of refactoring marriage-abroad. See doc/refactoring.md for more.
Use it instead of the `:no_document_download_link_if_os_resident_of_uk_countries` precalculate block. Part of refactoring marriage-abroad. See doc/refactoring.md for more. I struggled with the naming of this method as the logic seems confusing. The `NO_DOCUMENT_DOWNLOAD_LINK_IF_OS_RESIDENT_OF_UK_COUNTRIES` array contains a list of ceremony countries that _don't_ allow you to download the notice of and affidavit for marriage forms. We check that the ceremony country _isn't_ in this list in order to decide whether to display the download
The marriage-abroad regression tests now pass so I'm updating the
checksum data.
Updated using:
$ rails r script/generate-checksums-for-smart-answer.rb \
marriage-abroad
chrisroos
referenced
this pull request
Mar 22, 2016
Move this array of countries from the flow to the `MarriageAbroadDataQuery`.
chrisroos
referenced
this pull request
Mar 22, 2016
I _think_ this makes it clearer that the countries in this array are related to the countries listed in the conditionals just before this block of code in outcome_os_consular_cni. It also makes it clearer that 'italy' shouldn't be in this list as that now has its own outcome.
chrisroos
referenced
this pull request
Mar 22, 2016
This feels too specific to add to the `MarriageAbroadCalculator`.
chrisroos
referenced
this pull request
Mar 22, 2016
This should've been removed as part of PR #2349 but I overlooked it.
|
I've merging this given that all the feedback on PR #2388 was minor. |
chrisroos
added a commit
that referenced
this pull request
Mar 22, 2016
…-os-consular-cni-precalculate-blocks Remove marriage-abroad outcome_os_consular_cni precalculate blocks
chrisroos
deleted the
remove-marriage-abroad-outcome-os-consular-cni-precalculate-blocks
branch
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 #2388. See that PR for discussion.
I'm continuing to refactor marriage-abroad. This branch removes the
precalculateblocks from outcome_os_consular_cni by adding logic to theMarriageAbroadCalculator.