Skip to content

fix: initialize all tax columns to resolve Key error#53323

Open
ljain112 wants to merge 2 commits intofrappe:developfrom
ljain112:fix-itemised-report-groupby
Open

fix: initialize all tax columns to resolve Key error#53323
ljain112 wants to merge 2 commits intofrappe:developfrom
ljain112:fix-itemised-report-groupby

Conversation

@ljain112
Copy link
Collaborator

regression: #50979

 Traceback (most recent call last):
13:35:28 web.1            |   File "apps/frappe/frappe/app.py", line 121, in application
13:35:28 web.1            |     response = frappe.api.handle(request)
13:35:28 web.1            |   File "apps/frappe/frappe/api/__init__.py", line 63, in handle
13:35:28 web.1            |     data = endpoint(**arguments)
13:35:28 web.1            |   File "apps/frappe/frappe/api/v1.py", line 40, in handle_rpc_call
13:35:28 web.1            |     return frappe.handler.handle()
13:35:28 web.1            |            ~~~~~~~~~~~~~~~~~~~~~^^
13:35:28 web.1            |   File "apps/frappe/frappe/handler.py", line 53, in handle
13:35:28 web.1            |     data = execute_cmd(cmd)
13:35:28 web.1            |   File "apps/frappe/frappe/handler.py", line 86, in execute_cmd
13:35:28 web.1            |     return frappe.call(method, **frappe.form_dict)
13:35:28 web.1            |            ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13:35:28 web.1            |   File "apps/frappe/frappe/__init__.py", line 1131, in call
13:35:28 web.1            |     return fn(*args, **newargs)
13:35:28 web.1            |   File "apps/frappe/frappe/utils/typing_validations.py", line 49, in wrapper
13:35:28 web.1            |     return func(*args, **kwargs)
13:35:28 web.1            |   File "apps/frappe/frappe/__init__.py", line 493, in wrapper_fn
13:35:28 web.1            |     retval = fn(*args, **get_newargs(fn, kwargs))
13:35:28 web.1            |   File "apps/frappe/frappe/desk/query_report.py", line 242, in run
13:35:28 web.1            |     result = generate_report_result(
13:35:28 web.1            |     	report, filters, user, custom_columns, is_tree, parent_field, skip_total_calculation
13:35:28 web.1            |     )
13:35:28 web.1            |   File "apps/frappe/frappe/__init__.py", line 493, in wrapper_fn
13:35:28 web.1            |     retval = fn(*args, **get_newargs(fn, kwargs))
13:35:28 web.1            |   File "apps/frappe/frappe/desk/query_report.py", line 92, in generate_report_result
13:35:28 web.1            |     res = get_report_result(report, filters) or []
13:35:28 web.1            |           ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
13:35:28 web.1            |   File "apps/frappe/frappe/desk/query_report.py", line 67, in get_report_result
13:35:28 web.1            |     res = report.execute_script_report(filters)
13:35:28 web.1            |   File "apps/frappe/frappe/core/doctype/report/report.py", line 193, in execute_script_report
13:35:28 web.1            |     res = self.execute_module(filters)
13:35:28 web.1            |   File "apps/frappe/frappe/core/doctype/report/report.py", line 209, in execute_module
13:35:28 web.1            |     return frappe.get_attr(method_name)(frappe._dict(filters))
13:35:28 web.1            |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
13:35:28 web.1            |   File "apps/erpnext/erpnext/accounts/report/item_wise_sales_register/item_wise_sales_register.py", line 21, in execute
13:35:28 web.1            |     return _execute(filters)
13:35:28 web.1            |   File "apps/erpnext/erpnext/accounts/report/item_wise_sales_register/item_wise_sales_register.py", line 128, in _execute
13:35:28 web.1            |     add_sub_total_row(row, total_row_map, d.get(group_by_field, ""), tax_columns)
13:35:28 web.1            |     ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13:35:28 web.1            |   File "apps/erpnext/erpnext/accounts/report/item_wise_sales_register/item_wise_sales_register.py", line 761, in add_sub_total_row
13:35:28 web.1            |     total_row[f"{tax}_amount"] += flt(item[f"{tax}_amount"])
13:35:28 web.1            |                                       ~~~~^^^^^^^^^^^^^^^^^
13:35:28 web.1            | KeyError: '206c(1h)_amount'

Frappe Support Issue: https://support.frappe.io/desk/hd-ticket/61861

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: b74d3f2e-2c73-4d49-9e75-b07fd1512d0d

📥 Commits

Reviewing files that changed from the base of the PR and between 893b5db and 59eb8df.

📒 Files selected for processing (1)
  • erpnext/accounts/report/item_wise_purchase_register/item_wise_purchase_register.py

📝 Walkthrough

Walkthrough

Adds per-run default_taxes maps in both item-wise sales and purchase register report execution paths that prepopulate zero-initialized tax fields ({tax}_rate and {tax}_amount) for each tax column and merges a copy into each generated row so tax fields exist even when no tax details apply. Updates the sales report tests: create_sales_invoice now accepts item and taxes parameters and returns the created invoice; a new test test_grouped_report_handles_different_tax_descriptions creates invoices with distinct tax descriptions/rates and asserts grouped report tax amount columns are produced and populated.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: initialize all tax columns to resolve Key error' directly describes the main change: initializing tax columns to fix a KeyError, which matches the core fix in the changeset.
Description check ✅ Passed The description references regression #50979 and includes a detailed stack trace showing the KeyError: '206c(1h)_amount', providing context for why tax column initialization is necessary to resolve the issue.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 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.

@codecov
Copy link

codecov bot commented Mar 11, 2026

Codecov Report

❌ Patch coverage is 93.10345% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.35%. Comparing base (ac8f3d7) to head (59eb8df).
⚠️ Report is 3 commits behind head on develop.

Files with missing lines Patch % Lines
...e_purchase_register/item_wise_purchase_register.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #53323      +/-   ##
===========================================
+ Coverage    79.29%   79.35%   +0.05%     
===========================================
  Files         1171     1171              
  Lines       124333   124361      +28     
===========================================
+ Hits         98590    98686      +96     
+ Misses       25743    25675      -68     
Files with missing lines Coverage Δ
...em_wise_sales_register/item_wise_sales_register.py 81.08% <100.00%> (+24.78%) ⬆️
...se_sales_register/test_item_wise_sales_register.py 100.00% <100.00%> (ø)
...e_purchase_register/item_wise_purchase_register.py 72.72% <60.00%> (-0.55%) ⬇️

... and 3 files with indirect coverage changes

🚀 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant