LG -7015 Redirect user to "Sorry" page on TMX verification failure

[theabrad]

Why?
If a user fails threatmetrix verification we want to redirect them to a come back later page.

[zachmargolis]

I think we need to make sure it's also not nil

[zachmargolis]

can we get some tests with statuses like pass or nil?

[zachmargolis]

we'll change this once we have real content, right?

[matthinz]

I feel like this PR should also include a new stub page like /verify/setup_error that we'll use for the failure messaging (it can just say something like "Please contact support to continue account setup" or something)

[theabrad]

yep

[matthinz]

This should be proofing_device_profiling_decisioning_enabled, yes?

Suggested change

	return false unless IdentityConfig.store.proofing_device_profiling_collecting_enabled
	return false unless IdentityConfig.store.proofing_device_profiling_decisioning_enabled

[aduth]

Note that there are currently two implementations of the personal key step, controlled by the idv_api_enabled_steps feature flag. I'd expect we'd update both to keep the behaviors in sync.

The other:

identity-idp/app/controllers/api/verify/password_confirm_controller.rb

Lines 49 to 59 in 836b5ac

	def completion_url(result, user)
	if result.extra[:profile_pending]
	idv_come_back_later_url
	elsif in_person_enrollment?(user)
	idv_in_person_ready_to_verify_url
	elsif current_sp
	sign_up_completed_url
	else
	account_url
	end
	end

[zachmargolis]

ditto about checking if this is nil?

[theabrad]

Do you mean checking if proofing_component is nil or if threatmetrix_review_status is nil?

[zachmargolis]

I meant threatmetrix_review_status is nil

[theabrad]

I have a test for review status being nil and it returns true . I feel like an extra line checking for nil is unnecessary unless we change threatmetrix_review_status from 'pass' or 'fail' to a boolean in the future.

[zachmargolis]

ah I see, so my expectation is that review status being nil is not a failure, that this method should return falsy in that case? Becaue we don't know that it's a for sure negative? and there may be users who fit into a gap of "the feature flag was enabled but we didn't run the check for them" so they don't have data

[theabrad]

ah so if it is nil let them pass?

[zachmargolis]

yeah that's my expectation of how we could ensure the safest/smoothest launch

[jskinne3]

I'd like to add my support to the idea that a missing review_status in the API response (which I assume is what a nil value for proofing_component.threatmetrix_review_status indicates) should have (for now) the same behavior as pass.

[zachmargolis]

LGTM

[zachmargolis]

LGTM assuming we'll follow up with a new URL/page

[jskinne3]

added a comment

[jskinne3]

To me it feels like a confusing double negative to say that the user passes the test if device_profiling_failed is false. It could be named device_profiling_passed? and return true when the user passes. Or, I would also find the name blocked_by_device_profiling? clearer if you still want it to return false on a pass.