Skip to content

decouple ssavlp error handling#295

Closed
charlienparker wants to merge 15 commits intotrunkfrom
decouple-ssavlp-error-handling
Closed

decouple ssavlp error handling#295
charlienparker wants to merge 15 commits intotrunkfrom
decouple-ssavlp-error-handling

Conversation

@charlienparker
Copy link
Copy Markdown
Contributor

@charlienparker charlienparker commented Nov 18, 2025 •
edited
Loading

Ticket: https://app.clickup.com/t/868g4eejr

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved verification failure handling when both SSA and DHS verifications fail, now returning a consolidated error message instead of individual failures.

  • Refactor

    • Optimized verification request and response processing flows for improved reliability and centralized error tracking of applicant determinations.

@charlienparker charlienparker marked this pull request as ready for review November 18, 2025 12:54
coderabbitai[bot]
coderabbitai bot reviewed Nov 18, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb (1)

18-41: Fix arity mismatch in VLP applicant determination call

The method track_applicant_determination is defined with zero parameters in ResponseProcessorUtils (line 42) but is called with an argument @determined_applicants in VLP's process_verification_response.rb (line 21). This will raise an ArgumentError at runtime. The SSA variant correctly calls it with no arguments (line 19). The method accesses @determined_applicants as an instance variable instead.

Fix by removing the argument:

-            track_applicant_determination(@determined_applicants)
+            track_applicant_determination

Regarding parse_response (lines 38–40): While the method uses response_transaction without a nil-check, the preceding transaction_failed? call handles potential nil transactions safely via safe navigation. If defensive robustness against edge cases where both transactions are nil is desired, consider adding response_transaction&.json_payload, but this is not strictly required for the current PR.

🧹 Nitpick comments (10)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1)

39-51: VLP initial request generation logic and failure handling look solid

The FAA vs individual-market dispatch and the handle_failure wiring are consistent with the rest of the flow and should integrate cleanly with the call pipeline.

You might eventually want to reuse the APPLICATION_TYPE_FAA constant from ProcessApplicantRequests (or a shared constant) instead of hard-coding 'faa' here to avoid drift.

app/operations/fdsh/ssa_vlp/rj3/service_constants.rb (1)

10-25: Centralized key derivation matches SSA/VLP usage; consider guarding unknown services

The base_transaction_key / request_key / response_key helpers nicely centralize key construction and line up with "SSA" and "VLP" service_name implementations.

You might add an else branch in base_transaction_key that raises or logs for unsupported service_name values to avoid silently generating odd keys (e.g., :"_request") if a new service is wired up incorrectly.

app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb (1)

8-9: Sharing ServiceConstants here correctly drives request/response keys off service_name

Including ServiceConstants in RequestProcessorUtils is the right way to centralize key derivation; create_request_* / create_response_* now consistently use request_key / response_key tied to each service’s service_name.

Just ensure all classes including this module define a valid service_name (currently SSA/VLP do); if you add new services later, wiring a corresponding service_name + base_transaction_key entry will keep things consistent.

app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb (1)

16-38: SSA response processing now aligns with shared response utils; consider guarding missing response transactions

The SSA ProcessVerificationResponse#call correctly delegates to the shared validate_params, uses track_applicant_determination, and short-circuits via transaction_failed?, then transforms the JSON payload from response_transaction, which keeps it consistent with the new utilities.

For additional robustness (mirroring the VLP parse_response concern), you might explicitly handle a missing response_transaction or nil json_payload in transform_response, returning a Failure instead of risking a NoMethodError if the processor ever runs before a response transaction is persisted.

spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (1)

54-260: Specs thoroughly cover the new orchestration and partial-success behavior

The expanded #call and #request_verifications specs do a good job exercising:

  • All combinations of SSA/VLP eligibility and success/failure.
  • The new rule that only “both failures” produce an overall Failure.
  • That the public call path still returns "Successfully invoked verifications" when at least one verification succeeds or both are ineligible.

You can drop the unused mock_ssa_applicant_request / mock_vlp_applicant_request lets in the #request_verifications describe, since that block stubs request_ssa_verification/request_dhs_verification directly.

spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb (2)

101-166: Good coverage of VLP transformation failure paths; consider simplifying DB cleanup and call count

The shared examples and parametrized contexts exercise both FAA/IVL mappings and distinguish between transformation failure vs exception, which is exactly what generate_initial_verification_request needs.

Two minor points you may want to tighten up:

  • DatabaseCleaner.clean is being called both in the shared_examples before and again in the enclosing "when application type is …" context; combined with dbclean: :after_each, this is redundant noise and can be dropped from at least one place without changing behavior.
  • Each example in 'handles transformation failure' invokes subject.call(params) separately, which creates two VLP request transactions per example and increases coupling to transmittable state. You could DRY this by using a let(:result) or a before that calls subject.call(params) once per context and reusing that result in expectations.

Both are non-blocking and mostly about keeping specs lean and predictable.

168-195: Success-path scenario looks correct; DB cleaner here is likely redundant

The “successfully processes the VLP applicant request” example nicely asserts the full set of transmission/transaction state transitions and the success message, matching the ApplicantRequest#call contract.

Given the top-level dbclean: :after_each, the explicit DatabaseCleaner.clean in this context before is probably unnecessary and can be removed unless you’ve seen specific leakage issues here.

app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (2)

10-17: Determined-applicants wiring and error propagation look consistent with the new contract

The refactor to initialize @determined_applicants = {} in validate_params, thread it through process_ssa_verification_response / process_vlp_verification_response, and return [determined_application_payload, @determined_applicants] from call lines up with the new ProcessVerificationResponse specs (including missing-hash validation and tracking logic).

A couple of small, non-blocking thoughts:

  • If you ever want to support callers providing an existing determinations hash, validate_params could accept an optional params[:determined_applicants] || {} and still satisfy current callers.
  • Both SSA and DHS update methods share the same loop + short‑circuit pattern; if this class grows further, extracting a tiny helper that takes the processor class could reduce duplication, but it’s fine as-is.

Functionally, this looks solid and matches the behavior asserted in process_applicant_responses_spec.

Also applies to: 27-28, 31-55, 57-73

80-90: Evidence filtering behavior matches expectations; just be aware it mutates the application hash

filter_processed_evidences correctly keeps only evidences with non-empty request_results or verification_histories, and the specs exercise both normal and exception paths.

Note that this method mutates the passed application_hash in-place (by reassigning eligibility[:evidences]), while save_application_response has already persisted the pre-filter version. That seems intentional (filtered payload to EA; full payload in persistence), but if you ever need the original in memory after this, you may want to dup before filtering.

spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb (1)

583-650: transaction_failed? matrix is excellent; consider normalizing transaction key types in test data

The #transaction_failed? examples nicely encode the intended precedence:

  • When both request and response exist, response state drives the answer (only response :failed → true).
  • When only one exists, a :failed state on that transaction returns true.
  • Non-failed combinations return false.

One small nit: in this block, the test-created transactions use string keys ('ssa_verification_request' / 'ssa_verification_response'), whereas earlier in the file you create transactions with symbol keys (:ssa_verification_response). For consistency with production and to avoid any subtle key-type mismatches in the model layer, it would be slightly safer to use symbols here as well.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5b7b413 and d07500a.

📒 Files selected for processing (13)
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (2 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (4 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb (1 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (1 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/service_constants.rb (1 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (0 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb (2 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb (2 hunks)
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (2 hunks)
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb (1 hunks)
  • spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb (3 hunks)
  • spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb (1 hunks)
💤 Files with no reviewable changes (1)
  • app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings 
Learnt from: vkghub
Repo: ideacrew/fdsh_gateway PR: 268
File: app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb:119-123
Timestamp: 2025-07-15T16:28:40.423Z
Learning: In `app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb`, the application hash structure guarantees that top level keys like `:person_name` and `:demographics` will always be present, so direct hash access in `name_and_dob_match?` method is safe and doesn't require defensive navigation using `dig`.
📚 Learning: 2025-07-15T16:28:40.423Z 
Learnt from: vkghub
Repo: ideacrew/fdsh_gateway PR: 268
File: app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb:119-123
Timestamp: 2025-07-15T16:28:40.423Z
Learning: In `app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb`, the application hash structure guarantees that top level keys like `:person_name` and `:demographics` will always be present, so direct hash access in `name_and_dob_match?` method is safe and doesn't require defensive navigation using `dig`.

Applied to files:

  • app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb
  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb
  • spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb
  • app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb
  • app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb
  • app/operations/fdsh/ssa_vlp/rj3/service_constants.rb
  • app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb
📚 Learning: 2025-07-16T13:34:53.451Z 
Learnt from: ymhari
Repo: ideacrew/fdsh_gateway PR: 268
File: app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb:129-138
Timestamp: 2025-07-16T13:34:53.451Z
Learning: In `app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb`, the evidence appending approach in `update_evidences_with_failure` and `update_evidences_with_results` methods is intentionally designed to accumulate evidence entries rather than replace them, as confirmed by ymhari.

Applied to files:

  • app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb
  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb
  • spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb
  • app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb
  • app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb
📚 Learning: 2025-07-14T19:06:48.270Z 
Learnt from: ymhari
Repo: ideacrew/fdsh_gateway PR: 268
File: app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb:99-119
Timestamp: 2025-07-14T19:06:48.270Z
Learning: In `app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb`, the evidence appending approach in `update_evidences_with_failure` and `update_evidences_with_results` methods is intentionally designed to accumulate evidence entries rather than replace them, as confirmed by ymhari.

Applied to files:

  • app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb
  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb
  • spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb
  • app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb
📚 Learning: 2024-10-08T21:02:47.798Z 
Learnt from: TristanB17
Repo: ideacrew/fdsh_gateway PR: 191
File: spec/operations/fdsh/vlp/rx142/initial_verification/process_initial_verification_response_spec.rb:0-0
Timestamp: 2024-10-08T21:02:47.798Z
Learning: TristanB17 has clarified that an 'invalid response' in the context of `Fdsh::Vlp::Rx142::InitialVerification::ProcessInitialVerificationResponse` refers to a valid error response from CMS, which should still yield an `Attestation` object.

Applied to files:

  • app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb
  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb
  • app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb
  • app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb
📚 Learning: 2024-06-10T19:13:32.163Z 
Learnt from: saikumar9
Repo: ideacrew/fdsh_gateway PR: 219
File: app/models/alive_status/family.rb:45-78
Timestamp: 2024-06-10T19:13:32.163Z
Learning: The class `::AcaEntities::Operations::CreateFamily` is well-tested within its own gem, and its usage in the methods `request_family_entity` and `response_family_entity` of the `AliveStatus::Family` model is sufficiently covered in `spec/models/alive_status/family_spec.rb`.

Applied to files:

  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb
  • spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb
  • spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb
📚 Learning: 2024-10-08T21:02:47.798Z 
Learnt from: kristinmerbach
Repo: ideacrew/fdsh_gateway PR: 178
File: app/operations/fdsh/ridp/rj139/handle_primary_determination_request.rb:27-31
Timestamp: 2024-10-08T21:02:47.798Z
Learning: The user has clarified that there are contracts in place to ensure the payload structure for the `validate_params` method in the `HandlePrimaryDeterminationRequest` class, making additional validation unnecessary within this method.

Applied to files:

  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb
  • app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb
📚 Learning: 2024-11-18T17:13:17.915Z 
Learnt from: saipraveen18
Repo: ideacrew/fdsh_gateway PR: 262
File: app/operations/fdsh/h41/request/build_1095a_payload.rb:85-89
Timestamp: 2024-11-18T17:13:17.915Z
Learning: In the `fetch_insurance_provider_title` method within `app/operations/fdsh/h41/request/build_1095a_payload.rb`, it's acceptable to assume that the `modify_carrier_legal_names` feature is always enabled and that the mapping will always be present, so defensive checks for nil mappings are not necessary.

Applied to files:

  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb
🧬 Code graph analysis (11)
app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb (2)
app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (3)
  • validate_params (12-24)
  • track_applicant_determination (42-49)
  • response_transaction (30-32)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (1)
  • validate_params (21-29)
spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb (4)
app/models/transmittable/transaction.rb (1)
  • transmission (73-76)
app/models/transmittable/transmission.rb (1)
  • transactions (127-129)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (2)
  • call (15-31)
  • include (10-146)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)
  • eligibility (65-67)
app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb (2)
app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (3)
  • validate_params (12-24)
  • track_applicant_determination (42-49)
  • response_transaction (30-32)
app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb (1)
  • call (18-30)
spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb (4)
app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (2)
  • response_transaction (30-32)
  • request_transaction (26-28)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (2)
  • call (10-17)
  • include (7-101)
app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb (2)
  • call (16-28)
  • include (10-82)
app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb (2)
  • call (18-30)
  • include (10-87)
spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (5)
app/models/transaction.rb (1)
  • applicants (54-56)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1)
  • call (15-31)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)
  • call (18-24)
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (1)
  • call (15-28)
app/operations/fdsh/ssa_vlp/rj3/handle_verification_request.rb (1)
  • call (13-22)
app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb (2)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1)
  • include (10-146)
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (1)
  • include (10-93)
spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb (1)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1)
  • call (15-31)
app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (5)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (2)
  • include (7-101)
  • validate_params (21-29)
app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb (2)
  • include (10-82)
  • service_name (32-34)
app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb (2)
  • include (10-87)
  • service_name (34-36)
app/models/transmittable/transmission.rb (1)
  • transactions (127-129)
app/operations/fdsh/ssa_vlp/rj3/service_constants.rb (2)
  • request_key (19-21)
  • response_key (23-25)
app/operations/fdsh/ssa_vlp/rj3/service_constants.rb (2)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1)
  • service_name (35-37)
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (1)
  • service_name (32-34)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (4)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)
  • call (18-24)
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (1)
  • call (15-28)
app/operations/fdsh/ssa_vlp/rj3/vlp/request_verification.rb (1)
  • call (18-22)
app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb (1)
  • handle_failure (73-77)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (3)
app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb (1)
  • call (16-28)
app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb (1)
  • call (18-30)
app/operations/fdsh/ssa_vlp/rj3/handle_verification_request.rb (1)
  • call (13-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build-and-upload-image
  • GitHub Check: rspec
🔇 Additional comments (8)
app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (1)

8-49: Response utils now correctly handle request-level failures and track determined applicants

The updated validate_params, transaction accessors, transaction_failed?, and track_applicant_determination logic cleanly support:

  • Falling back to the request transaction’s state when no response exists.
  • Recording verification failure via record_verification_failure without breaking the monadic flow.
  • Accumulating @determined_applicants by service (:ssa / :vlp) only when the response transaction actually succeeded.

This matches the PR’s intent and stays consistent with the existing evidence update patterns.

app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)

21-51: New request_verifications orchestration correctly implements partial-success semantics

request_verifications runs SSA and DHS requests independently and fails only when both return Failure, while treating “not eligible” and single-service success as overall success. Wiring it into call via yield achieves the “don’t block on one service outage” behavior described in the PR.

spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb (3)

125-205: Strong coverage of core orchestration and determined_applicants; mocks are wired correctly

The #call examples cover:

  • Param validation failures with clear messages.
  • Happy path where both SSA and VLP processors succeed (including checking argument lists and double persistence).
  • Failure propagation when SSA or VLP returns Failure.
  • Exception paths where handle_error is exercised via AddError and UpdateProcessStatus.

The expectations on ProcessVerificationResponse#call arguments (including determined_applicants: {}) align with the implementation and the downstream specs. Using doubles here is a good choice to keep this spec focused on orchestration rather than SSA/VLP details.

269-345: filter_processed_evidences behavior and error handling are well-specified

The tests around #filter_processed_evidences:

  • Precisely assert that evidences lacking both request_results and verification_histories are dropped.
  • Verify that, when find_individual_market_eligibility raises, the class logs via AddError/UpdateProcessStatus and returns a failure with an informative message substring.

This gives good confidence that the evidence trimming and error reporting contracts won’t regress.

348-409: Helper behavior specs (find_individual_market_eligibility, handle_error) look correct and focused

The dedicated examples for #find_individual_market_eligibility and #handle_error are nicely minimal and fully aligned with the implementation:

  • find_individual_market_eligibility is tested for both hit and miss on the 'individual_market_eligibility' key.
  • handle_error is asserted to call both AddError and UpdateProcessStatus with the correct payload and to return a Failure of the message.

No issues here; this is helpful fine-grained coverage for the private helpers.

spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb (3)

80-90: New determined_applicants parameter and validation are consistent with downstream usage

Introducing let(:determined_applicants) and passing it into params matches how ProcessApplicantResponses now calls this operation. The additional invalid-params example for missing :determined_applicants ensures the contract is explicit and will fail fast if callers forget to provide the hash.

The combination of this spec and the ProcessApplicantResponses specs gives good coverage for the new parameter contract.

Also applies to: 116-121

292-487: Failed transaction handling semantics are clearly asserted and align with partial-failure goals

The "with failed transactions" contexts verify that:

  • A failed response transaction short-circuits normal processing, marks evidence current_state: :failed, and appends a verification_histories entry, while still returning Success.
  • A failed request transaction with no response transaction triggers the same behavior.
  • When both exist, response failure is prioritized over request success.
  • When neither transaction has a failed state, or when process status is missing, or when there are no transactions, the response is processed normally.

This matches the desired robustness: treat hub transport failures as evidence failures rather than raising, while allowing normal processing when transactions are in non-failed states. Nicely done.

489-581: Applicant determination tracking tests thoroughly cover edge cases

The applicant determination tracking block exercises:

  • Successful response -> applicant HBX ID added to determined_applicants[:ssa] only.
  • Failed or non-success states -> no additions.
  • Applicant with nil hbx_id -> no additions.
  • Nil determined_applicants hash -> no error.
  • Missing :ssa key -> key is created and populated.

This is a solid matrix ensuring track_applicant_determination is resilient to nil/missing keys and only records determinations for successful responses, which is exactly what the orchestrator needs when deciding partial success.

@charlienparker charlienparker added the cr-120+ PRs associated with the CR-120+, QHP Application, or Eligibility 3.0 label Nov 18, 2025
coderabbitai[bot]
coderabbitai bot reviewed Nov 18, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb (1)

18-30: Fix track_applicant_determination arity and harden parse_response against nil transactions

  • track_applicant_determination (from ResponseProcessorUtils) is defined without parameters and operates on @determined_applicants. Calling it as track_applicant_determination(@determined_applicants) will raise ArgumentError at runtime.
  • parse_response assumes response_transaction is present. When no matching transaction exists, response_transaction will be nil and this method will raise instead of returning a clean Failure.

Consider:

-          def call(params)
-            yield validate_params(params)
-
-            track_applicant_determination(@determined_applicants)
+          def call(params)
+            yield validate_params(params)
+
+            track_applicant_determination
             return handle_failed_transaction if transaction_failed?
@@
-          def parse_response
-            return Failure("Response JSON payload is blank") unless response_transaction.json_payload
-            Success(response_transaction.json_payload)
-          end
+          def parse_response
+            payload = response_transaction&.json_payload
+            return Failure("Response JSON payload is blank") unless payload
+
+            Success(payload)
+          end

Also applies to: 38-41

app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb (1)

16-27: Flow aligns with shared utils; add nil‑safety around response_transaction

The updated call flow correctly:

  • Delegates param validation to ResponseProcessorUtils and relies on its ivar setup.
  • Invokes track_applicant_determination prior to checking transaction_failed?, so successful SSA responses can be tracked per applicant.
  • Uses transaction_failed?/handle_failed_transaction to short‑circuit purely transaction‑level failures.

As in the VLP path, transform_response assumes response_transaction is present:

AcaEntities::Fdsh::Ssa::H3::Operations::SsaVerificationJsonResponse.new.call(response_transaction.json_payload)

If no matching transaction exists for the applicant, this will raise instead of letting the operation fail with a Failure. Consider mirroring the VLP fix here, e.g.:

-          def transform_response
-            AcaEntities::Fdsh::Ssa::H3::Operations::SsaVerificationJsonResponse.new.call(response_transaction.json_payload)
-          end
+          def transform_response
+            payload = response_transaction&.json_payload
+            return Failure('Response JSON payload is blank') unless payload
+
+            AcaEntities::Fdsh::Ssa::H3::Operations::SsaVerificationJsonResponse.new.call(payload)
+          end

Also applies to: 36-38

🧹 Nitpick comments (7)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1)

39-56: Centralized transformation failure handling looks good; minor DRY opportunity

The new generate_initial_verification_request correctly:

  • Selects the appropriate H92 transformation operation per application_type.
  • Pipes both operation failures and unexpected exceptions through handle_failure, ensuring the job/transmission/transaction end in a consistent :failed state.

If you want to tighten it up, you could DRY the duplicated handle_failure call by extracting the common context and message prefix, but behavior-wise this is solid.

app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb (1)

3-9: Guard against unknown service_name in ServiceConstants usage

Including ServiceConstants here is appropriate and simplifies key generation, given that callers define service_name as "SSA"/"VLP".

To avoid surprising behavior if another service starts including RequestProcessorUtils without defining a supported service_name, consider making base_transaction_key explicit for unknown values (e.g., raising, logging, or at least defaulting with a clear error) instead of returning nil and producing :"_request"/:"_response" keys.

app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)

18-24: Partial‑success semantics look correct; consider surfacing combined failure details

request_verifications correctly:

  • Invokes both SSA and DHS applicant requests independently.
  • Only returns Failure when both results are failures, giving you the intended “at least one service succeeded” behavior.

Right now, the combined failure case surfaces a generic "Both SSA and DHS verifications failed" message. Since the underlying ApplicantRequest calls are already recording detailed errors in transmittable state, that’s acceptable; if you ever need richer feedback from this operation itself, you could fold the individual failure messages into the combined error string.

Also applies to: 44-51

app/operations/fdsh/ssa_vlp/rj3/service_constants.rb (1)

7-25: Service key helpers are fine; add an explicit fallback for unknown services

The ServiceConstants helpers nicely centralize SSA/VLP request/response keys and remove duplication in the callers.

To make this safer for future reuse, you might want an explicit else branch in base_transaction_key (e.g., raise ArgumentError or log) when service_name is not "SSA" or "VLP". Right now an unknown service_name would return nil, yielding :"_request"/:"_response" keys and making misconfiguration harder to spot.

spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb (1)

101-167: Transformation failure coverage is solid; reconsider in‑example DatabaseCleaner.clean usage

The new shared examples and parameterized contexts do a good job of:

  • Exercising both FAA/IVL transformation paths.
  • Verifying that transformation failures and exceptions are surfaced via Failure(...) and drive both request transmission and transaction into a :failed state.

The repeated DatabaseCleaner.clean calls inside the before blocks (both in the shared examples and in the application_type context, plus in the success context) are likely redundant with dbclean: :after_each and can end up truncating data created by let! definitions in the same group. That’s brittle if you later introduce stricter DB constraints.

You can simplify by relying on the dbclean metadata and dropping the explicit DatabaseCleaner.clean calls here unless there’s a very specific isolation concern.

Also applies to: 170-171

spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb (1)

116-123: Core #call behavior and determined_applicants expectations look solid, but no assertions on populated determinations

The happy-path and failure-path tests for #call line up with the implementation: param validation messages, early failure on SSA/VLP monad failures, and success returning [determined_application_payload, determined_applicants].

Right now you only assert that determined_applicants is {} when using mocks that don’t touch the accumulator. Given the PR’s goal of wiring up partial success tracking, you might consider adding one spec that uses the real Ssa::ProcessVerificationResponse/Vlp::ProcessVerificationResponse (or a targeted fake) to assert that determined_applicants is actually populated with HBX IDs under the expected :ssa/:vlp keys. That would protect the new tracking behavior from regressions.

Also applies to: 125-179

spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (1)

150-198: Consider asserting that request objects are not instantiated when ineligible

In the "when neither SSA nor VLP is eligible" / "only SSA is eligible" / "only VLP is eligible" contexts you stub the eligibility predicates but don’t assert anything about whether the corresponding request methods or classes are invoked.

To tighten the tests and ensure you don’t accidentally instantiate and run real ApplicantRequest operations when eligibility is false, you could add expectations like:

expect(subject).not_to receive(:request_ssa_verification)
expect(subject).not_to receive(:request_dhs_verification)

in the “neither eligible” context (and similar expectations for the one‑service‑eligible contexts).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5b7b413 and d07500a.

📒 Files selected for processing (13)
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (2 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (4 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb (1 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (1 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/service_constants.rb (1 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (0 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb (2 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb (2 hunks)
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (2 hunks)
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb (1 hunks)
  • spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb (3 hunks)
  • spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb (1 hunks)
💤 Files with no reviewable changes (1)
  • app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2024-10-08T21:02:47.798Z 
Learnt from: kristinmerbach
Repo: ideacrew/fdsh_gateway PR: 178
File: app/operations/fdsh/ridp/rj139/handle_primary_determination_request.rb:27-31
Timestamp: 2024-10-08T21:02:47.798Z
Learning: The user has clarified that there are contracts in place to ensure the payload structure for the `validate_params` method in the `HandlePrimaryDeterminationRequest` class, making additional validation unnecessary within this method.

Applied to files:

  • app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb
  • app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb
  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb
📚 Learning: 2025-07-15T16:28:40.423Z 
Learnt from: vkghub
Repo: ideacrew/fdsh_gateway PR: 268
File: app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb:119-123
Timestamp: 2025-07-15T16:28:40.423Z
Learning: In `app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb`, the application hash structure guarantees that top level keys like `:person_name` and `:demographics` will always be present, so direct hash access in `name_and_dob_match?` method is safe and doesn't require defensive navigation using `dig`.

Applied to files:

  • app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb
  • app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb
  • app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb
  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb
  • spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb
  • app/operations/fdsh/ssa_vlp/rj3/service_constants.rb
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb
  • app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb
📚 Learning: 2024-06-10T19:13:32.163Z 
Learnt from: saikumar9
Repo: ideacrew/fdsh_gateway PR: 219
File: app/models/alive_status/family.rb:45-78
Timestamp: 2024-06-10T19:13:32.163Z
Learning: The class `::AcaEntities::Operations::CreateFamily` is well-tested within its own gem, and its usage in the methods `request_family_entity` and `response_family_entity` of the `AliveStatus::Family` model is sufficiently covered in `spec/models/alive_status/family_spec.rb`.

Applied to files:

  • spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb
📚 Learning: 2025-07-16T13:34:53.451Z 
Learnt from: ymhari
Repo: ideacrew/fdsh_gateway PR: 268
File: app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb:129-138
Timestamp: 2025-07-16T13:34:53.451Z
Learning: In `app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb`, the evidence appending approach in `update_evidences_with_failure` and `update_evidences_with_results` methods is intentionally designed to accumulate evidence entries rather than replace them, as confirmed by ymhari.

Applied to files:

  • spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb
  • app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb
  • app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb
  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb
  • spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb
📚 Learning: 2025-07-14T19:06:48.270Z 
Learnt from: ymhari
Repo: ideacrew/fdsh_gateway PR: 268
File: app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb:99-119
Timestamp: 2025-07-14T19:06:48.270Z
Learning: In `app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb`, the evidence appending approach in `update_evidences_with_failure` and `update_evidences_with_results` methods is intentionally designed to accumulate evidence entries rather than replace them, as confirmed by ymhari.

Applied to files:

  • spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb
  • app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb
  • app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb
  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb
  • spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb
📚 Learning: 2024-10-08T21:02:47.798Z 
Learnt from: TristanB17
Repo: ideacrew/fdsh_gateway PR: 191
File: spec/operations/fdsh/vlp/rx142/initial_verification/process_initial_verification_response_spec.rb:0-0
Timestamp: 2024-10-08T21:02:47.798Z
Learning: TristanB17 has clarified that an 'invalid response' in the context of `Fdsh::Vlp::Rx142::InitialVerification::ProcessInitialVerificationResponse` refers to a valid error response from CMS, which should still yield an `Attestation` object.

Applied to files:

  • app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb
  • app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb
  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb
  • app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb
📚 Learning: 2024-11-18T17:13:17.915Z 
Learnt from: saipraveen18
Repo: ideacrew/fdsh_gateway PR: 262
File: app/operations/fdsh/h41/request/build_1095a_payload.rb:85-89
Timestamp: 2024-11-18T17:13:17.915Z
Learning: In the `fetch_insurance_provider_title` method within `app/operations/fdsh/h41/request/build_1095a_payload.rb`, it's acceptable to assume that the `modify_carrier_legal_names` feature is always enabled and that the mapping will always be present, so defensive checks for nil mappings are not necessary.

Applied to files:

  • app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb
🧬 Code graph analysis (11)
app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb (2)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1)
  • include (10-146)
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (1)
  • include (10-93)
spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb (1)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1)
  • call (15-31)
app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb (1)
app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (3)
  • validate_params (12-24)
  • track_applicant_determination (42-49)
  • response_transaction (30-32)
app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (3)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (1)
  • validate_params (21-29)
app/models/transmittable/transmission.rb (1)
  • transactions (127-129)
app/operations/fdsh/ssa_vlp/rj3/service_constants.rb (2)
  • request_key (19-21)
  • response_key (23-25)
app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb (2)
app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (3)
  • validate_params (12-24)
  • track_applicant_determination (42-49)
  • response_transaction (30-32)
app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb (1)
  • call (18-30)
spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb (3)
app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (2)
  • response_transaction (30-32)
  • request_transaction (26-28)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (2)
  • call (10-17)
  • include (7-101)
app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb (2)
  • call (16-28)
  • include (10-82)
spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (3)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1)
  • call (15-31)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)
  • call (18-24)
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (1)
  • call (15-28)
spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb (4)
app/models/transmittable/transaction.rb (1)
  • transmission (73-76)
app/models/transmittable/transmission.rb (1)
  • transactions (127-129)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (2)
  • call (15-31)
  • include (10-146)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)
  • eligibility (65-67)
app/operations/fdsh/ssa_vlp/rj3/service_constants.rb (2)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1)
  • service_name (35-37)
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (1)
  • service_name (32-34)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (3)
app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb (1)
  • call (16-28)
app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb (1)
  • call (18-30)
app/operations/fdsh/ssa_vlp/rj3/handle_verification_request.rb (1)
  • call (13-22)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (2)
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (1)
  • call (15-28)
app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb (1)
  • handle_failure (73-77)
🔇 Additional comments (10)
spec/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response_spec.rb (2)

80-91: determined_applicants wiring and validation match the shared utils

Wiring determined_applicants into params and explicitly testing the “missing determined_applicants” case brings the spec in line with ResponseProcessorUtils.validate_params, which now requires this hash to be present.

The default { ssa: [], vlp: [] } setup in the happy‑path examples is a good fit for track_applicant_determination, and the invalid‑params example ensures missing data fails fast with a clear message.

Also applies to: 93-121

583-650: transaction_failed? behavior is well specified across state combinations

The dedicated #transaction_failed? examples that:

  • Create separate request/response transactions keyed as SSA request/response,
  • Vary latest_state across :succeeded, :failed, :received, :transmitted,
  • And test cases where one side is missing,

give a clear contract for how transaction failures are detected and how request vs response precedence works.

This matrix should make future refactors of transaction_failed? much safer.

spec/operations/fdsh/ssa_vlp/rj3/process_applicant_responses_spec.rb (2)

269-347: Evidence filtering tests accurately reflect filter_processed_evidences behavior

The #filter_processed_evidences examples correctly verify that only evidences with non-empty request_results or verification_histories survive, and that an exception in the filtering path results in AddError/UpdateProcessStatus calls plus a failure containing "Error while filtering processed evidences:".

This gives good coverage of both the main behavior and the error-handling branch in ProcessApplicantResponses#filter_processed_evidences.

381-408: #handle_error spec matches the new centralized error reporting contract

The #handle_error spec validates both side effects (AddError and UpdateProcessStatus called with the job) and the returned Failure(message). That lines up cleanly with the implementation and ensures downstream callers like update_ssa_verification_response and update_dhs_verification_response will propagate a failure monad with the same message they log.

spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (2)

54-88: #call specs correctly enforce partial‑success semantics

The #call examples where one service fails and the other succeeds, both fail, and neither is eligible all align with the new orchestration in ProcessApplicantRequests#call/#request_verifications:

  • One failure + one success ⇒ overall Success('Successfully invoked verifications').
  • Both failures ⇒ overall Failure('Both SSA and DHS verifications failed').
  • No eligible services ⇒ overall success with the same message.

This matches the PR objective of decoupling SSA and VLP so that a single-service outage doesn’t block the whole flow.

Also applies to: 90-148

201-261: #request_verifications unit tests nicely cover all aggregation combinations

The dedicated #request_verifications specs hit all four result combinations:

  • both success,
  • only SSA success,
  • only DHS success,
  • both failure → Failure('Both SSA and DHS verifications failed').

This isolates and documents the aggregation logic clearly, making future changes to partial‑success rules low risk.

app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (3)

10-17: #call return shape and @determined_applicants initialization are consistent with the orchestrator

call now returns [determined_application_payload, @determined_applicants], which matches the expectations in HandleVerificationRequest#call and the new spec. Initializing @determined_applicants to {} in validate_params is a clean way to seed the accumulator without leaking it into the monadic tuple.

This design keeps the public interface simple while still letting downstream services (via ResponseProcessorUtils#track_applicant_determination) mutate the shared accumulator.

Also applies to: 21-28

80-90: Evidence filtering and error propagation align with the new failure‑recording behavior

filter_processed_evidences now strips out evidences that have neither request_results nor verification_histories, which pairs well with ResponseProcessorUtils#record_verification_failure appending “failed” evidences with histories and the successful paths appending evidences with request_results. The spec confirms this behavior.

On exceptions, delegating to handle_error ensures you both record the problem via jobs and propagate a Failure(message) back to the caller, which is appropriate for a finalization step like this.

Also applies to: 92-100

31-52: Callers confirmed—no legacy call sites detected

Verification confirms that Ssa::ProcessVerificationResponse and Vlp::ProcessVerificationResponse are called only from this file (lines 58 and 67), eliminating the risk of legacy call sites hitting the 'Determined applicants hash not provided' failure.

app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (1)

12-24: Call sites verified to include determined_applicants parameter

Both Ssa::ProcessVerificationResponse and Vlp::ProcessVerificationResponse instantiations in app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (lines 58 and 67) correctly pass the required :determined_applicants parameter. No additional call sites exist that would trigger the validation failure.

@charlienparker charlienparker changed the title Decouple ssavlp error handling decouple ssavlp error handling Nov 18, 2025
@ideacrew ideacrew deleted a comment from coderabbitai bot Nov 18, 2025
@ideacrew ideacrew deleted a comment from coderabbitai bot Nov 18, 2025
@ideacrew ideacrew deleted a comment from coderabbitai bot Nov 18, 2025
@charlienparker charlienparker marked this pull request as draft November 18, 2025 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

bugfix cr-120+ PRs associated with the CR-120+, QHP Application, or Eligibility 3.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@charlienparker