fix: enhance item-wise tax detail handling and validation in GST transactions#3807
Conversation
WalkthroughReplace item-code keyed item-wise GST handling with item-name keyed structures across transaction processing, tax utilities, and post-install/patched compilation flows; adjust validation to compare provided item-wise tax details against computed taxes and update tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Patch as compile_docs
participant Doc as Document
participant Utils as TaxesController
participant ItemGST as ItemGSTDetails
Note right of Patch `#f6f3e8`: New param item_taxes added
Patch->>Doc: attach item_wise_tax_details (from item_taxes)
Doc->>Utils: set_temp_item_wise_tax_detail_object()
Utils->>Doc: build `_item_wise_tax_details` keyed by item_name
Doc->>ItemGST: get(docs, doctype, company)
ItemGST->>ItemGST: get_item_name_wise_tax_details()
ItemGST->>ItemGST: set_item_defaults() & compute taxes per item_name
ItemGST->>Doc: set_item_name_wise_tax_details() and validate
alt mismatch detected
ItemGST->>Doc: raise ValidationError("Item Wise Tax Details do not match...")
else success
ItemGST->>Doc: aggregate rounding & tax differences
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (15)📓 Common learnings📚 Learning: 2025-11-21T08:53:26.295ZApplied to files:
📚 Learning: 2025-10-01T10:54:11.096ZApplied to files:
📚 Learning: 2025-06-25T08:19:02.607ZApplied to files:
📚 Learning: 2025-04-25T10:12:24.584ZApplied to files:
📚 Learning: 2025-05-26T03:16:30.230ZApplied to files:
📚 Learning: 2025-06-25T08:16:18.010ZApplied to files:
📚 Learning: 2025-07-22T11:45:43.432ZApplied to files:
📚 Learning: 2025-06-19T08:21:29.308ZApplied to files:
📚 Learning: 2025-09-30T13:12:35.995ZApplied to files:
📚 Learning: 2025-05-29T15:22:04.761ZApplied to files:
📚 Learning: 2025-06-20T11:11:54.124ZApplied to files:
📚 Learning: 2025-06-25T08:19:20.439ZApplied to files:
📚 Learning: 2025-04-25T11:12:59.799ZApplied to files:
📚 Learning: 2025-11-11T12:44:44.690ZApplied to files:
🧬 Code graph analysis (1)india_compliance/gst_india/overrides/transaction.py (1)
⏰ 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)
🔇 Additional comments (8)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
india_compliance/patches/v14/update_item_gst_details_and_gst_trearment_in_bill_of_entry.py (1)
92-113: Bill of Entry tax query omitsgst_tax_type, breakingCustomItemGSTDetailsprocessing
CustomItemGSTDetails.is_gst_tax_rowfilters rows usingrow.gst_tax_typeandrow.get(self.tax_details_field()), andget_item_name_wise_tax_detailsexpectsgst_tax_typeto be present on each tax row. However,get_taxes_query()currently selects onlytax_amount,account_head,parent, anditem_wise_tax_rates. For the Bill of Entry patch run:
- Every tax row will fail
is_gst_tax_row, so no GST rows are processed.CustomItemGSTDetails().get(...)will then produce only default (zero) item tax details, andbuild_query_and_update_gst_detailswill overwrite item GST fields with zeros.You should include
gst_tax_typein the query (and optionallynameif needed later), for bothBill of Entry TaxesandIndia Compliance Taxes and Charges.Example adjustment:
def get_taxes_query(docs, doctype, taxes): return ( frappe.qb.from_(taxes) .select( - taxes.tax_amount, - taxes.account_head, - taxes.parent, - taxes.item_wise_tax_rates, + taxes.tax_amount, + taxes.account_head, + taxes.parent, + taxes.item_wise_tax_rates, + taxes.gst_tax_type, ) .where(taxes.parenttype == doctype) .where(taxes.parent.isin(docs)) )(Adapt field names if the Bill of Entry tax doctypes use a slightly different schema.)
🧹 Nitpick comments (4)
india_compliance/patches/post_install/improve_item_tax_template.py (1)
424-471:compile_docscorrectly aggregatesitem_wise_tax_detailsper voucher (small DRY opportunity)The extended
compile_docsnow initialises each doc withtaxes,items, anditem_wise_tax_detailsand appends rows from all three sources, which matches howItemGSTDetails.get_item_name_wise_tax_detailsexpects to consume data. The repeated initialisation blocks for taxes/items/item_taxes are slightly duplicated; if you touch this again, consider a tiny helper like_get_or_init_doc(response, parent, doctype)to centralise the default structure.india_compliance/gst_india/overrides/transaction.py (3)
138-158:validate_item_wise_tax_detailcorrectly targets problematic Actual charges, but depends on_item_wise_tax_detailsbeing populatedThe new validation only inspects
doc._item_wise_tax_detailsand throws when an “Actual” charge has an item-level amount but a zero/empty rate. That mirrors the intended error message and avoids touching non-Actual rows.Just be aware that this check is a no-op unless some upstream code (e.g.
CustomItemGSTDetails.set_temp_item_wise_tax_detail_objector ERPNext core) populates_item_wise_tax_details. If there are cases where item-wise details exist but_item_wise_tax_detailsis not set, you may want to either:
- Populate
_item_wise_tax_detailsearlier in the flow for those doctypes, or- Extend this validation to also look at
item_wise_tax_details/ other canonical structures.
1159-1232: Item-level recomputation via_item_wise_tax_detailslooks sound; just ensure row structure is consistentThe updated
set_item_name_wise_tax_detailsplusset_item_defaultscorrectly:
- Zero out all GST rate/amount fields per item.
- Accumulate
tax_differencesfrom GST tax rows usingtax_amount_field().- Walk
doc._item_wise_tax_detailsentries{ "item": ..., "tax": ..., "rate": ... }to compute per-item tax amounts and adjust the last taxable item for rounding.This is coherent with the new
_item_wise_tax_detailsstructure introduced inCustomItemGSTDetails. The main assumption is that every entry has a non-Noneitemandtax; if another caller ever inserts a malformed entry,item.update(...)andself.is_gst_tax_row(tax_row)will raise.If you expect any external producers of
_item_wise_tax_details, a small guard would harden this:for row in self.doc.get("_item_wise_tax_details") or []: item = row.get("item") tax_row = row.get("tax") if not item or not tax_row: continue ...Otherwise, the recomputation and rounding-difference handling look correct.
1253-1306: Guard against staleItem Wise Tax Detailrows referencing missing itemsThe refactored
get_item_name_wise_tax_detailsbuildsitem_map/tax_mapkeyed by row.name and then does:tax_row = tax_map.get(row.get("tax_row")) item = item_map.get(row.get("item_row")) ... item_taxes = tax_details[item.name]If an
Item Wise Tax Detailentry points to an item_row (or tax_row) that no longer exists indoc.items/doc.taxes(e.g. due to manual data fixes or earlier partial deletions),itemortax_rowcan beNone, and this will raise an exception.The previous JSON-based implementation explicitly skipped unknown items; mirroring that behaviour here would make the patch more robust:
for row in self.doc.get("item_wise_tax_details") or []: tax_row = tax_map.get(row.get("tax_row")) item = item_map.get(row.get("item_row")) if not tax_row or not item or not self.is_gst_tax_row(tax_row): continue ...That keeps the computation resilient to slightly inconsistent historical data, which is often what these patches are run against.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
india_compliance/gst_india/overrides/test_transaction.py(1 hunks)india_compliance/gst_india/overrides/transaction.py(7 hunks)india_compliance/gst_india/utils/taxes_controller.py(2 hunks)india_compliance/patches/post_install/improve_item_tax_template.py(3 hunks)india_compliance/patches/v14/update_item_gst_details_and_gst_trearment_in_bill_of_entry.py(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3344
File: india_compliance/gst_india/report/account_wise_summary/account_wise_summary.py:81-87
Timestamp: 2025-04-25T10:12:24.584Z
Learning: When processing taxes in GST India reports, charges after GST rows with charge_type "On Previous Row Total" should not be used to compute taxable value, so a break statement is used to exit the tax processing loop once such a row is encountered.
📚 Learning: 2025-06-25T08:19:02.607Z
Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3354
File: india_compliance/gst_india/report/summary_of_itc_availed/summary_of_itc_availed.py:278-281
Timestamp: 2025-06-25T08:19:02.607Z
Learning: In the Summary of ITC Availed report (india_compliance/gst_india/report/summary_of_itc_availed/summary_of_itc_availed.py), the summary dictionaries created by get_initial_summary() are never empty - they always contain tax field keys (igst_amount, cgst_amount, sgst_amount, cess_amount) with 0 values, so checking for empty dictionaries is unnecessary.
Applied to files:
india_compliance/gst_india/utils/taxes_controller.pyindia_compliance/gst_india/overrides/test_transaction.pyindia_compliance/gst_india/overrides/transaction.pyindia_compliance/patches/v14/update_item_gst_details_and_gst_trearment_in_bill_of_entry.pyindia_compliance/patches/post_install/improve_item_tax_template.py
📚 Learning: 2025-10-01T10:54:11.096Z
Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3709
File: india_compliance/gst_india/utils/exporter.py:174-174
Timestamp: 2025-10-01T10:54:11.096Z
Learning: In india_compliance/gst_india/doctype/gstr_1/gstr_1_export.py, FIELD_TRANSFORMATIONS dictionary contains lambdas that are only called internally by DataProcessor.apply_transformations() with a single argument. These are separate from header transforms used with ExcelExporter.insert_data() which accept two arguments (value, row).
Applied to files:
india_compliance/gst_india/utils/taxes_controller.pyindia_compliance/gst_india/overrides/test_transaction.pyindia_compliance/gst_india/overrides/transaction.pyindia_compliance/patches/v14/update_item_gst_details_and_gst_trearment_in_bill_of_entry.pyindia_compliance/patches/post_install/improve_item_tax_template.py
📚 Learning: 2025-09-24T06:52:03.847Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3691
File: india_compliance/gst_india/report/gst_balance/gst_balance.js:46-67
Timestamp: 2025-09-24T06:52:03.847Z
Learning: In India Compliance GST reports, the get_accounting_dimensions() function does not include project and cost_center by default, so these filters need to be explicitly added when required in report configurations.
Applied to files:
india_compliance/gst_india/utils/taxes_controller.py
📚 Learning: 2025-04-25T10:12:24.584Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3344
File: india_compliance/gst_india/report/account_wise_summary/account_wise_summary.py:81-87
Timestamp: 2025-04-25T10:12:24.584Z
Learning: When processing taxes in GST India reports, charges after GST rows with charge_type "On Previous Row Total" should not be used to compute taxable value, so a break statement is used to exit the tax processing loop once such a row is encountered.
Applied to files:
india_compliance/gst_india/utils/taxes_controller.pyindia_compliance/gst_india/overrides/transaction.py
📚 Learning: 2025-05-29T15:22:04.761Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3399
File: india_compliance/gst_india/utils/gstr_1/test_gstr_1_books_data.py:791-822
Timestamp: 2025-05-29T15:22:04.761Z
Learning: In the india_compliance test suite, the IntegrationTestCase framework automatically handles database rollbacks after test completion, which means functions that modify global state (like GST Settings and Item Tax Templates) in setUpClass won't persist beyond the test run and don't require manual cleanup.
Applied to files:
india_compliance/gst_india/overrides/test_transaction.py
📚 Learning: 2025-05-26T03:16:30.230Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3344
File: india_compliance/gst_india/report/gst_account_wise_summary/gst_account_wise_summary.py:0-0
Timestamp: 2025-05-26T03:16:30.230Z
Learning: In GST account-wise summary reports, the filtering logic `doc.company_gstin != IfNull(doc[counterparty_gstin_field], "")` is intentionally designed to exclude internal transfers (where company GSTIN equals party GSTIN) while including transactions with unregistered parties (empty GSTIN). Internal transfers are excluded as they are only for internal reporting purposes like stock transfers between branches.
Applied to files:
india_compliance/gst_india/overrides/transaction.py
📚 Learning: 2025-07-30T10:15:18.756Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3532
File: india_compliance/utils/change_log_utils.py:55-57
Timestamp: 2025-07-30T10:15:18.756Z
Learning: For e-Waybill update functions in india_compliance/gst_india/utils/e_waybill.py, HTML escaping in change log comments may not be required because the e-Waybill API data typically doesn't contain HTML tags and the risk of XSS through vehicle/transporter information fields is considered low by the project maintainers.
Applied to files:
india_compliance/patches/v14/update_item_gst_details_and_gst_trearment_in_bill_of_entry.py
📚 Learning: 2025-07-22T11:45:43.432Z
Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3532
File: india_compliance/gst_india/utils/e_waybill.py:327-336
Timestamp: 2025-07-22T11:45:43.432Z
Learning: In the e-waybill update functions (india_compliance/gst_india/utils/e_waybill.py), fields like place_of_change and state use hardcoded "-" values in old_values dictionaries because these values are not stored in the system, so there's no way to retrieve the actual previous values.
Applied to files:
india_compliance/patches/v14/update_item_gst_details_and_gst_trearment_in_bill_of_entry.py
📚 Learning: 2025-06-19T08:21:29.308Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3451
File: india_compliance/gst_india/doctype/gstr_1_beta/gstr_1_export.py:287-288
Timestamp: 2025-06-19T08:21:29.308Z
Learning: In GSTR-1 export code (india_compliance/gst_india/doctype/gstr_1_beta/gstr_1_export.py), date fields like invoice date (DOC_DATE) and shipping bill date (SHIPPING_BILL_DATE) are mandatory by GST regulations and must exist, so transform functions using strftime() don't need None checks.
Applied to files:
india_compliance/patches/v14/update_item_gst_details_and_gst_trearment_in_bill_of_entry.py
📚 Learning: 2025-09-29T05:06:10.320Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3693
File: india_compliance/gst_india/doctype/gst_invoice_management_system/gst_invoice_management_system.js:32-33
Timestamp: 2025-09-29T05:06:10.320Z
Learning: GST-related tool doctypes (like GST Invoice Management System, Purchase Reconciliation Tool, GSTR-1) use unconditional assignment of session default company in their setup() method: `frm.doc.company = frappe.defaults.get_user_default("Company");`. This is appropriate for these utility tools as they are meant to always start with the user's default company on each load, unlike regular documents that preserve saved company values.
Applied to files:
india_compliance/patches/v14/update_item_gst_details_and_gst_trearment_in_bill_of_entry.py
🧬 Code graph analysis (3)
india_compliance/gst_india/utils/taxes_controller.py (1)
india_compliance/gst_india/overrides/transaction.py (7)
set_temp_item_wise_tax_detail_object(1395-1397)get_tax_details(1387-1393)get(1130-1149)get_item_name_wise_tax_details(1233-1306)is_gst_tax_row(1367-1371)tax_amount_field(1400-1401)tax_details_field(1404-1405)
india_compliance/gst_india/overrides/transaction.py (1)
india_compliance/gst_india/utils/taxes_controller.py (4)
get_item_name_wise_tax_details(62-137)set_temp_item_wise_tax_detail_object(38-60)is_gst_tax_row(139-144)tax_amount_field(24-25)
india_compliance/patches/v14/update_item_gst_details_and_gst_trearment_in_bill_of_entry.py (1)
india_compliance/patches/post_install/improve_item_tax_template.py (1)
compile_docs(441-471)
⏰ 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
🔇 Additional comments (6)
india_compliance/patches/post_install/improve_item_tax_template.py (1)
355-402: Wiring in item-wise tax details into the GST recomputation looks correct
_update_gst_detailsnow fetchingItem Wise Tax Detailrows and passing them intocompile_docs, andget_taxes_for_docsselectingtaxes.nameplusbase_tax_amount_after_discount_amount, lines up with the newItemGSTDetailslogic (which relies on tax row names andtax_amount_field="base_tax_amount_after_discount_amount"). The data flow from GL → taxes/items/item_taxes →ItemGSTDetails.get()is coherent and should allow item-wise breakdowns without changing existing semantics.india_compliance/patches/v14/update_item_gst_details_and_gst_trearment_in_bill_of_entry.py (1)
74-83: Passing an emptyitem_taxeslist intocompile_docsis appropriate hereFor Bill of Entry, there is no separate
Item Wise Tax Detailtable, so callingcompile_docs(taxes, items, [], doctype=doctype)keeps this patch aligned with the new function signature without altering behaviour.india_compliance/gst_india/utils/taxes_controller.py (2)
38-61: Temporary_item_wise_tax_detailsstructure is consistent withItemGSTDetailsexpectations
set_temp_item_wise_tax_detail_objectbuildingdoc._item_wise_tax_detailsas a list of{item, tax, rate}entries is exactly whatItemGSTDetails.set_item_name_wise_tax_detailsconsumes. The guard for missing items (item_map.get(item_name)) avoids issues if tax JSON refers to non-existent rows.
1395-1397:ItemGSTDetails.set_temp_item_wise_tax_detail_objecthook is a sensible extension pointLeaving
ItemGSTDetails.set_temp_item_wise_tax_detail_objectas a no-op with a clear comment allows custom controllers (likeCustomItemGSTDetails) to inject their own_item_wise_tax_detailswithout impacting the core flow. This is a good, minimal hook design.india_compliance/gst_india/overrides/transaction.py (1)
1368-1372:is_gst_tax_rownow safely handles missing rowsAdding the early
if not row: returnguard and centralising the GST tax-type check intorow.get("gst_tax_type") and row.gst_tax_type in GST_TAX_TYPESprevents accidental crashes when callers passNoneor partially-populated tax rows. This is a good defensive improvement.india_compliance/gst_india/overrides/test_transaction.py (1)
666-695: Updated invalid item-wise tax details test aligns with the new_item_wise_tax_detailsflowPopulating
doc._item_wise_tax_detailswith a list of dicts containingitem,tax,rate,amount, andtaxable_amount, and asserting on the more specific message (“Item Wise Tax Details do not match with Taxes and Charges at the following rows...”), matches the new item-wise validation path introduced upstream. The shape of the test data is also compatible with your own helper methods that consume_item_wise_tax_details.Just keep in mind this test is tightly coupled to ERPNext’s exact error string; if that changes again in the dependent PR, you’ll need to update the regex here as well.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
india_compliance/gst_india/overrides/transaction.py(7 hunks)india_compliance/gst_india/utils/taxes_controller.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- india_compliance/gst_india/utils/taxes_controller.py
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3344
File: india_compliance/gst_india/report/account_wise_summary/account_wise_summary.py:81-87
Timestamp: 2025-04-25T10:12:24.584Z
Learning: When processing taxes in GST India reports, charges after GST rows with charge_type "On Previous Row Total" should not be used to compute taxable value, so a break statement is used to exit the tax processing loop once such a row is encountered.
📚 Learning: 2025-10-01T10:54:11.096Z
Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3709
File: india_compliance/gst_india/utils/exporter.py:174-174
Timestamp: 2025-10-01T10:54:11.096Z
Learning: In india_compliance/gst_india/doctype/gstr_1/gstr_1_export.py, FIELD_TRANSFORMATIONS dictionary contains lambdas that are only called internally by DataProcessor.apply_transformations() with a single argument. These are separate from header transforms used with ExcelExporter.insert_data() which accept two arguments (value, row).
Applied to files:
india_compliance/gst_india/overrides/transaction.py
📚 Learning: 2025-06-25T08:19:02.607Z
Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3354
File: india_compliance/gst_india/report/summary_of_itc_availed/summary_of_itc_availed.py:278-281
Timestamp: 2025-06-25T08:19:02.607Z
Learning: In the Summary of ITC Availed report (india_compliance/gst_india/report/summary_of_itc_availed/summary_of_itc_availed.py), the summary dictionaries created by get_initial_summary() are never empty - they always contain tax field keys (igst_amount, cgst_amount, sgst_amount, cess_amount) with 0 values, so checking for empty dictionaries is unnecessary.
Applied to files:
india_compliance/gst_india/overrides/transaction.py
📚 Learning: 2025-04-25T10:12:24.584Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3344
File: india_compliance/gst_india/report/account_wise_summary/account_wise_summary.py:81-87
Timestamp: 2025-04-25T10:12:24.584Z
Learning: When processing taxes in GST India reports, charges after GST rows with charge_type "On Previous Row Total" should not be used to compute taxable value, so a break statement is used to exit the tax processing loop once such a row is encountered.
Applied to files:
india_compliance/gst_india/overrides/transaction.py
📚 Learning: 2025-05-26T03:16:30.230Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3344
File: india_compliance/gst_india/report/gst_account_wise_summary/gst_account_wise_summary.py:0-0
Timestamp: 2025-05-26T03:16:30.230Z
Learning: In GST account-wise summary reports, the filtering logic `doc.company_gstin != IfNull(doc[counterparty_gstin_field], "")` is intentionally designed to exclude internal transfers (where company GSTIN equals party GSTIN) while including transactions with unregistered parties (empty GSTIN). Internal transfers are excluded as they are only for internal reporting purposes like stock transfers between branches.
Applied to files:
india_compliance/gst_india/overrides/transaction.py
📚 Learning: 2025-06-25T08:16:18.010Z
Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3354
File: india_compliance/gst_india/report/summary_of_itc_availed/summary_of_itc_availed.py:309-310
Timestamp: 2025-06-25T08:16:18.010Z
Learning: In the Summary of ITC Availed report (india_compliance/gst_india/report/summary_of_itc_availed/summary_of_itc_availed.py), creating both a main entry and an additional indented entry for categories without subcategories is by design and intentional behavior for the report structure.
Applied to files:
india_compliance/gst_india/overrides/transaction.py
📚 Learning: 2025-07-22T11:45:43.432Z
Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3532
File: india_compliance/gst_india/utils/e_waybill.py:327-336
Timestamp: 2025-07-22T11:45:43.432Z
Learning: In the e-waybill update functions (india_compliance/gst_india/utils/e_waybill.py), fields like place_of_change and state use hardcoded "-" values in old_values dictionaries because these values are not stored in the system, so there's no way to retrieve the actual previous values.
Applied to files:
india_compliance/gst_india/overrides/transaction.py
🧬 Code graph analysis (1)
india_compliance/gst_india/overrides/transaction.py (1)
india_compliance/gst_india/utils/taxes_controller.py (3)
build_item_wise_tax_detail_from_data(61-82)set_temp_item_wise_tax_detail_object(37-59)tax_amount_field(23-24)
⏰ 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
🔇 Additional comments (3)
india_compliance/gst_india/overrides/transaction.py (3)
1231-1233: LGTM: Clean initialization of item defaults.The method properly initializes GST tax fields for all items using a copy of
item_defaultsto avoid shared reference issues.
1372-1376: LGTM: Good null-safety improvement.The addition of null checks before accessing row attributes prevents potential AttributeError exceptions.
1275-1281: ****The stub at line 1404 is intentionally designed for subclass override. CustomItemGSTDetails in
india_compliance/gst_india/utils/taxes_controller.py(lines 61-82) properly overrides this method and populatesitem_wise_tax_detailswith the expected structure (item_row,tax_row,ratefields). The code works as designed through inheritance—no issue exists.Likely an incorrect or invalid review comment.
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (49.49%) is below the target coverage (85.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #3807 +/- ##
===========================================
- Coverage 69.58% 69.51% -0.07%
===========================================
Files 182 182
Lines 17970 17965 -5
===========================================
- Hits 12505 12489 -16
- Misses 5465 5476 +11
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
2 participants
depends on: frappe/erpnext#48692
depends_on: frappe/erpnext#50658
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.