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

BGSClient rework for static action -> service -> bean definitions #16736

Merged
merged 4 commits into from
May 17, 2024

Conversation

nihil2501
Copy link
Contributor

@nihil2501 nihil2501 commented May 14, 2024

Changes

  • Consolidate XML builder helper into BGSClient.perform_request
    • Simplified the calls to BGS in the POA request search and decide code
    • Now the body can either be passed in as an argument or a block can be provided that leverages this helper (but not both)
    • (This is what initiated the snowball into reconsidering namespaces)
  • Hardcode data namespace alias (based on this BGS-wide audit of beans with >1 namespace) since the alias is arbitrary (but it should be chosen so that it doesn't collide with any standard ones like soap)
  • No longer fetching WSDL in order to extract namespaces
    • To enable this, service action is remodeled as action -> service -> bean in order to get the bean definition's static namespaces (still nee to determine it truly is static)
    • Adapter function added to LocalBGSRefactored adapter to locate the right hardcoded definition (so all existing calls to LocalBGS would also stop fetching the WSDL if we migrate)
      • ClaimantWebService and EBenefitsBnftClaimStatusWebService already added in order to get local_bgs_refactored_spec (which duplicates local_bgs_spec) to pass, which was a decent test of the lookup function
  • BGSClient.healthcheck now accepts a proper service definition due to the above remodel
  • Unrelated change to proxy breakers_service to make that transparent in the migration as well

@nihil2501 nihil2501 added the claimsApi modules/claims_api label May 14, 2024
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 14, 2024 08:23 Inactive
Copy link

github-actions bot commented May 14, 2024

1 Error
🚫 This PR changes 676 LoC (not counting whitespace/newlines).

In order to ensure each PR receives the proper attention it deserves, those exceeding
500 will not be reviewed, nor will they be allowed to merge. Please break this PR up into
smaller ones.

If you have reason to believe that this PR should be granted an exception, please see the
Submitting pull requests for approval - FAQ.

File Summary

Files

  • modules/claims_api/app/clients/claims_api/bgs_client.rb (+23/-77)

  • modules/claims_api/app/clients/claims_api/bgs_client/definitions.rb (+116/-0)

  • modules/claims_api/app/clients/claims_api/bgs_client/request.rb (+43/-71)

  • modules/claims_api/app/clients/claims_api/bgs_client/request/envelope.rb (+42/-13)

  • modules/claims_api/app/clients/claims_api/bgs_client/service_action.rb (+0/-42)

  • modules/claims_api/app/services/claims_api/power_of_attorney_request_service/decide.rb (+7/-16)

  • modules/claims_api/app/services/claims_api/power_of_attorney_request_service/helpers/xml_builder.rb (+0/-32)

  • modules/claims_api/app/services/claims_api/power_of_attorney_request_service/search.rb (+12/-17)

  • modules/claims_api/lib/bgs_service/local_bgs_proxy.rb (+13/-6)

  • modules/claims_api/lib/bgs_service/local_bgs_refactored.rb (+34/-29)

  • modules/claims_api/lib/bgs_service/local_bgs_refactored/find_service_definition.rb (+52/-0)

  • modules/claims_api/spec/lib/claims_api/local_bgs_refactored_spec.rb (+7/-8)

  • modules/claims_api/spec/requests/v2/power_of_attorney_requests/decline/request_spec.rb (+2/-2)

  • modules/claims_api/spec/requests/v2/power_of_attorney_requests/index/request_spec.rb (+1/-1)

  • modules/claims_api/spec/support/bgs_client_spec_helpers.rb (+6/-4)

    Note: We exclude files matching the following when considering PR size:

    *.csv, *.json, *.tsv, *.txt, 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
    

Big PRs are difficult to review, often become stale, and cause delays.

Generated by 🚫 Danger

@nihil2501 nihil2501 force-pushed the dash/oren/API-34441/yet-another-bgs-client-refactor branch 2 times, most recently from a1297c7 to 1245b69 Compare May 14, 2024 08:31
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 14, 2024 08:38 Inactive
@nihil2501 nihil2501 force-pushed the dash/oren/API-34441/yet-another-bgs-client-refactor branch from 1245b69 to d51cec4 Compare May 15, 2024 05:08
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 15, 2024 05:09 Inactive
@nihil2501 nihil2501 force-pushed the dash/oren/API-34441/yet-another-bgs-client-refactor branch from d51cec4 to fddb992 Compare May 15, 2024 05:15
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 15, 2024 05:16 Inactive
@nihil2501 nihil2501 force-pushed the dash/oren/API-34441/yet-another-bgs-client-refactor branch from fddb992 to cc2ff69 Compare May 15, 2024 05:16
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 15, 2024 05:22 Inactive
@nihil2501 nihil2501 force-pushed the dash/oren/API-34441/yet-another-bgs-client-refactor branch from cc2ff69 to bf0d3cb Compare May 15, 2024 05:37
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 15, 2024 05:38 Inactive
@nihil2501 nihil2501 force-pushed the dash/oren/API-34441/yet-another-bgs-client-refactor branch from bf0d3cb to dfce133 Compare May 15, 2024 05:55
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 15, 2024 05:56 Inactive
@nihil2501 nihil2501 force-pushed the dash/oren/API-34441/yet-another-bgs-client-refactor branch 2 times, most recently from dfce133 to b529bcd Compare May 15, 2024 16:47
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 15, 2024 16:53 Inactive
@nihil2501 nihil2501 force-pushed the dash/oren/API-34441/yet-another-bgs-client-refactor branch 2 times, most recently from 008c027 to 22b88d4 Compare May 15, 2024 17:17
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 15, 2024 17:25 Inactive
@nihil2501 nihil2501 force-pushed the dash/oren/API-34441/yet-another-bgs-client-refactor branch from 22b88d4 to 7efeb15 Compare May 15, 2024 17:26
@nihil2501 nihil2501 changed the title wip BGSClient rework based on notion of static action -> service -> bean definition May 15, 2024
@nihil2501 nihil2501 requested a review from FonzMP May 15, 2024 17:29
@nihil2501 nihil2501 force-pushed the dash/oren/API-34441/yet-another-bgs-client-refactor branch 2 times, most recently from 06dbc52 to 0f8bb32 Compare May 15, 2024 17:53
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 15, 2024 18:00 Inactive
@nihil2501 nihil2501 force-pushed the dash/oren/API-34441/action-poa-request.spike branch from 9ccf78b to e2c8f4c Compare May 15, 2024 18:03
Base automatically changed from dash/oren/API-34441/action-poa-request.spike to master May 15, 2024 18:42
@nihil2501 nihil2501 force-pushed the dash/oren/API-34441/yet-another-bgs-client-refactor branch 2 times, most recently from ffd7078 to dd7c60a Compare May 15, 2024 19:07
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 15, 2024 19:18 Inactive
@nihil2501 nihil2501 changed the title BGSClient rework based on notion of static action -> service -> bean definition BGSClient rework based static action -> service -> bean definitions May 15, 2024
@nihil2501 nihil2501 changed the title BGSClient rework based static action -> service -> bean definitions BGSClient rework for static action -> service -> bean definitions May 15, 2024
…` definition

- Consolidate XML builder helper into `BGSClient.perform_request`
  - Now the body can either be passed in as an argument or a block can be provided that leverages this helper (but not both)
  - (This is what initiated the snowball into reconsidering namespaces)
- Hardcode data namespace alias (based on this [BGS-wide audit](https://github.com/department-of-veterans-affairs/bgs-catalog/blob/main/namespaces.xml) of beans with >1 namespace) since the alias is arbitrary
- No longer fetching WSDL in order to extract namespaces
  - To enable this, service action is remodeled as `action -> service -> bean` in order to get the bean definition's static namespaces (if it truly is static)
  - Adapter function added to `LocalBGSRefactored` adapter to locate the right hardcoded definition (so all existing calls to `LocalBGS` would also stop fetching the WSDL if we migrate)
- `BGSClient.healthcheck` now accepts a proper service definition due to the above remodel
- Unrelated change to proxy `breakers_service` to make that transparent in the migration as well
@nihil2501 nihil2501 force-pushed the dash/oren/API-34441/yet-another-bgs-client-refactor branch from dd7c60a to 1e8bbea Compare May 15, 2024 23:40
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 15, 2024 23:41 Inactive
@nihil2501 nihil2501 marked this pull request as ready for review May 16, 2024 16:38
@nihil2501 nihil2501 requested a review from a team as a code owner May 16, 2024 16:38
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 16, 2024 17:05 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to dash/oren/API-34441/yet-another-bgs-client-refactor/main/main May 16, 2024 20:00 Inactive
Copy link
Contributor

@FonzMP FonzMP left a comment

Choose a reason for hiding this comment

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

:shipit:

@nihil2501 nihil2501 merged commit ad5e33a into master May 17, 2024
18 of 19 checks passed
@nihil2501 nihil2501 deleted the dash/oren/API-34441/yet-another-bgs-client-refactor branch May 17, 2024 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
claimsApi modules/claims_api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants