Skip to content

LG-11982: selfie related analytics log with selfie attempts.#10456

Merged
dawei-nava merged 6 commits intomainfrom
dwang/LG-11982_log_selfie_attempts
Apr 19, 2024
Merged

LG-11982: selfie related analytics log with selfie attempts.#10456
dawei-nava merged 6 commits intomainfrom
dwang/LG-11982_log_selfie_attempts

Conversation

@dawei-nava
Copy link
Contributor

@dawei-nava dawei-nava commented Apr 17, 2024

🎫 Ticket

Link to the relevant ticket:
LG-11982

🛠 Summary of changes

Added another counter of selfieAttempts, which indicate SDK successfully detected face and took a selfie.

User may decide to retake and it will be counted as another selfieAttempts.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Step 1: Initial selfie doc auth flow
  • Step 2: Take selfie and retake for a couple of times
  • Step 3: Verify logs for events(see card) contains selfieAttempts counted.

@dawei-nava dawei-nava marked this pull request as ready for review April 18, 2024 12:55
@@ -339,6 +339,7 @@ function AcuantCapture(
const { isMobile } = useContext(DeviceContext);
Copy link
Contributor

Choose a reason for hiding this comment

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

I tested this out manually and it mostly worked as expected! However I did see one extra selfie attempt - so if I took 2 pictures, it recorded 3.

const { isMobile } = useContext(DeviceContext);
const { t, formatHTML } = useI18n();
const [captureAttempts, incrementCaptureAttempts] = useCounter(1);
const selfieAttempts = useRef(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

What does the 1 do here?

I wonder if this is adding the extra attempt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@night-jellyfish , tried to be consistent with captureAttempts which has a default 1. Was using 0 here, if this is the consent I can use 0.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. I would be curious what other folks think / see.

Personally I think a default of 1 is fine, but not if it increases the actual number of attempts to one extra than it actually is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@night-jellyfish , set to 0 now.

Copy link
Contributor

@night-jellyfish night-jellyfish left a comment

Choose a reason for hiding this comment

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

Tested manually again and saw the expected number of attempts! 🎉

Pretty neat too that if I add the selfie and then decide to retake again, it keeps the count.

Thanks for making the requested changes!

@dawei-nava dawei-nava merged commit 4c78157 into main Apr 19, 2024
@dawei-nava dawei-nava deleted the dwang/LG-11982_log_selfie_attempts branch April 19, 2024 12:48
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.

2 participants