Skip to content
This repository is currently being migrated. It's locked while the migration is in progress.
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ def alt_rev_validate_form_526_submission_values
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.

alt_rev_validate_form_526_claim_date
# ensure mailing address country is valid
alt_rev_validate_form_526_identification
# ensure disabilities are valid
Expand All @@ -42,6 +44,15 @@ def alt_rev_validate_form_526_submission_values

private

def alt_rev_validate_form_526_claim_date
return if claim_date <= Date.current

collect_error_messages(
source: '/claimDate',
detail: 'claimDate must not be in the future.'
)
end

def alt_rev_validate_form_526_change_of_address
return if form_attributes['changeOfAddress'].blank?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

require 'claims_api/v2/disability_compensation_shared_service_module'
require 'claims_api/lighthouse_military_address_validator'
require 'claims_api/disability_compensation_validations_helper'

module ClaimsApi
module V2
module DisabilityCompensationValidation # rubocop:disable Metrics/ModuleLength
include DisabilityCompensationSharedServiceModule
include ClaimsApi::DisabilityCompensationValidationsHelper
Comment thread
jeremyhunton-va marked this conversation as resolved.
include LighthouseMilitaryAddressValidator

DATE_FORMATS = {
Expand All @@ -25,6 +27,8 @@ def validate_form_526_submission_values(target_veteran)
return if form_attributes.empty?

validate_claim_process_type_bdd if bdd_claim?
# ensure 'claimDate', if provided, is not in the future
validate_form_526_claim_date
# ensure 'claimantCertification' is true
validate_form_526_claimant_certification
# ensure mailing address country is valid
Expand All @@ -51,12 +55,13 @@ def validate_form_526_submission_values(target_veteran)

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

@claim_date = if date_is_valid?(form_attributes['claimDate'], 'claimDate', true)
Date.parse(form_attributes['claimDate'])
else
Time.zone.today
end
def validate_form_526_claim_date
return if claim_date <= Date.current

collect_error_messages(
source: '/claimDate',
detail: 'claimDate must not be in the future.'
)
end

def validate_form_526_change_of_address
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1487,5 +1487,20 @@ def validate_field(field_path, expected_detail, expected_source)
expect(errors).to be_empty
end
end

context 'when claimDate is in the future' do
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.


test_526_validation_instance.send(:alt_rev_validate_form_526_claim_date)
errors = current_error_array

expect(errors).not_to be_empty
expect(errors[0][:detail]).to eq('claimDate must not be in the future.')
expect(errors[0][:source]).to eq('/claimDate')
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ def validate_field(field_path, expected_detail, expected_source)
# Ensure claimDate is not set
expect(subject.form_attributes).not_to have_key('claimDate')

expect(subject.send(:claim_date)).to eq(Time.zone.today)
expect(subject.send(:claim_date)).to eq(Date.current)

errors = test_526_validation_instance.send(:error_collection)

Expand All @@ -591,11 +591,26 @@ def validate_field(field_path, expected_detail, expected_source)
subject.form_attributes['claimDate'] = 'invalid-date'
subject.instance_variable_set(:@claim_date, nil)

expect(subject.send(:claim_date)).to eq(Time.zone.today)
expect(subject.send(:claim_date)).to eq(Date.current)
errors = test_526_validation_instance.send(:error_collection)

expect(errors).to be_empty
end
end

context 'when claimDate is in the future' do
it 'collects an error with the correct message' do
future_date = (Date.current + 1.day).iso8601
subject.form_attributes['claimDate'] = future_date
subject.instance_variable_set(:@claim_date, nil)

test_526_validation_instance.send(:validate_form_526_claim_date)
errors = test_526_validation_instance.send(:error_collection)

expect(errors).not_to be_empty
expect(errors[0][:detail]).to eq('claimDate must not be in the future.')
expect(errors[0][:source]).to eq('/claimDate')
end
end
end
end
Loading