Reports: remove partner_strings configuration parameter#12971
Reports: remove partner_strings configuration parameter#12971colton-lapp merged 12 commits intomainfrom
Conversation
astrogeco
force-pushed
the
improve-sp-cred-config
branch
from
67acf8e to
b8e573a
Compare
| if to_emails.empty? && bcc_emails.empty? | ||
| Rails.logger.warn "No email addresses received - #{@partner_strings.first} Credential Report NOT SENT" | ||
| Rails.logger.warn "No email addresses received - #{partner_strings.first} Credential Report NOT SENT" |
There was a problem hiding this comment.
use @agency_abbreviation here and in other places where we used to have partner_strings
|
@colton-lapp can you update the description of the PR and fill out the missing fields (they'll be visible once you click "edit")? |
|
Also add the gitlab ticket URL to one of the commits |
colton-lapp
changed the title
Replaces its use with a dynamic lookup based on issuers and using the 'agency_abbreviation' for report and file naming
…om sp_cred_metrics_report to for simplification
colton-lapp
force-pushed
the
improve-sp-cred-config
branch
from
1e81ac9 to
cfbb238
Compare
|
|
||
| @report_name = "#{@partner_strings.first.downcase}_monthly_cred_metrics" | ||
| @agency_abbreviation = report_config['agency_abbreviation'] | ||
| @report_name = "#{@agency_abbreviation.downcase}_monthly_cred_metrics" |
There was a problem hiding this comment.
Looks like @agency_abbreviation is expecting a list, should it be @agency_abbreviation.first.downcase
There was a problem hiding this comment.
Actually, I'm not sure what's expected here. In spec/jobs/reports/sp_cred_metrics_report_orchestrator_spec.rb it's a list but in spec/mailers/previews/report_mailer_preview.rb it's a string value
There was a problem hiding this comment.
the orchestrator processes a nested JSON with multiple configs, each of those configs should have a single agency_abbreviation string, then the orchestrator spins up a single job per config
MrNagoo
left a comment
MrNagoo
left a comment
There was a problem hiding this comment.
LGTM aside from typical style comments
| config = { | ||
| 'issuers' => ['Issuer_2', 'Issuer_3', 'Issuer_4'], | ||
| 'partner_strings' => ['Partner_1'], | ||
| # 'partner_strings' => ['Partner_1'], |
4 participants
The
sp_cred_metrics_report.rbpreviously utilized a config parameterpartner_stringsfor logging and emailing purposes (purely cosmetic). This PR replaces all cosmetic uses of thepartner_stringswith theagency_abbreviationconfig parameter instead. This simplifies the credential metrics report and makes it more in line with othersp_*reports.🎫 Ticket
Link to the relevant ticket:
https://gitlab.login.gov/lg-teams/Team-Data/reporting/-/issues/533
🛠 Summary of changes
partner_stringsinsp_cred_metrics_report.rbsp_cred_metrics_report_spec.rb,sp_cred_metrics_report_orchestrator_spec.rb)spec/mailers/previews/report_mailer_preview.rbas well to use new config for cred_metrics preview