return success on SSA/VLP request verifications#307
Merged
jacobkagon merged 2 commits intotrunkfrom Jan 15, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughUpdated SSA VLP verification handling: event name corrected to use Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ProcessApplicantRequests
participant SSAService
participant DHSService
participant TransmittableHandler
Caller->>ProcessApplicantRequests: invoke request_verifications
ProcessApplicantRequests->>SSAService: request_ssa_verification (if eligible)
alt DHS eligible
ProcessApplicantRequests->>DHSService: request_dhs_verification
end
SSAService-->>ProcessApplicantRequests: response (success/failure)
DHSService-->>ProcessApplicantRequests: response (success/failure)
ProcessApplicantRequests->>TransmittableHandler: let transmittable errors surface
ProcessApplicantRequests-->>Caller: return Success(nil)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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/process_applicant_requests.rb (1)
44-52: Remove unusedservice_resultsvariable to fix pipeline failure.The
service_resultsvariable is assigned but never used after the removal of the failure-checking logic. This triggers the RuboCopLint/UselessAssignmenterror reported in the pipeline.Proposed fix
def request_verifications - ssa_result = request_ssa_verification if eligible_for_invoking_ssa? - dhs_result = request_dhs_verification if eligible_for_invoking_dhs? - - service_results = [ssa_result, dhs_result].compact + request_ssa_verification if eligible_for_invoking_ssa? + request_dhs_verification if eligible_for_invoking_dhs? # processing failures are handled through Transmittable errors Success(nil) end
🧹 Nitpick comments (1)
spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (1)
292-296: Consider adding a test description clarifying the intentional design.The test now expects
Success(nil)when both verifications fail. While the expectation is correct, the test description could be enhanced to clarify that this is intentional behavior for downstream error handling.context 'when both verifications fail' do before do allow(subject).to receive(:eligible_for_invoking_ssa?).and_return(true) allow(subject).to receive(:eligible_for_invoking_dhs?).and_return(true) allow(subject).to receive(:request_ssa_verification).and_return(Dry::Monads::Failure('SSA failed')) allow(subject).to receive(:request_dhs_verification).and_return(Dry::Monads::Failure('DHS failed')) end - it 'returns success' do + it 'returns success with nil value (failures handled via Transmittable)' do result = subject.send(:request_verifications) expect(result).to be_success expect(result.value!).to eq(nil) end end
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/event_source/subscribers/fdsh/verification_requests/ssa_vlp_requested_subscriber.rbapp/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rbspec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb
🧰 Additional context used
🧠 Learnings (6)
📚 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/process_applicant_requests_spec.rbapp/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.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/process_applicant_requests_spec.rbapp/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.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:
spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rbapp/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb
📚 Learning: 2024-10-08T21:02:47.798Z
Learnt from: saikumar9
Repo: ideacrew/fdsh_gateway PR: 219
File: app/models/alive_status/family.rb:45-78
Timestamp: 2024-10-08T21:02:47.798Z
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_requests_spec.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/process_applicant_requests.rb
📚 Learning: 2026-01-14T20:40:53.472Z
Learnt from: saipraveen18
Repo: ideacrew/fdsh_gateway PR: 306
File: app/operations/fdsh/h36/request/clone_transaction_and_subject.rb:37-54
Timestamp: 2026-01-14T20:40:53.472Z
Learning: In app/operations/fdsh/h36/request/clone_transaction_and_subject.rb, the `publish` method uses fire-and-forget behavior where it always returns `Success(true)` regardless of individual event publication failures. This is intentional because events are processed asynchronously and errors won't be raised in the publishing method itself—individual failures are only logged.
Applied to files:
app/event_source/subscribers/fdsh/verification_requests/ssa_vlp_requested_subscriber.rb
🧬 Code graph analysis (1)
spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (1)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)
call(18-24)
🪛 GitHub Actions: RSpec
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb
[error] 48-48: RuboCop: Lint/UselessAssignment: Useless assignment to variable - service_results. (autocorrectable)
⏰ 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). (1)
- GitHub Check: build-and-upload-image
🔇 Additional comments (3)
spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (1)
141-145: Test expectations correctly reflect the new behavior.The updated tests properly verify that the operation now returns success even when all verification services fail. This aligns with the PR objective of ensuring EA receives a response regardless of verification outcomes.
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)
18-24: Verify the return value consistency betweencallandrequest_verifications.The
callmethod returnsSuccess('Successfully invoked verifications')on line 23, butrequest_verificationsreturnsSuccess(nil)on line 51. Sincerequest_verificationsis yielded withyield(line 21), thenilvalue is discarded and the final success message is returned. This is correct behavior, but the tests at line 292-295 expectnilwhen callingrequest_verificationsdirectly, which is inconsistent with the fullcallflow tests that expect'Successfully invoked verifications'.This is intentional and working correctly, just noting for clarity.
app/event_source/subscribers/fdsh/verification_requests/ssa_vlp_requested_subscriber.rb (1)
38-41: No action needed—event naming is correct.The change from
"events.fdsh.verifications.ssavlp.determined"to"events.fdsh.verifications.ssa_vlp.determined"is consistent with the rest of the codebase. The published event name with underscore matches the EVENT_NAME constant inhandle_verification_request.rb:11.The AMQP queue subscription at line 8 (
'enroll.verifications.ssavlp') is from a different event system (incoming events from enroll) and does not need to match the published event namespace (events.fdsh.verifications.*). These serve different purposes and are in separate namespaces.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb`:
- Around line 44-50: In request_verifications, add handling for the case where
neither eligible_for_invoking_ssa? nor eligible_for_invoking_dhs? is true:
create and surface a Transmittable-style failure so EA gets a status/evidence
record instead of silently succeeding; specifically, detect the "no verification
eligible" branch in request_verifications and invoke the same failure pipeline
used by the ApplicantRequest classes (use RequestProcessorUtils.handle_failure
or call Fdsh::Jobs::AddError and Fdsh::Jobs::UpdateProcessStatus with a clear
message and context) so a record is created for that applicant when no SSA or
DHS request is attempted.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb
🧰 Additional context used
🧠 Learnings (4)
📚 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/process_applicant_requests.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/process_applicant_requests.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/process_applicant_requests.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/process_applicant_requests.rb
⏰ 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
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
charlienparker
approved these changes
Jan 15, 2026
TristanB17
approved these changes
Jan 15, 2026
Contributor
TristanB17
left a comment
TristanB17
left a comment
There was a problem hiding this comment.
good catch with the event name consistency, lgtm!
jacobkagon
deleted the
CU_868gfyb67_handle_failure_when_ssavlp_request_fails
branch
sri49
pushed a commit
that referenced
this pull request
Feb 10, 2026
* return success on SSA/VLP request verifications * rubocop fix
jacobkagon
added a commit
that referenced
this pull request
Feb 11, 2026
* remove encrypted ssn check for ssa call eligibility check (#298) * only make ssa call when ssn field is present, remove citizenshp from list of ssa-applicable evidences * lint fix * refactor ssa call when no ssn (#301) * allow ssa processor to be called when an applicant is a citizen but lacks an ssn, fail the request in the processor for this case * spec coverage * lint fix * comment clarity * lint fixes --------- Co-authored-by: Jacob Kagon <69021620+jacobkagon@users.noreply.github.com> * query applicant by hbx id (#303) * return success on SSA/VLP request verifications (#307) * return success on SSA/VLP request verifications * rubocop fix --------- Co-authored-by: Charlie Parker <charlie.parker@ideacrew.com> Co-authored-by: Jacob Kagon <69021620+jacobkagon@users.noreply.github.com> Co-authored-by: vishal kalletla <vishuk1201@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://app.clickup.com/t/868gfyb67
Current Behavior:
If all applicants do not have an SSN, a failure is being returned and a response is not being published to EA
New Behavior:
Sends a failure response to EA even if all applicants do not have an SSN.
Summary by CodeRabbit
Bug Fixes
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.