Skip to content

feat: support taxable value overrides for custom resolvers - #4452

Open
vorasmit wants to merge 17 commits into
resilient-tech:developfrom
vorasmit:taxable_value_overrides
Open

vorasmit wants to merge 17 commits into
resilient-tech:developfrom
vorasmit:taxable_value_overrides

Conversation

@vorasmit

@vorasmit vorasmit commented Jun 23, 2026 •

Copy link
Copy Markdown
Member

Custom Taxable Overrides

Taxation in India could have

  • RSP (MRP) based taxes
  • Margin based taxes

This implementation gives standard implementation for above + guidelines on how it can be extended for other use cases.

Other Implications of above

RSP

Taxable Value: Selling Price (net of taxes)
Taxes: Based on MRP

  • e-Invoice: Govt allows extra taxes (non-proportional) to taxable value for select HSN Codes.
  • GSTR-1: Similar presentation (non-proportional) taxes in GSTR-1

Margin Scheme

Taxable Value: Sales - Purchase (Margin)
Taxes: Based on Margin

  • e-Invoice: Purchase Price becomes other charges
  • GSTR-1: Purchase price becomes difference of Invoice value and Taxable Value when reported in GSTR-1.

Recording / Screenshots

Screen.Recording.2026-06-25.at.12.08.31.PM.mov
image image image image

TODOs

  • Taxes inclusive in MRP?
  • How is difference in invoice value reported on e-Invoice / GSTR-1?
  • How do you handle cases where one item is on a specific charge type and other item is on different charge type? - Not allowed by design
  • Purchase Side - similarly extensible. Ignored for now unless a genuine usecase for this is identified.

Govt Advisory: https://tutorial.gst.gov.in/downloads/news/advisory_on_rsp_based_valuation_gstr-1_final_version.pdf
HSNs Allowed: https://einv-apisandbox.nic.in/downloads/RSP.pdf

Depends on: frappe/erpnext#56175

no-docs

@codacy-production

codacy-production Bot commented Jun 23, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 75 complexity

Metric Results
Complexity 75

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.

@vorasmit
vorasmit force-pushed the taxable_value_overrides branch from 58f0bfd to 3946323 Compare June 24, 2026 12:38
@vorasmit
vorasmit marked this pull request as ready for review June 25, 2026 06:37
@coderabbitai

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@greptile-apps

greptile-apps Bot commented Jun 25, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[High risk] Adds new tax calculation methods for GST compliance.

No outstanding finding blocks merging.

What we checked:

  • T-Rex produced a proof for a posted P1 finding and linked it to the review comment detailing the finding. T-Rex
  • T-Rex executed the GST charge toggle repro script to validate the On MRP behavior for POST /api/method/frappe.desk.form.save.savedocs, and collected before/after logs that show 403 Forbidden - No permission for Property Setter in both runs. T-Rex

Reviews (3) · Last reviewed commit: "chore: trim taxable value docstrings and..."

Comment thread india_compliance/gst_india/doctype/gst_settings/gst_settings.json Outdated
Comment thread india_compliance/public/js/taxes_controller.js Outdated
@vorasmit
vorasmit force-pushed the taxable_value_overrides branch from 73573c4 to f34754d Compare July 29, 2026 04:36

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


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 10fc248e-8100-4bfa-a8cf-94b2263e2a4e

📥 Commits

Reviewing files that changed from the base of the PR and between 73573c4 and f34754d.

📒 Files selected for processing (14)
  • india_compliance/gst_india/constants/custom_fields.py
  • india_compliance/gst_india/doctype/gst_settings/gst_settings.json
  • india_compliance/gst_india/doctype/gst_settings/gst_settings.py
  • india_compliance/gst_india/overrides/taxable_value.py
  • india_compliance/gst_india/overrides/test_transaction.py
  • india_compliance/gst_india/overrides/transaction.py
  • india_compliance/gst_india/setup/__init__.py
  • india_compliance/gst_india/setup/property_setters.py
  • india_compliance/gst_india/utils/test_e_invoice.py
  • india_compliance/gst_india/utils/tests.py
  • india_compliance/hooks.py
  • india_compliance/patches.txt
  • india_compliance/public/js/india_compliance.bundle.js
  • india_compliance/public/js/taxable_base_resolvers.js
🚧 Files skipped from review as they are similar to previous changes (9)
  • india_compliance/gst_india/utils/tests.py
  • india_compliance/gst_india/setup/property_setters.py
  • india_compliance/public/js/taxable_base_resolvers.js
  • india_compliance/gst_india/constants/custom_fields.py
  • india_compliance/gst_india/doctype/gst_settings/gst_settings.json
  • india_compliance/gst_india/overrides/transaction.py
  • india_compliance/gst_india/setup/init.py
  • india_compliance/gst_india/doctype/gst_settings/gst_settings.py
  • india_compliance/gst_india/overrides/taxable_value.py

Comment thread india_compliance/gst_india/overrides/test_transaction.py
@vorasmit
vorasmit force-pushed the taxable_value_overrides branch from e803e88 to 60a6388 Compare August 14, 2026 08:01
- apply item wise tax template
- fix return on margin type tax
Comment thread india_compliance/gst_india/constants/custom_fields.py Outdated
Comment thread india_compliance/gst_india/constants/custom_fields.py
val = request_data["ValDtls"]

self.assertEqual(item["AssAmt"], 100)
self.assertEqual(item["OthChrg"], 0)

@Anish59312 Anish59312 Aug 18, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

check for TotItemVal == 100

@ljain112

ljain112 commented Oct 6, 2026

Copy link
Copy Markdown
Member

@greptileai review

@greptile-apps

greptile-apps Bot commented Oct 6, 2026

Copy link
Copy Markdown

Comments Outside Diff

These findings could not be posted inline.

  • P1 Accounts User cannot save GST charge-type toggle changes ▶

    • Bug
      • GST Settings grants Accounts User write access, but enabling On MRP requires creating a Property Setter and disabling its existing override requires deleting one. The isolated before and after runs both reach the respective Frappe permission gates and return modeled 403 outcomes. The GST Settings change cannot be saved; the same root cause also applies to the On Margin toggle when it takes these paths.
    • Cause
      • india_compliance/gst_india/doctype/gst_settings/gst_settings.py:228-233 invokes toggle_charge_type_options during on_update. Its branches at india_compliance/gst_india/setup/property_setters.py:123-139 call frappe.delete_doc without ignore_permissions=True or frappe.make_property_setter, respectively. Frappe's frappe/__init__.py:1201-1256 inserts the setter without an ignore-permissions flag; frappe/model/document.py:733 checks create permission. Frappe's model/delete_doc.py:306-313 checks delete permission. gst_settings.json:709-727 grants Accounts User write, whereas Frappe's custom/doctype/property_setter/property_setter.json:135-157 does not grant that role create or delete.
    • Fix
      • Perform the charge-type Property Setter create/delete as an explicitly authorized internal operation during the GST Settings save, narrowly bypassing Property Setter permissions after GST Settings write permission has been checked.
  • P2 get tax amount ignores deemed taxable value for RSP resolver india_compliance/public/js/taxes_controller.js:211 ▶

    get_tax_amount uses item.taxable_value / 100 as the multiplier for every ad-valorem row. For an "On MRP" item the update_item_taxable_value path sets row.taxable_value to the net sale amount (because _dont_update_taxable_value is true), while the actual tax base is item._deemed_taxable_value (the RSP-derived base). The Python counterpart in ItemGSTDetails.get_item_tax_amount explicitly falls back to _deemed_taxable_value for this reason. If the custom controller is ever applied to a document with "On MRP" taxes, the client-side tax totals will be computed on the net amount instead of the deemed RSP base, diverging from the server-side calculation.

This branch has not been deployed

No deployments
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