Skip to content

Add count of users passing fraud review to Identity Verification Report#9251

Merged
soniaconnolly merged 4 commits intomainfrom
sonia-lg-11027-add-fraud-passed-to-idv-report
Sep 25, 2023
Merged

Add count of users passing fraud review to Identity Verification Report#9251
soniaconnolly merged 4 commits intomainfrom
sonia-lg-11027-add-fraud-passed-to-idv-report

Conversation

@soniaconnolly
Copy link
Contributor

🎫 Ticket

LG-11027

🛠 Summary of changes

Add a line to Identity Verification Report for users who have passed fraud review, using the new event added in #9194.

Incidentally fix spelling in the report.

📜 Testing Plan

  • Manually test on a prod instance?

changelog: Internal, Reporting, Add count of users passing fraud review to Identity Verification Report
@soniaconnolly soniaconnolly requested review from a team, jmhooper and zachmargolis September 21, 2023 22:54
Comment on lines +216 to +217
when Events::FRAUD_REVIEW_PASSED
event_users[Results::FRAUD_REVIEW_PASSED] << user_id if success == '1'
Copy link
Contributor

Choose a reason for hiding this comment

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

we could also add this filtering to the CW query itself? similar pattern as these:

        | filter (name = %{usps_enrollment_status_updated} and properties.event_properties.passed = 1)
                 or (name != %{usps_enrollment_status_updated})

in hindsight the other branches here could have maybe been done similarly

Copy link
Contributor

@zachmargolis zachmargolis Sep 22, 2023

Choose a reason for hiding this comment

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

I also just realized. this branch is redundant because we have

          event_users[event] << user_id

above. and since these are sets, it will not be duplicated

'IdV Reject: Doc Auth' => 3,
'IdV Reject: Phone Finder' => 1,
'IdV Reject: Verify' => 1,
'Fraud: Profile review passed' => 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

this is an "event", like a literal CW event name right? so we put it up above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the guidance. I was trying to figure out the patterns here, and adding success = 1 to the query is what I was missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the code to print out the query and then tested it out in cloudwatch. Fixed one problem, thanks for the suggestion.

soniaconnolly and others added 2 commits September 22, 2023 13:23
Started to amend this part of the query and left stray paren.

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Comment on lines 153 to +155
def successfully_verified_users
idv_final_resolution_verified + gpo_verification_submitted + usps_enrollment_status_updated
idv_final_resolution_verified + gpo_verification_submitted + usps_enrollment_status_updated +
fraud_review_passed
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In CloudWatch, I see that the Fraud: Profile review passed rows have the computed value identity_verified set to 1, so I checked whether adding the fraud_review_passed count here will double count them. Looks like identity_verified is only used for the final_resolution event, so the code should work as is. Tests pass.

@soniaconnolly soniaconnolly merged commit 5f75097 into main Sep 25, 2023
@soniaconnolly soniaconnolly deleted the sonia-lg-11027-add-fraud-passed-to-idv-report branch September 25, 2023 15:17
@solipet solipet mentioned this pull request Sep 26, 2023
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