LG-11148 | Adds monthly report on total verified users by n1zyy · Pull Request #9376 · 18F/identity-idp

n1zyy · 2023-10-12T21:37:57Z

changelog: Internal, Reporting, Monthly report now includes total verified users

🎫 Ticket

https://cm-jira.usa.gov/browse/LG-11148

🛠 Summary of changes

New report on total verified users as part of the monthly reporting framework.

📜 Testing Plan

Automated tests as part of this PR
Verify the nightly test email prior to acceptance

changelog: Internal, Reporting, Monthly report now includes total verified users

app/services/reporting/total_user_count_report.rb

n1zyy · 2023-10-12T21:44:00Z

app/services/reporting/total_user_count_report.rb

+
+    def verified_user_count
+      Profile.where(active: true).where('activated_at <= ?', report_date).count
+    end


I am boldly just assuming that the request for "verified users" means "IdV users," but hopefully @zachmargolis or someone can confirm if that is how others would interpret the term?

And to slightly further open the can of worms: I'm including the active: true limit to be consistent with the handbook, but the other report generated in this class is "total users all time" so I think one could make an argument that we shouldn't filter only on active: true profiles.

yeah that's a correct interpretation and yes, we should keep the handbook query here

n1zyy · 2023-10-12T21:44:23Z

spec/jobs/reports/monthly_key_metrics_report_spec.rb

+      total_user_count_s3_path,
+      total_verified_users_path,
+    ]
+  end


I built an array of all these expected S3 files so that...

n1zyy · 2023-10-12T21:44:43Z

spec/jobs/reports/monthly_key_metrics_report_spec.rb

+        path: path,
+        **s3_metadata,
+      ).exactly(1).time.and_call_original
+    end


...we can replace this with a happy little loop.

spec/services/reporting/total_user_count_report_spec.rb

zachmargolis · 2023-10-12T21:57:35Z

app/jobs/reports/monthly_key_metrics_report.rb

        total_user_count_report.total_user_count_emailable_report,
+        total_user_count_report.total_verified_users_emailable_report,


should we make these the same table? so it looks like

Total Users

Metric Value

Total Users 1,000

Total Verified Users 500

n1zyy · 2023-10-18T19:35:01Z

app/services/reporting/total_user_count_report.rb

+
+    def end_date
+      report_date.beginning_of_day
+    end


I worked with @olatifflexion to ~~steal his code~~ incorporate his work on LG-11150 into this, rather than having him deal with merge conflicts.

We confirmed with Cale in the original doc that "annual" here refers to the previous 12 months, not fiscal/calendar year.

zachmargolis · 2023-10-18T19:39:41Z

app/services/reporting/total_user_count_report.rb

+        ['All-time user count', 'Total verified users', 'Total annual users'],
+        [total_user_count, verified_user_count, annual_total_user_count],


I think a format like in this comment would be clearer?

#9376 (comment)

Oh! I actually agree. Will change that.

zachmargolis · 2023-10-18T19:52:37Z

app/services/reporting/total_user_count_report.rb

    end
+
+    def verified_user_count
+      Profile.where(active: true).where('activated_at <= ?', report_date).count


reminder to add Reports::BaseReport.transaction_with_timeout { ... } here

or around the generation of the whole report instead of each DB query

Thanks! That had completely slipped my mind.

And avoid merge conflicts all the while!

app/services/reporting/total_user_count_report.rb

* LG-11083: Enable USPS Public Endpoint (#9355) * changelog: Internal, In-Person Proofing, Enable public USPS post office search * Use EnrollmentHelper to switch between mock/real thing * Try behaves_like * Revert shared examples for now * Use full name * Update report mailer preview to be more realistic (#9419) **How**: stubs CloudwatchClient changelog: Internal, Reporting, Updates report preview to use live code * Add analytics section to frontend documentation (#9421) * Add analytics section to frontend documentation changelog: Internal, Documentation, Add analytics frontend documentation * link to correct javascript package * LG-11101: Support multiple valid MFA to satisfy authentication request (#9335) changelog: User-Facing Improvements, MFA, Avoid prompting for MFA in some scenarios where a recent MFA satisfies the requirement * LG-11148 | Adds monthly report on total verified users (#9376) changelog: Internal, Reporting, Monthly report now includes total verified users Also incorporates LG-11150 Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov> * Remove second MFA prompt exception for strict MFA requirement (#9422) changelog: User-Facing Improvements, MFA Setup, Add second MFA reminder screen for single-MFA accounts when signing in at AAL2 * LG-11126 Update Start over verifying your identity screen (#9313) * change text for start over verify screen * add translations for page * add changelog changelog: User-Facing Improvements, IdV By Mail, update text in start over verifying identity screen * remove unused i18n * create new translation with question mark added * current step indicator for user not in gpo flow yet * a missing period * Restore deleted translations, and rename start_over to start_over_new_address Co-authored-by: Doug Price <douglas.price@gsa.gov> * New template for confirm start over from request_letter Add source param to indicate whether referer is request_letter * Update specs to check for correct template Co-authored-by: Doug Price <douglas.price@gsa.gov> * Add before_letter route for new screen, don't use it yet And analytics * Lint, unused arg in analytics_events * alphabetization lint * Add suggested comment Co-authored-by: Matt Hinz <matt.hinz@gsa.gov> * lints --------- Co-authored-by: Douglas Price <douglas.price@gsa.gov> Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov> Co-authored-by: Matt Hinz <matt.hinz@gsa.gov> * LG-11198: Update address text (#9420) Update address text changelog: User-Facing Improvements, IdV, Update text for address * LG-10922: Display new headings for Hybrid Handoff page on AB test (#9316) * changelog: User-Facing Improvements, Doc Auth, Display new headings for Hybrid Handoff page on AB test Adds: * Conditional headers depending on which flag is on * Hybrid handoff show view test * Translations * LG-11235: Rename double address verification as ipp_enrollment_in_progress (#9390) * Removed double address verification replaced with ipp_enrollment_in_progress * changelog: Internal, In-person Proofing, change DAV references to reflect reality * Change test description to be closer to what is being changed in the controller * Addressing 50/50 state concerns in proofer and adjudicator * Addressing linter issues * Set missing initial value for dav * Moving arg with default value to end of list * Apply suggestions from code review Adding proper input to job_arguments hash. Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * Adding note about existing ticket for work post 50/50 state * Resolving Shannon's comments * Adding back in test for dav, need reader on adjudicator * Adding back in test for dav, need reader on adjudicator --------- Co-authored-by: jack.ryan@gsa.gov <johnaryan@fcoh2j-f4t79kf4.myfiosgateway.com> Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * Add --deflate option to data-pull and action-account scripts (#9424) changelog: Internal, Scripts, Add --deflate option to data-pull and action-account scripts --------- Co-authored-by: Matt Gardner <wilburnforce@gmail.com> Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Matt Wagner <mattwagner@navapbc.com> Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov> Co-authored-by: Alex Bradley <alexander.bradley@gsa.gov> Co-authored-by: Douglas Price <douglas.price@gsa.gov> Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov> Co-authored-by: Matt Hinz <matt.hinz@gsa.gov> Co-authored-by: jc-gsa <104452882+jc-gsa@users.noreply.github.com> Co-authored-by: Brittany Greaner <35475380+night-jellyfish@users.noreply.github.com> Co-authored-by: Jack Ryan <jackryan@navapbc.com> Co-authored-by: jack.ryan@gsa.gov <johnaryan@fcoh2j-f4t79kf4.myfiosgateway.com>

LG-11148 | Adds monthly report on total verified users

8b8abdc

changelog: Internal, Reporting, Monthly report now includes total verified users

n1zyy commented Oct 12, 2023

View reviewed changes

Now without one fewer egregiously-broken test. Oops.

0073046

n1zyy requested a review from a team October 12, 2023 21:49

zachmargolis reviewed Oct 12, 2023

View reviewed changes

zachmargolis approved these changes Oct 12, 2023

View reviewed changes

n1zyy added 5 commits October 16, 2023 16:32

Merge branch 'main' into mattw/LG-11148

ba042e4

cleanup

c82aecb

lintfix

dc824fa

Merge branch 'main' into mattw/LG-11148

a6529f8

Fix test I somehow broke?

a7bd27a

n1zyy commented Oct 18, 2023

View reviewed changes

zachmargolis reviewed Oct 18, 2023

View reviewed changes

n1zyy added 2 commits October 19, 2023 12:25

Merge branch 'main' into mattw/LG-11148

08f6257

Rotate table, add timeouts

f90cfc6

And avoid merge conflicts all the while!

zachmargolis approved these changes Oct 19, 2023

View reviewed changes

zachmargolis reviewed Oct 19, 2023

View reviewed changes

app/services/reporting/total_user_count_report.rb Outdated Show resolved Hide resolved

n1zyy and others added 3 commits October 19, 2023 14:50

Show annual report period

82cff22

Update table to have time range in it

e987ffa

Test fix

063cd29

zachmargolis approved these changes Oct 19, 2023

View reviewed changes

n1zyy merged commit c1f3dc2 into main Oct 20, 2023

n1zyy deleted the mattw/LG-11148 branch October 20, 2023 13:35

mdiarra3 mentioned this pull request Oct 23, 2023

Deploy RC 326 to Prod #9432

Merged

		total_user_count_report.total_user_count_emailable_report,
		total_user_count_report.total_verified_users_emailable_report,

		['All-time user count', 'Total verified users', 'Total annual users'],
		[total_user_count, verified_user_count, annual_total_user_count],

Conversation

n1zyy commented Oct 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎫 Ticket

🛠 Summary of changes

📜 Testing Plan

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

n1zyy Oct 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

n1zyy commented Oct 12, 2023 •

edited

Loading

n1zyy Oct 18, 2023 •

edited

Loading