Skip to content

Remove Double Address Verification from jobs - Deploy First#9854

Merged
JackRyan1989 merged 5 commits intomainfrom
jryan/lg-11352-dont-use-dav-in-jobs
Jan 8, 2024
Merged

Remove Double Address Verification from jobs - Deploy First#9854
JackRyan1989 merged 5 commits intomainfrom
jryan/lg-11352-dont-use-dav-in-jobs

Conversation

@JackRyan1989
Copy link
Contributor

@JackRyan1989 JackRyan1989 commented Jan 4, 2024

🎫 Ticket

🛠 Summary of changes

This PR compliments #9829.

The changes here should address steps 1 & 2 when removing a value from jobs:

Deploy 1: Remove all uses of the argument within the job, and add a default value
Deploy 2: Remove the argument in calls to the job (can be combined with the first deploy if there was already a default value)

https://handbook.login.gov/articles/manage-50-50-state.html#remove-an-argument-from-a-job-perform-method

📜 Testing Plan

Tests are already aware of the ipp_enrollment_in_progress variable, so no changes to tests are required. All should pass.

resolution_proofer.proof(applicant_pii)
end
end
# rubocop:enable Lint/UnusedMethodArgument
Copy link
Contributor

Choose a reason for hiding this comment

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

is the linter requiring this b/c the default value of ipp_enrollment_in_progress is now true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually no. It's required because we're passing in double_address_verification as an argument, but not using that value in the method.

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh ok thank you for clarifying that!

@aduth
Copy link
Contributor

aduth commented Jan 5, 2024

The changes here should address steps 1 & 2 when removing a value from jobs:

If Steps 1 and 2 are meant to be separate deploys, should the changes here include both, or should they be split across two pull requests?

@JackRyan1989
Copy link
Contributor Author

JackRyan1989 commented Jan 5, 2024

The changes here should address steps 1 & 2 when removing a value from jobs:
If Steps 1 and 2 are meant to be separate deploys, should the changes here include both, or should they be split across two pull requests?

Good q! My read of the text in the parens in the handbook:

(can be combined with the first deploy if there was already a default value)

is that since I have a default value set for double_address_verification in resolution_proofing_job, we can combine 1 and 2. Let me know if you think my read is incorrect or if I'm not adequately setting a default value. @aduth

@aduth
Copy link
Contributor

aduth commented Jan 5, 2024

Ah! I missed the parenthesized text. In that case, I think it should be okay 👍

Copy link
Contributor

@svalexander svalexander left a comment

Choose a reason for hiding this comment

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

LGTM

@JackRyan1989 JackRyan1989 merged commit e138a49 into main Jan 8, 2024
@JackRyan1989 JackRyan1989 deleted the jryan/lg-11352-dont-use-dav-in-jobs branch January 8, 2024 15:01
@amirbey amirbey mentioned this pull request Jan 9, 2024
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