Skip to content

remove encrypted ssn check for ssa call eligibility check#298

Merged
jacobkagon merged 2 commits intotrunkfrom
suppress-ssa-call-when-no-ssn
Dec 3, 2025
Merged

remove encrypted ssn check for ssa call eligibility check#298
jacobkagon merged 2 commits intotrunkfrom
suppress-ssa-call-when-no-ssn

Conversation

@charlienparker
Copy link
Copy Markdown
Contributor

@charlienparker charlienparker commented Dec 3, 2025

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

Summary by CodeRabbit

  • Changes

    • Social Security verification eligibility narrowed to require SSN evidence only (citizenship condition removed).
  • Tests

    • Expanded and reorganized tests to cover finer-grained eligibility branches for Social Security and DHS verification scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 3, 2025

Walkthrough

Removed citizenship evidence from SSA evidence keys and tightened SSA eligibility to require only an encrypted SSN. Tests were reorganized into finer-grained contexts to cover SSA and DHS eligibility branches explicitly.

Changes

Cohort / File(s) Summary
SSA Eligibility Logic
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb
SSA_EVIDENCE_KEYS changed from [:social_security_number_evidence, :citizenship_evidence] to [:social_security_number_evidence]. eligible_for_invoking_ssa? now requires encrypted SSN presence only (removed citizen status check).
Test Restructuring
spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb
Tests refactored into nested contexts for SSA and DHS evidences with explicit branches for encrypted SSN presence/absence, citizen/non-citizen statuses, applying-for-coverage flags, and corresponding true/false expectations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review focus:
    • eligible_for_invoking_ssa? logic change and any call sites assuming prior citizen-status behavior.
    • Test branches to ensure they align with new eligibility condition.

Possibly related PRs

Suggested labels

cr-120+

Suggested reviewers

  • TristanB17
  • jacobkagon
  • saipraveen18

Poem

🐰 I hopped through code with nimble feet,
Pulled out a check to make things neat,
SSN alone now leads the way,
Tests rearranged to prove the play,
A tidy patch — a carrot treat!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'remove encrypted ssn check for ssa call eligibility check' directly and accurately describes the main change: removing the encrypted SSN condition from the SSA eligibility check logic.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch suppress-ssa-call-when-no-ssn

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e093b13 and 91b9ea3.

📒 Files selected for processing (2)
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (2 hunks)
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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
  • spec/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.rb
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.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)
  • eligibility (62-64)
🪛 GitHub Actions: RSpec
spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb

[error] 313-313: Trailing whitespace detected. Layout/TrailingWhitespace (Correctable).

⏰ 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)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb (1)

82-87: LGTM!

The simplified eligibility check is clear and correct. SSA invocation now strictly requires an encrypted SSN to be present, which aligns with the PR objective. The early returns for eligibility and ssa_evidences? are maintained appropriately.

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

306-341: LGTM!

The restructured tests comprehensively cover all branches of eligible_for_invoking_ssa?:

  • With SSA evidences: encrypted SSN present (true) vs absent (false)
  • Without SSA evidences (false)

This aligns well with the updated production logic.


352-404: LGTM!

The DHS eligibility tests are well-structured with clear nested contexts covering all decision branches: evidence presence, coverage application status, and citizen status. The test expectations correctly mirror the production logic.

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

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

333-341: “No SSA evidences” branch looks correct and focused

This context cleanly asserts that eligible_for_invoking_ssa? returns false when ssa_evidences? is false, which matches the expected gating behavior.

If you want to make the dependency on eligibility explicit for readability, you could also stub eligibility here as you do in the “has SSA evidences” branch, but that’s optional.


352-393: DHS eligibility branches are well-factored and cover key combinations

The DHS contexts clearly encode the intended rules:

  • Requires DHS evidences (immigration_evidence? true).
  • Requires the applicant to be applying for coverage.
  • Returns true only for a non‑citizen status (e.g., 'alien_lawfully_present'), and false for 'us_citizen'.
  • Returns false when not applying for coverage.

The nested contexts make the decision table easy to read and should help prevent regressions.

As a small optional enhancement, you might add an example for another non‑eligible status (e.g., “not lawfully present”) if such a status exists in your domain, to lock down that behavior too.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 91b9ea3 and 6765c6c.

📒 Files selected for processing (1)
  • spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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.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)
  • eligibility (62-64)
⏰ 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 (2)
spec/operations/fdsh/ssa_vlp/rj3/process_applicant_requests_spec.rb (2)

395-405: “No DHS evidences” branch correctly forces ineligibility

This context verifies that eligible_for_invoking_dhs? returns false when immigration_evidence? is false, even with a valid eligibility, which matches the intended gating for DHS calls.

No changes needed here.


306-331: The implementation at line 86 of app/operations/fdsh/ssa_vlp/rj3/process_applicant_requests.rb still contains encrypted_ssn.present?, meaning the encrypted SSN check has not been removed. The spec at lines 306-331 correctly reflects the current implementation behavior—returning true when encrypted SSN is present and false when nil. There is no mismatch between the spec and implementation.

Likely an incorrect or invalid review comment.

Copy link
Copy Markdown
Contributor

@jacobkagon jacobkagon left a comment

Choose a reason for hiding this comment

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

Change looks good!

@jacobkagon jacobkagon enabled auto-merge (squash) December 3, 2025 18:39
Copy link
Copy Markdown
Contributor

@TristanB17 TristanB17 left a comment

Choose a reason for hiding this comment

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

lgtm!

@jacobkagon jacobkagon merged commit 454cc39 into trunk Dec 3, 2025
7 checks passed
@jacobkagon jacobkagon deleted the suppress-ssa-call-when-no-ssn branch December 3, 2025 21:31
sri49 added a commit that referenced this pull request Jan 16, 2026
jacobkagon pushed a commit that referenced this pull request Jan 20, 2026
* Revert "return success on SSA/VLP request verifications (#307)"

This reverts commit 7f1fbb6.

* Revert "refactor ssa call when no ssn (#301)"

This reverts commit 90913e3.

* Revert "remove encrypted ssn check for ssa call eligibility check (#298)"

This reverts commit 454cc39.
sri49 pushed a commit that referenced this pull request Feb 10, 2026
* only make ssa call when ssn field is present, remove citizenshp from list of ssa-applicable evidences

* lint 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants