feat: Template for P&L and BS of Schedule III#3993
feat: Template for P&L and BS of Schedule III#3993Abdeali099 wants to merge 15 commits intodevelopfrom
Conversation
india_compliance/income_tax_india/financial_report_template/account_categories.json
Outdated
Show resolved
Hide resolved
india_compliance/income_tax_india/financial_report_template/account_categories.json
Outdated
Show resolved
Hide resolved
india_compliance/income_tax_india/financial_report_template/account_categories.json
Outdated
Show resolved
Hide resolved
india_compliance/income_tax_india/financial_report_template/account_categories.json
Outdated
Show resolved
Hide resolved
india_compliance/income_tax_india/financial_report_template/account_categories.json
Outdated
Show resolved
Hide resolved
...rt_template/standard_balance_sheet_(schedule_iii)/standard_balance_sheet_(schedule_iii).json
Outdated
Show resolved
Hide resolved
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds two financial report templates (Standard Balance Sheet (Schedule III) and Standard Profit and Loss (Schedule III)) and their package initializers; updates patches to include a v16 sync patch and adds Estimated code review effort🎯 9 (High) | ⏱️ ~4–6 hours Suggested labelsbackport version-16-hotfix |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@india_compliance/income_tax_india/financial_report_template/standard_balance_sheet_`(schedule_iii)/standard_balance_sheet_(schedule_iii).json:
- Around line 90-121: Several Schedule III line items (reference_code values
SF_MONEY_WARRANTS, SHARE_APP_MONEY, NCL_DTL, NCA_CWIP, NCA_INTANGIBLE_DEV,
NCA_DTA, NCA_LOANS_ADV, CA_LOANS_ADV) are hardcoded with "calculation_formula":
"0" and "data_source": "Calculated Amount", which silently zeroes material
balances and understates parent aggregates (e.g., NCL_TOTAL, NCA_TOTAL,
NCA_FIXED_ASSETS, CA_TOTAL); fix by switching those entries to use
"data_source": "Account Data" and a proper calculation_formula that pulls the
mapped ERPNext account totals (or the appropriate aggregate function) when those
account_category mappings exist, and if mappings do not exist set
"hide_when_empty": 1 for each offending entry (or add a visible disclaimer row)
so users are warned; update the entries identified by their reference_code above
(and the repeated blocks noted at the other ranges) rather than line numbers.
In
`@india_compliance/income_tax_india/financial_report_template/standard_profit_and_loss_`(schedule_iii)/standard_profit_and_loss_(schedule_iii).json:
- Around line 366-413: DISCONTINUING_OPS and DISCONTINUING_TAX have inconsistent
reverse_sign values which will break the calculation_formula in
DISCONTINUING_AFTER_TAX when real account mappings are added; update the
reverse_sign on DISCONTINUING_TAX to match DISCONTINUING_OPS (make both
reverse_sign: 1) so both profit/loss items use the same sign convention and the
formula DISCONTINUING_AFTER_TAX = DISCONTINUING_OPS - DISCONTINUING_TAX yields
correct results.
- Around line 190-205: The EXP_DEPRECIATION row currently filters only by
["account_type","=","Depreciation"], which causes accounts that are both
Depreciation and account_category = "Cost of Goods Sold" to be counted again in
EXP_MATERIALS/EXP_STOCK_TRADE/EXP_FINANCE; fix by updating the JSON filters:
either add ["account_type","!=","Depreciation"] to each of EXP_MATERIALS,
EXP_STOCK_TRADE and EXP_FINANCE (so COGS buckets exclude depreciation) or modify
EXP_DEPRECIATION to also require ["account_category","!=","Cost of Goods Sold"]
— update the calculation_formula arrays for those entries (EXP_MATERIALS,
EXP_STOCK_TRADE, EXP_FINANCE or EXP_DEPRECIATION) to include the appropriate
exclusion condition.
In `@india_compliance/patches/v16/sync_financial_report_templates.py`:
- Line 1: The file sync_financial_report_templates.py has the wrong copyright
header; replace the existing top-line copyright (currently referencing 2025 and
Frappe Technologies Pvt. Ltd.) with the project's standard header "# Copyright
(c) 2026, Resilient Tech and contributors" so it matches the three new
__init__.py files in this PR and the repo convention.
- Around line 5-7: The patch imports the internal function _sync_templates_for
(aliased as sync_financial_report_templates) from
erpnext.accounts.doctype.financial_report_template.financial_report_template
which is unstable; remove this private import and instead implement syncing via
a stable mechanism—either call ERPNext's REST resource /api/resource/Financial
Report Template to CRUD templates programmatically or load them as fixtures
during deployment; replace usages of sync_financial_report_templates with the
new REST/fixture-based flow and ensure error handling and idempotency around
creating/updating Financial Report Template records.
...rt_template/standard_balance_sheet_(schedule_iii)/standard_balance_sheet_(schedule_iii).json
Show resolved
Hide resolved
...emplate/standard_profit_and_loss_(schedule_iii)/standard_profit_and_loss_(schedule_iii).json
Show resolved
Hide resolved
...emplate/standard_profit_and_loss_(schedule_iii)/standard_profit_and_loss_(schedule_iii).json
Show resolved
Hide resolved
india_compliance/patches/v16/sync_financial_report_templates.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@india_compliance/income_tax_india/financial_report_template/standard_profit_and_loss_`(schedule_iii)/standard_profit_and_loss_(schedule_iii).json:
- Around line 206-221: The EXP_OTHER entry's advanced_filtering is incorrectly
set to 0 even though its calculation_formula uses multiple account_name "not
like" conditions; update the EXP_OTHER object's "advanced_filtering" value from
0 to 1 so the account_name filters are applied (ensure you edit the object with
reference_code "EXP_OTHER" and its calculation_formula that contains the
account_name exclusions).
---
Duplicate comments:
In `@india_compliance/patches/v16/sync_financial_report_templates.py`:
- Line 1: Update the copyright header in
india_compliance/patches/v16/sync_financial_report_templates.py by replacing
"Frappe Technologies Pvt. Ltd. and contributors" with "Resilient Tech and
contributors" so the file reflects the correct copyright owner; ensure the year
(2026) remains unchanged and the rest of the header line is preserved exactly.
...emplate/standard_profit_and_loss_(schedule_iii)/standard_profit_and_loss_(schedule_iii).json
Show resolved
Hide resolved
12cad59 to
37a5175
Compare
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@india_compliance/patches/v16/sync_financial_report_templates.py`:
- Around line 5-7: You are importing and using the ERPNext-internal symbol
_sync_templates_for (aliased as sync_financial_report_templates) — this is an
accepted but fragile dependency, so add a clear inline comment documenting the
risk and wrap the import/use in a runtime guard: attempt to import or getattr
the symbol (_sync_templates_for) from
erpnext.accounts.doctype.financial_report_template.financial_report_template
inside a try/except, and if missing log/raise a clear, actionable error telling
integrators to verify the function exists (suggest the provided ripgrep command)
or to use the public replacement; ensure the code references the exact symbol
names _sync_templates_for and sync_financial_report_templates so future
reviewers can find and update the fallback.
- Line 1: Update the file header string "Copyright (c) 2026, Frappe Technologies
Pvt. Ltd. and contributors" to the project-standard copyright holder and format
(i.e. replace "Frappe Technologies Pvt. Ltd." with the correct organization name
used across the repo while keeping the updated year 2026), ensuring the exact
header line is changed to "Copyright (c) 2026, <Project Standard Copyright
Holder> and contributors".
Confidence Score: 2/5
|
| Filename | Overview |
|---|---|
| india_compliance/income_tax_india/financial_report_template/standard_balance_sheet_(schedule_iii)/standard_balance_sheet_(schedule_iii).json | New Balance Sheet template for Schedule III. Contains a semantic mismatch: "Short-Term Loans and Advances" row queries account_category = "Other Receivables". Several important categories (CWIP, DTA, DTL, Long-Term Loans) are hardcoded to 0, which will silently return zero regardless of actual account balances. |
| india_compliance/income_tax_india/financial_report_template/standard_profit_and_loss_(schedule_iii)/standard_profit_and_loss_(schedule_iii).json | New P&L template for Schedule III with a well-structured flow from Revenue → Expenses → Profit. Main concern is the name-based filtering for Employee Benefits and Other Expenses which is brittle and company-specific. The VARIANCE check row (hide_when_empty) is a useful validation feature. |
| india_compliance/patches/v16/sync_financial_report_templates.py | Simple patch to sync the new templates on install. Relies on a private function _sync_templates_for from ERPNext, and uses the wrong copyright entity ("Frappe Technologies" instead of "Resilient Tech"). |
| india_compliance/patches.txt | New patch entry appended correctly at the end of the [post_model_sync] section. |
| india_compliance/income_tax_india/financial_report_template/init.py | Standard empty __init__.py with copyright header, no issues. |
Comments Outside Diff (1)
-
india_compliance/income_tax_india/financial_report_template/standard_profit_and_loss_(schedule_iii)/standard_profit_and_loss_(schedule_iii).json, line 880-893 (link)Name-based filtering for employee expenses is brittle
EXP_EMPLOYEE(Employee Benefits Expense) usesaccount_name likepatterns — "Employee", "Salary", "Wages", "Staff", "Gratuity", "Provident Fund", "Bonus" — to classify accounts. Correspondingly,EXP_OTHERexcludes the same keywords from "Operating Expenses". A few concerns:- Companies naming their payroll account "Personnel Costs" or "Human Resources" would not be captured in
EXP_EMPLOYEEand would fall intoEXP_OTHERinstead. - An account like "Customer Bonus" or "Sales Staff Commission" could inadvertently match or not match, depending on the company's naming conventions.
- This is a template the user cannot easily override per account — they'd need to modify the JSON directly.
Consider whether ERPNext's Indian COA already has a dedicated
account_category(e.g.,"Employee Benefits Expense"or similar) that could be used instead of name-matching, making the template more reliable out of the box. - Companies naming their payroll account "Personnel Costs" or "Human Resources" would not be captured in
Last reviewed commit: "fix: use account cat..."
| "include_in_charts": 0, | ||
| "indentation_level": 1, | ||
| "italic_text": 0, | ||
| "reference_code": "CA_OTHER", | ||
| "reverse_sign": 0 | ||
| }, | ||
| { | ||
| "advanced_filtering": 0, | ||
| "balance_type": "", | ||
| "bold_text": 0, | ||
| "data_source": "Blank Line", |
There was a problem hiding this comment.
Account category mismatch for "Short-Term Loans and Advances"
The display_name is "e. Short-Term Loans and Advances" but the calculation_formula filters on account_category = "Other Receivables". Under Schedule III, these are distinct classifications — Short-Term Loans and Advances covers advances to subsidiaries, deposits, advance tax, etc., while Other Receivables typically covers miscellaneous receivables. Using "Other Receivables" here will pull the wrong set of accounts and may both miss genuine loans/advances and inadvertently include unrelated receivables.
Consider mapping this to a dedicated account category (e.g., "Loans and Advances" or the equivalent category used in the Indian chart of accounts) or, if such a category does not exist in the standard COA, document why "Other Receivables" is the intended proxy.
| "include_in_charts": 0, | |
| "indentation_level": 1, | |
| "italic_text": 0, | |
| "reference_code": "CA_OTHER", | |
| "reverse_sign": 0 | |
| }, | |
| { | |
| "advanced_filtering": 0, | |
| "balance_type": "", | |
| "bold_text": 0, | |
| "data_source": "Blank Line", | |
| "calculation_formula": "[\"account_category\", \"=\", \"Loans and Advances\"]", | |
| "display_name": "e. Short-Term Loans and Advances", |
| "hidden_calculation": 0, | ||
| "hide_when_empty": 0, | ||
| "include_in_charts": 0, | ||
| "indentation_level": 2, | ||
| "italic_text": 0, | ||
| "reference_code": "NCA_INTANGIBLE_DEV", | ||
| "reverse_sign": 0 | ||
| }, | ||
| { | ||
| "advanced_filtering": 0, | ||
| "balance_type": "Closing Balance", | ||
| "bold_text": 0, | ||
| "calculation_formula": "[\"account_category\", \"=\", \"Long-term Investments\"]", |
There was a problem hiding this comment.
Important Balance Sheet categories hardcoded to zero
Several Schedule III line items that many companies actively use are hardcoded to "calculation_formula": "0" with "data_source": "Calculated Amount", meaning they will always display as ₹0 regardless of what accounts exist in the books:
NCA_CWIP— Capital Work-in-Progress (reverse_sign: 0/ lines 452–465)NCA_INTANGIBLE_DEV— Intangible Assets Under Development (lines 467–481)NCL_DTL— Deferred Tax Liabilities (Net) (lines 177–191)NCA_DTA— Deferred Tax Assets (Net) (lines 498–513)NCA_LOANS_ADV— Long-Term Loans and Advances (lines 515–529)SF_MONEY_WARRANTS— Money Received Against Share Warrants (lines 112–127)SHARE_APP_MONEY— Share Application Money Pending Allotment (lines 128–143)
For NCA_CWIP, ERPNext's Indian COA has a "Capital Work in Progress" account type or similar category that could be mapped. Silently returning 0 for these items means users won't notice that data is missing, which could result in a materially incorrect Balance Sheet.
| from erpnext.accounts.doctype.financial_report_template.financial_report_template import ( | ||
| _sync_templates_for as sync_financial_report_templates, | ||
| ) |
There was a problem hiding this comment.
Importing a private function from ERPNext
_sync_templates_for is prefixed with an underscore, marking it as an internal/private function in the ERPNext codebase. Relying on private APIs is fragile — the function signature or existence could change across ERPNext releases without notice, silently breaking this patch.
If a public equivalent exists (or can be added to ERPNext), that would be safer. At minimum, this dependency should be noted in the PR/changelog so future upgraders are aware.
| @@ -0,0 +1,13 @@ | |||
| # Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and contributors | |||
There was a problem hiding this comment.
Copyright header names wrong entity
All other files added in this PR use "Resilient Tech and contributors" as the copyright holder, but this patch file uses "Frappe Technologies Pvt. Ltd. and contributors". This should be consistent with the rest of the app.
| # Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and contributors | |
| # Copyright (c) 2026, Resilient Tech and contributors |
|
Rebase and resolve conflicts |
3094fd3 to
823136b
Compare
Done 🆗 |
no-docsNote
Backport to V-16