Skip to content

Add mock ThreatMetrix Javascript implementation (LG-7174)#7018

Merged
zachmargolis merged 21 commits intomainfrom
margolis-mock-tmx-js
Sep 26, 2022
Merged

Add mock ThreatMetrix Javascript implementation (LG-7174)#7018
zachmargolis merged 21 commits intomainfrom
margolis-mock-tmx-js

Conversation

@zachmargolis
Copy link
Contributor

Background

Right now, in some sandbox environments, we're loading real JS from a 3rd party. Ideally, we'd mock this so we can control the behavior, or add clearer ways to test reject paths, etc

This adds that

  • a dropdown so users can select which device profiling behavior
  • also has an iframe fallback to no_result just like the actual vendor

Screenshots

screenshots
Screen Shot 2022-09-22 at 1 49 24 PM
Screen Shot 2022-09-22 at 1 49 37 PM

@zachmargolis zachmargolis requested a review from a team September 22, 2022 20:54
profiling behavior
</label>
<select
className="border-05 border-accent-warm-dark"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran the UI by @nickttng, who suggested square corners (aka not using usa-select) as well as the thicker border to hint "this is not part of normal Login.gov"

const { value } = event.target;

if (value === 'chaotic') {
document.body.style.transform = 'rotate(180deg)';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to have more easter eggs to show what we can do with JS like this. My first idea was to slowly rotate the whole page... but this was faster. I am open to removing if anybody feels that's better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added some more options in a7e8f75 👹

changelog: Internal, Device Profiling, Add ability to select mock device profiling response
@zachmargolis zachmargolis requested a review from aduth September 22, 2022 21:07
@@ -0,0 +1,52 @@
module Idv
module Steps
module ThreatMetrixStepHelper
Copy link
Contributor

Choose a reason for hiding this comment

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

@theabrad this might overlap with some of the refactoring / cleanup work you're doing

newrelic_browser_app_id: ''
newrelic_browser_key: ''
newrelic_license_key: ''
no_sp_device_profiling_enabled: true
Copy link
Contributor

Choose a reason for hiding this comment

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

👏 for adding this

)

result.response_body = JSON.parse(
response_body.gsub('REVIEW_STATUS', result.review_status.to_s),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is not wired up correctly--I'm not seeing REVIEW_STATUS in that file. In an case, can we do this without a gsub (parse the JSON, modify, then re-serialize?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in cbb1e17

@theabrad
Copy link
Contributor

Looks like this also covers LG-7233 and the changes I was working on.

return device_status
end

case SsnFormatter.format(ssn)
Copy link
Contributor

Choose a reason for hiding this comment

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

I vote for removing these magic SSNs and having the dropdown by the One True Way to trigger a certain device profiling result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cool I'll do that! My initial thought was to preserve any testing instructions we'd already sent people

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in cbb1e17

}

document.addEventListener('DOMContentLoaded', () => {
const ssnInput = document.getElementsByName('doc_auth[ssn]')[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be nice to output some debugging info to the console here:

Suggested change
const ssnInput = document.getElementsByName('doc_auth[ssn]')[0];
console.log(`Mock device profiling started for session ID ${loadSessionId()}`);
const ssnInput = document.getElementsByName('doc_auth[ssn]')[0];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have some ESLints that discourage console logging, and we have the UI element for the dropdown, would this be like to help us detect if the DOM elements we look for move and the UI fails to load?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'm going to skip this:

  • we have code in our feature specs that fails if it notices console.log output (which we usually only get in the case of errors) so adding an exception for this on every SSN page would be noisy
  • I added steps to the feature specs for the SSN step that select debug options from here and verify the result in redis, so if the JS breaks, our test suite should tell us

so I think we have guards that cover the things this helps with already

zachmargolis and others added 3 commits September 26, 2022 07:55
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Co-authored-by: Andrew Duthie <aduth@users.noreply.github.com>
- no more magic ssns, only results from redis
- set response_body.review_status correctly
@zachmargolis zachmargolis merged commit f43f000 into main Sep 26, 2022
@zachmargolis zachmargolis deleted the margolis-mock-tmx-js branch September 26, 2022 17:55
@solipet solipet mentioned this pull request Sep 26, 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.

4 participants