Skip to content

Api 55108 v2 val use claim date#27550

Merged
siddharthalamsal merged 14 commits intomasterfrom
api-55108-v2-val-use-claimDate
Apr 15, 2026
Merged

Api 55108 v2 val use claim date#27550
siddharthalamsal merged 14 commits intomasterfrom
api-55108-v2-val-use-claimDate

Conversation

@siddharthalamsal
Copy link
Copy Markdown
Contributor

@siddharthalamsal siddharthalamsal commented Apr 2, 2026

Summary

V2 validation was largely updated in #27361, this PR is just clean up

  • Removes duplicate claim_date function which was using time.date.now.
  • Now using claim_date from disability_compensation_validations_helper.rb
  • adds check for claimDate not in the future if provided.

Related issue(s)

API-55108
image

Testing done

  • New code is covered by unit tests
  • Describe what the old behavior was prior to the change
    • used time.date.now
  • testing:
    • the change is minimal, all tests are passing, should have caught anything
    • v2 526 submissions working as normal with new validation

What areas of the site does it impact?

modules/claims_api/app/controllers/concerns/claims_api/v2/alt_revised_disability_compensation_validation.rb
modules/claims_api/app/controllers/concerns/claims_api/v2/disability_compensation_validation.rb
modules/claims_api/spec/controllers/concerns/claims_api/v2/alt_revised_disability_compensation_validation_spec.rb
modules/claims_api/spec/lib/claims_api/v2/disability_compensation_validation_spec.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

@siddharthalamsal siddharthalamsal marked this pull request as ready for review April 3, 2026 15:44
@siddharthalamsal siddharthalamsal requested a review from a team as a code owner April 3, 2026 15:44
Copilot AI review requested due to automatic review settings April 3, 2026 15:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Cleans up Claims API v2 526 validation logic by removing the locally-defined claim_date method and reusing the shared implementation from ClaimsApi::DisabilityCompensationValidationsHelper, aligning v2 behavior with other validations.

Changes:

  • Include ClaimsApi::DisabilityCompensationValidationsHelper in the v2 disability compensation validation concern and remove the duplicate claim_date method.
  • Update v2 validation specs to expect Date.current (matching the helper fallback) instead of Time.zone.today.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
modules/claims_api/app/controllers/concerns/claims_api/v2/disability_compensation_validation.rb Removes duplicate claim_date and includes shared validations helper.
modules/claims_api/spec/lib/claims_api/v2/disability_compensation_validation_spec.rb Updates expectations to match shared helper fallback date behavior.

@siddharthalamsal siddharthalamsal marked this pull request as draft April 3, 2026 17:23
return if form_attributes.empty?

alt_rev_validate_claim_process_type_bdd if bdd_claim?
# ensure 'claimDate', if provided, is not in the future
Copy link
Copy Markdown
Contributor Author

@siddharthalamsal siddharthalamsal Apr 6, 2026

Choose a reason for hiding this comment

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

noticed claimDate business logic was not being checked before we used it for validations. This probably could have waited until the claimDate was actually added to the schema, so open to revert but I don't think it hurts anything.


private

def claim_date
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was a repeat of the helper file claim_date method

@siddharthalamsal siddharthalamsal added the claimsApi modules/claims_api label Apr 6, 2026
@siddharthalamsal siddharthalamsal marked this pull request as ready for review April 6, 2026 16:02
Copy link
Copy Markdown
Contributor

@jeremyhunton-va jeremyhunton-va left a comment

Choose a reason for hiding this comment

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

Code looks good to me! I'll pull this down after lunch and do a quick smoke test before I sign off.

jeremyhunton-va
jeremyhunton-va previously approved these changes Apr 7, 2026
Copy link
Copy Markdown
Contributor

@jeremyhunton-va jeremyhunton-va left a comment

Choose a reason for hiding this comment

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

LGTM!

@siddharthalamsal siddharthalamsal requested a review from a team April 8, 2026 13:37
stiehlrod
stiehlrod previously approved these changes Apr 8, 2026
# Conflicts:
#	modules/claims_api/app/controllers/concerns/claims_api/v2/disability_compensation_validation.rb
Copy link
Copy Markdown
Contributor

@jeremyhunton-va jeremyhunton-va left a comment

Choose a reason for hiding this comment

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

LGTM!

it 'collects an error with the correct message' do
future_date = (Date.current + 1.day).iso8601
test_526_validation_instance.form_attributes['claimDate'] = future_date
test_526_validation_instance.instance_variable_set(:@claim_date, nil)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The test manually resets @claim_date with instance_variable_set(:@claim_date, nil). This is fine for testing, but note that claim_date in the shared helper uses memoization (@claim_date = ...). If claim_date was previously called in the test setup with a non-future date, and @claim_date isn't reset, the validation would pass incorrectly. The test does reset it, so it works — but this is worth noting as a pattern that couples tests to internal implementation details.

@siddharthalamsal siddharthalamsal merged commit e1707fb into master Apr 15, 2026
42 of 44 checks passed
@siddharthalamsal siddharthalamsal deleted the api-55108-v2-val-use-claimDate branch April 15, 2026 16: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.

4 participants