Skip to content

FFS-4796 Create v2 invitations api - #2033

Open
ccvetter wants to merge 14 commits into
mainfrom
ffs-4796-v2-invitation-api
Open

ccvetter wants to merge 14 commits into
mainfrom
ffs-4796-v2-invitation-api

Conversation

@ccvetter

@ccvetter ccvetter commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

FFS-4796

Changes

Added V2 of the invitations api, which allows for the use of individual_id along with doc_id from V1.

Context for reviewers

For testing:
Generate an API token:

bin/run-command --workspace p-2033 app dev '["bin/rails", "users:create_api_token[la_ldh]"]'
export JSON_API_KEY=<token from last step>
curl --request POST \
  --url https://p-2033.navapbc.cloud/api/v2/invitations \
  --header "Authorization: Bearer $JSON_API_KEY" \
  --header "Content-Type: application/json" \
  --data '{
    "type": "income",
    "language": "en",
    "agency_partner_metadata": {
      "individual_id": "ABC1234"
    }
  }'

To test error response:

curl --request POST \
  --url https://p-2033.navapbc.cloud/api/v2/invitations \
  --header "Authorization: Bearer $JSON_API_KEY" \
  --header "Content-Type: application/json" \
  --data '{
    "type": "income",
    "language": "en",
    "agency_partner_metadata": {
      "case_number": "123456"
    }
  }'

which should return response:

{
  "errors": [
      { 
        "field": "agency_partner_metadata.individual_id",
        "message":"individual_id is required"
      }
  ]
}

Acceptance testing

Tag product and design in Slack for acceptance: @emmy-acceptance-testers

  • Acceptance testing in PR Environment
    • This change can be verified in a PR environment. Run this Github Action with the existing PR and most recent git sha.

AI Usage

  • USED_AI: I attest that I have read, understood, and take ownership of all AI-generated code in this PR.
  • NO_AI: I did not use AI.

Optional — for learning, not audited:

  • AI tools used: <e.g. GitHub Copilot, Claude Code — shows what's in use>
  • Prompt artifacts: <link a prompt/chat if worth keeping; otherwise skip>

Risk / Downtime:

Preview environment

Comment thread docs/app/rendered/database-schema.mmd
@ccvetter
ccvetter marked this pull request as ready for review September 3, 2026 17:14
@ccvetter
ccvetter requested a review from a team as a code owner September 3, 2026 17:14
@github-actions
github-actions Bot requested a review from daphnegold September 3, 2026 17:14
@ccvetter
ccvetter requested a review from bencalegari September 3, 2026 17:16
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

test-classifier: AI triage of failing tests

AI Test Classifier — triage of failing tests

Observed — these verdicts are grounded in the actual test run output.

Used the precomputed range origin/main..HEAD (verified: it contains PR #2033's API v2 invitations + individual_id change), so no base re-resolution was needed. Located the suite via .github/workflows/rspec.yml, installed gems from Gemfile.lock (Ruby 3.4.10 from the hosted toolcache; the 3.4.9 pin was temporarily relaxed to bundle and then reverted), started PostgreSQL, loaded the schema, ran npm install + rake assets:precompile as the CI setup action does, and ran 'bundle exec rspec spec --exclude-pattern e2e/**/*_spec.rb': 2551 examples, 0 failures. An earlier run before asset precompilation produced 20 Sprockets AssetNotFound failures; those are purely local provisioning artifacts that do not reproduce once assets are compiled (as CI does), so they are not reported as failures. i18n-tasks health is clean. The one genuine red CI check introduced by the diff is a rubocop Layout/TrailingWhitespace offense on a newly added comment line in app/app/controllers/api/v2/invitations_controller.rb.

Verdict Test Confidence Scope
APPLICATION_BUG RuboCop Layout/TrailingWhitespace (rubocop CI job) high change
Per-test rationale
  • APPLICATION_BUG · other — RuboCop Layout/TrailingWhitespace (rubocop CI job) (app/app/controllers/api/v2/invitations_controller.rb:61)
    The rubocop CI job fails on trailing whitespace in a comment line added by this PR ('...not required for all agencies '), reproduced locally with bundle exec rubocop on the changed files. The defect is in the newly added source line, not in any test, and is auto-correctable.

React 👍 if right / 👎 if wrong, and on a 👎 please reply to this comment with a one-line reason — that reply is the most useful tuning signal we get. Advisory, non-blocking.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

test-classifier: AI triage of failing tests

AI Test Classifier — triage of failing tests

Observed — these verdicts are grounded in the actual test run output.

Used the dispatcher-provided range origin/main..HEAD (verified it matches PR #2033; no re-resolution needed). Bootstrapped the CI environment (postgres 16 + role/db + schema load, npm install, rake assets:precompile — asset precompile is required or every view-rendering spec fails spuriously with Sprockets::Rails::Helper::AssetNotFound; Gemfile ruby pin 3.4.9 was temporarily bumped to the available 3.4.10 and restored). RSpec excluding spec/e2e (matching .github/workflows/rspec.yml) passed 2551/2551; Vitest passed 88/88; i18n-tasks health and the locale-sync script both passed. No test failed. RuboCop on the changed files reported one in-scope offense: a trailing whitespace at app/app/controllers/api/v2/invitations_controller.rb:61 that will turn the rubocop CI job red; it is autocorrectable and the fix belongs in the new application code. spec/e2e was not executed (excluded from the CI rspec job; needs chromedriver and Pinwheel/Argyle sandbox credentials not available here) and no e2e spec code changed in this diff.

Verdict Test Confidence Scope
APPLICATION_BUG rubocop Layout/TrailingWhitespace (rubocop CI workflow) high change
Per-test rationale
  • APPLICATION_BUG · other — rubocop Layout/TrailingWhitespace (rubocop CI workflow) (app/app/controllers/api/v2/invitations_controller.rb:61)
    The new V2 invitations controller introduced by this change leaves a trailing space at the end of the 'Filter out nil values for doc_id and individual_id...' comment, which the repo's rubocop CI job flags and fails on. This is not an rspec/vitest example but a lint check that this diff newly breaks; the fix is in the application code (autocorrectable with rubocop -a), not in any test.

React 👍 if right / 👎 if wrong, and on a 👎 please reply to this comment with a one-line reason — that reply is the most useful tuning signal we get. Advisory, non-blocking.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

test-classifier: AI triage of failing tests

AI Test Classifier — no action required

Observed — these verdicts are grounded in the actual test run output.

Ran the repo's real RSpec suite for PR #2033 (API v2 invitations endpoint + individual_id for LA LDH applicants) by reproducing .github/workflows/rspec.yml: Ruby 3.4.10, bundle install from Gemfile.lock, Postgres 16 container, rake db:schema:load, npm install, rake assets:precompile, then bundle exec rspec spec --exclude-pattern '{e2e/**/*_spec.rb,javascript/**/*}' (CI likewise excludes spec/e2e). Result: 2551 examples, 0 failures. The precomputed AI_REVIEW_DIFF_RANGE (origin/main..HEAD) was verified to contain this PR's change, so no base re-resolution was required. The companion i18n-tasks health check (also gated on app/**) is green. An initial narrower run showed 3 failures in spec/services/cbv_invitation_service_spec.rb ('The asset "application.css" is not present in the asset pipeline') caused solely by my skipping the assets:precompile step that CI performs; after precompiling, all examples in that file pass, so it is a local bootstrap artifact and is not reported as a failure. The browser-driven spec/e2e suite runs in a separate workflow requiring Chrome/chromedriver and was not executed. classifications is empty.

React 👍 if this is right (nothing needed triage) / 👎 if a real failure was missed, and on a 👎 please reply with a one-line reason. Advisory, non-blocking.

@bencalegari
bencalegari removed their request for review September 5, 2026 00:24
@ccvetter
ccvetter requested review from millerti and tdooner and removed request for daphnegold and tdooner September 8, 2026 11:52

@millerti millerti left a comment

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.

Just a few questions

Comment thread app/app/controllers/api/v2/invitations_controller.rb
Comment thread app/app/services/client_agency/la_ldh/report_fields.rb
Comment thread app/config/locales/es.yml Outdated
Comment on lines +114 to +115
doc_id_or_individual_id:
blank: Se requiere doc_id o individual_id

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.

Did you just use Google Translate? We need to actually go through our translator if so, because CMS expects human involvement.

I also notice that Individual ID is not translated yet. And actually, if these are just for errors to be seen by our state partners, we don't even need to translate them; English only is fine. But if it would show up on the site to an applicant in Emmy Income, we need to translate to Spanish. (And eventually if it would show up on the site for a CE user, we will also eventually want Spanish, but we are punting on that for now.)

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.

I used AI for the translation, mainly added because I get an i18 failure if it's not included in both en.yml and es.yml. I can change it to english or an empty string in es.yml if needed and also just change it to individual_id if we are not including doc_id in v2.

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.

Found the i18n-tasks file to ignore translation keys, so going to do that instead.

# Filter out nil values for doc_id and individual_id, since they are not required for all agencies
# and should not be included in the response if they are nil.
filtered = metadata.reject do |key, value|
%w[doc_id individual_id].include?(key) && value.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.

Should we switch to .blank? here? I'm worried that some fields might come back null instead of nil sometimes.

@millerti

Copy link
Copy Markdown
Contributor

Could use some thorough instructions for testing manually. Also, looks like you want testing in a PR env, but it doesn't look like you created one yet (see your PR description for a link to the GHA that creates one).

@emmybot-cms

emmybot-cms commented Sep 11, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions

Copy link
Copy Markdown
Contributor

test-classifier: AI triage of failing tests

AI Test Classifier — triage of failing tests

Observed — these verdicts are grounded in the actual test run output.

Re-resolved the base: the dispatcher's origin/main HEAD range spanned ~47 files of unrelated later-main commits, so I used the merge-base a6ee3a3 (git merge-base origin/main HEAD), which matches PR #2033 'FFS-4796 Create v2 invitations api' (13 files). Ran the repo's real suite per .github/workflows/rspec.yml (Ruby 3.4.10, Postgres 16 container, db:schema:load, assets:precompile): 2551 examples, 4 failures, all in the new Api::V2::InvitationsController spec; plus the repo's i18n-tasks health check fails with 1 missing translation. Three initial submits_controller_spec failures were my own bootstrap gap (assets not precompiled) — they reproduce on unmodified origin/main and pass after assets:precompile, so they are excluded. Verdicts: V2 hardcodes a strict metadata permit list that drops agency-required name fields (confirmed by experiment) and drops absent fields from the metadata echo (APPLICATION_BUG); one new example asserts a translation key the diff never adds (TEST_BUG); the i18n ignore path has a field/fields typo so the missing es string is not suppressed (APPLICATION_BUG).

Verdict Test Confidence Scope
APPLICATION_BUG Api::V2::InvitationsController#create creates an invitation with an associated cbv_applicant high change
APPLICATION_BUG Api::V2::InvitationsController#create creates an invitation using the client_agency_id in the access_token high change
APPLICATION_BUG Api::V2::InvitationsController#create when inviting a user in LA LDH returns the expected agency_partner_metadata medium change
TEST_BUG Api::V2::InvitationsController#create when inviting a user in LA LDH returns 422 when both doc_id and individual_id are nil for income high change
APPLICATION_BUG i18n-tasks health: Missing translations (1) — es cbv.applicant_informations.la_ldh.fields.individual_id.blank high change
Per-test rationale
  • APPLICATION_BUG · behavioral-drift — Api::V2::InvitationsController#create creates an invitation with an associated cbv_applicant (app/spec/controllers/api/v2/invitations_controller_spec.rb:44)
    V2's metadata_params hardcodes permit(:doc_id, :individual_id, :case_number, :date_of_birth), stripping first_name/middle_name/last_name which the sandbox agency config marks required, so creation 422s and no records are created. Confirmed by experiment: adding those keys to the permit list makes the example pass; V1 derives the list from the agency config instead of hardcoding it.
  • APPLICATION_BUG · behavioral-drift — Api::V2::InvitationsController#create creates an invitation using the client_agency_id in the access_token (app/spec/controllers/api/v2/invitations_controller_spec.rb:54)
    Same root cause as the sibling creation example: the hardcoded permit list drops the sandbox agency's required name attributes, so no CbvFlowInvitation/CbvApplicant is created. Verified fixed by permitting first_name/middle_name/last_name (change reverted).
  • APPLICATION_BUG · behavioral-drift — Api::V2::InvitationsController#create when inviting a user in LA LDH returns the expected agency_partner_metadata (app/spec/controllers/api/v2/invitations_controller_spec.rb:87)
    V2's allowed_metadata_params slices only the submitted params, so absent agency fields (case_number, date_of_birth) are omitted from the echoed metadata, whereas V1 builds the full hash over all agency-valid attributes and nils the rest. The new assertion and the V1 precedent both say all fields should be echoed; medium confidence because a human could decide V2 intentionally echoes only supplied fields.
  • TEST_BUG · behavioral-drift — Api::V2::InvitationsController#create when inviting a user in LA LDH returns 422 when both doc_id and individual_id are nil for income (app/spec/controllers/api/v2/invitations_controller_spec.rb:97)
    The app correctly returns 422 with field agency_partner_metadata.individual_id and message 'individual_id is required', but the test asserts on I18n key ...la_ldh.fields.doc_id_or_individual_id.blank, which the diff never adds, so it resolves to 'Translation missing'. Stale assertion from an earlier doc_id-or-individual_id design; update the test to the individual_id.blank key.
  • APPLICATION_BUG · other — i18n-tasks health: Missing translations (1) — es cbv.applicant_informations.la_ldh.fields.individual_id.blank (app/config/i18n-tasks.yml:157)
    The change adds the new key to en.yml only and tries to suppress the Spanish gap with ignore_missing 'cbv.applicant_informations.la_ldh.field.individual_id.blank' (singular 'field'), while the locale and controller use 'fields', so the ignore never matches and bundle exec i18n-tasks health fails. Fix the ignore path or add the es translation; the check itself is behaving correctly.

React 👍 if right / 👎 if wrong, and on a 👎 please reply to this comment with a one-line reason — that reply is the most useful tuning signal we get. Advisory, non-blocking.

@github-actions

Copy link
Copy Markdown
Contributor

test-classifier: AI triage of failing tests

AI Test Classifier — triage of failing tests

Observed — these verdicts are grounded in the actual test run output.

Re-resolved the base: AI_REVIEW_DIFF_RANGE ('origin/main HEAD') was misleading because origin/main is 23 commits ahead of this branch's merge-base, producing a 61-file two-dot diff full of unrelated reverse deltas; I used the merge-base three-dot range a6ee3a3...HEAD (14 files: new Api::V2::InvitationsController, individual_id column, LA LDH config/report field, v2 route, new v2 spec). Ran the repo's own CI recipe (Ruby 3.4.10 from the toolcache, Postgres 16, npm install, rake assets:precompile, rake db:schema:load, parallel_test -n 4 excluding spec/e2e). Result: 2551 examples, 2 failures, both in the spec added by this change. An initial run without assets:precompile yielded 595 Sprockets AssetNotFound failures; those are a missing-setup-step artifact and all pass once assets are compiled, so they are not classified. Both real failures are a 422 from Api::V2::InvitationsController#metadata_params hard-coding a four-key permit list that drops first_name/middle_name/last_name, so applicant validation fails for every agency whose valid attributes include names.

Verdict Test Confidence Scope
APPLICATION_BUG Api::V2::InvitationsController#create creates an invitation with an associated cbv_applicant high change
APPLICATION_BUG Api::V2::InvitationsController#create creates an invitation using the client_agency_id in the access_token high change
Per-test rationale
  • APPLICATION_BUG · behavioral-drift — Api::V2::InvitationsController#create creates an invitation with an associated cbv_applicant (app/spec/controllers/api/v2/invitations_controller_spec.rb:48)
    Api::V2::InvitationsController#metadata_params permits only :doc_id, :individual_id, :case_number, :date_of_birth, stripping first_name/last_name before allowed_metadata_params slices against CbvApplicant.valid_attributes_for_agency, so a sandbox create returns 422 ({"field":"cbv_applicant.first_name","message":"Enter the client's first name."}) and no records are created. The test asserts the same behavior V1 already provides (all 27 V1 examples pass), so the new controller is wrong, not the test.
  • APPLICATION_BUG · behavioral-drift — Api::V2::InvitationsController#create creates an invitation using the client_agency_id in the access_token (app/spec/controllers/api/v2/invitations_controller_spec.rb:58)
    Same root cause as the sibling example: the hard-coded permit list in metadata_params discards the name attributes, the CbvApplicant presence validations fail, and the controller returns 422 so CbvFlowInvitation.count changes by 0 before client_agency_id can be asserted. Deterministic across three runs with different seeds, so neither flaky nor environmental.

React 👍 if right / 👎 if wrong, and on a 👎 please reply to this comment with a one-line reason — that reply is the most useful tuning signal we get. Advisory, non-blocking.

@github-actions

Copy link
Copy Markdown
Contributor

test-classifier: AI triage of failing tests

AI Test Classifier — no action required

Observed — these verdicts are grounded in the actual test run output.

Re-resolved the diff base: the precomputed AI_REVIEW_DIFF_RANGE (origin/main HEAD) was stale/wrong — origin/main had advanced ~13 commits past this PR's base, surfacing 61 unrelated files. Used the merge-base a6ee3a3..HEAD instead, which exactly matches the 14 files reported by gh pr view 2033 (FFS-4796 Create v2 invitations api). Bootstrapped per the repo's own CI actions (Ruby 3.4.10 from the toolcache since the pinned 3.4.9 was unavailable, bundle install from Gemfile.lock, PostgreSQL 16 + rake db:schema:load, npm install + rake assets:precompile) and ran both CI test jobs: rspec spec --exclude-pattern e2e/** gave 2551 examples / 0 failures, and rspec ./spec/e2e with E2E_RUN_TESTS=1 E2E_AXE_MODE=full gave 23 examples / 0 failures. A targeted run of spec/controllers/api (64 examples, including all 7 new Api::V2::InvitationsController examples and all 27 V1 examples) also passed. An earlier run showed 593 failures, all Sprockets AssetNotFound for @uswds sprite.svg, caused solely by my not having run npm install/assets:precompile yet; all cleared after completing the documented bootstrap, so they are environment artifacts of my setup rather than failing tests and are not classified.

React 👍 if this is right (nothing needed triage) / 👎 if a real failure was missed, and on a 👎 please reply with a one-line reason. Advisory, non-blocking.

@github-actions

Copy link
Copy Markdown
Contributor

test-classifier: AI triage of failing tests

AI Test Classifier — no action required

Observed — these verdicts are grounded in the actual test run output.

Re-resolved the base: the dispatcher's AI_REVIEW_DIFF_RANGE (origin/main HEAD) was stale and surfaced 61 unrelated files, so I used the merge-base instead (git merge-base origin/main HEAD = a6ee3a3), which yields PR #2033's actual 14-file change adding a V2 invitations API with a new cbv_applicants.individual_id column. Bootstrapped from the repo's own CI definitions (Ruby 3.4.10, bundle install from Gemfile.lock, Postgres 16, db:schema:load, npm ci, assets:precompile) and ran the full non-e2e RSpec suite: 2551 examples, 0 failures. Vitest (88 tests), i18n-tasks health, and RuboCop (663 files) also pass. An initial run produced ~180 Sprockets::Rails::Helper::AssetNotFound failures in component and view-rendering specs; that was my own missing assets:precompile step (which .github/actions/setup-project runs before RSpec), not a property of the change, and all of those specs pass once assets are compiled, so they are excluded rather than reported as environment findings. spec/e2e/ was not run because the repo's own rspec.yml excludes it and the change under test touches no e2e spec. Nothing failed; classifications are empty.

React 👍 if this is right (nothing needed triage) / 👎 if a real failure was missed, and on a 👎 please reply with a one-line reason. Advisory, non-blocking.

@ccvetter
ccvetter requested a review from millerti September 14, 2026 12:40
Comment thread app/config/locales/en.yml
prompt: Your date of birth
super_one_html: Date of birth<sup>1</sup>
individual_id:
blank: individual_id is required

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.

One last thing: are we sure the error shouldn't say "individual or doc id is required"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants