LG-6777 capture native camera after failed twice by peggles2 · Pull Request #6727 · 18F/identity-idp

peggles2 · 2022-08-11T20:27:31Z

This pull request will force native camera to do the document capture after the sdk camera has failed twice.

-- What We are saving progress, but still trying to figure out where to inject the faile attempts check in order to trigger manual camera/upload flow.

-- What For clarity in the FailedCaptureContext interface, we've added a property called `forceNativeCamera` which is a boolean value that will update in the FailedCaptureContextProvider whenever the maxAttemptsBeforeNativeCamera is equal to or exceeds the current failedCaptures value. Additionally, we finally have some working tests for this. For the moment, we only get results by forcing maxAttemptsBeforeNativeCamera to be 0, then clicking the file input. This seems to do the trick. But we need to figure out how to test a couple of other scenarios, and how to force failures from that point in a test case.

changelog: Improvements, Acuant, updating max attempts before login

-- What This commit updates various JSDoc type annotations so that they accurately describe the new structure of the FailedCaptureAttempts context. Specifically, we added maxAttemptsBeforeNativeCamera and forceNativeCamera to this context in previous commits. Now the types and provider components accurately reflect those changes.

-- What Adding a test to ensure that logging is *not* called in the case where failed attempts is not equal or greater to the max attempts before native camera is forced. changelog: Improvements, Acuant Camera settings, updating max failures before native camera is triggered

solipet

Looks good - I'm going to try and test it locally, but that will wait until Monday...

solipet · 2022-08-12T21:12:23Z

app/javascript/packages/document-capture/components/acuant-capture.jsx

+        addPageAction(`IdV: Force native camera. Failed attempts: ${failedCaptureAttempts}`, {
+          field: name,
+        });


Since addPageAction logs to the event.log, the name passed should be constant and the failed attempts should go in the data hash that will be logged with it.

Suggested change

addPageAction(`IdV: Force native camera. Failed attempts: ${failedCaptureAttempts}`, {

field: name,

});

addPageAction('IdV: Force native camera.', {

field: name,

failed_attempts: $(failedCaptureAttempts),

});

Should we also allowlist this event? see EVENT_MAP for methods on AnalyticsEvents

so it doesn't need to get prefixed with "frontend"

so we can get documentation for it?

I think we should probably leave out the period as well in the suggested change, for consistency with other events?

Going to work on this now. What are the arguments passed to these ruby-side logging methods in AnalyticsEvents -- are they just the same as the dict/object we pass as the second arg to addPageAction on the frontend?

Going to work on this now. What are the arguments passed to these ruby-side logging methods in AnalyticsEvents -- are they just the same as the dict/object we pass as the second arg to addPageAction on the frontend?

Yeah, the properties we add in the JavaScript object here will be passed through as keyword arguments to the corresponding Ruby method.

identity-idp/app/controllers/frontend_log_controller.rb

Line 34 in e174eb1

analytics_method.bind_call(analytics, **payload)

Ok I believe I've dealt with this in 1b78fed and subsequent commits

changelog: Improvements, Acuant native camera, linting fixes

-- What We were not correctly calling `addPageAction` when trying to log the force native camera event from the frontend. Additionally, we are adding a method to the frontend log controller for this action so that it can have appropriate documentation. changelog: Improvement, Logging, loggin force native camera event

changelog: Improvements, Testing, removing unused features from tests

-- What After updating the IdV logging string for forcing native camera, we neede to also change the assumption of the tests that check for that string changelog: Improvements, Frontend Testing, fixing logging tests for failed capture

changelog: Improvements, Lintinf fixes, fixing linting in ruby analytics

app/javascript/packages/document-capture/components/acuant-capture.jsx

aduth · 2022-08-15T17:13:45Z

app/javascript/packages/document-capture/context/failed-capture-attempts.jsx

 * @prop {number} maxFailedAttemptsBeforeTips
+ * @prop {number} maxAttemptsBeforeNativeCamera


Will a user ever see these tips anymore? If not, is this a feature we'd want to remove?

@aduth good catch -- we are currently all discussing this and will get back to you / push changes if needed.

Ok. We have decided to open a separate ticket for dealing with the tips issue. It's going to take us a bit more time to sort that all out. As it stands with this branch, the tips should never be triggered (the default is 3 failed attempts vs our 2).

eric-gade · 2022-08-16T18:48:54Z

Question: since we have added a configurable yaml setting (doc_auth_max_attempts_before_native_camera: 2) in application.default.yml, do we need to also add this in the stored S3 prod settings somehow?

aduth · 2022-08-16T19:05:16Z

Question: since we have added a configurable yaml setting (doc_auth_max_attempts_before_native_camera: 2) in application.default.yml, do we need to also add this in the stored S3 prod settings somehow?

No, as long as it's either in the top-level or production: section of application.yml.default, it'll be used assuming there is no value in the S3 configuration that overrides it, since the latter is merged with the defaults.

Eric Gade and others added 9 commits August 4, 2022 16:58

Initial commit of LG-6777 work

0c3bbfb

-- What We are saving progress, but still trying to figure out where to inject the faile attempts check in order to trigger manual camera/upload flow.

LG-6777 - add maxAttemptsBeforeNativeCamera variable

3e6ae99

fix test

4d3a8f9

add test for analytics

1d51da5

Updaing test file to remove node error

5a4cfe6

Updating frontend tests for maxAttemptsBeforeNativeCamera

88a6a1e

changelog: Improvements, Acuant, updating max attempts before login

peggles2 changed the title ~~Lg 6777 capture native camera after failed twice~~ Lg-6777 capture native camera after failed twice Aug 11, 2022

peggles2 changed the title ~~Lg-6777 capture native camera after failed twice~~ LG-6777 capture native camera after failed twice Aug 11, 2022

peggles2 requested a review from a team August 11, 2022 22:08

solipet reviewed Aug 12, 2022

View reviewed changes

eric-gade added 5 commits August 15, 2022 08:59

Fixing linter errors in JS files

89ae00f

changelog: Improvements, Acuant native camera, linting fixes

Removing unneeded initialize in failed capture tests

102b543

changelog: Improvements, Testing, removing unused features from tests

Fixing logging check in JS failed capture tests

cc442ac

-- What After updating the IdV logging string for forcing native camera, we neede to also change the assumption of the tests that check for that string changelog: Improvements, Frontend Testing, fixing logging tests for failed capture

Fixing linter errors in Ruby analytics code

887c011

changelog: Improvements, Lintinf fixes, fixing linting in ruby analytics

aduth approved these changes Aug 15, 2022

View reviewed changes

peggles2 merged commit 602df44 into main Aug 16, 2022

peggles2 deleted the LG-6777-capture-native-camera-after-failed branch August 16, 2022 21:00

eric-gade mentioned this pull request Aug 17, 2022

LG-7291 TypeScript refactoring for 6777 #6769

Merged

jmhooper mentioned this pull request Aug 22, 2022

Parissa lg6777 #6696

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LG-6777 capture native camera after failed twice#6727

LG-6777 capture native camera after failed twice#6727
peggles2 merged 14 commits intomainfrom
LG-6777-capture-native-camera-after-failed

peggles2 commented Aug 11, 2022

Uh oh!

solipet left a comment

Uh oh!

solipet Aug 12, 2022

Uh oh!

zachmargolis Aug 12, 2022

Uh oh!

aduth Aug 15, 2022

Uh oh!

eric-gade Aug 15, 2022

Uh oh!

aduth Aug 15, 2022

Uh oh!

eric-gade Aug 15, 2022

Uh oh!

Uh oh!

aduth Aug 15, 2022

Uh oh!

eric-gade Aug 16, 2022

Uh oh!

eric-gade Aug 16, 2022

Uh oh!

eric-gade commented Aug 16, 2022

Uh oh!

aduth commented Aug 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		* @prop {number} maxFailedAttemptsBeforeTips
		* @prop {number} maxAttemptsBeforeNativeCamera

Conversation

peggles2 commented Aug 11, 2022

Uh oh!

solipet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-gade commented Aug 16, 2022

Uh oh!

aduth commented Aug 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants