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: Gross Profit report totals #28402

Merged
merged 7 commits into from
Nov 22, 2021

Conversation

GangaManoj
Copy link
Contributor

@GangaManoj GangaManoj commented Nov 15, 2021

Problems

  • When the Gross Profit report is Grouped By anything other than Invoice, there are two rows displaying the total values, only one of which is accurate.

Screenshot 2021-11-13 at 2 17 21 AM

  • When it's Grouped By Invoice, there is no row displaying the total values.
  • The Gross Profit Percentage value in the totals row is incorrect.
  • The custom totals row(the first one, the one that's actually right) is not displayed in bold.

Screenshot 2021-11-16 at 2 38 22 AM

Fixes

  • Remove the inaccurate totals row.
  • Add totals row for the report when Grouped By Invoice.
  • Fix incorrect Gross Profit Percentage value in the totals row.
  • Display totals row in bold.

Screenshot 2021-11-16 at 3 17 11 AM

Testing Info
  1. Open the Gross Profit report.
  2. Group by Invoice.
  3. Check if it has a Totals row.
  4. Now Group By everything else(Item Code, Customer, etc).
  5. Confirm that there's only one Totals row and that that row is displayed in bold.
  6. Verify that the Gross Profit Percentage displayed in the totals row is constant, irrespective of the value of the Group By filter.

@codecov
Copy link

codecov bot commented Nov 16, 2021

Codecov Report

Merging #28402 (5d8a85d) into develop (d0c240e) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop   #28402      +/-   ##
===========================================
- Coverage    55.27%   55.24%   -0.03%     
===========================================
  Files         1119     1119              
  Lines        66598    66604       +6     
===========================================
- Hits         36811    36796      -15     
- Misses       29787    29808      +21     
Impacted Files Coverage Δ
...pnext/accounts/report/gross_profit/gross_profit.py 0.00% <0.00%> (ø)
...payroll/doctype/income_tax_slab/income_tax_slab.py 83.33% <0.00%> (-16.67%) ⬇️
...eport/item_variant_details/item_variant_details.py 84.33% <0.00%> (-3.62%) ⬇️
erpnext/assets/doctype/asset/depreciation.py 84.44% <0.00%> (-2.97%) ⬇️
...wise_balance_history/batch_wise_balance_history.py 91.37% <0.00%> (-1.73%) ⬇️
erpnext/stock/report/stock_ledger/stock_ledger.py 75.60% <0.00%> (-1.63%) ⬇️
erpnext/portal/utils.py 28.98% <0.00%> (-1.45%) ⬇️
erpnext/stock/reorder_item.py 76.92% <0.00%> (-0.86%) ⬇️
erpnext/stock/report/stock_ageing/stock_ageing.py 90.51% <0.00%> (-0.73%) ⬇️
...next/accounts/doctype/subscription/subscription.py 81.74% <0.00%> (-0.57%) ⬇️
... and 5 more

@nextchamp-saqib nextchamp-saqib merged commit 7677d6d into frappe:develop Nov 22, 2021
@nextchamp-saqib
Copy link
Member

@Mergifyio backport version-13-hotfix

@mergify
Copy link
Contributor

mergify bot commented Nov 22, 2021

backport version-13-hotfix

✅ Backports have been created

nextchamp-saqib added a commit that referenced this pull request Nov 22, 2021
…-28402

fix: Gross Profit report totals (backport #28402)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants