Only allow SSNs in the 900 group to proof with the mock proofer#5576
Only allow SSNs in the 900 group to proof with the mock proofer#5576mitchellhenke merged 10 commits intomainfrom
Conversation
mitchellhenke
force-pushed
the
jmhooper-ssn-validation
branch
2 times, most recently
from
0fb4186 to
af2ae93
Compare
There was a problem hiding this comment.
Rubocop suggested start_with? is faster than doing match?(/\A900/)
zachmargolis
left a comment
zachmargolis
left a comment
There was a problem hiding this comment.
1-900-SHIP-IT ☎️ 🚢
LGTM
There was a problem hiding this comment.
question: is this supposed to start with 900?
There was a problem hiding this comment.
this is a weird test that I think was written to fail on duplicate SSNs, but we don't do that, so it fails for another reason so the test passes? 🤷🏼
I can probably set any test using this as a skip instead
There was a problem hiding this comment.
Yeah, it looks like we dropped that in #3647, so we could probably even delete these tests...
There was a problem hiding this comment.
Yeah, let's pull these
There was a problem hiding this comment.
Love to delete feature tests. Dropped them in 4129bed99
**Why**: So only real data i.e. not PII can be used in the int environment
mitchellhenke
force-pushed
the
jmhooper-ssn-validation
branch
from
4129bed to
187cea8
Compare
orenyk
left a comment
orenyk
left a comment
There was a problem hiding this comment.
One testing suggestion but otherwise looks good! 
| raise Proofing::TimeoutError, 'resolution mock timeout' | ||
|
|
||
| elsif applicant[:ssn].match?(/6666/) | ||
| elsif !ssn.start_with?('900') && !IdentityConfig.store.test_ssn_allowed_list.include?(ssn) |
There was a problem hiding this comment.
suggestion: it would be great to add a test for this branch (e.g. stub out IdentityConfig.store or something)
4 participants
Why: So only real data i.e. not PII can be used in the int environment