Remove the legacy doc auth steps and feature flags by jmhooper · Pull Request #4508 · 18F/identity-idp

jmhooper · 2020-12-11T22:58:55Z

Why: These steps are not in use anymore

This is a heavy diff. I recommend enabled the "Hide white space changes" in the "Files changed" tab when viewing it.

...and fix all the tests that break in the process The document capture step is enabled everywhere except for when our test suite runs (which seems unwise). This commit changes the config to make it enabled in the test suite. A few notes: - All of the legacy steps have a before action to flip the flag to disable the document capture step. I'm not fretting too much about those because the next PR I plan to open will remove them. - I moved some of the helper methods in the `DocAuthHelper` module into their own module for the document capture step. These new methods use the context on the page to complete the steps, rather than depending on the caller. - The document capture step tests have gotten a bit unwieldy. They have to test the fallback flow, the image upload API flow, and the S3 upload flow. I plan to come back around and break each of those cases up into their own spec file. I avoided that change here to keep the diff from getting too big.

**Why**: These steps are not in use anymore

app/views/idv/shared/_document_capture.html.erb

app/models/doc_capture.rb

**Why**: Because after #4508 is merged, we won't be using this table. We will be using document_capture_sessions instead.

jmhooper · 2020-12-18T14:27:32Z

This should be ready for review now. The total coverage check is failing, but I'm pretty sure that is because I removed a bunch of covered code. Diff coverage is at 96%.

stevegsa

Great job!

mitchellhenke

👏🏼

mitchellhenke · 2020-12-18T20:39:23Z

app/views/idv/shared/_document_capture.html.erb

+    <%= f.input(
+      :document_capture_session_uuid,
+      as: :hidden,
+      input_html: { value: flow_session[:document_capture_session_uuid] },


This isn't used by the lambdas or any callbacks, so I think it's ok to share this client-side, but wanted to confirm

Yeah, we actually give it to the React component root element on L19 above.

aduth

That's a lot of red! Great work 👍

spec/support/features/doc_capture_helper.rb

spec/javascripts/app/acuant/document_capture_fallback_spec.js

spec/forms/idv/document_capture_form_spec.rb

app/views/idv/doc_auth/_notices.html.erb

aduth · 2020-12-18T20:36:49Z

app/services/idv/steps/recover_step.rb

    class RecoverStep < DocAuthBaseStep
-      def call; end
+      def call
+        create_document_capture_session(document_capture_session_uuid_key)


I guess this should have always been here?

It happens in the WelcomeStep during doc auth flow, so I think so?
https://github.com/18F/identity-idp/blob/master/app/services/idv/steps/welcome_step.rb#L6

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

zachmargolis

LGTM, some small suggestions, the PR looks great overall!

app/services/idv/steps/upload_step.rb

spec/controllers/idv/capture_doc_controller_spec.rb

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

**Why**: Because after #4508 is merged, we won't be using this table. We will be using document_capture_sessions instead.

* Make AgencySeeder ignore the abbreviation attribute (#4617) Temporarily ignore the abbreviation attribute in agencies.yml to allow the config repo updates to be merged without breaking the IdP while we work on adding the Partnerships data model to the IdP schema. We will ultimately add the attribute to the `agencies` table and at that point remove this change. Also updates the localdev template YAML file and adds an explicit fixture file for testing, which is passed to the service object through dependency injection. * Add feature flag to send partial DOB to LexisNexis (LG-3706) (#4613) * Bump identity-proofer-gem to get Proofer::Result#transaction_id (#4624) * LG-4129: Add test case for cancelled IAL2 session (#4621) **Why**: Follow-up task from #4620 (comment) * LG-4097: Fix "Submit" button always appearing disabled on document capture review step (#4623) * Omit unknown errors when considering to allow step continue **Why**: Active errors contains both field-specific and general messages, e.g. received from a failed validation attempt. The latter will never become unresolved by changing field values. Only disable continue if the step is invalid OR if there are field-specific errors yet to be addressed. * Add ReviewIssuesStep validator * Fix lint error * Merge 3rd and 4th screens in mobile IAL2 flow (LG-3962) (#4626) * placeholder * remove mobile intro step * Update app/javascript/packages/document-capture/components/document-capture.jsx Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * update comment and remove unnecessary text * remove new line * fix merge Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * Move phone internationalization scripts to intl-tel-input pack (#4622) **Why**: Reduce size of main application bundle, which is loaded in all paths. Phone validation dependencies are quite large, and are only used in the context of the intl-tel-input pack (screens with phone input). * Add separate feature flag to disable VOIP checks entirely (#4629) * Retire the remote settings tooling (#4619) The remote setting tooling was added at a time when `service_providers.yml` was tracked in the IdP codebase. At that time it was not possible to update the configs for a service provider without changing the IdP code and deploying a new sha. With identity-idp-config, this is no longer true. This commit removes the code since now we can modify service provider configs by updating identity-idp-config and recycling to apply the changes. * Drop doc_captures table (#4519) **Why**: Because after #4508 is merged, we won't be using this table. We will be using document_capture_sessions instead. * LG-4114 Log Async Timeout events (#4618) * remove access rejected when capturing video (#4636) * LG-3769: Optimize image assets (#4635) * Add image optimization lint task * Remove "data-name" attribute from SVG * Optimize images * Restore keyframes to ID card SVG **Why**: Bug with SVGO removes keyframes. Comments are ignored by default, which are necessary for allowable exceptions to SVG inline styles. SVGO's `removeComments` will preserve comments prefixed with an exclamation point. See: svg/svgo#888 See: https://github.com/svg/svgo/blob/master/plugins/removeComments.js * Compress PNG images * Update lambda ref to get x-ray tracing functionality (#4637) * remove aal field from service_provider (LG-4119) (#4612) * remove aal field from service_provider * make remove_column reversible * Fix image upload 500 (LG-4154) (#4640) * add specs * do not attempt throttling if document capture session does not exist * do not raise exception on null byte in OIDC token code (#4641) * Remove migration that drops the AAL column (#4642) * Revert "remove aal field from service_provider (LG-4119) (#4612)" This reverts commit c9ce970. * Add AAL to the ignored column **Why**: We will be dropping this column in a future commit * Update proofer gem (#4646) **Why**: This version includes support for non-string attributes Co-authored-by: Oren Kanner <oren.kanner@gsa.gov> Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Jonathan Hooper <jonathan.hooper@gsa.gov> Co-authored-by: Steve Urciuoli <steve.urciuoli@gsa.gov> Co-authored-by: Doug Price <douglas.price@gsa.gov>

jmhooper added 7 commits December 11, 2020 13:11

no clue why i deleted this

b9958fb

no clue why i did this either

70a6002

fix smoke tests

48989b3

put geocoder stubs back the way i found them

cd44912

rebuild simulate_image_upload_api_submission

afca096

Remove the legacy doc auth steps and feature flags

7f4b11f

**Why**: These steps are not in use anymore

jmhooper changed the title ~~wip~~ [WIP] Remove the legacy doc auth steps and feature flags Dec 11, 2020

jmhooper added the status - work in progress label Dec 11, 2020

jmhooper commented Dec 11, 2020

View reviewed changes

app/views/idv/shared/_document_capture.html.erb Outdated Show resolved Hide resolved

Base automatically changed from jmhooper-document-capture-step-default-2 to master December 14, 2020 19:13

jmhooper added 7 commits December 15, 2020 10:52

A few test fixes

25b7b6c

Merge branch 'master' into jmhooper-remove-legacy-image-steps

f7600a1

fix the send link steps

191a825

fix more tests

efd6454

Merge branch 'master' into jmhooper-remove-legacy-image-steps

6f8a88b

Remove unused form and fix tests

1c112c2

Clean out old doc capture model code

59abdb1

jmhooper commented Dec 17, 2020

View reviewed changes

app/models/doc_capture.rb Show resolved Hide resolved

jmhooper added 6 commits December 17, 2020 13:02

fix some controller specs

f8f525e

more controller test fixes

b44fa92

More test cleanup

d7e07c6

Remove unused translations

712ee13

Put the time translations back the way they were

9d5a1c7

use session uuid

fdb3f05

jmhooper added a commit that referenced this pull request Dec 18, 2020

Drop doc_captures table

d86745d

**Why**: Because after #4508 is merged, we won't be using this table. We will be using document_capture_sessions instead.

jmhooper mentioned this pull request Dec 18, 2020

Drop doc_captures table #4519

Merged

jmhooper changed the title ~~[WIP] Remove the legacy doc auth steps and feature flags~~ Remove the legacy doc auth steps and feature flags Dec 18, 2020

jmhooper added status - ready for review and removed status - work in progress labels Dec 18, 2020

jmhooper requested review from aduth, mitchellhenke, stevegsa and zachmargolis December 18, 2020 14:26

stevegsa approved these changes Dec 18, 2020

View reviewed changes

mitchellhenke approved these changes Dec 18, 2020

View reviewed changes

aduth approved these changes Dec 18, 2020

View reviewed changes

jmhooper and others added 5 commits December 21, 2020 11:36

Update spec/support/features/doc_capture_helper.rb

c71b073

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

further cleanup

993c50d

Remove a test support file

2c83e26

Cleanup whitespace

f320b8e

remove notices code

7ad9633

zachmargolis approved these changes Dec 21, 2020

View reviewed changes

app/services/idv/steps/upload_step.rb Outdated Show resolved Hide resolved

spec/controllers/idv/capture_doc_controller_spec.rb Outdated Show resolved Hide resolved

jmhooper and others added 3 commits December 21, 2020 14:58

Update app/services/idv/steps/upload_step.rb

0862ad5

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

Update spec/controllers/idv/capture_doc_controller_spec.rb

93f8f22

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

Merge branch 'master' into jmhooper-remove-legacy-image-steps

63d3044

jmhooper merged commit 1fe7097 into master Dec 22, 2020

jmhooper deleted the jmhooper-remove-legacy-image-steps branch December 22, 2020 15:57

jmhooper added a commit that referenced this pull request Jan 27, 2021

Drop doc_captures table

eb814dd

**Why**: Because after #4508 is merged, we won't be using this table. We will be using document_capture_sessions instead.

jmhooper added a commit that referenced this pull request Jan 29, 2021

Drop doc_captures table (#4519)

1a95153

**Why**: Because after #4508 is merged, we won't be using this table. We will be using document_capture_sessions instead.

Conversation

jmhooper commented Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmhooper commented Dec 18, 2020

Uh oh!

stevegsa left a comment

Choose a reason for hiding this comment

Uh oh!

mitchellhenke left a comment

Choose a reason for hiding this comment

Uh oh!

mitchellhenke Dec 18, 2020

Choose a reason for hiding this comment

Uh oh!

jmhooper Dec 21, 2020

Choose a reason for hiding this comment

Uh oh!

aduth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aduth Dec 18, 2020

Choose a reason for hiding this comment

Uh oh!

mitchellhenke Dec 18, 2020

Choose a reason for hiding this comment

Uh oh!

zachmargolis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jmhooper commented Dec 11, 2020 •

edited

Loading