Skip to content

LG-14948 Try again opens selfie capture camera instead of redo doc auth page.#11661

Merged
theabrad merged 4 commits intomainfrom
abrad-lg-14948-try-again-bug
Dec 18, 2024
Merged

LG-14948 Try again opens selfie capture camera instead of redo doc auth page.#11661
theabrad merged 4 commits intomainfrom
abrad-lg-14948-try-again-bug

Conversation

@theabrad
Copy link
Contributor

🎫 Ticket

Link to the relevant ticket:
LG-14948

🛠 Summary of changes

Fixed a bug where if a user fail's doc auth and hits "Try again online" it immediatly opens up the selfie camera.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Go through Doc Auth with selfie with a failing ID
  • Click "Try again online"
  • Confirm that the selfie SDK does not open back up.

changelog: Bug Fixes, Doc Auth, Fix try again button opening up Selfie SDK
@theabrad theabrad force-pushed the abrad-lg-14948-try-again-bug branch from c276085 to c667986 Compare December 17, 2024 21:28
isSelfieDesktopTestMode: String(docAuthSelfieDesktopTestMode) === 'true',
showHelpInitially: true,
immediatelyBeginCapture: true,
immediatelyBeginCapture: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the only place we had been passing immediatelyBeginCapture ? Can we remove it altogether?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah looks like we don't need it. Had to confirm with the team before i outright removed it. Will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After testing we actually need immediatelyBeginCapture it might be in the acuant docs somewhere

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any references to that term in Acuant's source code. What were the issues you were bumping into?

@theabrad theabrad merged commit bed4b16 into main Dec 18, 2024
@theabrad theabrad deleted the abrad-lg-14948-try-again-bug branch December 18, 2024 20:28
@aduth
Copy link
Contributor

aduth commented Dec 18, 2024

@theabrad @amirbey Could one of you respond to the question at #11661 (comment) ? I don't see that we're ever setting this to anything other than false, nor is the value passed directly to Acuant. It seems that this is effectively dead code, and that removing it should have no functional impact.

@amirbey
Copy link
Contributor

amirbey commented Dec 18, 2024

👋🏿 @aduth - @theabrad initial attempt at removing the field created a behavior change ... due to timeliness, we moved forward with the working version. i've chatted with @theabrad and he will revisit this for a potential follow up PR; however, if we you are still encountering hiccups, @theabrad can you create a ticket to dig into it deeper to clean this up?

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