Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: COA importer importing all accounts as group #28145

Merged
merged 1 commit into from
Oct 29, 2021

Conversation

deepeshgarg007
Copy link
Member

Fixes: #28112

@Alchez
Copy link
Contributor

Alchez commented Oct 29, 2021

@deepeshgarg007, is there a better way to identify group and non-group accounts? Every time a column is added, this logic may need to be updated. I had brought this issue up earlier in #23687, but that went nowhere.

I still think manual setting should be the only way to set an account as a group account, but that may be a breaking change.

@codecov
Copy link

codecov bot commented Oct 29, 2021

Codecov Report

Merging #28145 (8ccd3fe) into develop (b01635e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop   #28145   +/-   ##
========================================
  Coverage    55.39%   55.40%           
========================================
  Files         1248     1248           
  Lines        67681    67695   +14     
========================================
+ Hits         37494    37505   +11     
- Misses       30187    30190    +3     
Impacted Files Coverage Δ
...ype/account/chart_of_accounts/chart_of_accounts.py 77.55% <100.00%> (-0.69%) ⬇️
erpnext/education/doctype/student/student.py 73.95% <0.00%> (-3.13%) ⬇️
...value/warehouse_wise_item_balance_age_and_value.py 92.59% <0.00%> (-2.47%) ⬇️
erpnext/stock/report/stock_ledger/stock_ledger.py 75.80% <0.00%> (-2.42%) ⬇️
...ion/doctype/course_enrollment/course_enrollment.py 45.09% <0.00%> (-1.97%) ⬇️
...ctype/accounting_dimension/accounting_dimension.py 64.34% <0.00%> (-1.56%) ⬇️
...e/period_closing_voucher/period_closing_voucher.py 88.23% <0.00%> (-1.48%) ⬇️
erpnext/stock/report/stock_ageing/stock_ageing.py 90.64% <0.00%> (-0.72%) ⬇️
...next/accounts/doctype/subscription/subscription.py 82.02% <0.00%> (-0.57%) ⬇️
... and 14 more

@deepeshgarg007
Copy link
Member Author

@deepeshgarg007, is there a better way to identify group and non-group accounts? Every time a column is added, this logic may need to be updated. I had brought this issue up earlier in #23687, but that went nowhere.

I still think manual setting should be the only way to set an account as a group account, but that may be a breaking change.

@Alchez I agree. Will have to see where there are implications of this change. Maybe you can raise a new PR again

@deepeshgarg007 deepeshgarg007 merged commit eb720d1 into frappe:develop Oct 29, 2021
deepeshgarg007 added a commit that referenced this pull request Oct 29, 2021
…-28145

fix: COA importer importing all accounts as group (backport #28145)
asoral pushed a commit to asoral/erpnext that referenced this pull request Nov 12, 2021
…fix/pr-28145

fix: COA importer importing all accounts as group (backport frappe#28145)
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.

[V13.13] Chart of Accounts Importer sets ALL ledgers as Group!
3 participants