Skip to content

revert secondary dhs call due to ssn#287

Merged
ymhari merged 3 commits intotrunkfrom
868fubywe_clickup
Oct 7, 2025
Merged

revert secondary dhs call due to ssn#287
ymhari merged 3 commits intotrunkfrom
868fubywe_clickup

Conversation

@ymhari
Copy link
Copy Markdown
Contributor

@ymhari ymhari commented Oct 6, 2025

https://app.clickup.com/t/868fubywe

Reverting commits:
5efd894
e49f98c

Summary by CodeRabbit

  • New Features
    • None
  • Bug Fixes
    • Use the latest verification response per applicant to avoid duplicate processing.
    • More robust applicant matching by comparing dates of birth as dates.
    • Clearer failure when no applicants are eligible for close-case processing.
  • Performance
    • Streamlined verification and close-case flows to reduce overhead.
  • Reliability
    • Removed automatic follow-up triggers to prevent unintended requests.
  • Tests
    • Updated tests to reflect streamlined flows and adjusted failure scenarios.

@ymhari ymhari added the cr-120+ PRs associated with the CR-120+, QHP Application, or Eligibility 3.0 label Oct 6, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 6, 2025

Walkthrough

Narrow VLP handling to a single latest verification transaction per applicant, remove automatic VLP triggering after saving SSA response payloads, derive VLP close-case payloads from the subject's last transaction, adjust control flow and tests to match the single-transaction approach.

Changes

Cohort / File(s) Summary
Process Applicant Responses
app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb
Stop iterating all :vlp_verification_response transactions; use find_vlp_response_transaction to obtain and process a single latest transaction per applicant.
SSA Applicant Request
app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb
Remove post-save VLP trigger; delete trigger_vlp_for_citizenship and can_trigger_vlp_for_citizenship; no VLP invocation after saving response payload.
VLP Close Case: ApplicantRequest
app/operations/fdsh/ssa_vlp/rj3/vlp/close_case/applicant_request.rb
validate_params no longer extracts/passes vlp_transaction; generate_request_payload fetches the last :vlp_verification_response from @subject.transactions.
VLP Close Case: HandleRequest
app/operations/fdsh/ssa_vlp/rj3/vlp/close_case/handle_request.rb
Filter applicants by payload hbx_ids; return Failure if none found; call ApplicantRequest with only applicant and job (no vlp_transaction).
Specs: SSA Applicant Request
spec/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request_spec.rb
Remove VLP request/response stubs and assertions; tests no longer expect VLP transaction side effects; focus on SSA request/response.
Specs: VLP Close Case ApplicantRequest
spec/operations/fdsh/ssa_vlp/rj3/vlp/close_case/applicant_request_spec.rb
Stop passing vlp_transaction in params; adjust failure expectation to payload-generation failure; simulate missing prior transaction.
Specs: VLP Close Case HandleRequest
spec/operations/fdsh/ssa_vlp/rj3/vlp/close_case/handle_request_spec.rb
Remove fixture creating a succeeded :vlp_verification_response transaction; tests updated to align with transaction-derived payload flow.
Response Processor Utils
app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb
Normalize DOB comparisons to dates with nil-safety (&.to_date) in name_and_dob_match?.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Processor as ProcessApplicantResponses
  participant Txns as applicant.transactions
  participant VLP as process_vlp_verification_response

  User->>Processor: update_dhs_verification_response(applications)
  loop per applicant
    Processor->>Txns: find last :vlp_verification_response
    alt transaction found
      Processor->>VLP: process(transaction)
      alt success
        Processor-->>User: update application_hash
      else failure
        Processor-->>User: early Failure
      end
    else no transaction
      Processor-->>User: skip applicant
    end
  end
Loading
sequenceDiagram
  autonumber
  participant Client
  participant HandleReq as VLP::CloseCase::HandleRequest
  participant Applicants as Applicants[]
  participant AppReq as VLP::CloseCase::ApplicantRequest
  participant Txns as applicant.transactions

  Client->>HandleReq: process_close_case_requests(payload, job)
  HandleReq->>Applicants: filter by payload.hbx_ids
  alt none found
    HandleReq-->>Client: Failure (no applicants)
  else found
    loop each applicant
      AppReq->>Txns: get last :vlp_verification_response
      alt exists
        AppReq-->>HandleReq: Success(payload)
      else missing
        AppReq-->>HandleReq: Failure (payload generation failed)
      end
    end
    HandleReq-->>Client: Aggregate results
  end
Loading
sequenceDiagram
  autonumber
  participant SSA as SSA::ApplicantRequest
  participant Store as ResponsePayloadStore

  SSA->>Store: save_response_payload(params)
  note right of SSA: removed call to trigger VLP after save
  SSA-->>Store: Success/Failure
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • vkghub
  • saipraveen18
  • kristinmerbach

Poem

I nibble the freshest carrot last in line,
hop to the newest trail and keep it fine.
No extra triggers scattering fur and dirt,
one tidy burrow — neat work, no spilt shirt. 🥕🐇

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 pull request title succinctly captures the primary action of this changeset, which is reverting the secondary DHS call added for SSN-based verification; this aligns with the main code modifications of removing the extra VLP transaction handling. Although there are additional related VLP flow adjustments, the core objective is to undo the secondary call, making the title fully relevant and clear.
✨ 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 868fubywe_clickup

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: 0

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

54-54: Use explicit ordering for deterministic query results

Replace the last call with an explicit order to ensure the most recent transaction is always selected:

-vlp_transaction = @subject.transactions.where(key: :vlp_verification_response).last
+vlp_transaction = @subject.transactions.where(key: :vlp_verification_response).order(created_at: :desc).first

Audit other similar patterns across the codebase with:

rg -n '\.where\([^)]+\)\.last' -g '*.rb' -A2 -B2
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dc39bc3 and 54f396e.

📒 Files selected for processing (7)
  • app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (2 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb (0 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/vlp/close_case/applicant_request.rb (3 hunks)
  • app/operations/fdsh/ssa_vlp/rj3/vlp/close_case/handle_request.rb (1 hunks)
  • spec/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request_spec.rb (0 hunks)
  • spec/operations/fdsh/ssa_vlp/rj3/vlp/close_case/applicant_request_spec.rb (2 hunks)
  • spec/operations/fdsh/ssa_vlp/rj3/vlp/close_case/handle_request_spec.rb (0 hunks)
💤 Files with no reviewable changes (3)
  • spec/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request_spec.rb
  • app/operations/fdsh/ssa_vlp/rj3/ssa/applicant_request.rb
  • spec/operations/fdsh/ssa_vlp/rj3/vlp/close_case/handle_request_spec.rb
🧰 Additional context used
🧬 Code graph analysis (4)
spec/operations/fdsh/ssa_vlp/rj3/vlp/close_case/applicant_request_spec.rb (1)
app/operations/fdsh/ssa_vlp/rj3/vlp/close_case/applicant_request.rb (1)
  • call (16-30)
app/operations/fdsh/ssa_vlp/rj3/vlp/close_case/handle_request.rb (1)
app/operations/fdsh/ssa_vlp/rj3/vlp/close_case/applicant_request.rb (1)
  • call (16-30)
app/operations/fdsh/ssa_vlp/rj3/vlp/close_case/applicant_request.rb (1)
app/models/transmittable/transmission.rb (1)
  • transactions (127-129)
app/operations/fdsh/ssa_vlp/rj3/process_applicant_responses.rb (1)
app/models/transmittable/transmission.rb (1)
  • transactions (127-129)
⏰ 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 (7)
app/operations/fdsh/ssa_vlp/rj3/vlp/close_case/applicant_request.rb (1)

17-17: LGTM!

The removal of vlp_transaction from the parameter list aligns with the new approach of deriving the transaction from the subject's history rather than passing it explicitly.

Also applies to: 38-38

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

48-56: LGTM!

The simplified approach of processing a single VLP verification response per applicant (instead of iterating over multiple transactions) aligns with the PR's objective to revert to the previous behavior.


78-78: Verify query ordering for deterministic results.

Similar to the concern in applicant_request.rb, using .last without explicit ordering could lead to non-deterministic behavior. Consider making the ordering explicit.

Consider applying this diff:

-applicant.transactions.where(key: :vlp_verification_response).last
+applicant.transactions.where(key: :vlp_verification_response).order(created_at: :desc).first
spec/operations/fdsh/ssa_vlp/rj3/vlp/close_case/applicant_request_spec.rb (2)

61-61: LGTM!

The test parameters correctly reflect the updated ApplicantRequest interface, which no longer requires vlp_transaction to be passed explicitly.


116-120: LGTM!

The test appropriately validates the new behavior where the VLP transaction is retrieved from the applicant's transaction history. Destroying the transaction and verifying the failure path ensures the implementation handles missing transactions correctly.

app/operations/fdsh/ssa_vlp/rj3/vlp/close_case/handle_request.rb (2)

55-56: LGTM!

The guard clause that returns a Failure when no applicants match the payload is good defensive programming and provides clear feedback when the payload doesn't align with the application's applicants.


58-60: LGTM!

The simplified call to ApplicantRequest with only applicant and job parameters is consistent with the changes across the codebase, where vlp_transaction is now derived from the applicant's transaction history rather than passed explicitly.

@ymhari ymhari enabled auto-merge (squash) October 6, 2025 20:20
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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54f396e and d3fdb80.

📒 Files selected for processing (1)
  • app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (1 hunks)
⏰ 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

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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d3fdb80 and c1876ee.

📒 Files selected for processing (1)
  • app/operations/fdsh/ssa_vlp/rj3/response_processor_utils.rb (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-15T16:28:40.423Z
Learnt from: vkghub
PR: ideacrew/fdsh_gateway#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/response_processor_utils.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

Copy link
Copy Markdown
Contributor

@vkghub vkghub left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@kristinmerbach kristinmerbach left a comment

Choose a reason for hiding this comment

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

LGTM

@ymhari ymhari merged commit 802c500 into trunk Oct 7, 2025
6 checks passed
@ymhari ymhari deleted the 868fubywe_clickup branch October 7, 2025 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants