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.

Sign up for GitHub

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

Jump to bottom

Fix: Set General Ledger 'Group By' filter as 'Group by Voucher(Consolidated)' when opened from Invoice #21673

Merged
merged 4 commits into from May 13, 2020
Merged

Fix: Set General Ledger 'Group By' filter as 'Group by Voucher(Consolidated)' when opened from Invoice #21673

merged 4 commits into from May 13, 2020

Conversation

AfshanKhan
Copy link
Contributor

Please provide enough information so that others can review your pull request:

Explain the details for making this change. What existing problem does the pull request solve?

Screenshots/GIFs

nabinhait
nabinhait requested changes May 11, 2020
View reviewed changes
erpnext/accounts/report/general_ledger/general_ledger.js Outdated
Comment on lines 56 to 58
if (!this.value) {
frappe.query_report.set_filter_value('group_by', "");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not make any sense, instead on change of voucher_no set group_by as "Group by Voucher (Consolidated)"

Suggested change
if (!this.value) {
frappe.query_report.set_filter_value('group_by', "");
}
frappe.query_report.set_filter_value('group_by', "Group by Voucher (Consolidated)");

@nabinhait nabinhait self-assigned this May 11, 2020
nabinhait
nabinhait reviewed May 11, 2020
View reviewed changes
Copy link
Member

@nabinhait nabinhait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AfshanKhan Always use semantic and descriptive title of the PR. Also, add some description of the PR.

@AfshanKhan AfshanKhan changed the title fix for issue #21419 Fix: v13 from purchase invoice view->Accounting Ledger, the expected Group by: Group by Voucher(Consolidated) filter field not set May 12, 2020
@nabinhait nabinhait changed the title Fix: v13 from purchase invoice view->Accounting Ledger, the expected Group by: Group by Voucher(Consolidated) filter field not set Fix: Set General Ledger 'Group By' filter as 'Group by Voucher(Consolidated)' when opened from Invoice May 13, 2020
@nabinhait nabinhait merged commit bd7e535 into frappe:develop May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nabinhait nabinhait nabinhait approved these changes

@deepeshgarg007 deepeshgarg007 Awaiting requested review from deepeshgarg007 deepeshgarg007 is a code owner

@nextchamp-saqib nextchamp-saqib Awaiting requested review from nextchamp-saqib

Assignees

@nabinhait nabinhait

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@AfshanKhan @nabinhait