Improve test coverage for in-person proofing#6680
Conversation
…rid document capture in-person opt-in
Avoid creating a new user instance
Avoid duplicate use resulting in foreign key constraint violation
| end | ||
|
|
||
| def begin_in_person_proofing(_user = user_with_2fa) | ||
| def begin_in_person_proofing(_user = nil) |
There was a problem hiding this comment.
The argument was previously unused, and the user_with_2fa method is non-trivial, so should help performance slightly to avoid calling it.
There was a problem hiding this comment.
Should we just remove the _user parameter for each of these methods where it's unused?
There was a problem hiding this comment.
Should we just remove the
_userparameter for each of these methods where it's unused?
I'm of two minds about it. I sorta wish we didn't ever need to pass the argument, but there are some instances where we do need to reference it (example), and I'd rather we be consistent about the signature with these complete_x_step helpers than to (a) leave a developer unsure if they need to pass it and (b) make it a hassle if ever the helper did need to reference the user (i.e. updating all call sites). So the thinking with this is to expect it to be passed if it's known, acknowledging that it's not actually used currently, but leaving it open so that it could be used, and having a consistent argument signature.
| end | ||
|
|
||
| def begin_in_person_proofing(_user = user_with_2fa) | ||
| def begin_in_person_proofing(_user = nil) |
There was a problem hiding this comment.
Should we just remove the _user parameter for each of these methods where it's unused?
2 participants
Why: Since this test coverage should have been included in its initial introduction in #6634, #6661.