Skip to content

Conversation

@ljain112
Copy link
Member

@ljain112 ljain112 commented Dec 3, 2025

closes: #3765
closes: frappe/erpnext#50686

Adds e-Waybill generation support for new ERPNext v16 Stock Entry purposes related to the Subcontracting Inward feature:

  • Subcontracting Delivery - Delivering finished goods back to the customer after job work
  • Return Raw Material to Customer - Returning unused raw materials to the customer

Changes:

  • Enable e-Waybill applicability for new Stock Entry purposes
  • Add sub supply type options (Others) with appropriate descriptions
  • Add address field mapping for customer billing
  • Add the customer_provided_value field to the Stock Entry Detail for calculating taxable value
  • Calculate customer-provided material value for GST taxable value computation

Summary by CodeRabbit

  • New Features

    • e‑Waybill now supports "Subcontracting Delivery" and "Return Raw Material to Customer".
    • New field to record customer‑provided material value for subcontracting inward entries.
  • Enhancements

    • Taxable values and totals now include customer‑provided material value for more accurate tax calculations.
    • Billing/Company field labels adjusted for inward subcontracting scenarios.
    • Broader, clearer address-related messaging for e‑Waybill checks.
  • Chores

    • Updated patch annotation reference.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 3, 2025

Walkthrough

Adds Subcontracting Inward support by treating "Subcontracting Delivery" and "Return Raw Material to Customer" as valid Stock Entry purposes for e‑Waybill rules, adds an additional_taxable_value child field, computes per-item additional taxable values server-side, and incorporates them into taxable-value calculations and client applicability checks.

Changes

Cohort / File(s) Summary
E‑Waybill client rules
india_compliance/gst_india/client_scripts/e_waybill_actions.js, india_compliance/gst_india/client_scripts/e_waybill_applicability.js, india_compliance/public/js/utils.js
Include "Subcontracting Delivery" and "Return Raw Material to Customer" in allowed Stock Entry purposes; add purpose-specific branches setting supply_typeOutward, sub_supply_type["Others"], and update client-side applicability checks.
Stock Entry client UX
india_compliance/gst_india/client_scripts/stock_entry.js
Replaced direct-purpose checks with is_subcontracting_entry(frm); added is_subcontracting_inward_entry(frm); adjusted party/company field labels and address messaging for subcontracting inward/outward flows.
Custom fields
india_compliance/gst_india/constants/custom_fields.py
Added additional_taxable_value (Currency) to Stock Entry Detail after taxable_value (read-only, no_copy, print_hide), gated on subcontracting inward purposes.
Server-side subcontracting extension
india_compliance/gst_india/overrides/subcontracting_transaction.py
Extended server-side e‑Waybill applicability to include the two new purposes; minor formatting tweak in validate.
Tax calculation & item aggregation
india_compliance/gst_india/utils/taxes_controller.py
Added set_additional_taxable_value and helpers to compute per-item additional_taxable_value for Subcontracting Delivery and Return Raw Material to Customer; update_item_taxable_value now includes this additional value with proper precision.
Client taxes controller
india_compliance/public/js/taxes_controller.js
update_item_taxable_value for Stock Entry rows now adds row.additional_taxable_value when computing amount returned to client.
Patch annotation
india_compliance/patches.txt
Bumped annotation marker for post_model_sync comment index.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User
    participant Browser as Client (client scripts)
    participant SE as Stock Entry (doc)
    participant Server as App Server
    participant Taxes as TaxesController
    participant SCIO as Subcontracting Inward DB

    User->>Browser: Create/Submit Stock Entry (Subcontracting Delivery / Return Raw Material)
    Browser->>SE: Save/Submit
    SE->>Server: validate(doc) / set_taxes_and_totals
    Server->>Taxes: set_additional_taxable_value(doc)

    alt Subcontracting Delivery
        Taxes->>SCIO: Query consumed customer-provided items by scio_detail
        SCIO-->>Taxes: rates & consumed_qty
        Taxes->>Taxes: item.additional_taxable_value = sum(rate × consumed_qty)
    else Return Raw Material to Customer
        Taxes->>SCIO: Query required items by scio_detail
        SCIO-->>Taxes: rate, qty, amount
        Taxes->>Taxes: item.additional_taxable_value = (rate × qty) − amount
    end

    Taxes->>Taxes: update_item_taxable_value (add additional value with precision)
    Server-->>SE: Validation completes (values updated)
    SE-->>User: Save/Submit response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Inspect DB queries and matching logic for scio_detail lookups in india_compliance/gst_india/utils/taxes_controller.py.
  • Verify numeric precision/rounding where additional_taxable_value is computed and added (server and client).
  • Confirm consistency of purpose lists and helper functions across client scripts and utils.
  • Validate custom field metadata and depends_on expression.

Possibly related PRs

Suggested labels

backport version-15-hotfix

Suggested reviewers

  • karm1000
  • vorasmit

Poem

🐰 I hopped through ledgers, quick and bright,
Adding tiny values in the moonlit night.
Delivery or return, each row I inspect,
Carrots become numbers — precise and correct.
E‑waybills hop forward, the books now connect.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title clearly and specifically summarizes the primary change: adding e-waybill support for the new Subcontracting Inward workflow with inward-facing purposes.
Linked Issues check ✅ Passed PR fully implements core requirements from #3765 and #50686: e-waybill applicability for new purposes, field mappings, additional_taxable_value tracking, and taxable value computations for both Subcontracting Delivery and Return Raw Material scenarios.
Out of Scope Changes check ✅ Passed All changes directly support Subcontracting Inward e-waybill functionality; no unrelated modifications detected beyond stated PR objectives.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (5)
india_compliance/gst_india/client_scripts/e_waybill_applicability.js (1)

241-255: Extending Stock Entry e-Waybill eligibility to new purposes is consistent

Including "Subcontracting Delivery" and "Return Raw Material to Customer" in both the applicability and API-enabled checks matches the backend is_e_waybill_applicable logic and the new subcontracting flows.

You do duplicate the same purpose array in is_e_waybill_applicable and is_e_waybill_api_enabled; consider extracting a shared constant (or at least keeping them adjacent) to avoid drift if more purposes are added later.

Also applies to: 324-335

india_compliance/gst_india/utils/taxes_controller.py (1)

150-158: Taxable value inclusion of customer_provided_value is correct

Using taxable_value = amount + customer_provided_value (with item-level precision) matches the subcontracting inward requirement and stays backward‑compatible: on items without customer_provided_value this simply adds zero.

If you expect CustomTaxController to ever be reused on doctypes without a taxable_value field, adding a short guard or comment about that assumption would make the intent clearer.

india_compliance/gst_india/client_scripts/stock_entry.js (1)

210-236: Bill To/Bill From mapping for new subcontracting purposes is sensible

For "Subcontracting Delivery" and "Return Raw Material to Customer", routing the party side to bill_to_address (with “Customer Address” label) and the company side to bill_from_address matches the described billing direction for inward subcontracting.

You might also consider whether the company handler (which currently only runs for "Send to Subcontractor") should auto-populate bill_from_address for these new purposes as well, to keep the UX consistent.

india_compliance/gst_india/overrides/subcontracting_transaction.py (2)

184-193: Customer‑provided value computation matches the subcontracting requirements

The set_customer_provided_value hook in validate and its helpers implement the described behavior:

  • For "Subcontracting Delivery", summing rate * consumed_qty from Subcontracting Inward Order Received Item rows with is_customer_provided_item = 1 by scio_item_detail, then mapping that to item.customer_provided_value, correctly yields the total value of consumed customer‑provided materials per FG line.

  • For "Return Raw Material to Customer", setting
    customer_provided_value = (SIO required_items.rate * qty) - amount
    so that taxable_value = amount + customer_provided_value = rate * qty
    ensures the e‑Waybill taxable value reflects the SIO rate rather than stock valuation.

A small robustness improvement would be to resolve the SIO name from either a document‑level field (if present) or from the first item that actually has subcontracting_inward_order, instead of assuming doc.items[0] always carries it; that would guard against edge cases where the first row is an unrelated item.

Also applies to: 408-530


408-418: Ruff warnings about × characters in docstrings/comments

The docstrings and comments here use the Unicode multiplication sign (×), which Ruff flags as ambiguous (RUF002/RUF003). Replacing × with a plain x (e.g., rate x qty) will quiet the linter without changing meaning.

This is purely cosmetic but easy to address and keeps CI clean.

Also applies to: 437-447, 487-493, 523-526

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 66fcda1 and 6b2908b.

📒 Files selected for processing (7)
  • india_compliance/gst_india/client_scripts/e_waybill_actions.js (1 hunks)
  • india_compliance/gst_india/client_scripts/e_waybill_applicability.js (2 hunks)
  • india_compliance/gst_india/client_scripts/stock_entry.js (1 hunks)
  • india_compliance/gst_india/constants/custom_fields.py (1 hunks)
  • india_compliance/gst_india/overrides/subcontracting_transaction.py (4 hunks)
  • india_compliance/gst_india/utils/taxes_controller.py (1 hunks)
  • india_compliance/patches.txt (1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3807
File: india_compliance/gst_india/overrides/transaction.py:142-157
Timestamp: 2025-11-21T08:53:26.329Z
Learning: In the india_compliance/gst_india/overrides/transaction.py file, the `_item_wise_tax_details` field used in `validate_item_wise_tax_detail()` is populated from ERPNext (not from within the India Compliance codebase) before validation runs, as part of cross-repository changes.
📚 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/gst_india/client_scripts/e_waybill_applicability.js
  • india_compliance/gst_india/client_scripts/e_waybill_actions.js
📚 Learning: 2025-07-30T10:16:09.615Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3532
File: india_compliance/gst_india/utils/e_waybill.py:484-490
Timestamp: 2025-07-30T10:16:09.615Z
Learning: In e-Waybill update functions (india_compliance/gst_india/utils/e_waybill.py), transporter IDs used in comments don't require HTML escaping because they come from the government e-Waybill API system which already validates and sanitizes data, rejecting HTML tags. The values are system-controlled rather than direct user input.

Applied to files:

  • india_compliance/gst_india/client_scripts/e_waybill_applicability.js
📚 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/client_scripts/e_waybill_applicability.js
  • india_compliance/gst_india/client_scripts/e_waybill_actions.js
  • india_compliance/gst_india/constants/custom_fields.py
📚 Learning: 2025-11-21T08:53:26.329Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3807
File: india_compliance/gst_india/overrides/transaction.py:142-157
Timestamp: 2025-11-21T08:53:26.329Z
Learning: In the india_compliance/gst_india/overrides/transaction.py file, the `_item_wise_tax_details` field used in `validate_item_wise_tax_detail()` is populated from ERPNext (not from within the India Compliance codebase) before validation runs, as part of cross-repository changes.

Applied to files:

  • india_compliance/gst_india/client_scripts/e_waybill_applicability.js
  • india_compliance/gst_india/client_scripts/stock_entry.js
  • india_compliance/patches.txt
  • india_compliance/gst_india/client_scripts/e_waybill_actions.js
  • india_compliance/gst_india/utils/taxes_controller.py
  • india_compliance/gst_india/constants/custom_fields.py
  • india_compliance/gst_india/overrides/subcontracting_transaction.py
📚 Learning: 2025-08-12T16:57:46.264Z
Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3595
File: india_compliance/gst_india/overrides/delivery_note.py:30-38
Timestamp: 2025-08-12T16:57:46.264Z
Learning: In the delivery_note.py override, the onload function already checks if doc.get("ewaybill") exists before proceeding with e-waybill related operations.

Applied to files:

  • india_compliance/gst_india/client_scripts/e_waybill_applicability.js
📚 Learning: 2025-11-11T12:44:44.690Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3784
File: india_compliance/gst_india/client_scripts/company.js:25-28
Timestamp: 2025-11-11T12:44:44.690Z
Learning: In india_compliance, when using erpnext.company.set_custom_query(frm, [field_name, filters]) in Company doctype client scripts, the function automatically adds company and is_group filters. The filters object parameter should only contain additional filters beyond these defaults (e.g., root_type). For default_gst_expense_account, no root_type filter is needed, so an empty object {} is passed.

Applied to files:

  • india_compliance/gst_india/client_scripts/stock_entry.js
📚 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/patches.txt
📚 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.py
🪛 Ruff (0.14.7)
india_compliance/gst_india/overrides/subcontracting_transaction.py

412-412: Docstring contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?

(RUF002)


416-416: Docstring contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?

(RUF002)


441-441: Docstring contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?

(RUF002)


490-490: Docstring contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?

(RUF002)


492-492: Docstring contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?

(RUF002)


523-523: Comment contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?

(RUF003)


525-525: Comment contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?

(RUF003)

⏰ 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 (4)
india_compliance/patches.txt (1)

5-8: Patch marker bump to #67 looks fine

Aligning the comment index with the new custom field additions is fine and has no runtime impact.

india_compliance/gst_india/client_scripts/e_waybill_actions.js (1)

541-555: Sub‑supply type mapping for new Stock Entry purposes

Using supply_type = "Outward" with sub_supply_type = ["Others"] and descriptive sub_supply_desc values ("Job Work Delivery", "Return Raw Material") for the two new purposes is consistent with the existing pattern for special cases and integrates correctly with the generation dialog.

Please just double‑check against the latest NIC e‑Waybill portal specifications that these flows are indeed expected to be reported under “Others” rather than specific job‑work enums, to avoid downstream rejection.

india_compliance/gst_india/constants/custom_fields.py (1)

750-776: customer_provided_value field definition lines up with its intended use

Defining customer_provided_value as a read‑only, non‑copied, non‑printed currency field right after taxable_value is appropriate for storing customer‑provided material value solely for GST/e‑Waybill computations.

This matches the server logic in the subcontracting override and the taxable value calculation in CustomTaxController.

india_compliance/gst_india/overrides/subcontracting_transaction.py (1)

41-46: Purpose constant and e-Waybill applicability extension are coherent

Defining SUBCONTRACTING_INWARD_PURPOSES and using the same purpose names in both set_customer_provided_value and is_e_waybill_applicable keeps the logic consistent across tax computation and eligibility checks.

The updated Stock Entry purpose list ("Subcontracting Delivery", "Return Raw Material to Customer") is aligned with the client‑side applicability rules and should ensure these inward subcontracting flows are treated as e‑Waybill‑capable wherever GST Settings permit it.

Also applies to: 639-661

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
india_compliance/gst_india/overrides/subcontracting_transaction.py (2)

43-46: Avoid duplicating purpose literals; reuse SUBCONTRACTING_INWARD_PURPOSES in is_e_waybill_applicable.

You’re hard‑coding "Subcontracting Delivery" and "Return Raw Material to Customer" both in SUBCONTRACTING_INWARD_PURPOSES and in is_e_waybill_applicable. To prevent future drift if these labels change, consider driving the applicability check from the same tuple:

-SUBCONTRACTING_INWARD_PURPOSES = (
-    "Subcontracting Delivery",
-    "Return Raw Material to Customer",
-)
+SUBCONTRACTING_INWARD_PURPOSES = (
+    "Subcontracting Delivery",
+    "Return Raw Material to Customer",
+)

...

-    if doc.purpose not in [
-        "Material Transfer",
-        "Material Issue",
-        "Send to Subcontractor",
-        "Subcontracting Delivery",
-        "Return Raw Material to Customer",
-    ]:
+    if doc.purpose not in (
+        "Material Transfer",
+        "Material Issue",
+        "Send to Subcontractor",
+    ) + SUBCONTRACTING_INWARD_PURPOSES:
         return False

Also applies to: 620-626


410-418: Ruff RUF002: replace × in docstrings with ASCII * or x.

Ruff is flagging the multiplication sign × in these docstrings as ambiguous. To keep lint noise down and improve readability in plain terminals, consider switching to * or plain x:

-        - customer_provided_value = SUM(received_items.rate × consumed_qty)
+        - customer_provided_value = SUM(received_items.rate * consumed_qty)
@@
-        - customer_provided_value = (SIO required_items.rate × qty) - item.amount
+        - customer_provided_value = (SIO required_items.rate * qty) - item.amount
@@
-    Customer_provided_value = SUM(received_items.rate * received_items.consumed_qty)
+    Customer_provided_value = SUM(received_items.rate * received_items.consumed_qty)
@@
-    customer_provided_value = (SIO rate × qty) - amount
+    customer_provided_value = (SIO rate * qty) - amount

Also applies to: 439-439, 473-473

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6b2908b and 5ebcbc1.

📒 Files selected for processing (1)
  • india_compliance/gst_india/overrides/subcontracting_transaction.py (4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3807
File: india_compliance/gst_india/overrides/transaction.py:142-157
Timestamp: 2025-11-21T08:53:26.329Z
Learning: In the india_compliance/gst_india/overrides/transaction.py file, the `_item_wise_tax_details` field used in `validate_item_wise_tax_detail()` is populated from ERPNext (not from within the India Compliance codebase) before validation runs, as part of cross-repository changes.
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3532
File: india_compliance/gst_india/utils/e_waybill.py:484-490
Timestamp: 2025-07-30T10:16:09.615Z
Learning: In e-Waybill update functions (india_compliance/gst_india/utils/e_waybill.py), transporter IDs used in comments don't require HTML escaping because they come from the government e-Waybill API system which already validates and sanitizes data, rejecting HTML tags. The values are system-controlled rather than direct user input.
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.
📚 Learning: 2025-11-21T08:53:26.329Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3807
File: india_compliance/gst_india/overrides/transaction.py:142-157
Timestamp: 2025-11-21T08:53:26.329Z
Learning: In the india_compliance/gst_india/overrides/transaction.py file, the `_item_wise_tax_details` field used in `validate_item_wise_tax_detail()` is populated from ERPNext (not from within the India Compliance codebase) before validation runs, as part of cross-repository changes.

Applied to files:

  • india_compliance/gst_india/overrides/subcontracting_transaction.py
🪛 Ruff (0.14.7)
india_compliance/gst_india/overrides/subcontracting_transaction.py

412-412: Docstring contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?

(RUF002)


416-416: Docstring contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?

(RUF002)


473-473: Docstring contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?

(RUF002)

⏰ 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 (2)
india_compliance/gst_india/overrides/subcontracting_transaction.py (2)

185-186: Placement of set_customer_provided_value in validate looks correct.

Calling set_customer_provided_value(doc) at the top of validate ensures item‑level customer_provided_value is populated before CustomTaxController(...).set_taxes_and_totals() recomputes taxable values and totals, while the helper itself is safely no‑op for non‑Stock Entry doctypes.


408-498: Edge-case handling in _set_return_raw_material_customer_value warrants defensive improvements.

The formulas correctly implement the intended calculations:

  • Subcontracting Delivery: sums rate × consumed_qty for customer-provided items, keyed by scio_item_detail
  • Return Raw Material: calculates (SIO rate × qty) - amount to derive customer-provided value

One edge case deserves attention: if an item references a scio_detail that doesn't exist in required_items, required_items.get(item.scio_detail, 0) returns 0, resulting in customer_provided_value = -item.amount—an incorrect negative value.

Make the function more robust by explicitly skipping rows where the SIO reference is missing:

 for item in doc.items:
-    if item.get("scio_detail"):
-        sio_value = required_items.get(item.scio_detail, 0) * flt(item.qty)
-        item.customer_provided_value = sio_value - flt(item.amount)
+    if not item.get("scio_detail"):
+        continue
+
+    if item.scio_detail not in required_items:
+        continue
+
+    sio_value = required_items[item.scio_detail] * flt(item.qty)
+    item.customer_provided_value = sio_value - flt(item.amount)

Additionally, deduplicate scio_details to avoid redundant IN-filter values when multiple items reference the same SIO row.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
india_compliance/gst_india/client_scripts/stock_entry.js (1)

210-222: Consider exposing helpers on india_compliance namespace for cross-module access.

The helper functions are correctly implemented and properly distinguish between outbound subcontracting ("Send to Subcontractor") and inward subcontracting ("Subcontracting Delivery", "Return Raw Material to Customer") flows. However, if other client scripts (e.g., e_waybill_actions.js, e_waybill_applicability.js) need to check for subcontracting entry types, these helpers should be exposed on the india_compliance namespace.

If cross-module access is needed, apply this diff:

 function is_subcontracting_entry(frm) {
     return [
         "Send to Subcontractor",
         "Subcontracting Delivery",
         "Return Raw Material to Customer",
     ].includes(frm.doc.purpose);
 }

 function is_subcontracting_inward_entry(frm) {
     return ["Subcontracting Delivery", "Return Raw Material to Customer"].includes(
         frm.doc.purpose
     );
 }
+
+// Expose helpers for use in other modules
+Object.assign(india_compliance, {
+    is_subcontracting_entry,
+    is_subcontracting_inward_entry
+});
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ebcbc1 and b65e751.

📒 Files selected for processing (5)
  • india_compliance/gst_india/client_scripts/e_waybill_applicability.js (3 hunks)
  • india_compliance/gst_india/client_scripts/stock_entry.js (3 hunks)
  • india_compliance/gst_india/overrides/subcontracting_transaction.py (4 hunks)
  • india_compliance/gst_india/utils/taxes_controller.py (1 hunks)
  • india_compliance/public/js/utils.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • india_compliance/gst_india/client_scripts/e_waybill_applicability.js
  • india_compliance/gst_india/overrides/subcontracting_transaction.py
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3807
File: india_compliance/gst_india/overrides/transaction.py:142-157
Timestamp: 2025-11-21T08:53:26.329Z
Learning: In the india_compliance/gst_india/overrides/transaction.py file, the `_item_wise_tax_details` field used in `validate_item_wise_tax_detail()` is populated from ERPNext (not from within the India Compliance codebase) before validation runs, as part of cross-repository changes.
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3532
File: india_compliance/gst_india/utils/e_waybill.py:484-490
Timestamp: 2025-07-30T10:16:09.615Z
Learning: In e-Waybill update functions (india_compliance/gst_india/utils/e_waybill.py), transporter IDs used in comments don't require HTML escaping because they come from the government e-Waybill API system which already validates and sanitizes data, rejecting HTML tags. The values are system-controlled rather than direct user input.
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.
📚 Learning: 2025-11-21T08:53:26.329Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3807
File: india_compliance/gst_india/overrides/transaction.py:142-157
Timestamp: 2025-11-21T08:53:26.329Z
Learning: In the india_compliance/gst_india/overrides/transaction.py file, the `_item_wise_tax_details` field used in `validate_item_wise_tax_detail()` is populated from ERPNext (not from within the India Compliance codebase) before validation runs, as part of cross-repository changes.

Applied to files:

  • india_compliance/gst_india/utils/taxes_controller.py
  • india_compliance/public/js/utils.js
  • india_compliance/gst_india/client_scripts/stock_entry.js
📚 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.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/public/js/utils.js
📚 Learning: 2025-07-30T10:16:09.615Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3532
File: india_compliance/gst_india/utils/e_waybill.py:484-490
Timestamp: 2025-07-30T10:16:09.615Z
Learning: In e-Waybill update functions (india_compliance/gst_india/utils/e_waybill.py), transporter IDs used in comments don't require HTML escaping because they come from the government e-Waybill API system which already validates and sanitizes data, rejecting HTML tags. The values are system-controlled rather than direct user input.

Applied to files:

  • india_compliance/public/js/utils.js
📚 Learning: 2025-11-11T12:44:44.690Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3784
File: india_compliance/gst_india/client_scripts/company.js:25-28
Timestamp: 2025-11-11T12:44:44.690Z
Learning: In india_compliance, when using erpnext.company.set_custom_query(frm, [field_name, filters]) in Company doctype client scripts, the function automatically adds company and is_group filters. The filters object parameter should only contain additional filters beyond these defaults (e.g., root_type). For default_gst_expense_account, no root_type filter is needed, so an empty object {} is passed.

Applied to files:

  • india_compliance/gst_india/client_scripts/stock_entry.js
📚 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/gst_india/client_scripts/stock_entry.js
⏰ 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/public/js/utils.js (1)

550-556: LGTM!

The addition of "Subcontracting Delivery" and "Return Raw Material to Customer" to the e-waybill applicability whitelist correctly enables e-waybill support for the new Subcontracting Inward purposes.

india_compliance/gst_india/client_scripts/stock_entry.js (1)

236-246: LGTM!

The new branch correctly handles the inward subcontracting flow where the company bills to the customer. The field mappings appropriately swap bill_to_address and bill_from_address compared to the default outbound flow, and the labels clearly indicate the direction of billing.

india_compliance/gst_india/utils/taxes_controller.py (1)

152-157: Verify that customer_provided_value is properly populated before this calculation.

The logic correctly adds customer_provided_value to the taxable value with appropriate precision handling using flt() with item precision. The defensive use of item.get("customer_provided_value", 0) handles missing values safely. Confirm that this field is set during Stock Entry validation (likely in subcontracting_transaction.py or ERPNext doctype configuration) before update_item_taxable_value is called.

@ljain112 ljain112 force-pushed the feat-ewaybill-inward branch from b65e751 to 555f9c3 Compare December 4, 2025 12:48
@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

❌ Patch coverage is 28.57143% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.41%. Comparing base (f259e9d) to head (2bf9563).

Files with missing lines Patch % Lines
...dia_compliance/gst_india/utils/taxes_controller.py 28.57% 35 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3822      +/-   ##
===========================================
- Coverage    69.53%   69.41%   -0.12%     
===========================================
  Files          182      182              
  Lines        17972    18020      +48     
===========================================
+ Hits         12496    12509      +13     
- Misses        5476     5511      +35     
Files with missing lines Coverage Δ
...ia_compliance/gst_india/constants/custom_fields.py 100.00% <ø> (ø)
.../gst_india/overrides/subcontracting_transaction.py 84.78% <ø> (ø)
...dia_compliance/gst_india/utils/taxes_controller.py 69.71% <28.57%> (-16.12%) ⬇️

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
india_compliance/gst_india/constants/custom_fields.py (1)

763-775: additional_taxable_value field definition is consistent; consider narrowing visibility

The additional_taxable_value Currency field mirrors taxable_value in options and flags and is correctly positioned after it, which fits the subcontracting inward use‑case.

Optionally, to avoid noise on unrelated Stock Entries, you could add a depends_on expression so this field is only shown for subcontracting inward purposes (e.g., "Subcontracting Delivery" / "Return Raw Material to Customer").

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 555f9c3 and faef75f.

📒 Files selected for processing (4)
  • india_compliance/gst_india/client_scripts/stock_entry.js (4 hunks)
  • india_compliance/gst_india/constants/custom_fields.py (1 hunks)
  • india_compliance/gst_india/overrides/subcontracting_transaction.py (2 hunks)
  • india_compliance/gst_india/utils/taxes_controller.py (3 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3807
File: india_compliance/gst_india/overrides/transaction.py:142-157
Timestamp: 2025-11-21T08:53:26.329Z
Learning: In the india_compliance/gst_india/overrides/transaction.py file, the `_item_wise_tax_details` field used in `validate_item_wise_tax_detail()` is populated from ERPNext (not from within the India Compliance codebase) before validation runs, as part of cross-repository changes.
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3532
File: india_compliance/gst_india/utils/e_waybill.py:484-490
Timestamp: 2025-07-30T10:16:09.615Z
Learning: In e-Waybill update functions (india_compliance/gst_india/utils/e_waybill.py), transporter IDs used in comments don't require HTML escaping because they come from the government e-Waybill API system which already validates and sanitizes data, rejecting HTML tags. The values are system-controlled rather than direct user input.
📚 Learning: 2025-11-21T08:53:26.329Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3807
File: india_compliance/gst_india/overrides/transaction.py:142-157
Timestamp: 2025-11-21T08:53:26.329Z
Learning: In the india_compliance/gst_india/overrides/transaction.py file, the `_item_wise_tax_details` field used in `validate_item_wise_tax_detail()` is populated from ERPNext (not from within the India Compliance codebase) before validation runs, as part of cross-repository changes.

Applied to files:

  • india_compliance/gst_india/constants/custom_fields.py
  • india_compliance/gst_india/utils/taxes_controller.py
  • india_compliance/gst_india/client_scripts/stock_entry.js
  • india_compliance/gst_india/overrides/subcontracting_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/utils/taxes_controller.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/gst_india/client_scripts/stock_entry.js
  • india_compliance/gst_india/overrides/subcontracting_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/client_scripts/stock_entry.js
  • india_compliance/gst_india/overrides/subcontracting_transaction.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/gst_india/client_scripts/stock_entry.js
📚 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/gst_india/overrides/subcontracting_transaction.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/overrides/subcontracting_transaction.py
📚 Learning: 2025-07-30T10:16:09.615Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3532
File: india_compliance/gst_india/utils/e_waybill.py:484-490
Timestamp: 2025-07-30T10:16:09.615Z
Learning: In e-Waybill update functions (india_compliance/gst_india/utils/e_waybill.py), transporter IDs used in comments don't require HTML escaping because they come from the government e-Waybill API system which already validates and sanitizes data, rejecting HTML tags. The values are system-controlled rather than direct user input.

Applied to files:

  • india_compliance/gst_india/overrides/subcontracting_transaction.py
🧬 Code graph analysis (1)
india_compliance/gst_india/client_scripts/stock_entry.js (1)
india_compliance/gst_india/client_scripts/e_waybill_actions.js (1)
  • frm (985-985)
⏰ 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/subcontracting_transaction.py (1)

523-529: New Stock Entry purposes correctly included for e‑Waybill applicability

Allowing "Subcontracting Delivery" and "Return Raw Material to Customer" in the Stock Entry purpose whitelist aligns with the subcontracting inward requirements and keeps the existing guardrails (GST Settings + doctype) intact. Please just confirm these strings exactly match the ERPNext v16 purpose values to avoid silent non‑applicability.

india_compliance/gst_india/utils/taxes_controller.py (1)

113-170: Taxable value flow correctly extended to include additional_taxable_value

Calling set_additional_taxable_value() before update_item_taxable_value() ensures item.taxable_value is amount + additional_taxable_value for Stock Entries, while remaining a no‑op for other doctypes. This integrates cleanly with existing calculate_total_taxable_value() and downstream tax computations.

india_compliance/gst_india/client_scripts/stock_entry.js (1)

75-85: Inward/outward subcontracting address handling looks consistent with new purposes

  • The new alert message (“Party Address is required to create e‑Waybill”) is clearer and purpose‑agnostic.
  • company(frm) using is_subcontracting_entry(frm) correctly extends default company‑address population to "Subcontracting Delivery" and "Return Raw Material to Customer" alongside "Send to Subcontractor".
  • is_subcontracting_entry / is_subcontracting_inward_entry helpers centralize purpose checks and are used consistently.
  • The new get_field_and_label branch for inward subcontracting correctly flips the semantics so that:
    • party_field = bill_to_address with “(same as Customer Address)” label,
    • company_field = bill_from_address,
      matching the “company bills to the customer” requirement.

Overall this wiring should make the e‑Waybill address UI align with both outward and inward subcontracting flows.

Also applies to: 114-131, 210-222, 227-246

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
india_compliance/gst_india/utils/taxes_controller.py (1)

355-365: Past review concern addressed—LGTM.

The guard if not scio_rate: continue prevents the problematic scenario where a missing SCIO record would result in negative additional_taxable_value. Items without valid SCIO linkage now retain their initialized value of 0.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between faef75f and 0fc335b.

📒 Files selected for processing (1)
  • india_compliance/gst_india/utils/taxes_controller.py (4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3807
File: india_compliance/gst_india/overrides/transaction.py:142-157
Timestamp: 2025-11-21T08:53:26.329Z
Learning: In the india_compliance/gst_india/overrides/transaction.py file, the `_item_wise_tax_details` field used in `validate_item_wise_tax_detail()` is populated from ERPNext (not from within the India Compliance codebase) before validation runs, as part of cross-repository changes.
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3532
File: india_compliance/gst_india/utils/e_waybill.py:484-490
Timestamp: 2025-07-30T10:16:09.615Z
Learning: In e-Waybill update functions (india_compliance/gst_india/utils/e_waybill.py), transporter IDs used in comments don't require HTML escaping because they come from the government e-Waybill API system which already validates and sanitizes data, rejecting HTML tags. The values are system-controlled rather than direct user input.
📚 Learning: 2025-11-21T08:53:26.329Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3807
File: india_compliance/gst_india/overrides/transaction.py:142-157
Timestamp: 2025-11-21T08:53:26.329Z
Learning: In the india_compliance/gst_india/overrides/transaction.py file, the `_item_wise_tax_details` field used in `validate_item_wise_tax_detail()` is populated from ERPNext (not from within the India Compliance codebase) before validation runs, as part of cross-repository changes.

Applied to files:

  • india_compliance/gst_india/utils/taxes_controller.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/utils/taxes_controller.py
📚 Learning: 2025-08-20T12:52:11.346Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3620
File: india_compliance/gst_india/api_classes/nic/e_invoice.py:167-175
Timestamp: 2025-08-20T12:52:11.346Z
Learning: In the GST India API classes (india_compliance/gst_india/api_classes/base.py), all JSON responses are automatically converted to frappe._dict objects using `response.json(object_hook=frappe._dict)` at line 151. This means that any dictionary objects in API responses already support attribute access and don't need manual wrapping in frappe._dict.

Applied to files:

  • india_compliance/gst_india/utils/taxes_controller.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
🔇 Additional comments (6)
india_compliance/gst_india/utils/taxes_controller.py (6)

2-2: LGTM!

Standard library import, appropriate for the per-key cost accumulation pattern used in _set_subcontracting_delivery_additional_value.


114-119: LGTM!

Correct sequencing—set_additional_taxable_value() is invoked before update_item_taxable_value(), ensuring the additional value is populated before it's consumed in the taxable value calculation.


152-159: LGTM!

The taxable value computation correctly incorporates the additional value with a safe default, and applies appropriate precision for the target field.


161-171: LGTM!

Clean implementation with proper doctype guard, explicit initialization, and clear dispatch to purpose-specific handlers.


291-317: LGTM!

Well-structured implementation: graceful handling of empty data, safe numeric operations with flt(), and efficient cost aggregation using defaultdict.


339-348: LGTM!

Query correctly filters by name (since scio_detail in return scenarios references the received item directly), and the frappe._dict wrapper efficiently converts the result to a lookup dictionary.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
india_compliance/gst_india/utils/taxes_controller.py (1)

342-377: Use is None for scio_rate check to avoid treating 0 as “missing”

In _set_return_raw_material_additional_value, this block:

scio_rate = received_items.get(scio_detail)
if not scio_rate:
    continue

skips lines where the SCIO Received Item’s rate is 0, not just when the rate is absent. That’s slightly different from the earlier suggested fix (which used if rate is None) and changes semantics for legitimate 0‑rate cases: taxable_value will remain item.amount instead of being driven to 0 via (0 * qty) - amount.

To keep the “skip only when there is no SCIO record” behaviour while still guarding against missing links, consider tightening the check:

-        scio_rate = received_items.get(scio_detail)
-        if not scio_rate:
-            continue
+        scio_rate = received_items.get(scio_detail)
+        if scio_rate is None:
+            # Leave additional_taxable_value at 0 if SCIO link is missing
+            continue

This matches the intent of the previous review while still preventing under‑valuation when the SCIO link is missing.

🧹 Nitpick comments (1)
india_compliance/public/js/taxes_controller.js (1)

135-141: Additional taxable value handling for Stock Entry is consistent with server‑side logic

The client now mirrors the server: for Stock Entry rows, taxable_value is base (qty × rate / transfer_qty × basic_rate) plus additional_taxable_value, using the same precision key. This looks correct and keeps other doctypes untouched.

As a tiny clean‑up, you could avoid recomputing precision twice:

-        const calculateAmount = (qty, rate, precisionType) => {
-            let amount = flt(flt(qty) * flt(rate), precision(precisionType, row));
-
-            if (this.frm.doc.doctype === "Stock Entry") {
-                amount += flt(row.additional_taxable_value, precision(precisionType, row));
-            }
-            return amount;
-        };
+        const calculateAmount = (qty, rate, precisionType) => {
+            const prec = precision(precisionType, row);
+            let amount = flt(flt(qty) * flt(rate), prec);
+
+            if (this.frm.doc.doctype === "Stock Entry") {
+                amount += flt(row.additional_taxable_value, prec);
+            }
+            return amount;
+        };
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67f2f81 and 2bf9563.

📒 Files selected for processing (2)
  • india_compliance/gst_india/utils/taxes_controller.py (4 hunks)
  • india_compliance/public/js/taxes_controller.js (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3807
File: india_compliance/gst_india/overrides/transaction.py:142-157
Timestamp: 2025-11-21T08:53:26.329Z
Learning: In the india_compliance/gst_india/overrides/transaction.py file, the `_item_wise_tax_details` field used in `validate_item_wise_tax_detail()` is populated from ERPNext (not from within the India Compliance codebase) before validation runs, as part of cross-repository changes.
📚 Learning: 2025-11-21T08:53:26.329Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3807
File: india_compliance/gst_india/overrides/transaction.py:142-157
Timestamp: 2025-11-21T08:53:26.329Z
Learning: In the india_compliance/gst_india/overrides/transaction.py file, the `_item_wise_tax_details` field used in `validate_item_wise_tax_detail()` is populated from ERPNext (not from within the India Compliance codebase) before validation runs, as part of cross-repository changes.

Applied to files:

  • india_compliance/public/js/taxes_controller.js
  • india_compliance/gst_india/utils/taxes_controller.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/utils/taxes_controller.py
📚 Learning: 2025-08-20T12:52:11.346Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3620
File: india_compliance/gst_india/api_classes/nic/e_invoice.py:167-175
Timestamp: 2025-08-20T12:52:11.346Z
Learning: In the GST India API classes (india_compliance/gst_india/api_classes/base.py), all JSON responses are automatically converted to frappe._dict objects using `response.json(object_hook=frappe._dict)` at line 151. This means that any dictionary objects in API responses already support attribute access and don't need manual wrapping in frappe._dict.

Applied to files:

  • india_compliance/gst_india/utils/taxes_controller.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
🔇 Additional comments (2)
india_compliance/gst_india/utils/taxes_controller.py (2)

114-120: Order of additional taxable value calculation in set_taxes_and_totals is correct

Calling set_additional_taxable_value() before update_item_taxable_value() ensures that item.additional_taxable_value is fully derived (or zeroed for non‑subcontracting Stock Entries) before item.taxable_value is recomputed, and the early return on non‑Stock Entry doctypes keeps other flows unaffected. The server‑side taxable_value logic now aligns with the updated JS controller.

Also applies to: 152-172


291-340: [rewritten comment]
[classification tag]

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.

India Compliance support for Subcontracting Inward [ERPNext v16] Eway bill for Subcontracting Inward!

1 participant