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 reports Invoices with -ve qty for Invoices with Cr Notes #34456

Merged
merged 3 commits into from
Mar 17, 2023

Conversation

ruthra-kumar
Copy link
Member

@ruthra-kumar ruthra-kumar commented Mar 15, 2023

Issue

  1. Create Invoice with 2 rows of same item.
  2. Make a credit note for [1]
  3. GP report for [1] will have -ve qty and incorrect selling amount due to the -ve quantity

Cr note is overallocated to the Parent Invoice while calculating Average rate. This only happens if multiple rows have the same item.

Changes in this PR:

  1. Overallocation issue mentioned above has been fixed.
  2. Cr notes with 'return_against'(parent invoice) will be ignored by the report. Item quantities of those Cr notes are applied against the Original Invoice, there is no need to report them separately.
  3. Standalone CR Notes will be reported as usual.

Invoice SINV-23-00056 with multiple rows of same item.
Screenshot 2023-03-15 at 11 18 26 AM

Cr Note, SINV-23-00058, against SINV-23-00056
Screenshot 2023-03-15 at 11 18 18 AM

Standalone Cr Note SINV-23-00057
Screenshot 2023-03-15 at 11 18 31 AM

Before Fix:

  1. Item quantities of SINV-23-00056 goes to -ve due to overallocation mentioned above.
  2. It's Cr Note SINV-23-00058 is also reported as a separate Invoice

without_fix

After Fix:

  1. SINV-23-00056 has correct qty(0) as the Cr note should negate the Original invoice.
  2. Cr Note SINV-23-00058 is not reported.

with_fix

@ruthra-kumar ruthra-kumar marked this pull request as ready for review March 15, 2023 06:03
Cr Notes 'qty' are overallocated to parent invoice, when there are
mulitple instances of same item in Invoice.
2 New test cases added.
1. Standalone Cr notes will be reported as normal Invoices
2. Cr notes against an Invoice will not overallocate qty if there are
multiple instances of same item
@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Merging #34456 (cc61dae) into develop (1d767d5) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #34456      +/-   ##
===========================================
+ Coverage    63.86%   63.87%   +0.01%     
===========================================
  Files          810      810              
  Lines        59520    59526       +6     
===========================================
+ Hits         38010    38022      +12     
+ Misses       21510    21504       -6     
Impacted Files Coverage Δ
...pnext/accounts/report/gross_profit/gross_profit.py 87.76% <100.00%> (+2.09%) ⬆️

... and 2 files with indirect coverage changes

@ruthra-kumar ruthra-kumar merged commit 6b0dc62 into frappe:develop Mar 17, 2023
@ruthra-kumar
Copy link
Member Author

@Mergifyio backport version-14-hotfix version-13-hotfix

@mergify
Copy link
Contributor

mergify bot commented Mar 17, 2023

backport version-14-hotfix version-13-hotfix

✅ Backports have been created

@ruthra-kumar
Copy link
Member Author

@Mergifyio backport version-14-hotfix

@mergify
Copy link
Contributor

mergify bot commented Mar 19, 2023

backport version-14-hotfix

❌ No backport have been created

  • Backport to branch version-14-hotfix failed

Git reported the following error:

To https://github.com/frappe/erpnext
 ! [rejected]              mergify/bp/version-14-hotfix/pr-34456 -> mergify/bp/version-14-hotfix/pr-34456 (non-fast-forward)
error: failed to push some refs to 'https://github.com/frappe/erpnext'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant