Skip to content

increment failed capture event if we get the review warning screen#6909

Merged
eric-gade merged 13 commits intomainfrom
lg-7300-native_camera_file_2_submission_attempts
Sep 16, 2022
Merged

increment failed capture event if we get the review warning screen#6909
eric-gade merged 13 commits intomainfrom
lg-7300-native_camera_file_2_submission_attempts

Conversation

@peggles2
Copy link
Contributor

@peggles2 peggles2 commented Sep 2, 2022

No description provided.

@peggles2 peggles2 force-pushed the lg-7300-native_camera_file_2_submission_attempts branch from dd165e5 to b74a750 Compare September 2, 2022 21:23
@eric-gade eric-gade requested a review from a team September 12, 2022 18:48
peggles2 and others added 6 commits September 12, 2022 15:18
changelog: Improvements, Native Camera, Use native camera after 2 failed submissions
-- What
We need to track failed submissions in addition to failed captures,
and trigger forceNativeCamera accordingly.

The easiest way to accomplish this without touching the backend is to
simply add this as a property to the failed capture context, then
increment it on the review page.

In a subsequent commit, we should explore not even incrementing this,
but simply checking the upload remainingAttempts against the
configured maximum value, and setting forceNativeCamera based on that.

Updating tests accordingly.

changelog: Improvements, Acuant Capture, updating ability to force
native camera
-- What
This commit has two main changes:
1. We are renaming `max_attempts_before_native_camera` and its JS
equivalent `maxAttemptsBeforeNativeCamera` to
`max_capture_attempts_before_native_camera` etc.
2. We are adding a new configuration option called
`max_submission_attempts_before_native_camera`, and linking submission
attempt failure counting to check for exceeding this value when
determining the `forceNativeCamera` boolean in the
FailedCaptureContext.

changelog: Improvements, Document Submission, adding force native
camera option for failed submissions
changelog: Improvements, Document Capture, adding native camera option
for failed submissions
@eric-gade eric-gade force-pushed the lg-7300-native_camera_file_2_submission_attempts branch from 776f5e6 to eaa6f5a Compare September 12, 2022 19:40
eric-gade added 4 commits September 12, 2022 15:44
-- What
With the new distinction between failed_capture_attempts and
failed_submission_attempts -- and the corresponding way that these are
logged from the frontend -- we need to update the new explicit
analytics method signature on the Ruby side.

changelog: Improvements, Document Capture, adding native camera
capability to failed submission attempts
changelog: Improvements, Document Capture, adding native camera
capability to failed submissions
changelog: Improvements, Document Capture, adding native camera
capability to failed submissions
changelog: Improvements, Document Capture, adding native camera
capability to failed submissions
@eric-gade eric-gade marked this pull request as ready for review September 13, 2022 12:23
@aduth
Copy link
Contributor

aduth commented Sep 14, 2022

I'm noticing a strange behavior when testing this on desktop. After submitting twice with a failing result, I can choose to upload a new image, but as soon as I select an image, the prompt to select an image reappears, leading to a situation where I can't change the image.

pr-6909.mov

These changes are targeted at mobile, but the same component is also used on desktop.

I think what might be happening is that the call to forceUpload() in startCaptureOrTriggerUpload is redundant in this case, since on desktop the function would be triggered by the default file selection, so calling forceUpload only serves to show a second file selection prompt.

function startCaptureOrTriggerUpload(event: MouseEvent) {
if (event.target === inputRef.current) {
if (forceNativeCamera) {
trackEvent('IdV: Native camera forced after failed attempts', {
field: name,
failed_attempts: failedCaptureAttempts,
});
return forceUpload();

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
@eric-gade
Copy link
Contributor

eric-gade commented Sep 14, 2022

Aha, yes, good catch there. Prior to this, it was only Acuant SDK failures that would meet the criteria (and therefore only in mobile environments). We need to now ensure that we are checking for mobile explicitly.

Oddly, I can reproduce this behavior in Chrome, but not FF

-- What
Previously, Acuant SDK failures were the only way to set
forceNativeCamera. These _always_ occur in the context of a mobile
device, due to how the SDK loads. However, we are now giving this
option to people who fail to _submit_ more than twice, which can also
occur on desktop.

The upshot is that Chrome browsers were triggering a file picker twice
in a row on desktop environments, because we were not explicitly
checking to ensure that the current environment was mobile before
calling forceUpload(). This has now been rectified.

I have also updated the tests accordingly.

changelog: Improvements, Document Capture, adding native camera option
for failed submissions
function startCaptureOrTriggerUpload(event: MouseEvent) {
if (event.target === inputRef.current) {
if (forceNativeCamera) {
if (forceNativeCamera && isMobile) {
Copy link
Contributor

@aduth aduth Sep 15, 2022

Choose a reason for hiding this comment

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

Checking mobile seems to be only indirectly related to the logic associated with starting Acuant capture. In fact, I hadn't realized that we're already evaluating whether to use Acuant on the lines immediately following this condition (shouldStartAcuantCapture), so the logic associated with forcing native camera in this condition seems a bit redundant. I think we should consider forceNativeCamera as part of that assignment and avoid the call to forceUpload altogether.

    if (event.target === inputRef.current) {
      const isAcuantCaptureCapable = hasCapture && !acuantFailureCookie;
      const isEnvironmentCapture = capture !== 'user';
      const shouldStartAcuantCapture =
        isAcuantCaptureCapable &&
        isEnvironmentCapture &&
        !isForceUploading.current &&
        !forceNativeCamera;
      const shouldStartSelfieCapture =
        isAcuantLoaded && capture === 'user' && !isForceUploading.current;

      if (isAcuantCaptureCapable && isEnvironmentCapture && forceNativeCamera) {
        trackEvent('IdV: Native camera forced after failed attempts', {
          field: name,
          failed_capture_attempts: failedCaptureAttempts,
          failed_submission_attempts: failedSubmissionAttempts,
        });
      }

      if (!allowUpload || shouldStartSelfieCapture || shouldStartAcuantCapture) {
        event.preventDefault();
      }

      // ...

changelog: Improvements, Document Capture, enabling native camera
after failed submissions
@eric-gade eric-gade force-pushed the lg-7300-native_camera_file_2_submission_attempts branch from 1b6d241 to f5b3514 Compare September 15, 2022 17:30
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@eric-gade eric-gade merged commit 6c52a06 into main Sep 16, 2022
@eric-gade eric-gade deleted the lg-7300-native_camera_file_2_submission_attempts branch September 16, 2022 13:20
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