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

refactor: Exchange rate revaluation to handle accounts with zero account balance #33165

Merged

Conversation

ruthra-kumar
Copy link
Member

@ruthra-kumar ruthra-kumar commented Nov 29, 2022

Exchange Rate Revaluation will handle Foreign accounts with zero balance on account currency but has balance on company currency.

Changes in this PR:

  1. New Journal type Exchange Gain/Loss. This type skips validations: validate_debit_and_credit_amount and set_amounts_in_company_currency which helps to submit GL's with '0' debit/credit value in either currencies.
  2. def convert_to_presentation_currency(gl_entries, currency_info, company):
    has been modified to handle GL's from [1].
  3. Exchange Rate Revaluation will now create 2 Journals.

Screenshot 2022-12-02 at 9 43 16 PM

Discussion thread:
https://discuss.erpnext.com/t/how-to-maintain-correct-account-balance-in-multiple-currencies-in-case-of-foreign-bank-account/96839

Solution:
https://discuss.erpnext.com/t/how-to-maintain-correct-account-balance-in-multiple-currencies-in-case-of-foreign-bank-account/96839/9?u=ruthra

Conder a scenario where a Foreign currency account has balance in company currency but not in account currency. 'HDFC USD - TC' in the below screenshot.
Screenshot 2022-12-02 at 9 41 15 PM

ERR can submit a journal to move only the company currency balance to Exchange Gain/Loss account.
Screenshot 2022-12-02 at 9 43 16 PM

Screenshot 2022-12-02 at 9 43 31 PM

Screenshot 2022-12-02 at 9 43 43 PM

General Ledger:
Company/Base Currency:
Screenshot 2022-12-03 at 7 23 24 AM

Account Currency:
Screenshot 2022-12-03 at 7 23 29 AM

@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Nov 29, 2022
@ruthra-kumar ruthra-kumar changed the title Exchange rate reval refactor refactor: Exchange rate revaluation to handle accounts with zero account balance Nov 29, 2022
@ruthra-kumar ruthra-kumar force-pushed the exchange_rate_reval_refactor branch from 56dc7be to ed9df5f Compare November 29, 2022 15:05
@codecov
Copy link

codecov bot commented Nov 29, 2022

Codecov Report

Merging #33165 (8a730dd) into develop (4076999) will decrease coverage by 0.05%.
The diff coverage is 52.02%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #33165      +/-   ##
===========================================
- Coverage    64.10%   64.04%   -0.06%     
===========================================
  Files          818      818              
  Lines        58670    58834     +164     
===========================================
+ Hits         37609    37681      +72     
- Misses       21061    21153      +92     
Impacted Files Coverage Δ
erpnext/accounts/general_ledger.py 94.31% <0.00%> (-0.72%) ⬇️
...ange_rate_revaluation/exchange_rate_revaluation.py 55.60% <49.34%> (-14.09%) ⬇️
...xt/accounts/doctype/journal_entry/journal_entry.py 68.30% <85.71%> (+0.14%) ⬆️
erpnext/accounts/doctype/gl_entry/gl_entry.py 69.10% <100.00%> (ø)
erpnext/accounts/report/utils.py 80.59% <100.00%> (-0.57%) ⬇️
erpnext/erpnext_integrations/exotel_integration.py 68.04% <0.00%> (-3.73%) ⬇️
erpnext/stock/doctype/stock_entry/stock_entry.py 82.23% <0.00%> (-0.26%) ⬇️
erpnext/stock/doctype/pick_list/pick_list.py 76.53% <0.00%> (-0.21%) ⬇️
erpnext/selling/doctype/sales_order/sales_order.py 80.10% <0.00%> (-0.18%) ⬇️
erpnext/hooks.py 100.00% <0.00%> (ø)
... and 11 more

@ruthra-kumar ruthra-kumar force-pushed the exchange_rate_reval_refactor branch from 30efb82 to d07b78a Compare December 2, 2022 16:09
@ruthra-kumar ruthra-kumar marked this pull request as ready for review December 3, 2022 01:55
@ruthra-kumar ruthra-kumar linked an issue Dec 5, 2022 that may be closed by this pull request
@ruthra-kumar ruthra-kumar marked this pull request as draft December 18, 2022 02:26
@ruthra-kumar ruthra-kumar force-pushed the exchange_rate_reval_refactor branch 3 times, most recently from 6827c04 to e97667b Compare December 30, 2022 09:31
@ruthra-kumar ruthra-kumar marked this pull request as ready for review December 30, 2022 09:46
@ruthra-kumar ruthra-kumar force-pushed the exchange_rate_reval_refactor branch from e97667b to 014f180 Compare January 2, 2023 05:25
@ruthra-kumar ruthra-kumar force-pushed the exchange_rate_reval_refactor branch from 014f180 to 8a730dd Compare January 2, 2023 06:04
@deepeshgarg007 deepeshgarg007 merged commit 914b230 into frappe:develop Jan 2, 2023
@deepeshgarg007 deepeshgarg007 added the backport version-14-hotfix backport to version 14 label Jan 2, 2023
mergify bot pushed a commit that referenced this pull request Jan 2, 2023
…unt balance (#33165)

* refactor: new type for JE - Exchange Gain or Loss

* refactor: skip few validations for Exchanage Gain Or Loss type Jour

* refactor: ERR create 2 journals for handling zero and non-zero compa

1. Additional check box accounts table to identify accounts with zero balance
2. Accounts with zero balance only in either of the 2 currencies will be handled on separate Journal

* refactor: skips few validation for allowing 0 debit/credit

* fix: General Ledger presentaion currency

* test: fix test case in general ledger

* test: fix failing test case in AR report

(cherry picked from commit 914b230)

# Conflicts:
#	erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py
ruthra-kumar added a commit that referenced this pull request Jan 2, 2023
…unt balance (#33165)

* refactor: new type for JE - Exchange Gain or Loss

* refactor: skip few validations for Exchanage Gain Or Loss type Jour

* refactor: ERR create 2 journals for handling zero and non-zero compa

1. Additional check box accounts table to identify accounts with zero balance
2. Accounts with zero balance only in either of the 2 currencies will be handled on separate Journal

* refactor: skips few validation for allowing 0 debit/credit

* fix: General Ledger presentaion currency

* test: fix test case in general ledger

* test: fix failing test case in AR report

(cherry picked from commit 914b230)
deepeshgarg007 pushed a commit that referenced this pull request Jan 2, 2023
…unt balance (#33165)

refactor: Exchange rate revaluation to handle accounts with zero account balance (#33165)

* refactor: new type for JE - Exchange Gain or Loss

* refactor: skip few validations for Exchanage Gain Or Loss type Jour

* refactor: ERR create 2 journals for handling zero and non-zero compa

1. Additional check box accounts table to identify accounts with zero balance
2. Accounts with zero balance only in either of the 2 currencies will be handled on separate Journal

* refactor: skips few validation for allowing 0 debit/credit

* fix: General Ledger presentaion currency

* test: fix test case in general ledger

* test: fix failing test case in AR report

(cherry picked from commit 914b230)

Co-authored-by: ruthra kumar <ruthra@erpnext.com>
# Handle Accounts with '0' balance in Account/Base Currency
for d in [x for x in account_details if x.zero_balance]:

# TODO: Set new balance in Base/Account currency
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this TODO valid?
I'm trying to find why this is generated a non balanced journal entry.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's Invalid. Looks like I forgot to remove it.

@barredterra barredterra mentioned this pull request Mar 7, 2023
@mohsinalimat
Copy link
Contributor

@ruthra-kumar @deepeshgarg007 is it possible to backport to version-13?

@ruthra-kumar
Copy link
Member Author

@mohsinalimat
There are no plans to backport this refactor to v13 at the moment.

rizkyGP added a commit to greenphyto/erpnext that referenced this pull request Oct 3, 2023
…unt balance (frappe#33165)

* refactor: new type for JE - Exchange Gain or Loss

* refactor: skip few validations for Exchanage Gain Or Loss type Jour

* refactor: ERR create 2 journals for handling zero and non-zero compa

1. Additional check box accounts table to identify accounts with zero balance
2. Accounts with zero balance only in either of the 2 currencies will be handled on separate Journal

* refactor: skips few validation for allowing 0 debit/credit

* fix: General Ledger presentaion currency

* test: fix test case in general ledger

* test: fix failing test case in AR report
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14 needs-tests This PR needs automated unit-tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exchange Rate Revaluation Error
4 participants