Skip to content

Rewrite the IRS Monthly Credential Metrics Report#12527

Merged
astrogeco merged 25 commits intomainfrom
irs_monthly_cred_report_refactor
Sep 29, 2025
Merged

Rewrite the IRS Monthly Credential Metrics Report#12527
astrogeco merged 25 commits intomainfrom
irs_monthly_cred_report_refactor

Conversation

@astrogeco
Copy link
Copy Markdown
Contributor

@astrogeco astrogeco commented Sep 24, 2025

🎫 Ticket

Link to the relevant ticket:
Team-Data/reporting

🛠 Summary of changes

Instead of copying over multiple components of the combined_invoice_supplement_report_v2.rb inherit from it and narrow the partner and issuers to the required ones.

Leverages the existing irs_credential_tenure_report_config parameter and expands it into a JSON containing the various parameters needed.

📜 Testing Plan

Test format in local and sandbox environments, test data in dm environment and compare to DW query outputs

  • Update specs
  • Format correctness
  • Data correctness evidence
  • Test that job runs successfully based on the schedule

👀 Screenshots

Before:
After:

@@ -616,6 +616,7 @@ test:
hmac_fingerprinter_key: a2c813d4dca919340866ba58063e4072adc459b767a74cf2666d5c1eef3861db26708e7437abde1755eb24f4034386b0fea1850a1cb7e56bff8fae3cc6ade96c
hmac_fingerprinter_key_queue: '["old-key-one", "old-key-two"]'
identity_pki_disabled: true
irs_issuers: '["urn:gov:gsa:openidconnect.profiles:sp:sso:agency_name:app_name"]'
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.

we dont need an irs_monthly_cred_emails config here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm using irs_credentials_emails in line 246

Inherit from CombinedInvoiceSupplement

Use json config in IdentityConfigStore
… BaseReport

Add mock issuer and partner string

Fix default config param definition location
Remove unneeded comments
…on logic

Fix report so that it only outputs the report month instead of all months

Add safety by rejecting blank config parameters

Fix partner string comparison
@astrogeco astrogeco force-pushed the irs_monthly_cred_report_refactor branch from 81b7c24 to a8fd168 Compare September 26, 2025 03:07
@astrogeco astrogeco marked this pull request as ready for review September 26, 2025 14:32
Copy link
Copy Markdown
Contributor

@tau-usds tau-usds left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Copy Markdown
Contributor

@MrNagoo MrNagoo left a comment

Choose a reason for hiding this comment

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

Approve from a 'make it work' stand point. Probably some things to refactor.

@astrogeco astrogeco force-pushed the irs_monthly_cred_report_refactor branch from 075ec10 to f74f7b4 Compare September 26, 2025 21:59
@astrogeco astrogeco force-pushed the irs_monthly_cred_report_refactor branch from f74f7b4 to 89f737c Compare September 26, 2025 22:12
@astrogeco astrogeco merged commit 123fecb into main Sep 29, 2025
1 check passed
@astrogeco astrogeco deleted the irs_monthly_cred_report_refactor branch September 29, 2025 17:35
mitchellhenke pushed a commit that referenced this pull request Sep 30, 2025
* Remove unused config setting for approving IPP enrollments (#12477)

* Remove unused config setting for approving IPP enrollments

I had originally submitted PR #12014 to allow immediate approval of
IPP enrollments in `int`, to make it easier for partners to test IPP in
the sandbox environment. At the time, I thought this was the only way to
do it, after seeing PR #11653 that introduced the ability to manually
approve IPP enrollments in local development only.

However, it turns out the route that displays the IPP enrollments is
behind another config setting that requires enabling test routes. We
didn't think it was a good idea to enable test routes in `int`, and we
then realized that solving our problem was as easy as changing these two
settings that were causing the 1.5 hour delay between completing IPP and
receiving the confirmation email:

```
get_usps_proofing_results_job_cron: '0/1 * * * *'
in_person_results_delay_in_hours: 0
```

whereas the default values in our lower environments were:

```
get_usps_proofing_results_job_cron: '0/30 * * * *'
in_person_results_delay_in_hours: 1
```
which resulted in having to wait 1.5 hours to complete IPP testing.

changelog: Internal, IPP, Remove unused config setting

* Team Data 1105(feature) Add Fraud Ops Tracking for data warehouse and FCMS (#12503)

(feature) Add Fraud Ops Tracking for data warehouse and FCMS

* changelog: Internal, Reporting, Tracking for Fraud Ops (Team Data-1105)

* Handle SAML request errors from saml_idp gem (#12520)

Before, the saml_idp gem would return a 403 response if it found
specific errors with the SAML request, such as a missing issuer, or a
missing AuthnRequest. The problem is that this did not provide any
helpful information to partners while testing their integration.

To improve this, we updated the `saml_idp` gem to populate the
`saml_request` object with errors and not do anything else. This allows
the IdP to consume those errors and handle them appropriately, by
rendering them in the existing `/saml/auth/error` template.

This allows partners to quickly understand what part of their SAML
Request is  invalid, and makes it easier for them to fix the problem.
If they don't  know how to fix it themselves, they can let us know the
error message they see, which makes it easier and faster for us to help
our partners.

changelog: User-Facing Improvements, SAML Request Handling, Display SAML request errors so partners can understand what they did wrong

* Update cron schedules for reports to improve timing and avoid overlaps (#12531)

* changelog: internal, reporting, issue-#1113 stagger jobs Update cron schedules for reports to improve timing and avoid overlaps

* Prevent users from skipping choose_id_type (#12502)

* Prevent users from skipping ID type selection when accessing document capture directly

changelog: Bug Fixes, Identity Verification, Prevent users from skipping ID type selection when accessing document capture directly

* Fix failing specs after choose_id_type enforcement

* Use session state for choose_id_type enforcement

* Prevent hybrid mobile users from skipping choose_id_type step

* Update hybrid mobile flow test

* Refactor ID type enforcement based on PR feedback

* move logic from step_info preconditions to a dedicated before_action

* fix failing test

* patch failing test

* fix issue in mobile flow

* prevent users from skipping choose ID type step

* Simplify document_capture preconditions and update test helper

* address pr feedback

* Rewrite the IRS Monthly Credential Metrics Report (#12527)

* changelog: Bug Fixes, Reporting, Simplifies execution and logic of the monthly credential report by using a single-partner/single-issuer invocation of the CombinedInvoiceSupplementReportV2

Report Source Changes
* Add partner strings config
* Refactor data fetching and generation
* Add MAU and other fields

Email Report Changes
* Update corresponding mailer preview invocation
* Remove reference to key metrics report in email text
* Transpose Data table

Spec Updates
* Add csv file as a fixture for testing row and column logic
* Remove bucket testing since we are relying on default one from BaseReport

See https://gitlab.login.gov/lg-teams/Team-Data/reporting/-/issues/222

* GL_Data_reporting: remove cred tenure from Fraud metric (#12534)

* changelog: Internal, Reporting, remove credential tenure metric from fraud

* changelog: Internal, Reporting, fix lint issue

* Lg-16684 HowToverify mobile is always required (#12536)

* changelog: Internal, Doc Auth, removing mobile required and updating logic to always have mobile required on how to verify presenter

* Updating context title and resolving PR comments

* Update pinpoint supported countries (#12539)

* Update to reflect drift in footnote numbering

changelog: Internal, Pinpoint, Update script to reflect drift in footnote numbering

Looks like
https://docs.aws.amazon.com/sms-voice/latest/userguide/phone-numbers-sms-by-country.html#sms-support-note-9
became
https://docs.aws.amazon.com/sms-voice/latest/userguide/phone-numbers-sms-by-country.html#sms-support-note-8.

* Remove country that ceases to exist

See https://en.wikipedia.org/wiki/Netherlands_Antilles

* Fix flaky feature test (#12526)

changelog: Internal, Testing, Fix flaky feature test

* LG-16724 Add methods to pii passport and state_id structs (#12533)

Added methods to pii passport and state_id structs for determining if
residential address is needed and constructing pii address from internal
object data.

changelog: Internal, Remote IdV, Add methods to pii passport and state_id structs

* Enable freezing string literals when running tests in CI (#12541)

changelog: Internal, Continuous Integration, Enable freezing string literals when running tests in CI

* Bump libphonenumber-js from 1.12.22 to 1.12.23 (#12538)

Bumps [libphonenumber-js](https://gitlab.com/catamphetamine/libphonenumber-js) from 1.12.22 to 1.12.23.
- [Changelog](https://gitlab.com/catamphetamine/libphonenumber-js/blob/master/CHANGELOG.md)
- [Commits](https://gitlab.com/catamphetamine/libphonenumber-js/compare/v1.12.22...v1.12.23)

---
updated-dependencies:
- dependency-name: libphonenumber-js
  dependency-version: 1.12.23
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* One Account Feature Tests (#12514)

* feature tests

* sign in spec

* changelog: Internal, One Account, Basic Feature tests for flow

* spec

* update sign in spec

* make sure setting is at 100

* make sure to reload ab test

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Moncef Belyamani <moncef.belyamani@gsa.gov>
Co-authored-by: Aaron <aaron.nagucki@gmail.com>
Co-authored-by: Sree Appachi <guruparan18@users.noreply.github.com>
Co-authored-by: Malik Warren <33402370+Mawar2@users.noreply.github.com>
Co-authored-by: Gerardo E. Cruz-Ortiz <59618057+astrogeco@users.noreply.github.com>
Co-authored-by: shilen <shilen.patel@gsa.gov>
Co-authored-by: A Shukla <abir.shukla@gsa.gov>
Co-authored-by: Vraj Mohan <vraj.mohan@gsa.gov>
Co-authored-by: Shane Chesnutt <shane.chesnutt@gsa.gov>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Malick Diarra <malick.diarra@gsa.gov>
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.

4 participants