Fix account coa#25
Conversation
WalkthroughServer-side account validation now coercively checks is_group using cint before decisions about root/group. Client-side COA tree UI guards were relaxed to allow creating root accounts and adding children to root nodes when nodes are expandable and not hidden. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant UI as COA Tree UI
participant S as Account DocType (Server)
participant DB as Database
U->>UI: Click "New" or "Add Child"
UI->>S: Submit account creation request
S->>S: validate_root_details() / validate_group_or_ledger()<br/>- use cint(is_group) for root/group checks
S->>DB: Insert / Update account
DB-->>S: Success / Error
S-->>UI: Result (success or validation error)
UI-->>U: Display outcome
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
erpnext/accounts/doctype/account/account.py (1)
170-183: Standardizeis_groupchecks to usecintThe truthiness checks on
self.is_groupcan misbehave if the field is passed as"0"/"1". Normalize both occurrences to avoid subtle bugs:• erpnext/accounts/doctype/account/account.py:170
• erpnext/accounts/doctype/account/account.py:262@@ erpnext/accounts/doctype/account/account.py - if self.is_group: + if cint(self.is_group): db_value = self.get_doc_before_save() if db_value: @@ - elif self.is_group: + elif cint(self.is_group): if self.account_type and not self.flags.exclude_account_type_check: throw(_("Cannot covert to Group because Account Type is selected."))erpnext/accounts/doctype/account/account_tree.js (1)
230-244: Ensure “New” action respects ignore_root_company_validationThe primary “New” button currently always blocks when a root company is selected. To match the “Add Child” toolbar behavior (which allows creation when frappe.flags.ignore_root_company_validation is true), swap the condition order so that “New” also skips validation if that flag is set.
Locations to update:
- erpnext/accounts/doctype/account/account_tree.js, inside the post_render → set_primary_action block
Revised diff:
@@ post_render: function (treeview) { - if (root_company) { - frappe.throw(__("Please add the account to root level Company - {0}"), [ - root_company, - ]); - } else { - treeview.new_node(); - } + // Allow account creation against child companies when flagged + if (!root_company || frappe.flags.ignore_root_company_validation) { + treeview.new_node(); + } else { + frappe.throw(__("Please add the account to root level Company - {0}"), [ + root_company, + ]); + }
🧹 Nitpick comments (1)
erpnext/accounts/doctype/account/account.py (1)
207-215: Add a regression test for is_group provided as a stringTo prevent future regressions, add a test that creates a root account with is_group="0"/"1" as strings and asserts correct validation behavior (root requires group; child inherits root_type).
I can draft a pytest for Account.validate_root_details and set_root_and_report_type if you want.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
erpnext/accounts/doctype/account/account.py(1 hunks)erpnext/accounts/doctype/account/account_tree.js(1 hunks)
⏰ 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). (1)
- GitHub Check: Patch Test
🔇 Additional comments (2)
erpnext/accounts/doctype/account/account.py (1)
213-214: Robust root validation with cint(self.is_group) — good fixCasting is_group avoids the "0" string pitfall and enforces that root must be a group. This aligns with the PR objective and prevents the incorrect Root Type validation from surfacing.
erpnext/accounts/doctype/account/account_tree.js (1)
257-258: Allowing Add Child on root nodes is consistent with permitting root-level maintenanceRemoving the extra guard and relying on node.expandable and node.hide_add is appropriate. This matches the backend constraint that roots must be groups.
Please verify the UX flow when the selected root node is a ledger (non-expandable). The button should be disabled as intended due to node.expandable=false.
1 participant
The fix was incorrect. Root account creation is allowed from the charts of accounts.
The problem was an incorrect validation message.
Root Account should always be a group account.
But due to an incorrect validation error, a missing root type was thrown.
Issue: 0 being passed as a string.
Before:
image
After:
image
Summary by CodeRabbit
New Features
Bug Fixes