Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove proxyquire #4212

Merged
merged 1 commit into from Sep 17, 2020
Merged

Remove proxyquire #4212

merged 1 commit into from Sep 17, 2020

Conversation

aduth
Copy link
Member

@aduth aduth commented Sep 16, 2020

Why: While occasionally useful to mock out the behavior of underlying modules, it can give a false sense of security that the behavior of a mocked implementation will match the experience and expectations of the user-facing behavior.

I'd heard some feedback through the grapevine (@jmhooper) that the use of proxyquire in the JavaScript tests wasn't instilling a lot of confidence.

After #4211, there was only a single test spec which was using this dependency, and it was easy enough to move toward a more integration/functional-style testing approach.

This test was also written at a time prior to the introduction of broad availability of a DOM in the JavaScript tests (#3946), and can now more easily assert against real(-ish) DOM element content.

**Why**: While occasionally useful to mock out the behavior of underlying modules, it can give a false sense of security that the behavior of a mocked implementation will match the experience and expectations of the user-facing behavior.
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

@aduth aduth merged commit c480507 into master Sep 17, 2020
@aduth aduth deleted the aduth-remove-proxyquire branch September 17, 2020 12:02
solipet added a commit that referenced this pull request Sep 23, 2020
* Notify via email when resetting password from RISC (LG-3120) (#4177)

* Combine reset password classes
* Remove rake task and script
* Update email copy

* Remove docker branch builds, leave "build-release-container" (#4181)

* convert idv cac success template to erb

* convert backup code setup confirm delete template to erb

* convert backup code setup confirm edit template to erb

* convert piv cac auth setup error template to erb

* Add more randomness to Agency factories (#4186)

**Why**: We have a unique index on agency name

* fix: Gemfile & Gemfile.lock to reduce vulnerabilities (#4187)

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-RUBY-ACTIONVIEW-632514

* refactor account page a bit

* placeholder

* lint

* rubocop

* add redirect

* fix backup codes spec

* add translations to mobile menu

* move two factor to own controller

* move connected accounts to own controller

* move history to own controller

* fix indent

* fix menu not showing on tablets

* fix some specs

* fix some specs

* rubocop

* fix greeting and header nav

* fix more specs

* double quotes in html attributes

* update redirects for piv/cac setup

* use USWDS breakpoints

* convert navigation class method to being instantiated

* rubocop

* csscop

* fix piv cac spec

* remove semi-circle from user-access image

* internationalization for new account banner

* normalize yaml

* update events page to align with new account page design

* review changes

* LG-3118 Fix bugs discovered while testing AAL3 flows (#4182)

This commit fixes 2 bugs:

1.) The WebAuthn setup was not marking the session as AAL3, so the user was needed to interact with their token twice
2.) The "Don't have your <method>" text was appearing on the PIV/WebAuthn verification screen when the user did not have an option to change methods

* LG-3334: Support failure testing for document capture in development environments (#4173)

* Allow AcuantCapture to receive non-image files in non-production environments

**Why**: As a developer, I expect that I can upload files which simulate expected failures which can occur during document capture, so that I can reliably test non-happy-path flows.

* Return error from YAML in non-production image upload

* rubocop

* Fix client test specs

* Pass mock client as detail to root upload context

* Pass through non-image files to client implementation

See: https://github.com/18F/identity-idp/pull/4173/files#r485970570

* Remove unused i18n keys

* update user access svg to hide bottom corners

* LG-3118 Update the webauthn verification headers (#4190)

This was discovered during LG-3118. The headers on the WebAuthn verification screen are supposed to ask you to connect instead of present your key.

* Separate links to Privacy Act Statement from Practices (LG-3467) (#4188)

* update banner to for mobile design changes

* LG-873 Add x509_issuer to piv/cac response data for OIDC (#4174)

* Change "Sign in" to "Continue" on user authorization confirmation (#4189)

This was found as part of LG-3118. The "Sign in" button on this page should say "Continue".

While I was in there I noticed the rest of the text on the page was not translated so I went ahead and took care of that.

* LG-3342: Rate limit image uploads during doc auth API response (#4185)

* LG-3342: Rate limit image uploads during doc auth API response

**Why:** As a legitimate user doing doc auth, I want the image uploads in the react flow to be uploaded, so that users attempting to steal an identity have a harder time proofing.

* Pass image form status as argument to render_form_response

* Memoize image upload form throttle increment to avoid duplicate counting

* Increment throttle by submission

See: #4185 (comment)

* Assign fallback status at time of use

See: #4185 (comment)
Co-Authored-By: Zach Margolis <zbmargolis@gmail.com>

* Create ImageUploadResponsePresenter to handle image upload response

* Add test spec for ImageUploadResponsePresenter

Co-authored-by: Zach Margolis <zbmargolis@gmail.com>

* do not show account banner on mobile

* Forget remembered browsers after RISC password reset (LG-3120) (#4195)

* Update CSS style guide image path (#4198)

* LG-3294 Replace raw NET::HTTP with Faraday equivalent. (#4197)

LG-3294 Replace raw NET::HTTP with Faraday equivalent for the piv_cac_service.

* Add maintenance window notification for Acuant (LG-3451) (#4202)

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

* LG-3320 Prevent uploading selfie (#4199)

* Use image-path CSS helper function to cache asset (#4203)

* LG-3463: Rate limit image uploads during doc auth: Client response (#4193)

* LG-3463: Rate limit image uploads during doc auth: Client response

**Why**: As a user who has exceeded the allowable limit of doc auth attempts, I expect to be presented with a message explaining why I'm no longer able to submit my documents.

With these changes, when the client detects there are no more attempts, the user is redirected to the error state.

See: #4185 (LG-3342)

* Return redirect URL in 429 response

* Update test spec

* Use ref callback to initialize FullScreen modal focus trap (#4200)

**Why**: This may resolve an issue where focus trap's event handling is not removed after a FullScreen dialog is hidden, under the assumption that the previous implementation could leave potential for desync between modalRef and useEffect, or between the useEffect's unsubscribe behavior and trapRef.current.deactivate.

* LG-3487: Remove file size validation for doc auth images (#4205)

**Why**: As a user, I expect that even if my mobile device captures photos with low quality, I will be allowed to submit them in case Acuant will still select them, so that I am not blocked from proofing.

* LG-3473: Log analytics for the image upload API (#4201)

* LG-3473: Log analytics for the image upload API

**Why**: As a user, I want login.gov to log analytics for the doc auth image upload API, so that if I am having an issue the login.gov team can debug it

* rubocop

* Log analytics using analytics.track_event

* Rename doc auth image upload analytics constants

**Why**: Clarity

* Omit braces in last hash arguments

**Why**: See #4201 (comment)

* Tweak PIV/CAC and WebAuthn verification controllers (#4191)

This was a discovery during LG-3118. This commit has 2 fixes:

1. The presenters renders the help text for AAL3 only when the user could only use the current method. They should render the help text if an AAL3 sign in is required
2. Remove duplicate paragraph from the PIV/CAC screen

* Remove redundant "Connect your key" header (#4204)

**Why**: We changed the context of the header on the page, making this header duplicative

* Remove unused "verify-modal" JavaScript pack (#4210)

**Why**: #2259 replaced modal dialogs with dedicated alert screens, and this code is now unused.

* LexisNexis doc_auth client now available for use (LG-3263) (#4184)

* LexisNexis doc_auth client now available for use (LG-3263)
Co-authored-by: amathews <amathews@fearsol.com>

* LG-3476: Upgrade focus-trap from ^2.3.0 (2.4.6) to 6.0.1 (#4211)

* Upgrade focus-trap from ^2.3.0 (2.4.6) to 6.0.1

* Simplify components index file

**Why**:

- Exports unused
- TrapModal variable as pass-through is unnecessary, since direct access to Modal import is available

* Remove proxyquire (#4212)

**Why**: While occasionally useful to mock out the behavior of underlying modules, it can give a false sense of security that the behavior of a mocked implementation will match the experience and expectations of the user-facing behavior.

* Fix mobile nav rendering (#4209)

* make sure nav is rendered in a valid place

* render mobile nav as child of body

* Update app/views/layouts/base.html.erb

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

* add usa-overlay div

* add title to new account pages

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

* Remove unused "if checked" JavaScript form validation (#4214)

* Remove unused submit auto-enable form validation (#4216)

**Why**: While some forms implement custom auto-enable behavior for submit buttons, they do so by bypassing the default auto-enable behavior. There does not appear to be any instances of forms which rely on this core behavior.

* LG-3339: Extract submit-with-spinner pack from form-validation.js (#4219)

* Extract submit-with-spinner pack from form-validation.js

* Add comment for custom event dispatching

* Define element-closest as dependency of polyfill package

* Convert a few more templates to erb (#4220)

* convert edit password templates to erb

* convert event disavowal new template to erb

* convert pii_review template to erb

* convert idv review new template to erb

* Update app/views/shared/_pii_review.html.erb

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* Update app/views/shared/_pii_review.html.erb

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* Update app/views/shared/_pii_review.html.erb

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* Update app/views/shared/_pii_review.html.erb

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* remove extra space

* fix specs

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* update rubocop to 0.91.0 (#4218)

* Convert some more slim templates (#4222)

* remove unused code in layout

* convert idv come back later template to erb

* convert verify_personal_key template to erb

* convert devise passwords new template to erb

* convert page took too long template to erb

* convert in person find usps template to erb

* fix unnecessary space

* Use Acuant SDK for selfie capture (#4224)

* Use AcuantPassiveLiveness for mobile selfie capture

* Cache image data blob

* Fix test specs

* LG-3339: Make functionality in form-validation.js opt-in (#4221)

* LG-3339: Make functionality in form-validation.js opt-in

**Why**: As a developer, I want the behavior of forms to be predictable and enable automatic validations only when intended, so that I'm not caught in a situation where automatic validations occur unexpectedly and break user behavior.

* Pass through simple_form_for arguments as given

* Rename idp_simple_form_for as validated_form_for

* Print form validation script once with data attribute

* Explicitly include Webpacker helper

* Print javascript_pack_tag using variadic arguments

* Add test spec for form-validation script

* Add test spec for script helper

* Rename print_javascript_pack_once_tags to render_javascript_pack_once_tags

* Use validated_form_for globally

* Preserve this reference in I18n calls

* Substitute classlist.js with classlist-polyfill (#4225)

* Pass fields used to check if liveness is enabled to the hybrid flow (LG-3471) (#4223)

Pass fields used to check if liveness is enabled to the hybrid flow (LG-3471)

* LG-3543 Update doc captures to avoid showing the selfie in the hybrid… (#4230)

The selfie step currently appears in the hybrid flow even if SPs don't request it. This commit backports the changes from #4223 to the old doc captures table so this bug is fixed for the old liveness flow.

Co-authored-by: Doug Price <dprice@fearless.tech>
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov>
Co-authored-by: Snyk bot <snyk-bot@snyk.io>
Co-authored-by: Jonathan Hooper <jonathan.hooper@gsa.gov>
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Co-authored-by: Steve Urciuoli <steve.urciuoli@gsa.gov>
Co-authored-by: Zach Margolis <zbmargolis@gmail.com>
Co-authored-by: Shade L. Jenifer <shade.jenifer@gsa.gov>
Co-authored-by: Alex Mathews <44584810+amathews-fs@users.noreply.github.com>
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.

None yet

2 participants