Skip to content

Ask VA | Updating regex for inquiry ID to be more future-proof#27805

Merged
jerekshoe merged 4 commits intomasterfrom
2324-check-inquiry-id-shape-1
Apr 17, 2026
Merged

Ask VA | Updating regex for inquiry ID to be more future-proof#27805
jerekshoe merged 4 commits intomasterfrom
2324-check-inquiry-id-shape-1

Conversation

@jerekshoe
Copy link
Copy Markdown
Contributor

@jerekshoe jerekshoe commented Apr 16, 2026

Summary

After talking to the CRM team, we found out that the oldest inquiry IDs end in a 5 digit sequence and the newest ones end in a 7 digit sequence. It's unlikely to happen soon, but at some point the end segment of the inquiry ID may end up being 8 digits long. In order to be as flexible as possible, this PR updates the end segment to accept any sequence of digits between 5 and 10 digits long (so 5, 6, 7 and 8 will work, while leaving us with extra leeway for future-proofing)

  • This work is behind a feature toggle (flipper): NO
  • (Summarize the changes that have been made to the platform)
  • (If bug, how to reproduce)
  • (What is the solution, why is this the solution?)
  • (Which team do you work for, does your team own the maintenance of this component?)
  • (If introducing a flipper, what is the success criteria being targeted?)

Related issue(s)

Testing done

Specs have been updated to remove a now-valid case of 8 digits for the end segment

  • New code is covered by unit tests
  • Describe what the old behavior was prior to the change
  • Describe the steps required to verify your changes are working as expected. Exclusively stating 'Specs run' is NOT acceptable as appropriate testing
  • If this work is behind a flipper:
    • Tests need to be written for both the flipper on and flipper off scenarios. Docs.
    • What is the testing plan for rolling out the feature?

Screenshots

Note: Optional

What areas of the site does it impact?

(Describe what parts of the site are impacted andifcode touched other areas)

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?

@jerekshoe jerekshoe self-assigned this Apr 16, 2026
@jerekshoe jerekshoe added the ask-va AskVA team label Apr 16, 2026
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

Updates Ask VA inquiry ID validation to support newer/longer trailing numeric sequences and adjusts request specs accordingly, so /inquiries/:id/status can accept evolving CRM inquiry number formats.

Changes:

  • Broadened inquiry ID regex to allow a trailing digit segment of 5+ digits.
  • Updated the controller’s validation comment/error text to reflect the new expected format.
  • Updated request spec “invalid id format” examples to remove now-valid cases.

Reviewed changes

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

File Description
modules/ask_va_api/app/controllers/ask_va_api/v0/inquiries_controller.rb Expands INQUIRY_ID_FORMAT and updates validation messaging for inquiry ID format.
modules/ask_va_api/spec/requests/ask_va_api/v0/inquiries_spec.rb Adjusts invalid inquiry ID cases to align with the expanded format.
Comments suppressed due to low confidence (1)

modules/ask_va_api/spec/requests/ask_va_api/v0/inquiries_spec.rb:499

  • This spec update removes previously-invalid 5- and 8-digit trailing segments from the “bad id” list, but it doesn’t add a positive assertion that 5- and 8-digit inquiry IDs are accepted. Consider adding explicit examples that GET /status succeeds for a 5-digit and an 8-digit trailing segment to lock in the intended new behavior.
    context 'when the id format is invalid' do
      %w[invalid 12345 A-1234567-123456 A-12345678-1234 A-12345678-12L4567 A12345678123456].each do |bad_id|
        it "returns bad_request for '#{bad_id}'" do
          get "/ask_va_api/v0/inquiries/#{bad_id}/status"
          expect(response).to have_http_status(:bad_request)
          expect(JSON.parse(response.body)['error']).to include('Invalid inquiry ID format')
        end

Comment thread modules/ask_va_api/app/controllers/ask_va_api/v0/inquiries_controller.rb Outdated
Comment thread modules/ask_va_api/app/controllers/ask_va_api/v0/inquiries_controller.rb Outdated
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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

modules/ask_va_api/spec/requests/ask_va_api/v0/inquiries_spec.rb:500

  • This invalid-ID spec now treats an 11-digit trailing segment as invalid, which implicitly matches the new 5–10 digit controller regex. If the intended support is only 5–8 digits per the PR description, add coverage for 9+ digits being rejected (and keep the invalid examples aligned with the controller’s actual range).
    context 'when the id format is invalid' do
      %w[invalid 12345 A-1234567-123456 A-12345678-1234 A-12345678-12345678901 A-12345678-12L4567
         A12345678123456].each do |bad_id|
        it "returns bad_request for '#{bad_id}'" do
          get "/ask_va_api/v0/inquiries/#{bad_id}/status"
          expect(response).to have_http_status(:bad_request)
          expect(JSON.parse(response.body)['error']).to include('Invalid inquiry ID format')
        end

@jerekshoe jerekshoe marked this pull request as ready for review April 17, 2026 15:21
@jerekshoe jerekshoe requested review from a team as code owners April 17, 2026 15:21
Copy link
Copy Markdown
Contributor

@mfloyd-iat mfloyd-iat left a comment

Choose a reason for hiding this comment

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

LGTM - this was the adjustment he mentioned yesterday to expand the digits to allow for production traffic numbers as they differ from staging traffic numbers

Copy link
Copy Markdown
Contributor

@acastillo-ironarch acastillo-ironarch left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work getting the details hashed out

@jerekshoe jerekshoe enabled auto-merge (squash) April 17, 2026 15:43
@jerekshoe jerekshoe merged commit fe87cee into master Apr 17, 2026
48 of 50 checks passed
@jerekshoe jerekshoe deleted the 2324-check-inquiry-id-shape-1 branch April 17, 2026 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ask-va AskVA team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants