Skip to content

LG-7415: Allow using mock TMX proofer when TMX JS disabled#6885

Merged
matthinz merged 3 commits intomainfrom
matthinz/lg-7415-tmx-collecting-fix
Sep 1, 2022
Merged

LG-7415: Allow using mock TMX proofer when TMX JS disabled#6885
matthinz merged 3 commits intomainfrom
matthinz/lg-7415-tmx-collecting-fix

Conversation

@matthinz
Copy link
Contributor

Why: We're trying to test ThreatMetrix in int, with collecting (TMX's third-party JS embed) disabled, but decisioning enabled via a mock, and we ran into an issue where the mock was not being called.

How: Rather than only generating a threatmetrix_session_id value when collecting is enabled, we now always generate that ID and store it in flow_session. This does not affect the logic around including third party Javascript on the page (if collecting is disabled, no Javascript embed will be included).

@matthinz matthinz requested a review from a team August 31, 2022 19:17
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM, are there any specs we'd want to update to make sure the session ID is generated but the JS is still not included?

@theabrad
Copy link
Contributor

theabrad commented Aug 31, 2022

You probably have to remove the spec in spec/services/idv/steps/ssn_step_spec.rb that checks if a session_id is not added when proofing_device_profiling_collecting is disabled

https://github.com/18F/identity-idp/blob/main/spec/services/idv/steps/ssn_step_spec.rb#L104-L112

and

https://github.com/18F/identity-idp/blob/main/spec/services/idv/steps/ipp/ssn_step_spec.rb#L72-L79

@matthinz
Copy link
Contributor Author

are there any specs we'd want to update to make sure the session ID is generated but the JS is still not included?

This should be covered by https://github.com/18F/identity-idp/blob/matthinz/lg-7415-tmx-collecting-fix/spec/views/idv/shared/_ssn.html.erb_spec.rb#L79-L88

Copy link
Contributor

@theabrad theabrad left a comment

Choose a reason for hiding this comment

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

LGTM

@matthinz matthinz force-pushed the matthinz/lg-7415-tmx-collecting-fix branch 2 times, most recently from 31d8a9e to c6cf814 Compare August 31, 2022 22:09
To support scenarios where `proofing_device_profiling_collecting_enabled` is `false` but `proofing_device_profiling_decisioning_enabled` is `true`, ensure we're generating `session_id` values for ThreatMetrix, even if we don't end up embedding the TMX javascript.

changelog: Upcoming Features, ThreatMetrix, Allow using mock TMX proofer when TMX JS disabled
@matthinz matthinz force-pushed the matthinz/lg-7415-tmx-collecting-fix branch from c6cf814 to 0adbcd9 Compare August 31, 2022 23:51
@matthinz matthinz merged commit 27a574b into main Sep 1, 2022
@matthinz matthinz deleted the matthinz/lg-7415-tmx-collecting-fix branch September 1, 2022 01:54
@zachmargolis zachmargolis mentioned this pull request Sep 7, 2022
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