Me/dpc 4201 log waivers #2246
Me/dpc 4201 log waivers #2246
Conversation
| @@ -165,12 +165,26 @@ def check_code | |||
| status: :bad_request) | |||
| end | |||
|
|
|||
| # rubocop:disable Metrics/AbcSize | |||
There was a problem hiding this comment.
Please refactor instead of disabling
| Rails.logger.info(['Organization has a waiver', | ||
| { actionContext: LoggingConstants::ActionContext::Registration, | ||
| actionType: LoggingConstants::ActionType::OrgHasWaiver, | ||
| providerOrganization: @organization.id }]) |
There was a problem hiding this comment.
Maybe add invitation id?
Organization id should already be added automagically by logger.
dpc-portal/app/jobs/verify_ao_job.rb
Outdated
| { actionContext: LoggingConstants::ActionContext::BatchVerificationCheck, | ||
| actionType: LoggingConstants::ActionType::AoHasWaiver, | ||
| authorizedOfficial: link.user.id, | ||
| providerOrganization: link.provider_organization.id }]) |
There was a problem hiding this comment.
Should use attributes for user and organization found in app/models/current_attributes
There was a problem hiding this comment.
Is that filled in when running this job?
There was a problem hiding this comment.
Nope, but it would be nice to use the same format
|
|
||
| { success: true, ao_role: } | ||
| { success: true, ao_role: response[:ao_role], has_ao_waiver: response[:has_ao_waiver], |
There was a problem hiding this comment.
Should be able to just have response.merge(success: true)
| ao_role = get_authorized_official_role(organization_npi, identifier_type, identifier) | ||
| check_individual_med_sanctions(ao_role['ssn']) | ||
| ao_role | ||
| ao_role_response = get_authorized_official_role(organization_npi, identifier_type, identifier) |
There was a problem hiding this comment.
The logic is fine, but I find the names very confusing.
Maybe something like
role_and_waivers = get_ao_role_and_wavers
individual_sanctions = check...
role_and_waivers.merge(individual_sanctions)
And change the keys so the merge works?
…pc-app into me/dpc-4201-log-waivers
dpc-portal/app/jobs/verify_ao_job.rb
Outdated
| Rails.logger.info(['Authorized official has a waiver', | ||
| { actionContext: LoggingConstants::ActionContext::BatchVerificationCheck, | ||
| actionType: LoggingConstants::ActionType::AoHasWaiver, | ||
| current_user: link.user.id, |
There was a problem hiding this comment.
In current_attibutes, we are storing these as
organization: {
id: org.id,
dpc_api_organization_id: org.dpc_api_organization_id
}
and
current_user: {
id: user.id,
external_id: user.uid,
pac_id: user.pac_id
}
dpc-portal/app/jobs/verify_ao_job.rb
Outdated
|
|
||
| private | ||
|
|
||
| def log_waivers(role_and_waivers, link) |
There was a problem hiding this comment.
log_waivers is essentially the same for both jobs (you can get the org from the link), so it should probably be shared by moving it to app/concerns/verification.rb
dpc-portal/app/jobs/verify_ao_job.rb
Outdated
| @@ -12,6 +12,7 @@ def perform | |||
| @start = Time.now | |||
| service = AoVerificationService.new | |||
| links_to_check.each do |link| | |||
| CurrentAttributes.save_organization_attributes(link.provider_organization, link.user) | |||
There was a problem hiding this comment.
I think you need to save_user_attributes too
There was a problem hiding this comment.
Doh! Total oversight on my part.
🎫 Ticket
https://jira.cms.gov/browse/DPC-4201
🛠 Changes
ℹ️ Context
We want to be able to monitor how many AOs and organizations are using the DPC API with a waiver. This is the first step in doing so.
🧪 Validation
Created RSpec tests.