Skip to content

fix: complete session only after document transfer#1310

Open
bhavyajain0810 wants to merge 3 commits into
viru0909-dev:mainfrom
bhavyajain0810:fix/session-completion-document-transfer-770
Open

fix: complete session only after document transfer#1310
bhavyajain0810 wants to merge 3 commits into
viru0909-dev:mainfrom
bhavyajain0810:fix/session-completion-document-transfer-770

Conversation

@bhavyajain0810

@bhavyajain0810 bhavyajain0810 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Description

This PR fixes a data-loss risk in the Vakil-Friend completeSession() flow.

Previously, a session could be marked as COMPLETED before uploaded documents were transferred to the newly created case. If the transfer failed, the session could remain permanently completed while its evidence was not linked correctly.

The updated flow transfers documents before changing the session status. If document transfer fails, the method throws an exception before the session is finalized.

Closes #770

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • No breaking change introduced
  • Documentation update is not required for this backend-only fix

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented the document-transfer ordering where necessary
  • Documentation changes are not applicable for this backend-only fix
  • My changes generate no new whitespace warnings
  • I verified that document transfer occurs before the session is marked as COMPLETED
  • Document-transfer failure prevents the session from being finalized
  • Local Maven tests could not be run because Maven is not installed or configured on my system

Testing

  • Ran git diff --check successfully
  • Verified transferDocumentsBeforeCompletion(...) executes before ChatSessionStatus.COMPLETED
  • Verified that document-transfer failure throws before the session status is updated
  • Confirmed that this PR changes only VakilFriendService.java

Screenshots

Not applicable, backend-only change.

@vercel

vercel Bot commented Jun 10, 2026

Copy link
Copy Markdown

@bhavyajain0810 is attempting to deploy a commit to the CodeBlooded's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

Copy link
Copy Markdown
Contributor

Hi @bhavyajain0810, thanks for contributing to Nyay Setu! 🎉

I have automatically:

  • 👤 Assigned this PR to you.
  • 🏷️ Applied the gssoc:approved label.

Our workflows will now analyze your changes to classify:

  • 📈 PR Difficulty: level:*
  • 🧩 PR Type: type:*
  • 🌟 PR Quality: quality:*

Tip

Ensure your PR description references the issue it resolves (e.g. Closes #123). This allows the bot to inherit any additional labels from that issue!

Happy coding! 🚀

@bhavyajain0810

Copy link
Copy Markdown
Contributor Author

Hi @viru0909-dev,

The CI failures appear to be caused by unresolved conflict markers already present in unrelated base files, not by this PR.

I verified my PR diff using:

git diff --name-only upstream/main...HEAD

and this PR only changes:

backend/nyaysetu-backend/src/main/java/com/nyaysetu/backend/service/VakilFriendService.java

The failing backend/NLP checks are pointing to unrelated files from the base branch, such as SecurityConfig.java, application.properties, and nlp-orchestrator/requirements.txt, which contain conflict markers like <<<<<<< HEAD, =======, and >>>>>>> origin/main.

Since this PR is scoped to #770, I have not included unrelated cleanup changes. Please let me know if you want me to handle those base conflict markers in a separate PR.

@viru0909-dev viru0909-dev left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

🤖 Automated PR Review — Changes Requested 🔴

Hey @bhavyajain0810! Thanks for the PR. The automated review found 1 issue(s) that must be resolved before this can be approved.

❌ Failed Checks

Status Check Details
Checklist Completed 2 checklist item(s) are still unchecked. Complete all checklist items before requesting a review.

✅ Passed Checks

Status Check Details
Title Quality Title looks good.
Description Quality Description is well-structured.
Issue Linked Issue linked via closing keyword.
No Merge Conflicts Mergeability unknown (GitHub still computing) — skipping.
Branch Up-to-date Branch is recent (last updated 1.6 days ago).
CI / Tests No CI checks found (possibly no workflows configured).

📋 How to resolve

  1. Read each failed check above carefully.
  2. Push the required fixes to your branch fix/session-completion-document-transfer-770.
  3. The bot will re-review automatically on the next push.

If this PR has been idle for 3+ more days after this message, it will be scheduled for closure.


🤖 This review was generated automatically by the NYAY-SETU PR Review Bot.

@viru0909-dev viru0909-dev left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Review Summary

This PR has been reviewed and requires changes before it can be approved. The issues below must be resolved.

Issues to Resolve

Result Check Notes
FAIL Description Description is missing: a Checklist section with checkboxes.
FAIL Checklist No checklist found in the description.

Passing Checks

Result Check Notes
PASS Title Title format is acceptable.
PASS Issue Link Issue is linked with a closing keyword.
PASS Merge Conflicts Conflict status is not yet computed by GitHub — skipping.
PASS Branch Freshness Branch was last updated 2.0 days ago.
PASS CI / Tests No CI checks are configured or results are not yet available.
PASS Code Quality No code quality issues detected in the diff.

To proceed: push the required fixes to fix/session-completion-document-transfer-770 and the PR will be re-evaluated on the next review run.

@viru0909-dev viru0909-dev left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Review Summary

This PR has been reviewed and requires changes before it can be approved. The issues below must be resolved.

Issues to Resolve

Result Check Notes
FAIL Description Description is missing: a Checklist section with checkboxes.
FAIL Checklist No checklist found in the description.

Passing Checks

Result Check Notes
PASS Title Title format is acceptable.
PASS Issue Link Issue is linked with a closing keyword.
PASS Merge Conflicts Conflict status is not yet computed by GitHub — skipping.
PASS Branch Freshness Branch was last updated 0.9 days ago.
PASS CI / Tests No CI checks are configured or results are not yet available.
PASS Code Quality No code quality issues detected in the diff.

To proceed: push the required fixes to fix/session-completion-document-transfer-770 and the PR will be re-evaluated on the next review run.

@bhavyajain0810 bhavyajain0810 force-pushed the fix/session-completion-document-transfer-770 branch from 83659a7 to 6f02a34 Compare June 16, 2026 13:06
@bhavyajain0810

Copy link
Copy Markdown
Contributor Author

Hi @viru0909-dev,

I have updated the PR checklist and retriggered the workflows with commit 6f02a34.

The PR Template Compliance / check-checklist check is now passing, so the checklist issue from the previous review has been resolved.

The remaining environment-related failure appears to be from the unrelated base-branch issues documented earlier; this PR remains limited to VakilFriendService.java.

Could you please re-review the PR and dismiss the outdated changes-requested review if everything looks good?

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Session Permanently Marked COMPLETED Before Document Transfer Succeeds in completeSession()

2 participants