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

Add DisabilityMaxRating client, configuration, and tests with get_ratings update #20483

Merged
merged 17 commits into from
Feb 3, 2025

Conversation

asiisii
Copy link
Contributor

@asiisii asiisii commented Jan 27, 2025

Summary

  • This work is behind a feature toggle (flipper): YES
  • This PR introduces the disability_526_max_cfi_service_switch feature flag to control the switch between the Virtual Regional Office (VRO) client and the new Max Ratings service for fetching disability max ratings.
  • This update includes the implementation of the DisabilityMaxRating::Client along with its configuration to interact with the Max Ratings service.
  • The get_ratings method in the ClaimFastTracking::MaxRatingAnnotator has been updated to use the new client when the feature toggle is enabled and fallback to the VRO client when disabled.
  • Unit tests have been added for both the updated get_ratings method and the new client.
  • I am part of the Employee Experience team (EE).

Related issue(s)

Testing done

  • New code is covered by unit tests.
  • The following tests were added:
    • Flipper ON: Ensures the new client is used to fetch max ratings.
    • Flipper OFF: Ensures the existing VRO client is used.
  • Verified the feature toggle behavior using unit tests for both scenarios.
  • Verified that the DisabilityMaxRating::Client correctly interacts with the Max Ratings service.

What areas of the site does it impact?

  • This change impacts the disability compensation max ratings functionality, specifically the backend service used to fetch max ratings.

Acceptance criteria

  • I added unit tests for both the Flipper ON and OFF scenarios.
  • No errors or warnings in the console.
  • Events are being sent to the appropriate logging solution.
  • No sensitive information (PII/credentials/internal URLs) is captured in logging or hardcoded.
  • Documentation has been updated if necessary.
  • Verified that all authenticated routes work as expected.

@asiisii asiisii changed the title add DisabilityMaxRating client, configuration, and tests with get_rat… Add DisabilityMaxRating client, configuration, and tests with get_ratings update Jan 27, 2025
Copy link

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: spec/lib/disability_max_rating/client_spec.rb

Copy link

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: spec/lib/disability_max_rating/client_spec.rb

@asiisii asiisii marked this pull request as ready for review January 28, 2025 15:41
@asiisii asiisii requested review from a team as code owners January 28, 2025 15:41
@asiisii asiisii requested review from nanotone and dfitchett January 28, 2025 15:42
@va-vfs-bot va-vfs-bot temporarily deployed to ee/am/3966/main/main January 28, 2025 16:03 Inactive
@dfitchett dfitchett requested a review from gabezurita January 28, 2025 16:10
.github/CODEOWNERS Outdated Show resolved Hide resolved
@va-vfs-bot va-vfs-bot temporarily deployed to ee/am/3966/main/main January 28, 2025 20:41 Inactive
@asiisii asiisii requested a review from a team as a code owner January 28, 2025 21:15
Copy link

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: spec/support/vcr_cassettes/disability_max_rating/max_ratings_multiple.yml

Copy link
Contributor

@dfitchett dfitchett left a comment

Choose a reason for hiding this comment

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

One last thing!

lib/disability_max_ratings/client.rb Outdated Show resolved Hide resolved
dfitchett
dfitchett previously approved these changes Jan 30, 2025
Copy link
Contributor

@rmtolmach rmtolmach left a comment

Choose a reason for hiding this comment

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

Looks good! Except, can you update your spec file changes to mock the new flipper?

spec/models/form_profile_spec.rb Outdated Show resolved Hide resolved
spec/support/vcr.rb Show resolved Hide resolved
@rmtolmach rmtolmach requested a review from a team January 30, 2025 18:54
Copy link
Contributor

@gabezurita gabezurita left a comment

Choose a reason for hiding this comment

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

LGTM with some minor nits/questions.

@dfitchett dfitchett requested a review from rmtolmach February 3, 2025 21:01
Copy link
Contributor

@dfitchett dfitchett left a comment

Choose a reason for hiding this comment

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

Great work 🚀

@asiisii asiisii merged commit 062d788 into master Feb 3, 2025
21 of 22 checks passed
@asiisii asiisii deleted the ee/am/3966 branch February 3, 2025 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create vets-api client for disability-max-ratings-api
6 participants