Skip to content

fix: always set correct gst treatment#3751

Merged
vorasmit merged 1 commit intoresilient-tech:developfrom
ljain112:fix-gst-treatment-incorrect
Oct 16, 2025
Merged

fix: always set correct gst treatment#3751
vorasmit merged 1 commit intoresilient-tech:developfrom
ljain112:fix-gst-treatment-incorrect

Conversation

@ljain112
Copy link
Copy Markdown
Member

@ljain112 ljain112 commented Oct 16, 2025

Issue: incorrect gst treatment if the item tax template is changed from the backend or the item tax template is updated from exempted to taxable.

Steps to replicate:

  • Set the item tax template in the item as 18%.
  • Create a Sales Invoice and save it.
  • Item tax template will be 18% and gst treatment will be taxable.
  • Change the item tax template in Item to Exempted
  • Again, save the invoice.

GST treatment will be Taxable, but the Item Tax Template will be Exempted because erpnext changed it.

Summary by CodeRabbit

  • Refactor
    • Optimized GST treatment mapping logic for improved transaction processing efficiency.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 16, 2025

Walkthrough

Updated GST treatment assignment in transaction.py: update_gst_treatment_map now directly builds self.gst_treatment_map by querying Item Tax Template names; set_default_treatment now unconditionally assigns item.gst_treatment from that map before applying fallback defaults.

Changes

Cohort / File(s) Summary
GST Treatment Mapping & Defaults
india_compliance/gst_india/overrides/transaction.py
Removed intermediate collection/conditional branching in update_gst_treatment_map and directly populated self.gst_treatment_map from Item Tax Template names; set_default_treatment now unconditionally sets item.gst_treatment = self.gst_treatment_map.get(item.item_tax_template) and then applies default_treatment when needed.

Sequence Diagram(s)

sequenceDiagram
  participant TX as transaction.py
  participant DB as ItemTaxTemplate DB
  participant Item as Item

  rect rgb(235, 248, 255)
    Note over TX,DB: New flow — direct map build
    TX->>DB: query names for item_templates set
    DB-->>TX: returns mapping (item_tax_template -> gst_treatment)
    TX->>Item: item.gst_treatment = gst_treatment_map.get(item.item_tax_template)
    alt no treatment
      TX->>Item: apply default_treatment
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • vorasmit
  • karm1000

Poem

🐰 Hopping through mappings, neat and spry,

I fetch treatments from templates high.
No branching burrows, just one bright path,
Defaults ready for any aftermath.
A little hop, a tidy map — bye-bye math!

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 PR title "fix: always set correct gst treatment" directly aligns with the primary objective of the changeset. The PR addresses a bug where GST treatment on Sales Invoices remained incorrect when an Item's tax template was changed in the backend. The title succinctly captures the fix's core purpose—ensuring GST treatment is always correctly set in response to tax template changes. The title is concise, clear, and specific enough for a teammate to understand the main change without additional context.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 847a328 and 31690a4.

📒 Files selected for processing (1)
  • india_compliance/gst_india/overrides/transaction.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • india_compliance/gst_india/overrides/transaction.py
⏰ 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). (4)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Python Unit Tests
  • GitHub Check: Mergify Merge Protections
  • GitHub Check: Summary

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.

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.59%. Comparing base (50820eb) to head (31690a4).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3751      +/-   ##
===========================================
- Coverage    69.60%   69.59%   -0.02%     
===========================================
  Files          182      182              
  Lines        17974    17967       -7     
===========================================
- Hits         12511    12504       -7     
  Misses        5463     5463              
Files with missing lines Coverage Δ
...ndia_compliance/gst_india/overrides/transaction.py 83.35% <100.00%> (-0.14%) ⬇️

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ljain112 ljain112 force-pushed the fix-gst-treatment-incorrect branch from 847a328 to 31690a4 Compare October 16, 2025 10:22
@ljain112 ljain112 requested review from karm1000 and vorasmit October 16, 2025 10:28
@vorasmit vorasmit merged commit 6965ff1 into resilient-tech:develop Oct 16, 2025
14 checks passed
mergify Bot added a commit that referenced this pull request Oct 16, 2025
…tfix/pr-3751

fix: always set correct gst treatment (backport #3751)
mergify Bot added a commit that referenced this pull request Oct 16, 2025
…tfix/pr-3751

fix: always set correct gst treatment (backport #3751)
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.

2 participants