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

[15.0][IMP] account_financial_report: Add Amount cur. to General Ledger totals if the account has not set currency #1236

Conversation

victoralmau
Copy link
Member

Add Amount cur. to General Ledger totals if the account has not set currency

Related to #1235 (comment)

@Tecnativa

@pedrobaeza pedrobaeza added this to the 15.0 milestone Oct 17, 2024
@pedrobaeza
Copy link
Member

The initial balance in currency is incorrect:
image

@victoralmau victoralmau force-pushed the 15.0-imp-account_financial_report-general_ledger_total branch from 2240fe7 to 61a0d33 Compare October 18, 2024 12:27
@pedrobaeza pedrobaeza requested a review from alexis-via October 25, 2024 06:53
@pedrobaeza
Copy link
Member

@victoralmau
Copy link
Member Author

Ok, I see what you mean, it shows -1224.01 instead of -601.74, but is it really an error? IMO no, I explain what happens and why.

1224.01=622.27 + 601.74

622.27 is something that although it is not shown on screen in the Amount cur. column (although in the Cumul. Bal. column)., it has been defined in https://github.com/OCA/account-financial-reporting/blob/15.0/account_financial_report/report/general_ledger.py#L150.
The real problem (IMO) is that defining that value that way is “wrong” because we should only add the values of the lines if the currency is different from the company currency (but by performance it is not) or if there is a currency defined in the account. I think changing that value now would confuse some people. What do you think?

@pedrobaeza
Copy link
Member

Yeah, I understand, but it's incorrect. You have to only sum in the total if the currency is different from the company one.

@victoralmau victoralmau force-pushed the 15.0-imp-account_financial_report-general_ledger_total branch from 61a0d33 to 32ca933 Compare October 30, 2024 08:11
@victoralmau
Copy link
Member Author

Changes done.

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Tested and working correctly:

imagen

@pedrobaeza
Copy link
Member

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 15.0-ocabot-merge-pr-1236-by-pedrobaeza-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit fb90b3d into OCA:15.0 Nov 26, 2024
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 897cde8. Thanks a lot for contributing to OCA. ❤️

@pedrobaeza pedrobaeza deleted the 15.0-imp-account_financial_report-general_ledger_total branch November 26, 2024 15:40
victoralmau added a commit to Tecnativa/account-financial-reporting that referenced this pull request Nov 28, 2024
…Show foreign currency checkbox is not checked.

Related to OCA#1236

TT51996
victoralmau added a commit to Tecnativa/account-financial-reporting that referenced this pull request Nov 28, 2024
…als if the account has not set currency

Related to OCA#1235 (comment)

[FIX] account_financial_report: Avoid error in General ledger if the Show foreign currency checkbox is not checked.

Related to OCA#1236

TT51996
victoralmau added a commit to Tecnativa/account-financial-reporting that referenced this pull request Nov 29, 2024
…als if the account has not set currency

Related to OCA#1235 (comment)

[FIX] account_financial_report: Avoid error in General ledger if the Show foreign currency checkbox is not checked.

Related to OCA#1236

TT51996
chaule97 pushed a commit to chaule97/account-financial-reporting that referenced this pull request Nov 29, 2024
…als if the account has not set currency

Related to OCA#1235 (comment)

[FIX] account_financial_report: Avoid error in General ledger if the Show foreign currency checkbox is not checked.

Related to OCA#1236

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

Successfully merging this pull request may close these issues.

4 participants