fix: Defer e-Invoice/e-Waybill portal cancellation to after-commit job by vorasmit · Pull Request #4546 · resilient-tech/india-compliance

vorasmit · 2026-07-06T10:07:26Z

Summary

Refactored the e-Invoice and e-Waybill cancellation flow to defer irreversible portal operations to an after-commit job. This ensures that if a Sales Invoice cancellation rolls back, the portal state remains untouched and doesn't diverge from the database state.

Key Changes

Deferred portal cancellation: Moved auto_cancel_e_invoice and auto_cancel_e_waybill calls from synchronous cancel_e_waybill_e_invoice hook to a new cancel_e_invoice_e_waybill_after_commit function enqueued with enqueue_after_commit=True
Separated concerns:
- cancel_e_waybill_e_invoice now only validates prerequisites and enqueues the job
- cancel_e_invoice_e_waybill_after_commit handles the actual portal cancellation with full gating logic
- _cancel_e_invoice no longer calls doc.cancel() (portal-only operation)
Updated UI flow: Modified e_invoice_actions.js to clarify that the cancel dialog performs portal cancellation only; the SI is cancelled by a separate request
Enhanced test coverage:
- Updated test_auto_cancel_e_waybill_on_si_cancel to explicitly test the deferred job execution
- Added test_portal_cancel_not_triggered_when_si_cancel_rolls_back to verify portal remains untouched on cancellation failure
- Updated test_cancel_e_invoice to verify two-step flow: portal cancel first, then SI cancel

Implementation Details

Portal cancellation is now idempotent and safe to retry
If SI cancellation fails/rolls back before commit, the enqueued job is discarded automatically
Settings validation and auto-cancel gating now live in the deferred job, not the hook
Maintains backward compatibility with existing e-Invoice/e-Waybill cancellation logic

https://claude.ai/code/session_01VQModrsTicyLZjFRMp54UG

…fecycle Portal (NIC/GSP) cancellation is irreversible, but today it runs inside the Sales Invoice cancel transaction. If a later step fails and the transaction rolls back (bulk cancel, linked docs, frozen period, ...), the local record of the cancellation is erased while the IRN/e-Waybill stay cancelled on the portal -> local and portal diverge. Fix the two failure windows without any cross-connection commit tricks: - Auto cancel (outer-transaction rollback): `cancel_e_waybill_e_invoice` no longer hits the portal synchronously inside `before_cancel`. It enqueues `cancel_e_invoice_e_waybill_after_commit` with `enqueue_after_commit=True`, so the portal call runs only if the cancellation commits, in its own isolated transaction (plain commit is safe there). On failure it logs and leaves `einvoice_status = "Pending Cancellation"` for retry/reconciliation; it never raises. - Manual "Cancel IRN & Invoice" button: `_cancel_e_invoice` now only performs the irreversible portal cancellation + local record; `cancel_e_invoice` cancels the Sales Invoice afterwards inside a savepoint, so a `doc.cancel()` failure rolls back ONLY that step and keeps the recorded portal cancellation. Tests (need a bench to run): - test_portal_cancel_not_triggered_when_si_cancel_rolls_back: forces the SI cancel to fail and asserts no portal call was made and local state is intact. - test_auto_cancel_e_waybill_on_si_cancel: updated for the sync -> async contract (drives the after-commit job explicitly). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQModrsTicyLZjFRMp54UG

Follow-up to the decoupling: instead of the cancel API cancelling the Sales Invoice itself (guarded by a savepoint), the API is now portal-only and the document cancellation is a separate request. This removes doc.cancel() from the API entirely and eliminates the savepoint. - `cancel_e_invoice` now only cancels the e-Waybill + IRN on the portal and records it (its own request/transaction). It no longer calls doc.cancel(). - The cancel dialog (e_invoice_actions.js) issues the document cancel as a second request after the portal cancel commits: * standalone "Cancel e-Invoice" button -> frappe.client.cancel * form's native cancel (before_cancel hook) -> let the pending native cancellation proceed via the existing continueCancellation callback (branch on whether a callback was passed, to avoid a double cancel). Because step 1 clears the IRN, the document cancel's before_cancel hook finds nothing to cancel and does not re-enqueue a portal call. Since portal cancel and document cancel are now separate transactions, a document-cancel failure can never roll back the (irreversible) portal cancellation -- no savepoint needed. test_e_invoice._cancel_e_invoice helper updated to the two-step flow and now asserts the intermediate portal-only state (docstatus 1, irn cleared, einvoice_status "Cancelled") before cancelling the document. Bulk cancel action intentionally not included in this change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQModrsTicyLZjFRMp54UG

No behaviour change. Trim the verbose comments/docstrings added for the portal-first cancel decoupling down to short, blunt notes; keep only the non-obvious "why" (separate transactions => no divergence). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQModrsTicyLZjFRMp54UG

Replace the raw frappe.client.cancel call with frm.savecancel() in the standalone "Cancel e-Invoice" button path. Safe because the portal-cancel response clears irn in place on frm.doc (frappe.model.update_in_locals), so the re-triggered before_cancel early-returns (no re-dialog) and the synced modified avoids a timestamp mismatch. frm.savecancel sets frappe.validated itself and gives the full client cancel flow (linked-docs handling, after_cancel, refresh). Note: savecancel adds the standard "Permanently Cancel?" confirm. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQModrsTicyLZjFRMp54UG

The RQ job runner (execute_job) already commits on success, rolls back + logs on failure, and auto-retries deadlocks/lock timeouts. The explicit commit/try/except in cancel_e_invoice_e_waybill_after_commit was redundant, and swallowing the exception disabled the runner's deadlock retry. Just call auto_cancel_e_invoice / auto_cancel_e_waybill and let the runner manage the transaction. "Pending Cancellation" (set by the cancel itself, already committed) survives a job rollback for reconciliation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQModrsTicyLZjFRMp54UG

Collapse the remaining multi-line docstrings/comments added in this PR to blunt one-liners and drop ones that just restate the code. No behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQModrsTicyLZjFRMp54UG

…ncel The enable/auto_cancel/irn/ewaybill checks in cancel_e_waybill_e_invoice were identical to the guards inside auto_cancel_e_invoice / auto_cancel_e_waybill, which the job runs anyway. Keep only the cheap "anything on the portal + API enabled" gate to avoid enqueuing a no-op job on every cancel; let the job do the settings/auto-cancel gating. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQModrsTicyLZjFRMp54UG

coderabbitai · 2026-07-06T10:07:34Z

📝 Walkthrough

Walkthrough

Sales Invoice cancellation of e-Invoice/e-Waybill is changed from a synchronous call to an after-commit enqueued job (cancel_e_invoice_e_waybill_after_commit), gated on IRN/e-Waybill presence and API enablement. The direct doc.cancel() side-effect is removed from the internal e-invoice cancellation helper, with docstrings clarified accordingly. The client-side cancel dialog now hides before the server call and invokes either a provided callback or frm.savecancel() instead of always refreshing the form. Tests are updated to mock enqueue behavior, invoke the new helper directly, and assert intermediate cancellation state.

Changes

Refactored cancel_e_waybill_e_invoice to enqueue an after-commit background job instead of synchronous auto-cancel calls.
Added cancel_e_invoice_e_waybill_after_commit(docname) performing the deferred portal cancellation logic.
Removed doc.cancel() call from _cancel_e_invoice; updated related docstrings.
Updated client-side cancel dialog to hide before server call and conditionally run callback or frm.savecancel().
Updated tests to mock frappe.enqueue, invoke the new after-commit helper directly, and assert intermediate portal-cancelled invoice state.

Related PRs: None identified.

Suggested labels: enhancement, e-invoicing, e-waybill

Suggested reviewers: None identified.

Poem
A rabbit hops past commit's gate,
No more sync calls, we now await —
Enqueue the job, let commit land,
Then cancel portal, close at hand.
Dialogs hide, forms savecancel true,
Tests confirm the whole flow through. 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: deferring portal cancellation to an after-commit job.
Description check	✅ Passed	The description is detailed and directly matches the cancellation flow changes in this PR.

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.}

codacy-production · 2026-07-06T10:10:11Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 5 complexity

Metric Results

Complexity 5

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

greptile-apps · 2026-07-06T10:17:06Z

Confidence Score: 5/5

Safe to merge; the deferral logic is correctly implemented and the key invariants (no synchronous portal call, job discarded on rollback) are verified by tests.

The separation of portal cancel from SI cancel is well-reasoned and internally consistent. The deferred job correctly re-checks API availability and feature flags, handles already-cleared IRN/eWaybill gracefully, and the 9999 error-code path makes portal cancel idempotent. The JS callback order (model sync before savecancel) means the before_cancel guard sees the updated frm.doc.irn and does not re-open the dialog. No data-loss or state-divergence paths were found.

No files require special attention.

Important Files Changed

Filename	Overview
india_compliance/gst_india/utils/e_invoice.py	New cancel_e_invoice_e_waybill_after_commit deferred job added; _cancel_e_invoice no longer calls doc.cancel(), cleanly separating portal cancel from SI cancel
india_compliance/gst_india/overrides/sales_invoice.py	cancel_e_waybill_e_invoice refactored to enqueue a deferred job; now also does an early irn/ewaybill check before calling is_api_enabled()
india_compliance/gst_india/client_scripts/e_invoice_actions.js	Dialog primary action now hides before API call and calls frm.savecancel() for standalone cancellation; dialog labels still reflect the old combined flow
india_compliance/gst_india/utils/test_e_waybill.py	test_auto_cancel_e_waybill_on_si_cancel updated to patch enqueue and call the deferred job directly; test_portal_cancel_is_deferred_to_after_commit added to verify deferral semantics
india_compliance/gst_india/utils/test_e_invoice.py	test_cancel_e_invoice updated to verify two-step flow: portal cancel leaves SI submitted, then SI cancel completes

_{Reviews (2): Last reviewed commit: "test(e-waybill): run deferred cancel job..." | Re-trigger Greptile}

CI failure fixes: - The unit-test harness runs the enqueued after-commit job during the test, and a direct doc.cancel() exception does not auto-roll back (no request handler), so the two new/updated tests asserted the wrong thing. - Revert test_auto_cancel_e_waybill_on_si_cancel to its original end-state assertions (the deferred job runs during the test, so the e-Waybill ends cancelled). - Replace the rollback test with test_portal_cancel_is_deferred_to_after_commit: call cancel_e_waybill_e_invoice directly with frappe.enqueue mocked and assert no synchronous portal call + enqueue_after_commit=True. Deterministic, no dependency on worker/rollback timing. Review feedback (Greptile P2): re-check is_api_enabled() inside cancel_e_invoice_e_waybill_after_commit, since the API could be disabled between enqueue and execution; skip cleanly instead of failing with a config error. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQModrsTicyLZjFRMp54UG

…er race test_auto_cancel_e_waybill_on_si_cancel was flaky: the after-commit job runs on a worker (separate connection), so its db_set clearing si.ewaybill wasn't visible to the test's transaction (while the Log's is_cancelled update was), causing `assertEqual(si.ewaybill, "")` to fail. Suppress the worker dispatch during si.cancel() (mock frappe.enqueue) and run cancel_e_invoice_e_waybill_after_commit in-process so its writes land in the test's own transaction. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQModrsTicyLZjFRMp54UG

claude added 7 commits July 6, 2026 08:52

greptile-apps Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread india_compliance/gst_india/utils/e_invoice.py

claude added 2 commits July 6, 2026 10:26

vorasmit assigned Abdeali099 Jul 6, 2026

vorasmit added the squash label Jul 7, 2026

Abdeali099 changed the title ~~Defer e-Invoice/e-Waybill portal cancellation to after-commit job~~ fix: Defer e-Invoice/e-Waybill portal cancellation to after-commit job Jul 8, 2026

vorasmit assigned ljain112 and unassigned Abdeali099 Jul 8, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Defer e-Invoice/e-Waybill portal cancellation to after-commit job#4546

fix: Defer e-Invoice/e-Waybill portal cancellation to after-commit job#4546
vorasmit wants to merge 9 commits into
developfrom
claude/einvoice-ewaybill-api-analysis-ubilvq

vorasmit commented Jul 6, 2026

Uh oh!

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

codacy-production Bot commented Jul 6, 2026

Uh oh!

greptile-apps Bot commented Jul 6, 2026 •

edited

Loading

Important Files Changed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

vorasmit commented Jul 6, 2026

Summary

Key Changes

Implementation Details

Uh oh!

coderabbitai Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

codacy-production Bot commented Jul 6, 2026

Up to standards ✅

Uh oh!

greptile-apps Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading

greptile-apps Bot commented Jul 6, 2026 •

edited

Loading