-
Notifications
You must be signed in to change notification settings - Fork 163
GL_Data_Reporting-398: Add SP verification report #12670
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
b1eb586 to
fba2bfa
Compare
astrogeco
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add this report to job config. Discuss specific report changes in #12685
|
@astrogeco I thought to do that, but as of now if we will add that to job config, once deployed it will start sending an email. Do we want that? Also, sp_verification_report_configs is not yet available in any environment, once this will merged and deployed I can see that in secrets and edit that right |
yes, we want it to send internal emails only |
|
Before the PR deploys we should add the new parameters in the secrets manager |
@astrogeco I have added in dev and int |
|
|
@shilenpatel1 Did you test for mutiple server providers? Please include full screenshot of report showing the overview section where the SP is shown. |
@koseni123 I tested it with multiple service p[roviders also, it worked! I put the screenshots in ticker. Also, there is full screenshot of the report. |
koseni123
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@astrogeco I have made all requested changes, can you please review this PR? |
3a842b2 to
e2d94c1
Compare
changelog: Internal, Reporting, generecize VerificationReport see https://gitlab.login.gov/lg-teams/Team-Data/reporting/-/issues/252
e2d94c1 to
eb63aca
Compare
| args: -> { [Time.zone.yesterday.end_of_day, :both] }, | ||
| }, | ||
|
|
||
| # Send previous week's verification reports to partners |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change the comment to indicate this is currently for testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comment
|
Left some comments in #12742 |
* changelog: Internal, Reporting, Fix comment see https://gitlab.login.gov/lg-teams/Team-Data/reporting/-/issues/252
* changelog: Internal, Reporting, Fix spec file see https://gitlab.login.gov/lg-teams/Team-Data/reporting/-/issues/252
* changelog: Internal, Reporting, Update spec file see https://gitlab.login.gov/lg-teams/Team-Data/reporting/-/issues/252
* changelog: Internal, Reporting, Update spec file see https://gitlab.login.gov/lg-teams/Team-Data/reporting/-/issues/252

🎫 Ticket
GL_Data_Reporting-398
Link to the relevant ticket:
GL_Data_Reporting-398
🛠 Summary of changes
Write a brief description of what you changed.
--> Implement SPVerification report job that expands the capability of sending the report to differnet agency, not just the IRS.
--> Implement SPVerificationReport at lib/reporting/sp_verification_report.rb, that generates the actual report
--> Added sp_verification_report_configs in identity_config and set the default value for that.
📜 Testing Plan
Provide a checklist of steps to confirm the changes.
Testing with giving Single Service providers
Send email to internal:
Send email to both:
Testing with giving multiple Service providers
Here, is the rails console local testing:
Here is the report output in browser, I got 2 reports one for each service provider:
👀 Screenshots
If relevant, include a screenshot or screen capture of the changes.
Testing inside Local Rails Console:
Before:
After: