refactor ssa call when no ssn #301
Conversation
…acks an ssn, fail the request in the processor for this case
WalkthroughAdds SSA eligibility checks: expands SSA evidence keys to include Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb(2 hunks)app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb(2 hunks)spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb(2 hunks)spec/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request_spec.rb(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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.rbspec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_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:
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rbspec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.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/ssa/applicant_request.rb
🧬 Code graph analysis (4)
spec/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request_spec.rb (3)
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (1)
call(15-29)app/operations/fdsh/ssa_vlp/rj3/ssa/request_verification.rb (1)
call(14-18)app/models/transaction.rb (1)
applicants(54-56)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (1)
encrypted_ssn(37-46)
spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (1)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)
eligibility(62-64)
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (2)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)
encrypted_ssn(74-80)app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb (1)
handle_failure(73-77)
🪛 GitHub Actions: RSpec
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb
[error] 42-42: RuboCop: Layout/IndentationWidth: Use 2 (not 1) spaces for indentation. ( autocorrectable )
[error] 43-43: RuboCop: Layout/ElseAlignment: Align else with if. ( autocorrectable )
[error] 45-45: RuboCop: Layout/EndAlignment: end at 45, 30 is not aligned with if at 41, 31. ( autocorrectable )
[error] 52-52: RuboCop: Layout/ArgumentAlignment: Align the arguments of a method call if they span more than one line. ( autocorrectable )
[error] 53-53: RuboCop: Layout/ArgumentAlignment: Align the arguments of a method call if they span more than one line. ( 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 (6)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (2)
16-16: LGTM - Evidence keys expansion looks correct.Adding
citizenship_evidencetoSSA_EVIDENCE_KEYSaligns with the broader SSA eligibility logic that now considers citizenship status.
86-88: Logic correctly enables downstream failure handling for US citizens without SSN.The condition
encrypted_ssn.present? || citizen_status == US_CITIZEN_STATUSintentionally allows US citizens through even without an SSN, enabling the downstreamcheck_ssa_eligibilitymethod inApplicantRequestto properly handle the failure with appropriate status updates. The comment accurately describes this design.spec/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request_spec.rb (1)
152-178: Good test coverage for the missing SSN scenario.The tests properly validate both the failure result and the side effects (transmission/job status transitions to failed). The test setup correctly creates an applicant entity without SSN to exercise the new
check_ssa_eligibilityflow.app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (1)
19-19: LGTM - Pre-flight eligibility check is well-placed in the flow.Adding
check_ssa_eligibilityafter transmission/transaction creation but before payload generation ensures proper status updates occur when SSN is missing, rather than failing silently or with unclear errors downstream.spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (2)
306-357: Comprehensive test coverage for SSA eligibility conditions.The tests cover all the key permutations:
- US citizen with/without SSN
- Non-citizen with SSN
- Negative cases (no eligibility, no evidences, no SSN + non-citizen)
This properly validates the
encrypted_ssn.present? || citizen_status == US_CITIZEN_STATUSlogic.
368-420: Good test coverage for DHS eligibility conditions.The tests properly validate all non-citizen status types and negative paths. The parameterized test at lines 378-387 efficiently covers all
NON_CITIZEN_STATUSvalues.
charlienparker
added
the
cr-120+
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (3)
15-21: Pre‑flight SSA eligibility check placement looks correct; consider removing the unused local.Running
check_ssa_eligibilityafter creating the request transmission/transaction aligns with howhandle_failureexpects those objects. The assigned_eligiblevalue is never used, though, so you could simplify to just binding the monad:yield check_ssa_eligibilityThis keeps the Do-notation behavior while avoiding an unused ivar/local.
37-46:encrypted_ssnimplementation matches other flow but duplicates logic; consider centralizing.This helper mirrors
encrypted_ssninFdsh::SsaVlp::Rj3::ProcessApplicantRequests(FAA vs non‑FAA selection) and the TODO already notes the duplication. To avoid future drift (e.g., if the FAA check or entity shape changes), consider extracting this into a shared helper/module or delegating to the existing method so eligibility and payload builders stay in sync.
48-54: Eligibility rule currently hard‑requires an encrypted SSN; confirm this matches business intent and upstream gating.
check_ssa_eligibilityshort‑circuits the flow with a failure wheneverencrypted_ssnis blank, regardless of any other evidence (e.g., citizenship). That cleanly enforces “no SSA request without SSN” at this layer and fits the PR title, but it also means any future upstream rule that allows SSA in limited no‑SSN cases would be blocked here.If there’s a possibility of broader eligibility (as hinted by the expanded SSA evidence keys in
process_applicant_requests.rb), consider either:
- computing the eligibility boolean in one place upstream and having this method simply respect that flag, or
- explicitly documenting here that SSA is only allowed when
encrypted_ssnis present so future changes don’t silently conflict.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb(2 hunks)app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb
🧰 Additional context used
🧠 Learnings (2)
📚 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/ssa/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/applicant_request.rb
🧬 Code graph analysis (1)
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (2)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)
encrypted_ssn(74-80)app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb (1)
handle_failure(73-77)
⏰ 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
Ticket: https://app.clickup.com/t/868gfyb67
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.