Api 41369 remove v1 base method bgs ext

[rockwellwindsor-va]

Summary

• Adds feature flag
• Adds definition entry
• Adds service file
• Adds feature flipper
• updates v1 526 validations to use flipper when enabled
• Updates related spec tests

Related issue(s)

API-41369

Testing done

• New code is covered by unit tests

Testing Notes

Testing the Service

• rails c
• require 'bgs_service/standard_data_service'
• 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 <--- This will should a HUGE list of existing contentions

Submission Testing

• V1 526 submission / classificationCode on a disability or disabilities w/ the claims_api_526_validations_v1_local_bgs Flipper enabled

What areas of the site does it impact?

modified:   config/features.yml
modified:   modules/claims_api/app/controllers/concerns/claims_api/disability_compensation_validations.rb
modified:   modules/claims_api/spec/requests/v1/forms/526_spec.rb
new file:   modules/claims_api/lib/bgs_service/standard_data_service.rb
modified:   modules/claims_api/app/clients/claims_api/bgs_client/definitions.rb

Acceptance criteria

• I fixed|updated|added unit tests and integration tests for each feature (if applicable).
• No error nor warning in the console.
• Events are being sent to the appropriate logging solution
• Documentation has been updated (link to documentation)
• No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
• Feature/bug has a monitor built into Datadog (if applicable)
• If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
• I added a screenshot of the developed feature

Requested Feedback

(OPTIONAL)What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?

[github-actions]

1 Warning

⚠️

This PR changes 248 LoC (not counting whitespace/newlines). In order to ensure each PR receives the proper attention it deserves, we recommend not exceeding 200. Expect some delays getting reviews. File Summary Files config/features.yml (+4/-0) modules/claims_api/app/clients/claims_api/bgs_client/definitions.rb (+10/-0) modules/claims_api/app/controllers/concerns/claims_api/disability_compensation_validations.rb (+10/-1) modules/claims_api/lib/bgs_service/standard_data_service.rb (+14/-0) modules/claims_api/spec/requests/v1/forms/526_spec.rb (+121/-88) Note: We exclude files matching the following when considering PR size: *.csv, *.json, *.tsv, *.txt, *.md, Gemfile.lock, app/swagger, modules/mobile/docs, spec/fixtures/, spec/support/vcr_cassettes/, modules/mobile/spec/support/vcr_cassettes/, db/seeds, modules/vaos/app/docs, modules/meb_api/app/docs, modules/appeals_api/app/swagger/, *.bru, *.pdf Big PRs are difficult to review, often become stale, and cause delays.

Generated by 🚫 Danger

[mchristiansonVA]

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

[rockwellwindsor-va]

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/`

[rockwellwindsor-va]

@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

[rockwellwindsor-va]

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

[rockwellwindsor-va]

in line with slack discussion adjusted the bean_name c3ee1a3

Good callout @mchristiansonVA to help keep these consistent

[mchristiansonVA]

Tested locally via 526 v1 POST and observed successful call to StandardDataService with flipper enabled when a classificationCode is added to a disability; validation functioned as expected. Looks good, just a minor question on the service bean_name.

[rjohnson2011]

Please update Flipper specs in accordance to VA best practices.

[FonzMP]

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

[rockwellwindsor-va]

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.

[rockwellwindsor-va]

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

[mchristiansonVA]

Tested locally again after rename of bean_name string; working as expected. Local runs of spec tests all run successfully as well. LGTM!

Flipper issue resolved.