Skip to content

feat(ims): declare ITC reduction for credit notes on IMS save (CR25787E)#4473

Open
vorasmit wants to merge 6 commits into
developfrom
claude/elegant-gates-mxg6xh
Open

feat(ims): declare ITC reduction for credit notes on IMS save (CR25787E)#4473
vorasmit wants to merge 6 commits into
developfrom
claude/elegant-gates-mxg6xh

Conversation

@vorasmit

@vorasmit vorasmit commented Jun 25, 2026

Copy link
Copy Markdown
Member

Problem

GST IMS release CR25787E (Aug 2025) made the ITC-reduction declaration mandatory on accept of "specified records" — Credit Notes, CN amendments, and B2B/Debit-Note downward amendments. Our IMS save (convert_data_to_gov_format) never sent itcRedReq / declIgst/Cgst/Sgst/Cess, so the portal partially rejected exactly those records while plain B2B accepts succeeded.

What this does

Phase 1 — make Save succeed (backend)

  • New fields on GST Inward Supply: itc_reduction_required, declared_igst/cgst/sgst/cess, remarks, is_itc_reduction_blocked, is_remarks_blocked (read-only; synced via bench migrate).
  • Download captures the new GET-response fields. A re-download keeps a record's un-uploaded declared values (the portal returns stale values until we upload), mirroring how ims_action is preserved.
  • On accept of a matched specified record, declared ITC is computed from books (linked Purchase Invoice tax) and stored: per head, ≤ ₹1 book-vs-supplier diff → use the supplier value (books may carry rounding noise), else min(books, supplier) (can't exceed the document — "usually less only"); CGST is kept equal to SGST.
  • Save emits the declared block only for the four specified categories (IMSB2BA/B2BDNA/B2BCN/B2BCNA), on accept, when the govt hasn't blocked it (isItcRedReqBlocked); remarks is sent on reject/pending. Govt-blocked records are suppressed.
  • Credit-note reject sends the action only — no declared block (declaration is accept-only).
  • Unmatched specified accepts (no linked PI) are skipped and deferred with a notice.
  • update_action is scoped to the company's own invoices (record-level isolation).

Phase 2 — review & correct (frontend)

  • On accept, records where books differ from the supplier value by > ₹1 per head open a single review dialog (the rest auto-declare silently). One table, pre-filled with the books value (clamped to [0, supplier]), with "use books / use supplier" bulk-fill, CGST/SGST kept equal, supplier-controlled values HTML-escaped, and an alert if dismissed without confirming.
  • The single detail-view accept is just the N = 1 case of the same dialog.
  • update_action takes declared_overrides; user corrections are stored and take precedence over the books default (clamped server-side; blocked records skipped).

Decisions (confirmed with the requester)

Question Decision
Unmatched specified accept Skip + defer to the Phase 2 dialog
CN reject Action only, no declared block
Govt-blocked record Suppress itcRedReq/declared (and remarks)
Rounding tolerance ≤ ₹1 per tax head (repo's existing reconciliation convention)

Tests

  • _declared_from_books (tolerance snap / cap / books-lower / zero), gov-format emit/suppress/reject/non-specified, defer-undeclarable filter, set_declared_itc on accept, declared-override capping, re-download preservation.
  • Download mapping asserted on b2bcn in test_ims.py.

Out of scope

ecoma (ECO-document downward amendment) — not yet exposed in the app's IMS download sections; defer until GSTN wires it in.

no-docs: GST-portal compliance fix; the user-facing ITC-reduction dialog can be documented separately in india-compliance-docs if desired.

🤖 Generated with Claude Code

https://claude.ai/code/session_0121aivgGxwtympU3ojykJmh

claude added 2 commits June 25, 2026 11:30
Govt made itcRedReq and declared ITC amounts mandatory on accept of
credit notes and downward amendments, so IMS save partially failed for
those records.

- Capture new GET fields (itcRedReq, declIgst/Sgst/Cgst/Cess, remarks,
  isItcRedReqBlocked, isRemarksBlocked) on GST Inward Supply.
- On accept of a matched specified record, store declared ITC from
  books (linked PI tax): snap to supplier value within Re 1, else cap
  at document tax (usually less); CGST = SGST.
- Emit the declared block on save only for specified records, on
  accept, when govt allows; emit remarks on reject/pending.
- Skip and defer specified accepts with no linked PI to Phase 2.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0121aivgGxwtympU3ojykJmh
Phase 2 of CR25787E. On accept, records where books differ from the
supplier value beyond rounding (> Re 1 per tax head) now open a single
review dialog instead of silently using books.

- Exceptions-only: records matching within Re 1 still auto-declare the
  supplier value; only the divergent ones are surfaced.
- One table, pre-filled with books value (capped at supplier), with
  "use books / use supplier" bulk-fill; CGST kept equal to SGST and
  values clamped to the supplier tax.
- update_action accepts declared_overrides; user corrections are stored
  on GST Inward Supply and take precedence over the books default.
- Expose declared/blocked fields to the dashboard for the dialog.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0121aivgGxwtympU3ojykJmh
@codacy-production

codacy-production Bot commented Jun 25, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 102 complexity

Metric Results
Complexity 102

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

greptile-apps Bot commented Jun 25, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

Safe to merge after reviewing the P2 comments; none block correct portal submissions under the primary code paths covered by the test suite.

The core declaration logic is sound and all previous blocking issues are addressed. Remaining concerns are edge-case re-accepts silently discarding user overrides, potential itcRedReq=Y emission for upward amendments if the portal does not set isItcRedReqBlocked=Y, and None stored in numeric fields for absent portal declX values.

init.py (set_declared_itc and _declared_from_books interaction with itc_reduction_required), gst_invoice_management_system.py (re-accept override reset), and ims.py (None defaults for absent portal fields).

Important Files Changed

Filename Overview
india_compliance/gst_india/doctype/gst_invoice_management_system/init.py Adds ITC reduction declaration logic (set_declared_itc, apply_declared_overrides, defer_undeclarable_itc_reduction, is_specified_record, _declared_from_books). Core math is correct; _cap_declared properly applies [0, supplier] clamping. Minor concern: set_declared_itc may overwrite a portal-supplied itc_reduction_required=0 for records where the portal says no reduction is needed but is_itc_reduction_blocked is N.
india_compliance/gst_india/doctype/gst_invoice_management_system/gst_invoice_management_system.py update_action now adds company_gstin scoping, calls set_declared_itc then apply_declared_overrides in correct order, and applies defer_undeclarable_itc_reduction at the upload path. Addresses all previously flagged security/isolation issues.
india_compliance/gst_india/doctype/gst_invoice_management_system/gst_invoice_management_system.js Adds ITCReductionDialog with bulk-fill buttons, per-head clamping, and CGST=SGST equalization. onhide feedback added. Bulk-fill buttons don't trigger change events so equalization feedback is absent in the table until Confirm is clicked (harmless since confirm() always equalizes).
india_compliance/gst_india/utils/gstr_2/ims.py EMITS_ITC_REDUCTION class flag added to B2BA, B2BDNA, B2BCN, B2BCNA. set_itc_reduction correctly suppresses declared block for non-specified, govt-blocked, and non-accept cases. declX fields mapped as None for absent portal fields — should default to 0.
india_compliance/gst_india/doctype/gst_inward_supply/gst_inward_supply.py preserve_pending_itc_declaration correctly guards declared ITC fields from re-download overwrite when there is a pending action, while allowing portal flags (is_itc_reduction_blocked) to refresh.
india_compliance/gst_india/doctype/gst_inward_supply/gst_inward_supply.json Adds ITC Reduction section with six new read-only fields. Schema change requires bench migrate.
india_compliance/gst_india/doctype/gst_invoice_management_system/test_gst_invoice_management_system.py Good coverage: _declared_from_books, gov-format emit/suppress/reject/non-specified, defer_undeclarable filter, set_declared_itc on accept, preserve_pending_itc_declaration, and declared-override capping.
india_compliance/gst_india/utils/gstr_2/test_ims.py Asserts download mapping of new ITC reduction fields on b2bcn record.
india_compliance/gst_india/data/test_ims.json Adds itcRedReq/isItcRedReqBlocked/isRemarksBlocked/declX/remarks fields to the b2bcn fixture, matching the CR25787E portal API shape.

Reviews (3): Last reviewed commit: "fix(ims): preserve pending declared ITC ..." | Re-trigger Greptile

Comment thread india_compliance/gst_india/doctype/gst_invoice_management_system/__init__.py Outdated
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds ITC reduction fields and handling for GST IMS invoices. It updates schema and query shapes, emits declared ITC values during IMS conversion, adds an accepted-action dialog for declared overrides, stores those values server-side, defers undeclarable uploads, and extends tests.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.98% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding ITC reduction handling for IMS save under CR25787E.
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.
Description check ✅ Passed The description clearly matches the ITC-reduction IMS backend, frontend, and test changes in this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
india_compliance/gst_india/doctype/gst_invoice_management_system/gst_invoice_management_system.py (1)

149-193: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Constrain declared overrides to the selected company-scoped invoices.

The new declared_overrides map is client-controlled, but Lines 190-193 pass all keys through to apply_declared_overrides. A crafted whitelisted call can persist declared ITC values for records outside the selected action set.

Proposed fix
         invoice_names = frappe.parse_json(invoice_names)
+        allowed_invoice_names = set(
+            frappe.get_all(
+                "GST Inward Supply",
+                filters={"name": ["in", invoice_names], "company_gstin": self.company_gstin},
+                pluck="name",
+            )
+        )
+        invoice_names = [name for name in invoice_names if name in allowed_invoice_names]
         GSTR2 = frappe.qb.DocType("GST Inward Supply")
         if declared_overrides:
             if isinstance(declared_overrides, str):
                 declared_overrides = frappe.parse_json(declared_overrides)
-            apply_declared_overrides(declared_overrides)
+            declared_overrides = {
+                name: values
+                for name, values in declared_overrides.items()
+                if name in allowed_invoice_names
+            }
+            if declared_overrides:
+                apply_declared_overrides(declared_overrides)

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 133dc606-448d-44a3-a282-fd9d74a4e74e

📥 Commits

Reviewing files that changed from the base of the PR and between c3bf414 and 40bd0c4.

📒 Files selected for processing (8)
  • india_compliance/gst_india/data/test_ims.json
  • india_compliance/gst_india/doctype/gst_invoice_management_system/__init__.py
  • india_compliance/gst_india/doctype/gst_invoice_management_system/gst_invoice_management_system.js
  • india_compliance/gst_india/doctype/gst_invoice_management_system/gst_invoice_management_system.py
  • india_compliance/gst_india/doctype/gst_invoice_management_system/test_gst_invoice_management_system.py
  • india_compliance/gst_india/doctype/gst_inward_supply/gst_inward_supply.json
  • india_compliance/gst_india/utils/gstr_2/ims.py
  • india_compliance/gst_india/utils/gstr_2/test_ims.py

Comment thread india_compliance/gst_india/doctype/gst_invoice_management_system/__init__.py Outdated
Comment thread india_compliance/gst_india/doctype/gst_invoice_management_system/__init__.py Outdated
claude added 4 commits June 25, 2026 14:17
- Scope update_action to the company's own invoices (record-level
  isolation) and filter declared_overrides to that set, so a crafted
  request cannot touch another company's records.
- Clamp declared amounts to [0, document] on both server paths and in
  the dialog's confirm(); keep CGST = SGST via min so neither head
  exceeds its supplier cap.
- Skip GSTN-blocked records in set_declared_itc so their stored
  declaration is not overwritten.
- Escape supplier name and bill no before injecting into the dialog.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0121aivgGxwtympU3ojykJmh
Match the repo's pinned ruff 0.15.8 (line-length 110, import sort) and
prettier 3.1.0 to fix the linters check.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0121aivgGxwtympU3ojykJmh
Satisfy frappe-semgrep missing-argument-type-hint on the whitelisted
endpoint.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0121aivgGxwtympU3ojykJmh
- Keep the user's un-uploaded declared ITC when IMS is re-downloaded
  (the portal returns stale values until we upload), mirroring how
  ims_action is already preserved across re-download.
- Skip GSTN-blocked records in apply_declared_overrides.
- Make the set_declared_itc test deterministic by computing the
  expected declaration from the actual linked PI tax; add a unit test
  for the re-download preservation.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0121aivgGxwtympU3ojykJmh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants