Skip to content

Fix Network Error in Dev by Adding missing selfie analytics event#10244

Merged
charleyf merged 7 commits intomainfrom
charley/fix-selfie-log
Mar 19, 2024
Merged

Fix Network Error in Dev by Adding missing selfie analytics event#10244
charleyf merged 7 commits intomainfrom
charley/fix-selfie-log

Conversation

@charleyf
Copy link
Copy Markdown
Contributor

@charleyf charleyf commented Mar 13, 2024

🛠 Summary of changes

I'm looking into some black screen on open problems that selfie is happening. The first thing that caught my eye was a logger error. It's unrelated to the black screen, but this PR fixes that error.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • On main get to the front/back/selfie upload page.
  • Open Safari and use the "Develop" menu to view the console for the page you have open on your iPhone.
  • Open to the "network" tab in safari so you can see the requests your iPHone is making.
  • Click the selfie box.
  • Note that there's a network request from logger that fails with a 400.
  • Switch to this branch: charley/fix-selfie-log
  • Repeat the steps above, there will now be no failed network requests from logger.

@charleyf charleyf marked this pull request as ready for review March 13, 2024 19:52
source:,
use_alternate_sdk:,
liveness_checking_required: nil,
**_extra
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is captureAttempts easy to add in?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is captureAttempts easy to add in?

👍 I'd hope we could add all of the properties here we're expecting to be logged.

@aduth aduth self-requested a review March 14, 2024 16:29
end
# rubocop:enable Naming/VariableName,Naming/MethodParameterName

# rubocop:disable Naming/VariableName,Naming/MethodParameterName,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For new analytics events, I'd wonder if it's worth trying to address some of what's being identified in these linters, i.e. naming the event properties with snake case. I suspect it might be made difficult by multiple events sharing the same common payload, but it seems like we will want to address that at some point anyways.

source:,
use_alternate_sdk:,
liveness_checking_required: nil,
**_extra
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is captureAttempts easy to add in?

👍 I'd hope we could add all of the properties here we're expecting to be logged.

@charleyf charleyf changed the title Add missing selfie analytics event Fix Network Error in Dev by Adding missing selfie analytics event Mar 15, 2024
@charleyf
Copy link
Copy Markdown
Contributor Author

I'm going to leave a bit of time for comments, but my plan is to merge this shortly (~noon/1pm) as is and revisit in LG-12178.

I would prefer to add the correct properties to this event, remove the rubocop:disable and generally put more time into this, but I don't think that's where I should be putting my time right now.

Reasoning:

  • I put this PR up because I want to remove a console error I'm seeing in dev that's making the black screen issue (recent slack) more confusing.
  • This event the way I implemented it matches the existing events idv_front_image_clicked and idv_back_image_clicked.
  • The ticket here: https://cm-jira.usa.gov/browse/LG-12178
    • Contains the various improvements we want to make. I suspect we're going to need to make that improvement to all three of the clicked events: idv_front_image_clicked, idv_back_image_clicked, and idv_selfie_image_clicked`.
    • I've gone and updated that ticket with a link to this PR and also some notes about what we've discussed here.

@charleyf charleyf merged commit 65a4854 into main Mar 19, 2024
@charleyf charleyf deleted the charley/fix-selfie-log branch March 19, 2024 14:29
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