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: Calculate depreciation_amount accurately #27585

Merged
merged 48 commits into from
Nov 2, 2021

Conversation

GangaManoj
Copy link
Contributor

@GangaManoj GangaManoj commented Sep 19, 2021

Proposed Changes

  • While modifying the Depreciation Schedule of an Asset after its sale, the Depreciation Amount is not calculated correctly if Depreciation Entries have been posted for it already. This PR ensures that the Depreciation Amount calculated will be accurate.
Example

Initial Asset Value = Rs 100,000
Expected Value After Useful Life = Rs 10,000
Number of Depreciation = 3
Frequency of Depreciation = 12

Screenshot 2021-09-20 at 4 50 24 AM

Before:

On selling the Asset on 31/12/2021, the Depreciation Amount for the second row should have been 30,000, but instead it's being calculated incorrectly as 20,000.

Screenshot 2021-09-20 at 4 52 22 AM

After:

Screenshot 2021-09-20 at 4 53 26 AM

  • If the asset is then returned, the Depreciation Schedule should be reset, but the Depreciation Amount for subsequent rows are not being calculated accurately. This PR fixes that as well.
Example

Before:

On returning the Asset, the Depreciation Amount for the third row should have been 30,000, but instead it's being calculated incorrectly as -30,000.

Screenshot 2021-09-27 at 10 24 42 PM

After:

Screenshot 2021-09-20 at 4 50 24 AM

  • If an Asset was set to be sold in the future on the day it was scheduled to be depreciated, once the Asset gets returned, the Depreciation Entry posted on its sale would not be reversed, even if the posting date for the Credit Note is in the future. This PR adds an extra condition to let the Depreciation Entry be cancelled in cases like this.
Explanation

Screenshot 2021-09-20 at 4 50 24 AM

If this Asset is set to be sold on 31-12-2021(which is in the future right now), an additional Depreciation Entry would be made for that date. On returning this Asset, the Depreciation Schedule should be reset and the Depreciation Entry posted on its sale should be reversed, but because the sale was made on the scheduled date for depreciation, the Depreciation Entry does not get reversed.

With the new condition, the Depreciation Entry made on sale will be reversed if the sale was set to happen in the future(i.e. if the Posting Date for the Sales Invoice is in the future) but gets returned, even if the date of sale = scheduled date for depreciation.

  • Add more tests for Depreciation Schedule.
  • Categorise tests into test suites.

@nextchamp-saqib nextchamp-saqib added CI-failing Unit tests or patch tests are failing. needs-tests This PR needs automated unit-tests. labels Sep 20, 2021
@GangaManoj GangaManoj changed the title fix: Calculate depreciation_left accurately fix: Calculate depreciation_amount accurately Sep 21, 2021
@GangaManoj GangaManoj removed the needs-tests This PR needs automated unit-tests. label Sep 21, 2021
@codecov
Copy link

codecov bot commented Sep 25, 2021

Codecov Report

Merging #27585 (7681600) into develop (c8d0d9e) will increase coverage by 0.25%.
The diff coverage is 84.09%.

@@             Coverage Diff             @@
##           develop   #27585      +/-   ##
===========================================
+ Coverage    55.24%   55.50%   +0.25%     
===========================================
  Files         1255     1248       -7     
  Lines        67935    67753     -182     
===========================================
+ Hits         37534    37605      +71     
+ Misses       30401    30148     -253     
Impacted Files Coverage Δ
erpnext/accounts/doctype/gl_entry/gl_entry.py 73.77% <ø> (ø)
erpnext/regional/india/utils.py 64.47% <0.00%> (-1.39%) ⬇️
...xt/accounts/doctype/sales_invoice/sales_invoice.py 78.76% <80.00%> (+2.09%) ⬆️
erpnext/assets/doctype/asset/asset.py 74.19% <88.00%> (+2.58%) ⬆️
...xt/accounts/doctype/journal_entry/journal_entry.py 68.27% <100.00%> (+0.04%) ⬆️
erpnext/stock/report/stock_ageing/stock_ageing.py 89.92% <0.00%> (-3.60%) ⬇️
...eport/item_variant_details/item_variant_details.py 84.52% <0.00%> (-3.58%) ⬇️
...xt/stock/report/stock_analytics/stock_analytics.py 91.08% <0.00%> (-2.98%) ⬇️
erpnext/stock/reorder_item.py 75.42% <0.00%> (-2.55%) ⬇️
...wise_balance_history/batch_wise_balance_history.py 91.52% <0.00%> (-1.70%) ⬇️
... and 42 more

@nextchamp-saqib nextchamp-saqib removed the CI-failing Unit tests or patch tests are failing. label Sep 26, 2021
@blewsky
Copy link

blewsky commented Oct 5, 2021

Will that also fix #27431 ?

@nextchamp-saqib
Copy link
Member

@Mergifyio backport version-13-hotfix

@mergify
Copy link
Contributor

mergify bot commented Dec 3, 2021

backport version-13-hotfix

✅ Backports have been created

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

3 participants