feat: e-Waybill bulk actions in Delivery Note#3842
feat: e-Waybill bulk actions in Delivery Note#3842ljain112 wants to merge 20 commits intoresilient-tech:developfrom
Conversation
📝 WalkthroughWalkthroughAdds Delivery Note list-view wiring; centralizes and expands bulk e‑Waybill/e‑Invoice client actions; refactors e‑Waybill dialog UI into modular builders; threads dialog values through bulk enqueue/generation; enhances server-side e‑waybill validation and tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant ListView as Client ListView
participant Dialog as Client Dialog
participant ClientAPI as Client API
participant Server as Server
participant Job as Background Job
participant EWayAPI as External E‑Waybill API
rect rgba(173,216,230,0.5)
Note over ListView,Dialog: Client-side bulk action flow
end
User->>ListView: select documents + trigger bulk action
ListView->>Dialog: open bulk action dialog (document/transporter fields)
Dialog->>ListView: confirm values
ListView->>ClientAPI: enqueue_bulk_generation(method, {docnames, values})
ClientAPI->>Server: create background job (method, docnames, values)
Server->>Job: start generate_e_waybills job
Job->>Server: validate_is_e_waybill_api_enabled(doctype)
Job->>Job: for each doc: apply values → validate_sub_supply_type()
Job->>EWayAPI: call external E‑Waybill API per document
EWayAPI-->>Job: respond (success / error)
Job->>Server: persist results/logs
Server->>User: notify job status / provide links to logs/API requests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3842 +/- ##
===========================================
+ Coverage 69.52% 69.69% +0.16%
===========================================
Files 182 182
Lines 17973 18088 +115
===========================================
+ Hits 12496 12606 +110
- Misses 5477 5482 +5
... and 5 files with indirect coverage changes 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
india_compliance/gst_india/utils/e_waybill.py (2)
1591-1684: Sub-supply validation: add explicit “invalid sub_supply_type” handling (avoidNonein errors).
On Line 1626-1629, iftransaction_details.sub_supply_typedoesn’t map back viaSUB_SUPPLY_TYPES,sub_supply_typebecomesNoneand the thrown error becomes confusing (and_validate_sub_supply_description()silently skips). Add a guard right after the reverse lookup.@@ sub_supply_type = next( (k for k, v in SUB_SUPPLY_TYPES.items() if v == sub_supply_type), None, ) + if not sub_supply_type: + frappe.throw(_("Invalid Sub Supply Type for e-Waybill generation"))
94-119: Fix type-checking fordocnamesandvaluesparameters to handle xcall serialization.Line 109 assumes
docnamesis always a string, but frappe.xcall deserializes list selections as a Python list, causingAttributeErroron.startswith("["). Additionally, line 115-117 passesvaluesas a plain dict toupdate_transaction, which uses only attribute access (e.g.,values.transporter,values.vehicle_no), inconsistent with the safer pattern ingenerate_e_waybill(line 145) which always callsfrappe.parse_json(values).Apply type-safe handling for both parameters:
- docnames = frappe.parse_json(docnames) if docnames.startswith("[") else [docnames] + if isinstance(docnames, str): + docnames = frappe.parse_json(docnames) if docnames.startswith("[") else [docnames] + elif isinstance(docnames, (list, tuple)): + docnames = list(docnames) + else: + docnames = [docnames]if values: update_transaction( doc, - frappe.parse_json(values) if isinstance(values, str) else values, + frappe.parse_json(values) if isinstance(values, str) else frappe._dict(values), )india_compliance/gst_india/client_scripts/e_waybill_actions.js (2)
340-529: Fix TDZ risk (din onchange) + stop using hard-codedsplice(5, ...)for port_address insertion.
- Line 368-372 and Line 423-424 reference
dbefore it’s initialized; ifonchangefires during dialog creation, this can throw.- Line 503-518:
splice(5, 0, ...)will insert at the wrong position now that fields are conditional (and it currently lands inside the Doc Details block).function get_generate_e_waybill_dialog(opts, frm, transporter_only = false) { @@ - const fields = []; + const fields = []; + let d; @@ onchange: () => { if (ewaybill_defaults.supply_type.length > 1) { - update_sub_supply_type_options(d, frm); + if (d) update_sub_supply_type_options(d, frm, is_foreign_transaction); } }, }, @@ onchange: () => update_gst_tranporter_id(d), }, @@ - if ( - ["Sales Invoice", "Delivery Note"].includes(frm.doctype) && - is_foreign_transaction - ) { - fields.splice(5, 0, { + if (["Sales Invoice", "Delivery Note"].includes(frm.doctype) && is_foreign_transaction) { + const transporter_idx = fields.findIndex(f => f.fieldname === "transporter"); + const insert_at = transporter_idx >= 0 ? transporter_idx + 1 : 0; + fields.splice(insert_at, 0, { label: "Origin Port / Border Checkpost Address", fieldname: "port_address", fieldtype: "Link", options: "Address", default: frm.doc.port_address, reqd: frm.doc?.__onload?.shipping_address_in_india != true, get_query: () => { return { filters: { country: "India", }, }; }, }); } @@ - const d = new frappe.ui.Dialog(opts); + d = new frappe.ui.Dialog(opts);
531-668: Ensureget_sub_suppy_type_options()always returns a defined object (avoid undefined access in dialog defaults).
In the “default_supply_types[key]” branch (Line 664-665), an unexpected(doctype, is_return)combination returnsundefined, which then breaks callers doingewaybill_defaults.supply_type.length.@@ - return default_supply_types[key]; + const out = default_supply_types[key]; + if (!out) { + frappe.throw?.(__("Unsupported configuration for e-Waybill generation")) || + console.error("Unsupported e-Waybill defaults for", key); + } + return out;
🧹 Nitpick comments (1)
india_compliance/gst_india/client_scripts/delivery_note_list.js (1)
1-10: Guard against missingfrappe.listview_settings[DOCTYPE]to avoid hard crash.
If the script ever loads whenfrappe.listview_settings["Delivery Note"]isn’t initialized, Line 3 throws. A small defensive init makes it safer and consistent with other Frappe patterns.const DOCTYPE = "Delivery Note"; -const erpnext_onload = frappe.listview_settings[DOCTYPE].onload; +frappe.listview_settings[DOCTYPE] = frappe.listview_settings[DOCTYPE] || {}; +const erpnext_onload = frappe.listview_settings[DOCTYPE].onload; frappe.listview_settings[DOCTYPE].onload = function (list_view) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
india_compliance/gst_india/client_scripts/delivery_note_list.js(1 hunks)india_compliance/gst_india/client_scripts/e_waybill_actions.js(7 hunks)india_compliance/gst_india/client_scripts/sales_invoice_list.js(1 hunks)india_compliance/gst_india/utils/e_waybill.py(4 hunks)india_compliance/hooks.py(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
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: 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.
📚 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/delivery_note_list.jsindia_compliance/gst_india/client_scripts/sales_invoice_list.jsindia_compliance/hooks.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/sales_invoice_list.jsindia_compliance/gst_india/utils/e_waybill.pyindia_compliance/gst_india/client_scripts/e_waybill_actions.js
📚 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/sales_invoice_list.jsindia_compliance/hooks.pyindia_compliance/gst_india/utils/e_waybill.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/utils/e_waybill.pyindia_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/utils/e_waybill.pyindia_compliance/gst_india/client_scripts/e_waybill_actions.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/e_waybill_actions.js
🧬 Code graph analysis (3)
india_compliance/gst_india/client_scripts/delivery_note_list.js (1)
india_compliance/gst_india/client_scripts/sales_invoice_list.js (2)
DOCTYPE(1-1)erpnext_onload(3-3)
india_compliance/gst_india/client_scripts/sales_invoice_list.js (1)
india_compliance/gst_india/client_scripts/delivery_note_list.js (1)
DOCTYPE(1-1)
india_compliance/gst_india/client_scripts/e_waybill_actions.js (1)
india_compliance/gst_india/utils/__init__.py (1)
is_foreign_transaction(376-377)
⏰ 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/client_scripts/sales_invoice_list.js (1)
9-9: Nice consolidation: list view now delegates bulk wiring to the shared helper.
Line 9 removes per-doctype duplication and aligns Sales Invoice with the new centralized bulk actions.india_compliance/hooks.py (1)
110-121: Delivery Note list scripts wiring is correct.
All paths exist, load order matches Sales Invoice (applicability → actions → list), andsetup_bulk_e_waybill_actionsis defined once ine_waybill_actions.jsand called appropriately from both list scripts.
…or same GSTIN scenarios
…larity and consistency
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
india_compliance/gst_india/client_scripts/e_waybill_actions.js(6 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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.
📚 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_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_actions.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_actions.js
📚 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_actions.js
📚 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_actions.js
🧬 Code graph analysis (1)
india_compliance/gst_india/client_scripts/e_waybill_actions.js (1)
india_compliance/gst_india/utils/__init__.py (1)
is_foreign_transaction(376-377)
⏰ 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/client_scripts/e_waybill_actions.js (3)
1495-1511: LGTM!Centralized e-Waybill enablement logic provides a clean, single source of truth for doctype-specific settings checks.
1698-1732: Helpful user feedback with tracking links.The implementation provides comprehensive tracking links for background jobs, API requests, and error logs, making it easy for users to monitor bulk generation progress.
492-495: Verify thatis_foreign_transactionis properly defined and accessible.The function
is_foreign_transaction(frm)is invoked at lines 492-495 but its definition is not apparent in this file. If it is defined in another module or through Frappe's server-side methods, ensure it is either:
- Imported into this file's scope, or
- Called via
frappe.call()as a server-side method rather than a direct client-side function callAdditionally, verify that the JavaScript version can accept a
frmobject as a parameter, as the Python utility typically expects(gst_category, place_of_supply)parameters.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
india_compliance/gst_india/client_scripts/e_waybill_actions.js (2)
670-688: Add defensive checks for missing field.Line 684 retrieves the field but lines 685-687 use it without verifying it exists. If the dialog structure changes or the field is conditionally rendered, this will throw a TypeError.
Apply this diff:
function update_sub_supply_type_options(dialog, frm) { const supply_type = dialog.get_value("supply_type"); if (!supply_type) return; const temp_frm = { ...frm, doc: { ...frm.doc, is_return: supply_type === "Inward" ? 1 : 0, }, }; const options = get_sub_supply_type_options(temp_frm); const sub_supply_options = options?.sub_supply_type || []; + if (!sub_supply_options.length) return; + const sub_supply_field = dialog.get_field("sub_supply_type"); + if (!sub_supply_field) return; + sub_supply_field.df.options = sub_supply_options.join("\n"); sub_supply_field.refresh(); - dialog.set_value("sub_supply_type", sub_supply_options[0]); + if (sub_supply_options[0]) { + dialog.set_value("sub_supply_type", sub_supply_options[0]); + } }
1638-1670: Fix off-by-one error in invalid document check.Line 1647 checks
> 1which skips showing the message when exactly one document is invalid. This should check for any invalid documents.Apply this diff:
- if (r.message.invalid_log.length > 1) { + if (r.message.invalid_log.length > 0) { const invalid_docs = r.message.invalid_log.map( doc => `${doc.link} - ${doc.reason}` );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
india_compliance/gst_india/client_scripts/e_waybill_actions.js(6 hunks)india_compliance/gst_india/utils/e_waybill.py(5 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
📚 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/utils/e_waybill.pyindia_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/utils/e_waybill.pyindia_compliance/gst_india/client_scripts/e_waybill_actions.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/utils/e_waybill.pyindia_compliance/gst_india/client_scripts/e_waybill_actions.js
📚 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/e_waybill.pyindia_compliance/gst_india/client_scripts/e_waybill_actions.js
📚 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/utils/e_waybill.pyindia_compliance/gst_india/client_scripts/e_waybill_actions.js
📚 Learning: 2025-05-26T03:12:27.472Z
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:12:27.472Z
Learning: frappe.qb returns objects of type frappe._dict which provides safe dot notation access, meaning accessing non-existent attributes won't raise AttributeError.
Applied to files:
india_compliance/gst_india/utils/e_waybill.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/e_waybill.py
🧬 Code graph analysis (2)
india_compliance/gst_india/utils/e_waybill.py (2)
india_compliance/gst_india/utils/__init__.py (1)
is_api_enabled(732-741)india_compliance/gst_india/utils/transaction_data.py (3)
_throw(189-191)_throw(600-618)_throw(658-664)
india_compliance/gst_india/client_scripts/e_waybill_actions.js (1)
india_compliance/gst_india/utils/__init__.py (1)
is_foreign_transaction(376-377)
⏰ 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 (8)
india_compliance/gst_india/utils/e_waybill.py (3)
94-111: LGTM! Clean integration of values parameter and API validation.The addition of the
valuesparameter and the upfrontvalidate_is_e_waybill_api_enabledcheck properly gate bulk generation. The values are correctly passed through to the enqueued job where they'll be parsed.
114-138: LGTM! Values parsing resolves the previous AttributeError concern.The unconditional use of
frappe.parse_json(values)at line 124 ensures thatvaluesis always converted to afrappe._dict, whether it arrives as a JSON string (direct API call) or plain dict (from RQ deserialization). This prevents the AttributeError whenupdate_transactionaccesses fields via attribute notation.
1625-1694: LGTM! Robust sub-supply type validation with clear error messaging.The validation pipeline correctly enforces GST e-Waybill rules:
- Required description for "Others" type
- Different allowed types for same vs. different GSTIN scenarios
- Separate rules for Inward vs. Outward supply types
The error messages are actionable and list the allowed options.
india_compliance/gst_india/client_scripts/e_waybill_actions.js (5)
340-522: LGTM! Clean refactor enabling dialog reuse for transporter-only updates.The
transporter_onlyparameter elegantly enables the same dialog to serve both full e-Waybill generation and bulk transporter updates. The conditional field construction is well-structured, and the dynamic supply type handling with the onchange callback provides good UX.
524-529: LGTM! Clean refactor extracting foreign transaction logic.This helper consolidates the repeated foreign transaction check and matches the backend utility function signature, promoting consistency across the codebase.
531-668: LGTM! Comprehensive refactor handling all doctype scenarios.The refactored function correctly handles multiple doctypes with appropriate supply type logic:
- Delivery Note with and without company_gstin
- Stock Entry with purpose-based logic
- Sales Invoice foreign transaction handling
- Default mappings for Purchase and Subcontracting doctypes
The broader options provided when company_gstin is unavailable (line 560-582) give users flexibility, while backend validation ensures correctness.
1502-1518: LGTM! Centralized enablement check for all supported doctypes.This helper consolidates the e-Waybill enablement logic across Sales Invoice, Delivery Note, Purchase doctypes, and Stock Entry/Subcontracting scenarios. The structure is clean and matches the backend validation.
1524-1636: LGTM! Comprehensive bulk operations with proper job tracking and validation.The bulk operation framework is well-implemented:
- Proper permission checks before exposing actions
- Status validation with user-friendly messaging for invalid documents
- Job tracking with links to background job, API requests, and error logs
- Clean separation of concerns between setup, validation, and execution
Line 1597 correctly prevents empty arrays from being passed to callbacks.
Also applies to: 1671-1783
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
india_compliance/gst_india/utils/e_waybill.py (1)
1108-1135: Fix f-string in translatable message for proper i18n.Line 1132 uses an f-string inside
_(), which prevents translation tools from extracting the message template.Apply this diff:
- _throw(_("Please enable e-Waybill for {0} in GST Settings").format(doctype)) + _throw(_("Please enable e-Waybill for {0} in GST Settings").format(doctype))Wait, this is actually already correct in the current code. The f-string issue from the past comment appears to be resolved. The current code uses
.format(doctype)which is the correct pattern.Actually, re-reading the past comment, it seems to reference line 1132 having an f-string. Looking at the current line 1132:
_throw(_("Please enable e-Waybill for {0} in GST Settings").format(doctype))This is using
.format(), not an f-string. So this appears to already be correct! Let me verify if the past comment was about a different version...Looking at the past comment more carefully, it says to replace:
_throw(f"Please enable e-Waybill for {doctype} in GST Settings")with:
_throw(_("Please enable e-Waybill for {0} in GST Settings").format(doctype))And the current code already has the corrected version! So this issue was already fixed. I should approve this instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
india_compliance/gst_india/utils/e_waybill.py(5 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
📚 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/utils/e_waybill.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/utils/e_waybill.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/utils/e_waybill.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/utils/e_waybill.py
📚 Learning: 2025-05-26T03:12:27.472Z
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:12:27.472Z
Learning: frappe.qb returns objects of type frappe._dict which provides safe dot notation access, meaning accessing non-existent attributes won't raise AttributeError.
Applied to files:
india_compliance/gst_india/utils/e_waybill.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/e_waybill.py
🧬 Code graph analysis (1)
india_compliance/gst_india/utils/e_waybill.py (2)
india_compliance/gst_india/utils/__init__.py (1)
is_api_enabled(732-741)india_compliance/gst_india/utils/e_invoice.py (1)
_throw(491-493)
⏰ 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/gst_india/utils/e_waybill.py (4)
94-111: LGTM!The addition of the optional
valuesparameter and the per-doctype API enablement check viavalidate_is_e_waybill_api_enabled(doctype)properly gates bulk operations before enqueueing.
114-138: LGTM!The unconditional use of
frappe.parse_json(values)at line 124 correctly handles both JSON strings (direct API calls) and plain dicts (deserialized by RQ afterfrappe.enqueue), ensuringupdate_transaction()always receives afrappe._dictwith attribute access support.
1108-1135: LGTM!The function correctly validates e-Waybill API enablement at both global and per-doctype levels. Line 1132 properly uses
.format(doctype)for i18n rather than an f-string.
1672-1694: This concern is not valid — the code is safe.The
supply_typeretrieved at line 1677 will always be either "I" or "O" becauseupdate_transaction_details()populatestransaction_details["supply_type"]from thedefault_supply_typesdictionary, which explicitly sets it to one of these values for every document type and return status. Even ifsupply_typewere somehowNone, line 1678's conditional defaults to"Outward", and_get_applicable_sub_supply_types()would still return a valid non-empty list (both "Inward" and "Outward" have entries for bothsame_gstinvalues in thetype_map). The scenario described—an empty allowed list causing all sub supply types to be rejected—cannot occur with the current code structure.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
india_compliance/gst_india/client_scripts/e_waybill_actions.js (1)
1755-1787: Bulk operations implemented correctly.The bulk e-Waybill print, generation, and validation flows are well-structured with proper error handling and user feedback. The integration with background jobs and API request tracking (lines 1856-1890) provides good observability.
The
validate_doc_statusfunction (lines 1892-1934) appropriately filters documents by status and prompts users to confirm when excluding invalid documents.Note: Line 1764 checks
if (r.message.invalid_log.length > 1)which skips the message when exactly one document is invalid. While you previously indicated this pattern is out of scope, this particular code is new in this PR (all lines marked with~). Consider changing to> 0in a future cleanup if this behavior is unintended.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
india_compliance/gst_india/client_scripts/e_waybill_actions.js
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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.
📚 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_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_actions.js
📚 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_actions.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_actions.js
📚 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_actions.js
🧬 Code graph analysis (1)
india_compliance/gst_india/client_scripts/e_waybill_actions.js (1)
india_compliance/gst_india/utils/__init__.py (1)
is_foreign_transaction(376-377)
⏰ 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/gst_india/client_scripts/e_waybill_actions.js (4)
294-296: LGTM - Clean secondary action handling.The extraction of dialog values is clear and maintains proper functionality.
340-814: Excellent refactoring - clean separation and reusability.The extraction of field builders into dedicated helper functions significantly improves code maintainability:
get_transporter_fields,get_bulk_transporter_part_a_fields, andget_bulk_transporter_part_b_fieldseliminate duplication between single-document and bulk-operation dialogsget_document_detail_fieldsandget_sub_supply_type_optionsprovide proper sub-supply type handling based on doctype and contextis_foreign_transactionhelper appropriately duplicates server-side logic for client-side use (fromindia_compliance/gst_india/utils/__init__.py)- The expanded Delivery Note logic (lines 660-708) correctly handles both cases: with and without
company_gstin, providing appropriate fallback optionsThe defensive coding at line 809 with optional chaining (
options?.sub_supply_type || []) properly guards against edge cases.
1628-1644: LGTM - Clear doctype-specific enablement logic.The function properly gates e-Waybill features based on both global settings and doctype-specific configurations. The logic is straightforward and maintainable.
1646-1754: Well-structured bulk action setup.The setup functions follow a consistent pattern with proper permission and feature-flag checks. The fix at line 1715 correctly prevents callbacks with empty arrays by checking both truthiness and length.
The modular approach makes it easy to add bulk actions for additional doctypes in the future.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
india_compliance/gst_india/utils/e_waybill.py (1)
1640-1650: Consider validating sub_supply_type code before reverse lookup.Lines 1640-1643 use
next()to reverse-lookup the key fromSUB_SUPPLY_TYPESby value. Ifsub_supply_typecontains an invalid code not inSUB_SUPPLY_TYPES.values(),next()returnsNone, leading to the error message "Invalid Sub Supply Type: {code}" at lines 1646-1650.While this works, adding an explicit check before the lookup would produce a clearer error:
# Validate that the code exists in SUB_SUPPLY_TYPES values if sub_supply_type not in SUB_SUPPLY_TYPES.values(): frappe.throw( _("Invalid Sub Supply Type code: {0}").format(sub_supply_type) )However, based on your earlier clarification that the UI restricts to valid options and the government API validates edge cases, this additional check is optional.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
india_compliance/gst_india/client_scripts/e_waybill_actions.jsindia_compliance/gst_india/utils/e_waybill.py
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
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.
📚 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/utils/e_waybill.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/utils/e_waybill.pyindia_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/utils/e_waybill.pyindia_compliance/gst_india/client_scripts/e_waybill_actions.js
📚 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/e_waybill.pyindia_compliance/gst_india/client_scripts/e_waybill_actions.js
📚 Learning: 2025-05-26T03:12:27.472Z
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:12:27.472Z
Learning: frappe.qb returns objects of type frappe._dict which provides safe dot notation access, meaning accessing non-existent attributes won't raise AttributeError.
Applied to files:
india_compliance/gst_india/utils/e_waybill.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/e_waybill.py
📚 Learning: 2025-06-20T11:11:54.124Z
Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3459
File: india_compliance/gst_india/report/hsn_wise_summary_of_inward_supplies/hsn_wise_summary_of_inward_supplies.js:50-59
Timestamp: 2025-06-20T11:11:54.124Z
Learning: In the India Compliance codebase, for convenience features like automatic checkbox setting in report filters, extensive error handling may not be necessary if users can still manually control the feature when the automatic functionality fails.
Applied to files:
india_compliance/gst_india/utils/e_waybill.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_actions.js
🧬 Code graph analysis (1)
india_compliance/gst_india/utils/e_waybill.py (1)
india_compliance/gst_india/utils/__init__.py (2)
load_doc(58-64)is_api_enabled(732-741)
⏰ 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 (14)
india_compliance/gst_india/utils/e_waybill.py (4)
94-111: LGTM! Values parameter correctly forwarded to enqueue.The
valuesparameter is properly threaded through tofrappe.enqueue, and the subsequent parsing ingenerate_e_waybills(line 119) ensures it's converted tofrappe._dictfor attribute access inupdate_transaction.
114-127: LGTM! Values parsing ensures consistent frappe._dict type.Unconditional
frappe.parse_json(values)at line 119 guarantees thatupdate_transactionreceives afrappe._dictwith attribute access support, resolving the concern from the past review.
1110-1137: LGTM! Centralized API enablement validation.The function properly checks global e-waybill enablement and doctype-specific settings. The setting map covers all permitted doctypes, and error messages are correctly translatable.
1681-1703: LGTM! Sub supply type validation correctly enforces GSTIN-based rules.The validation properly checks whether the sub supply type is allowed based on:
- Supply direction (Inward vs Outward)
- GSTIN relationship (same vs different)
The error message at lines 1694-1702 clearly communicates the context and allowed types.
india_compliance/gst_india/client_scripts/e_waybill_actions.js (10)
340-441: LGTM! Transporter fields refactored into reusable function.The extraction of transporter fields into
get_transporter_fields(frm)reduces duplication and maintains all original functionality including defaults, validations, and onchange handlers.
530-581: LGTM! Document detail fields properly encapsulated.The function correctly generates Supply Type, Document Type, Sub Supply Type, and Sub Supply Description fields with appropriate defaults, dependencies, and dynamic behavior based on
get_sub_supply_type_options.
583-626: LGTM! Dialog refactored to use field helper functions.The refactoring properly composes fields from
get_document_detail_fieldsandget_transporter_fields, and correctly handles the conditionalport_addressfield for foreign transactions in Sales Invoice and Delivery Note.
649-654: LGTM! Foreign transaction check is correct.The function properly identifies foreign transactions by checking both
gst_categoryandplace_of_supply, using optional chaining for safety.
659-707: LGTM! Delivery Note sub supply type logic aligns with backend validation.The enhanced logic for Delivery Note correctly handles:
- With company_gstin (lines 659-684): Determines
same_gstinand provides appropriate options based on whether it's a return and whether GSTINs match.- Without company_gstin (lines 685-707): Allows selection of both "Inward" and "Outward" supply types, with comprehensive sub supply type options that cover all backend-validated scenarios.
The options align with the backend validation in
_get_applicable_sub_supply_types(lines 1662-1679 of e_waybill.py).
795-813: LGTM! Sub supply type options correctly updated.The function properly:
- Constructs
temp_frmwith modifiedis_returnbased on selected supply type- Spreads all
frmandfrm.docproperties to preserve context- Updates the dialog field options with defensive optional chaining
1628-1644: LGTM! Doctype-specific e-waybill enablement check.The function correctly mirrors the backend validation in
validate_is_e_waybill_api_enabled, checking global settings and doctype-specific flags.
1710-1719: LGTM! Bulk action helper properly validates document selection.The function correctly:
- Validates document status using
validate_doc_status- Checks both
submitted_docsexistence and length before invoking callback (line 1715)- Prevents empty
docnamesfrom being sent to the backend
1789-1796: LGTM! Optional values parameter correctly handled.The function conditionally adds
valuestoargsonly when truthy (lines 1790-1791), correctly supporting the optional parameter pattern in the backendenqueue_bulk_e_waybill_generation.
1805-1854: LGTM! Bulk generation dialog properly structured.The dialog construction:
- Uses
get_bulk_generate_dialog_fieldsto build field list (line 1809)- Includes toggle for updating transporter details (lines 1834-1838)
- Sets
valuestonullwhen toggle is unchecked (lines 1813-1815), preventing unnecessary backend processing- Properly composes document detail and transporter fields
…nd remove duplication
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
india_compliance/gst_india/utils/test_e_waybill.py (1)
982-1003: Test correctly validates same-GSTIN rules; consider adding positive test cases for completeness.The test properly exercises the validation logic—"Job Work" is confirmed as invalid for same-GSTIN outward supplies. The test approach is sound:
- Manual attribute assignment (
transaction_details,bill_from,bill_to) is an appropriate pattern for unit testing validation in isolation.- The validation method correctly checks that "Job Work" is not in the allowed list for same-GSTIN scenarios.
Two optional enhancements:
Readability: Replace magic number
4with the constant fromSUB_SUPPLY_TYPES:SUB_SUPPLY_TYPES["Job Work"]. This requires importing the constant but improves clarity.Test coverage: Add a positive assertion verifying that valid types—such as
5("For Own Use")—are accepted with same GSTIN in outward supplies.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
india_compliance/gst_india/utils/test_e_waybill.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 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/test_e_waybill.py
🧬 Code graph analysis (1)
india_compliance/gst_india/utils/test_e_waybill.py (1)
india_compliance/gst_india/utils/e_waybill.py (1)
validate_sub_supply_type(1627-1653)
⏰ 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: Python Unit Tests
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@india_compliance/gst_india/client_scripts/e_waybill_actions.js`:
- Around line 1700-1717: In show_bulk_generate_e_waybill_dialog, do not set
values = null when update_transporter_details is false because that discards
required document fields; instead construct a payload that always includes
document detail keys (e.g., supply_type, sub_supply_type, sub_supply_desc) and
only omit transporter-related keys before calling
enqueue_bulk_e_waybill_generation; modify the primary_action to either remove
transporter fields from values or build a new object with the document-detail
properties and conditionally add transporter info, referencing
show_bulk_generate_e_waybill_dialog, get_bulk_generate_dialog_fields, the values
variable, and enqueue_bulk_e_waybill_generation to locate where to change the
logic.
🧹 Nitpick comments (1)
india_compliance/gst_india/utils/e_waybill.py (1)
1060-1063: Consider defensive handling for invalid sub_supply_type.If an invalid
sub_supply_typeis passed (e.g., via direct API call),SUB_SUPPLY_TYPES[values.sub_supply_type]at line 1061 will raise aKeyErrorbefore the more descriptive validation invalidate_sub_supply_typeruns.♻️ Optional: Add validation before lookup
if doc.doctype in ("Delivery Note", "Stock Entry", "Subcontracting Receipt"): + if values.sub_supply_type not in SUB_SUPPLY_TYPES: + frappe.throw(_("Invalid Sub Supply Type: {0}").format(values.sub_supply_type)) doc._sub_supply_type = SUB_SUPPLY_TYPES[values.sub_supply_type]
2 participants
TODOs
no-docs
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.