-
Notifications
You must be signed in to change notification settings - Fork 7.3k
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
feat(India): Bootstrapped GST Setup #25415
feat(India): Bootstrapped GST Setup #25415
Conversation
…into bootstraped_gst_setup
'account_name': account.get('account_name'), | ||
'account_number': account.get('account_number', ''), | ||
'company': company_name | ||
}, | ||
or_filters={ | ||
'account_name': account.get('account_name'), | ||
'account_number': account.get('account_number') | ||
'company': company_name, | ||
'root_type': root_type, | ||
'is_group': 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Company is both in filters
and or_filters
.
The intention for having account_name
and account_number
in or_filters
was that each should be unique and one is enough to identify the account. Having both in filters, especially 'account_number' == ''
might not work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@deepeshgarg007 I agree with Raffael, account_number
and account_name
should be in or_filters
doc.flags.ignore_links = True | ||
doc.flags.ignore_validate = True | ||
doc.insert(ignore_permissions=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is all this ignoring really necessary? I don't remember getting any errors, at least for the german setup, and if there are any they might be worth fixing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not to ignore any errors but to reduce the setup time. India has a lot of fixtures that are created during setup which takes a lot of time. Have not done any profiting yet so cannot say if this helps or not. Will remove if there is no significant change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Could you please add a comment mentioning this?
if(me.values.bank_account){ | ||
frappe.call({ | ||
async: false, | ||
method: "erpnext.accounts.doctype.account.chart_of_accounts.chart_of_accounts.validate_bank_account", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this is removed? The code was there because many people enter just "Bank Accounts" in Bank field in Setup Wizard and it raises duplication error.
'account_name': account.get('account_name'), | ||
'account_number': account.get('account_number', ''), | ||
'company': company_name | ||
}, | ||
or_filters={ | ||
'account_name': account.get('account_name'), | ||
'account_number': account.get('account_number') | ||
'company': company_name, | ||
'root_type': root_type, | ||
'is_group': 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@deepeshgarg007 I agree with Raffael, account_number
and account_name
should be in or_filters
docs: frappe/erpnext_documentation#362