Skip to content
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

Api 41369 remove v1 base method bgs ext #19620

Merged
merged 16 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
5918d16
API-41369-remove-v1-base-method-bgs-ext
rockwellwindsor-va Nov 25, 2024
e6147da
API-41369-remove-v1-base-method-bgs-ext
rockwellwindsor-va Nov 25, 2024
58c5d40
API-41369-remove-v1-base-method-bgs-ext
rockwellwindsor-va Nov 25, 2024
655f0c9
API-41369-remove-v1-base-method-bgs-ext
rockwellwindsor-va Nov 26, 2024
af89f46
Merge branch 'master' into API-41369-remove-v1-base-method-bgs-ext
rockwellwindsor-va Nov 26, 2024
e2f0d4f
Merge branch 'master' into API-41369-remove-v1-base-method-bgs-ext
rockwellwindsor-va Dec 2, 2024
5210653
Merge branch 'master' into API-41369-remove-v1-base-method-bgs-ext
rockwellwindsor-va Dec 2, 2024
6575479
Removes chained in method brought over from older bgs ext gem method
rockwellwindsor-va Dec 2, 2024
3f047a3
Merge branch 'master' into API-41369-remove-v1-base-method-bgs-ext
rockwellwindsor-va Dec 2, 2024
6032b6f
Merge branch 'master' into API-41369-remove-v1-base-method-bgs-ext
rockwellwindsor-va Dec 2, 2024
4fa545f
Merge branch 'master' into API-41369-remove-v1-base-method-bgs-ext
rockwellwindsor-va Dec 2, 2024
c7c429f
Merge branch 'master' into API-41369-remove-v1-base-method-bgs-ext
rockwellwindsor-va Dec 4, 2024
849b3be
Adjust how flipper is handled in rspec tests
rockwellwindsor-va Dec 4, 2024
6b41d96
Merge branch 'master' into API-41369-remove-v1-base-method-bgs-ext
rockwellwindsor-va Dec 5, 2024
dffabf6
Merge branch 'master' into API-41369-remove-v1-base-method-bgs-ext
rockwellwindsor-va Dec 6, 2024
c3ee1a3
Updates bean_name value in StadardDataService in an effort to maintai…
rockwellwindsor-va Dec 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions config/features.yml
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,10 @@ features:
actor_type: user
description: When enabled, sends poa forms to BD via the refactored logic
enable_in_development: true
claims_api_526_validations_v1_local_bgs:
actor_type: user
description: Enables the method calls in the v1 526 validations use local_bgs
enable_in_development: true
claims_api_use_person_web_service:
actor_type: user
description: Uses person web service rather than local bgs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,20 @@ module FindPersonBySSN
end
end

##
# StandardDataService
#
module StandardDataService
DEFINITION =
Bean.new(
path: 'StandardDataService',
namespaces: Namespaces.new(
target: 'http://services.mapd.benefits.vba.va.gov/',
data: nil
)
)
end

##
# StandardDataWebServiceBean
#
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require 'common/exceptions'
require 'brd/brd'
require 'bgs_service/standard_data_service'

module ClaimsApi
module DisabilityCompensationValidations # rubocop:disable Metrics/ModuleLength
Expand Down Expand Up @@ -424,7 +425,15 @@ def validate_form_526_disability_classification_code!
def bgs_classification_ids
return @bgs_classification_ids if @bgs_classification_ids.present?

contention_classification_type_codes = bgs_service.data.get_contention_classification_type_code_list
contention_classification_type_codes = if Flipper.enabled?(:claims_api_526_validations_v1_local_bgs)
contention_service = ClaimsApi::StandardDataService.new(
external_uid: Settings.bgs.external_uid,
external_key: Settings.bgs.external_key
)
contention_service.get_contention_classification_type_code_list
else
bgs_service.data.get_contention_classification_type_code_list
end
@bgs_classification_ids = contention_classification_type_codes.pluck(:clsfcn_id)
end

Expand Down
18 changes: 18 additions & 0 deletions modules/claims_api/lib/bgs_service/standard_data_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# frozen_string_literal: true

module ClaimsApi
class StandardDataService < ClaimsApi::LocalBGS
def bean_name
'StandardDataService'
Copy link
Contributor

Choose a reason for hiding this comment

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

Tyler had a similar question on the CorporateUpdateWebService PR: Should this be StandardDataServiceBean/StandardDataService or similar, for consistency with our other services?

Copy link
Contributor Author

@rockwellwindsor-va rockwellwindsor-va Dec 6, 2024

Choose a reason for hiding this comment

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

Ah yep, I can see this now, going to start a discussion on this since looking at the catalog I'm not sure all of these will be exactly the same set up/structure (note the difference in the image between my service here and another one that appears to have that top level at the ....ServiceBean/`

Screenshot 2024-12-06 at 9 56 42 AM

Copy link
Contributor Author

@rockwellwindsor-va rockwellwindsor-va Dec 6, 2024

Choose a reason for hiding this comment

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

@mchristiansonVA I feel like the answer to your question here is 'no' but worth making sure so I'll kick off a thread to discuss

Copy link
Contributor Author

@rockwellwindsor-va rockwellwindsor-va Dec 6, 2024

Choose a reason for hiding this comment

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

Oh, I think what I was overlooking is that they may both be the same, which is a little confusing to me but maybe StandardDataService/StandardDataService is what I need here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in line with slack discussion adjusted the bean_name c3ee1a3

Good callout @mchristiansonVA to help keep these consistent

end

def get_contention_classification_type_code_list
body = Nokogiri::XML::DocumentFragment.parse <<~EOXML
<getContentionClassificationTypeCodeList/>
EOXML
Comment on lines +10 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

should we redis cache this? I'm not sure if these change frequent enough for us to need to make this call per request?

Copy link
Contributor Author

@rockwellwindsor-va rockwellwindsor-va Dec 4, 2024

Choose a reason for hiding this comment

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

Ya that is a good question, it is a lot of data that gets returned. I do not believe we have been doing that previously, I also would want to verify that the data we get there does not need to be specifically 'up-to-date' when it goes back into the sidekiq job. If it does not then we could do that but if it does then I do not think we could but I'm not sure.

I think an investigation ticket may be warranted to determine that, unless there is a more obvious way to know that I am unaware of.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Revisiting this, I got this call confused with one in the other PR I have, I think we probably could benefit from caching this in Redis


response = make_request(endpoint: bean_name, action: 'getContentionClassificationTypeCodeList', body:)
response[:return]
end
end
end
212 changes: 124 additions & 88 deletions modules/claims_api/spec/requests/v1/forms/526_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require 'rails_helper'
require_relative '../../../rails_helper'
require 'bgs_service/standard_data_service'

RSpec.describe 'ClaimsApi::V1::Forms::526', type: :request do
let(:headers) do
Expand Down Expand Up @@ -2274,34 +2275,45 @@ def obj.class
context "when 'disabilites.secondaryDisabilities.classificationCode' is invalid" do
let(:classification_type_codes) { [{ clsfcn_id: '1111' }] }

before do
expect_any_instance_of(BGS::StandardDataService)
.to receive(:get_contention_classification_type_code_list).and_return(classification_type_codes)
end
[true, false].each do |flipped|
context "when feature flag is #{flipped}" do
before do
if flipped
Flipper.enable(:claims_api_526_validations_v1_local_bgs)
expect_any_instance_of(ClaimsApi::StandardDataService)
.to receive(:get_contention_classification_type_code_list).and_return(classification_type_codes)
else
Flipper.disable(:claims_api_526_validations_v1_local_bgs)
expect_any_instance_of(BGS::StandardDataService)
.to receive(:get_contention_classification_type_code_list).and_return(classification_type_codes)
end
end

it 'raises an exception' do
mock_acg(scopes) do |auth_header|
VCR.use_cassette('claims_api/brd/countries') do
json_data = JSON.parse data
params = json_data
disabilities = [
{
disabilityActionType: 'NONE',
name: 'PTSD (post traumatic stress disorder)',
diagnosticCode: 9999,
secondaryDisabilities: [
it 'raises an exception' do
mock_acg(scopes) do |auth_header|
VCR.use_cassette('claims_api/brd/countries') do
json_data = JSON.parse data
params = json_data
disabilities = [
{
disabilityActionType: 'SECONDARY',
name: 'PTSD',
serviceRelevance: 'Caused by a service-connected disability.',
classificationCode: '2222'
disabilityActionType: 'NONE',
name: 'PTSD (post traumatic stress disorder)',
diagnosticCode: 9999,
secondaryDisabilities: [
{
disabilityActionType: 'SECONDARY',
name: 'PTSD',
serviceRelevance: 'Caused by a service-connected disability.',
classificationCode: '2222'
}
]
}
]
}
]
params['data']['attributes']['disabilities'] = disabilities
post path, params: params.to_json, headers: headers.merge(auth_header)
expect(response).to have_http_status(:bad_request)
params['data']['attributes']['disabilities'] = disabilities
post path, params: params.to_json, headers: headers.merge(auth_header)
expect(response).to have_http_status(:bad_request)
end
end
end
end
end
Expand All @@ -2310,34 +2322,45 @@ def obj.class
context "when 'disabilites.secondaryDisabilities.classificationCode' does not match name" do
let(:classification_type_codes) { [{ clsfcn_id: '1111' }] }

before do
expect_any_instance_of(BGS::StandardDataService)
.to receive(:get_contention_classification_type_code_list).and_return(classification_type_codes)
end
[true, false].each do |flipped|
context "when feature flag is #{flipped}" do
before do
if flipped
Flipper.enable(:claims_api_526_validations_v1_local_bgs)
ericboehs marked this conversation as resolved.
Show resolved Hide resolved
expect_any_instance_of(ClaimsApi::StandardDataService)
.to receive(:get_contention_classification_type_code_list).and_return(classification_type_codes)
else
Flipper.disable(:claims_api_526_validations_v1_local_bgs)
expect_any_instance_of(BGS::StandardDataService)
.to receive(:get_contention_classification_type_code_list).and_return(classification_type_codes)
end
end

it 'raises an exception' do
mock_acg(scopes) do |auth_header|
VCR.use_cassette('claims_api/brd/countries') do
json_data = JSON.parse data
params = json_data
disabilities = [
{
disabilityActionType: 'NONE',
name: 'PTSD (post traumatic stress disorder)',
diagnosticCode: 9999,
secondaryDisabilities: [
it 'raises an exception' do
mock_acg(scopes) do |auth_header|
VCR.use_cassette('claims_api/brd/countries') do
json_data = JSON.parse data
params = json_data
disabilities = [
{
disabilityActionType: 'SECONDARY',
name: 'PTSD',
serviceRelevance: 'Caused by a service-connected disability.',
classificationCode: '1111'
disabilityActionType: 'NONE',
name: 'PTSD (post traumatic stress disorder)',
diagnosticCode: 9999,
secondaryDisabilities: [
{
disabilityActionType: 'SECONDARY',
name: 'PTSD',
serviceRelevance: 'Caused by a service-connected disability.',
classificationCode: '1111'
}
]
}
]
}
]
params['data']['attributes']['disabilities'] = disabilities
post path, params: params.to_json, headers: headers.merge(auth_header)
expect(response).to have_http_status(:bad_request)
params['data']['attributes']['disabilities'] = disabilities
post path, params: params.to_json, headers: headers.merge(auth_header)
expect(response).to have_http_status(:bad_request)
end
end
end
end
end
Expand Down Expand Up @@ -2458,52 +2481,65 @@ def obj.class

describe "'disabilites' validations" do
describe "'disabilities.classificationCode' validations" do
let(:classification_type_codes) { [{ clsfcn_id: '1111' }] }
[true, false].each do |flipped|
context "when feature flag is #{flipped}" do
before do
if flipped
Flipper.enable(:claims_api_526_validations_v1_local_bgs)
expect_any_instance_of(ClaimsApi::StandardDataService)
.to receive(:get_contention_classification_type_code_list).and_return(classification_type_codes)
else
Flipper.disable(:claims_api_526_validations_v1_local_bgs)
expect_any_instance_of(BGS::StandardDataService)
.to receive(:get_contention_classification_type_code_list).and_return(classification_type_codes)
end
end

before do
expect_any_instance_of(BGS::StandardDataService)
.to receive(:get_contention_classification_type_code_list).and_return(classification_type_codes)
end
let(:classification_type_codes) { [{ clsfcn_id: '1111' }] }

context "when 'disabilites.classificationCode' is valid" do
it 'returns a successful response' do
mock_acg(scopes) do |auth_header|
VCR.use_cassette('claims_api/bgs/claims/claims') do
VCR.use_cassette('claims_api/brd/countries') do
json_data = JSON.parse data
params = json_data
disabilities = [
{
disabilityActionType: 'NEW',
name: 'PTSD (post traumatic stress disorder)',
classificationCode: '1111'
}
]
params['data']['attributes']['disabilities'] = disabilities
post path, params: params.to_json, headers: headers.merge(auth_header)
expect(response).to have_http_status(:ok)
context "when 'disabilites.classificationCode' is valid" do
it 'returns a successful response' do
mock_acg(scopes) do |auth_header|
VCR.use_cassette('claims_api/bgs/claims/claims') do
VCR.use_cassette('claims_api/brd/countries') do
json_data = JSON.parse data
params = json_data
disabilities = [
{
disabilityActionType: 'NEW',
name: 'PTSD (post traumatic stress disorder)',
classificationCode: '1111'
}
]
params['data']['attributes']['disabilities'] = disabilities
post path, params: params.to_json, headers: headers.merge(auth_header)
expect(response).to have_http_status(:ok)
end
end
end
end
end
end
end

context "when 'disabilites.classificationCode' is invalid" do
it 'responds with a bad request' do
mock_acg(scopes) do |auth_header|
VCR.use_cassette('claims_api/brd/countries') do
json_data = JSON.parse data
params = json_data
disabilities = [
{
disabilityActionType: 'NEW',
name: 'PTSD (post traumatic stress disorder)',
classificationCode: '2222'
}
]
params['data']['attributes']['disabilities'] = disabilities
post path, params: params.to_json, headers: headers.merge(auth_header)
expect(response).to have_http_status(:bad_request)
context "when 'disabilites.classificationCode' is invalid" do
it 'responds with a bad request' do
mock_acg(scopes) do |auth_header|
VCR.use_cassette('claims_api/brd/countries') do
VCR.use_cassette('claims_api/bgs/stadard_service_data') do
json_data = JSON.parse data
params = json_data
disabilities = [
{
disabilityActionType: 'NEW',
name: 'PTSD (post traumatic stress disorder)',
classificationCode: '2222'
}
]
params['data']['attributes']['disabilities'] = disabilities
post path, params: params.to_json, headers: headers.merge(auth_header)
expect(response).to have_http_status(:bad_request)
end
end
end
end
end
end
Expand Down
Loading