frappe / erpnext

Free and Open Source Enterprise Resource Planning (ERP)
https://erpnext.com
GNU General Public License v3.0
20.3k stars 7.07k forks source link

[v12.1.6] Decimals in Currencies not working #19255

Closed ophl55 closed 4 years ago

ophl55 commented 4 years ago

Installed Apps ERPNext: v12.1.6 (version-12) Frappe Framework: v12.0.16 (version-12)

I’ve recently updated to v12.1.6. Afterwards the currencies are not working as expected.

The issue is about decimals:

decimals

Quantaties, percentages, etc. are working as expected. Only currencies are wrong. The same happens when I use points instead of comma.

Please see also: https://discuss.erpnext.com/t/v12-1-6-decimals-in-currencies-not-working/53486 https://discuss.erpnext.com/t/decimals-behaving-strangely/53506

toofun666 commented 4 years ago

It is the same on mobile, too. It also happens in Journal Entries. Please check onLostFocus and onMouseClick events.

toofun666 commented 4 years ago

Looks like nobody cares about this obvious error or noone is updating their systems...

mkreckovic commented 4 years ago

I am using ERPnext for almost two years. It's a great software. But currently it is unusable because of this error. Please prioritize this issue.

asem89 commented 4 years ago

ERPNext is great but this bug broke the system for us. Please fix the decimals in currencies.

mkreckovic commented 4 years ago

Anybody there? This introduces serious uncertainty about the use of ERPNext. We have not been able to use this software for 10 days, and we think about alternatives. Some official answer would mean a lot at this point.

ophl55 commented 4 years ago

I assume, that the team is busy with organizing the conference. I think there will be a bunch of bug fixes the next days as there are contributing sessions during the conference.

I found out that the currencies work with the indian number format. So you can type ,512,03 if you want to have 512,03. Understanding the indian number format allows you to work with the system until the bug is fixed.

Hope that helps you.

ophl55 commented 4 years ago

Anybody there? This introduces serious uncertainty about the use of ERPNext. We have not been able to use this software for 10 days, and we think about alternatives. Some official answer would mean a lot at this point.

It's a great software and the bug will be fixed soon. ;)

mkreckovic commented 4 years ago

I assume, that the team is busy with organizing the conference. I think there will be a bunch of bug fixes the next days as there are contributing sessions during the conference.

Aaa that's about it :) I follow erpnext blog but I missed this one.

It's a great software and the bug will be fixed soon. ;)

It really is! I know that they are doing their best and that this will be resolved soon.

@ophl55 thanks for the tip, it works, now we can wait for fix :)

barredterra commented 4 years ago

According to @toofun666 , this is the offending PR:

https://github.com/frappe/frappe/pull/8497#issue-321121247

surajshetty3416 commented 4 years ago

https://github.com/frappe/frappe/pull/8563 This should fix the issue.

Please update your instance.

mseibert commented 4 years ago

@lasalesi told me that this is not yet fixed. Maybe he'll have time to add his thoughts on why.

mseibert commented 4 years ago

I also think, that someone offering a pull request with a fix would be something, that the European community should come up with.

dulhaver commented 4 years ago

as this impacting an entire market (every country using , and not . as decimal separators) and apparently it is not yet fixed (I hear, without being able to confirm myself yet, neither the latest release v12.2.2 is good to go yet) I don't see why this issue is closed.

re-opening therefore.

TurkerTunali commented 4 years ago

Thank you @vrms . We still have this issue.

mseibert commented 4 years ago

@rmehta @asbasawaraj This issue has gobbled up quite some frustration in the German community. It would be super cool, if you could look into this an either fix or comment so that we get a better understanding.

It is true. An invoice over 1000 Euro needs to be formatted like this: 1000,00 Euro. Currently your model is this: 1000.00 Euro. Germans can understand this. But it's weird and a lot of assholes will send you the invoice back and ask you to correct it (with commas) just to earn some time before having to pay. Such "games" are pretty common in Germany.

On top of this people who work in ERPNext will type 1000,00 and earn 100000 instead, which is really frustrating for them. It seems to be a quick fix with a huge positive impact for people from DE, AT and CH.

marcogabriel commented 4 years ago

Just to mention it: in DE, AT and CH, customers might see this as a showstopper and decide not to implement ERPNext.

itsdaveit commented 4 years ago

Just stopped my Upgrade and reverted to v11 until this is fixed.

rmehta commented 4 years ago

IMO this problem is already fixed. Please check the "System Settings" and "Currency" settings in your instances.

@nextchamp-saqib please confirm

rmehta commented 4 years ago

@vrms if you are re-opening, please independently verify if this issue exists and steps to reproduce it.

mseibert commented 4 years ago

Looks good in our system.

image

https://p198.p4.n0.cdn.getcloudapp.com/items/E0uE7mBX/Screen+Recording+2019-12-07+at+07.30+AM.gif?v=e0acc65e3eea99954205aa0c9a01c35d

ERPNext: v12.1.8 (master) Frappe Framework: v12.0.17 (master)

barredterra commented 4 years ago

@rmehta The number format shouldn't be overwritten by the currency, at least not by default. It would be totally normal to show 1,200.00 € to one user/customer and 1.200,00 € to another. -> The number format should be tied to the locale – anybody can see what he likes.


However, here's how to make the problem go away:

Now you can enter EUR values like 1.200,00 and ERPNext will understand them correctly.

dulhaver commented 4 years ago

I can't verify the problem myself in v12.2.0 at the moment because the bench update in my LXD container fails unfortunately.

I can however confirm the above workaround works on: ERPNext: v12.1.8 (version-12) Frappe Framework: v12.0.18 (version-12)

where it did not work before for me (even though it looks like the same context as @mseibert reports it was working.

If it can made to work through that workaround, but not by default I think this Issue should not be closed. Can someone check on a fresh v12.2.0 instance?

dulhaver commented 4 years ago

created a fresh v12.2.2. instance ran and see the error still with a company with a Comma as Decimal divisor.

Decimal Comma error v12 2 2_191209 09-47

The workaround mentioned above by @barredterra works also though.


In my eyes the existence of a workaround does not qualify as a fix, and likewise this issue can not be regarded as solved as per the high standards we are trying to live up to.

rmehta commented 4 years ago

We had a discussion on this and have decided to drop the formatting from Currency and use System Settings.

IMHO people should just use the international convention for number separators 😜

On 09-Dec-2019, at 2:30 PM, gunnar notifications@github.com wrote:

created a fresh v12.2.2. instance ran and see the error still with a company with a Comma as Decimal divisor.

https://user-images.githubusercontent.com/7608112/70421092-6a910a80-1a69-11ea-9348-b601d314adb4.gif The workaround mentioned above by @barredterra https://github.com/barredterra works also though.

In my eyes the existence of a workaround does not qualify as a fix, and likewise this issue can not be regarded as solved as per the high standards we are trying to live up to.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/frappe/erpnext/issues/19255?email_source=notifications&email_token=AABCGLF335D76KA2NDR3FXLQXYCKHA5CNFSM4I5WX7T2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGILOCY#issuecomment-563132171, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABCGLASVAH6W46DS624AY3QXYCKHANCNFSM4I5WX7TQ.

zefanja commented 4 years ago

IMHO people should just use the international convention for number separators

There are more countries using comma (green) compared to dot (blue) :smile:

grafik

(https://en.wikipedia.org/wiki/Decimal_separator#/media/File:DecimalSeparator.svg)

ArnulfKoch commented 4 years ago

We had a discussion on this and have decided to drop the formatting from Currency and use System Settings. IMHO people should just use the international convention for number separators 😜

It is essential that this works absolutely error-free. If you really plan to enter the German-speaking market (or any "comma country" on the map), then the localization just has to fit:

  1. Number format
  2. Correct translation (correct context-dependent meaning of words, correct grammar)

Without that, we are laughed at ("they can't even get the translation done, but they want to manage our business-critical processes").

mseibert commented 4 years ago

@rmehta I am unsure whether your comment was meant to be a joke. You added a smiley. Maybe it was only to soothe us. If you are serious about that, I recommend that you switch to the comma instead of the point and watch the American and Indian users and their reaction. You probably can feel how they would react.

If you do not plan to support the needs of German companies in cases where our standard deviates from what you are used to or think is easy or preferable, you should know, that this will never be an acceptable solution here ever. If your goal is to support users and service providers in Germany, you may want to rethink your decision.

And I also doubt that this type of humor is well received anywhere.

mseibert commented 4 years ago

I do not expect Frappe to fix this here or that Frappe supports the German market. I think, that German users and service providers should be capable of maintaining and customizing the software to their needs.

But when @rmehta writes: "We had a discussion on this and have decided to drop the formatting from Currency and use System Settings." I get the feeling, that Frappe wants to rule how the platform shall work. That's irritating for me. Is this a misunderstanding? There should be a place for basic German needs in an ERP software in ERPNext. Is that available through the "system settings" then? And does that mean, that I have to decide whether I want to have German customers or customers from USA and cannot have them both, because I will either have German formatted or American formatted offers? Or is it only that I have to train my German ERP users internally to use international formatting while the customer gets his formatting? My knowledge may be too shallow to understand. Sorry.

dulhaver commented 4 years ago

It is essential that this works absolutely error-free. If you really plan to enter the German-speaking market (or any "comma country" on the map), then the localization just has to fit:

Number format Correct translation (correct context-dependent meaning of words, correct grammar)

I agree to that as soon it was ...

"If you we really plan to enter the German-speaking market ..."

and with 'we' in this context, I mean even more the Comma Decimal users then the Community on the whole. Quite a view active in this thread, including yourself @ArnulfKoch [correct me whether I am wrong), want to provide paid services around ERPNext in the german, or Comma driven market. Ideally we will be supported by Frappe PvT. in this endeavor (and dropping this issue, or not recognizing the gravity of the Comma Problem I wouldn't count as supportive).

There is not obligation to actually provide the fix on Frappé's side (even though I would consider it smart to take ownership for this on their end) in my eyes.

Another thing that come to my mind is that this is not a new feature that is asked for. Comma Decimals worked until something broke it. And things that worked and where used, and needed, and then broke, normally are getting fixed again.

rmehta commented 4 years ago

Basically there were two places where you could set the formatting in currency and global. For whatever reason the currency setting was not implemented and was reported as a bug. When we fixed this "bug", it overrode the system settings which was the design.

What "we" (maintainers, not frappe) finally decided was to reintroduce the bug and ignore the currency settings, since this was clearly not optimal.

The problem was that no one was accurately able to identify the issue and it kept boiling over in the forum. If someone had put it clearly and made a suggestion, rather than fixing the blame, it could have been fixed much faster.

On 09-Dec-2019, at 3:56 PM, Martin Seibert notifications@github.com wrote:

I do not expect Frappe to fix this here or that Frappe supports the German market. I think, that German users and service providers should be capable of maintaining and customizing the software to their needs.

But when @rmehta https://github.com/rmehta writes: "We had a discussion on this and have decided to drop the formatting from Currency and use System Settings." I get the feeling, that Frappe wants to rule how the platform shall work. That's irritating for me. There should be a place for basic German needs in an ERP software in ERPNext.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/frappe/erpnext/issues/19255?email_source=notifications&email_token=AABCGLGDQG3GQ5IKL4OFB7LQXYMOLA5CNFSM4I5WX7T2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGITXCA#issuecomment-563166088, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABCGLBZOG2DVPBQDKKPGDDQXYMOLANCNFSM4I5WX7TQ.

dulhaver commented 4 years ago

The problem was that no one was accurately able to identify the issue and it kept boiling over in the forum. If someone had put it clearly and made a suggestion, rather than fixing the blame, it could have been fixed much faster.

I agree to this fully harted. I think it does not serve anyone to get upset, angry or righteous about this

mseibert commented 4 years ago

@rmehta All good for me. I agree, that we need more people doing and fixing things in Europe.

dulhaver commented 4 years ago

I suggest we keep this issue open, fix it once possible and advertise the workaround wherever this comes up.

dulhaver commented 4 years ago

As I wouldn't really know how else I could contribute I just created a bounty for fixing this issue https://www.bountysource.com/issues/81449218-v12-1-6-decimals-in-currencies-not-working

mseibert commented 4 years ago

@vrms I have added 25 USD for the very same reason. Problem that I see. If no one collects the money it's gone with the bountysource-Website. Questionable process, right?

dulhaver commented 4 years ago

I am not all too sure what happens if the bounty is not claimed, or cancelled to be honest @mseibert. Just wasn't aware of a better alternative right on the spot.

I don't think bountysource will keep the money to themselves, but in a worst case give it back to you bountysource account where you can re-purpose it to another bounty.

Generally I think it would be a meaningful project to investigate on such a process (co-funding development costs), find the best platform for such, and write up a standard procedure for the ERPNext context. But that would be another story, I guess.

barredterra commented 4 years ago

https://github.com/frappe/frappe/issues/9001

This might fix our problems, but it's a big project which will require lots of correctly formatted money.

dulhaver commented 4 years ago

do you mean that only this frappe/frappe#9001 can fix the problem and likewise this issue here would be obsolete because it is unfixable on it's own @barredterra ?

barredterra commented 4 years ago

No, the proposal is just a systematic solution, not a "fix".

dulhaver commented 4 years ago

so, coming up with a PR for fixing this problem now, we may need to have a bit more funds available then the current $50, we have on the bounty thus far in order to attract a developer to come up with a mergeable PR, I guess

TurkerTunali commented 4 years ago

IMHO setting an empty "Number Format" for the default currency and setting "#.###,##" as "Number Format" in the "System Settings" works as expected for the decimals, floats and currencies.

nextchamp-saqib commented 4 years ago

This PR reverts all the changes done by me to the currency and float controls. It has been merged and everything should work like it was. This won't be a permanent fix but now people won't have to find workarounds.

asbasawaraj commented 4 years ago

@ophl55 I also faced this issue and this is how I fixed it.

Step 1: Set the 'Number Format' in 'System Settings' as shown below.

Screen Shot 2019-12-12 at 2 53 39 PM

Step 2: Set the 'Number Format' in 'Currency' DocType as shown below.

Screen Shot 2019-12-12 at 2 56 27 PM

Can you please confirm if this works for you.

johanlives commented 4 years ago

@vrms I have added 25 USD for the very same reason. Problem that I see. If no one collects the money it's gone with the bountysource-Website. Questionable process, right?

Backers can request a refund to their Bountysource account or transfer to a new issue

toofun666 commented 4 years ago

Basically there were two places where you could set the formatting in currency and global. For whatever reason the currency setting was not implemented and was reported as a bug. When we fixed this "bug", it overrode the system settings which was the design. What "we" (maintainers, not frappe) finally decided was to reintroduce the bug and ignore the currency settings, since this was clearly not optimal. The problem was that no one was accurately able to identify the issue and it kept boiling over in the forum. If someone had put it clearly and made a suggestion, rather than fixing the blame, it could have been fixed much faster. On 09-Dec-2019, at 3:56 PM, Martin Seibert @.***> wrote: I do not expect Frappe to fix this here or that Frappe supports the German market. I think, that German users and service providers should be capable of maintaining and customizing the software to their needs. But when @rmehta https://github.com/rmehta writes: "We had a discussion on this and have decided to drop the formatting from Currency and use System Settings." I get the feeling, that Frappe wants to rule how the platform shall work. That's irritating for me. There should be a place for basic German needs in an ERP software in ERPNext. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#19255?email_source=notifications&email_token=AABCGLGDQG3GQ5IKL4OFB7LQXYMOLA5CNFSM4I5WX7T2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGITXCA#issuecomment-563166088>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABCGLBZOG2DVPBQDKKPGDDQXYMOLANCNFSM4I5WX7TQ.

Why does anyone think that why was this broken in the first place? Nobody had this issue before some ignorant developer decided to mess with it! The solution now is to keep the code in the kernel and ask everyone to set their own settings? Did you make any impact analysis at all?

toofun666 commented 4 years ago

@ophl55 I also faced this issue and this is how I fixed it.

Step 1: Set the 'Number Format' in 'System Settings' as shown below.

Screen Shot 2019-12-12 at 2 53 39 PM

Step 2: Set the 'Number Format' in 'Currency' DocType as shown below.

Screen Shot 2019-12-12 at 2 56 27 PM

Can you please confirm if this works for you.

We did not have to mess with these settings before! Trace back the code and revert the changes! Then, we will need to discuss in detail what the system and display settings need to be based on: user interface setting, system setting or OS settings.

That is serious stuff and is one of the core features. This can kill the entire project. I think nobody realizes the importance of this.

toofun666 commented 4 years ago

This PR reverts all the changes done by me to the currency and float controls. It has been merged and everything should work like it was. This won't be a permanent fix but now people won't have to find workarounds.

You really messed up for good! We did not need any workarounds before you meddled with the code. Some users having trouble in pasting from Excel is just a minor issue that needs to be dealt with on the user side. Just because you know how to code does not mean that you understand the I8n and business impact. So disappointing for the future of this project.

toofun666 commented 4 years ago

IMHO setting an empty "Number Format" for the default currency and setting "#.###,##" as "Number Format" in the "System Settings" works as expected for the decimals, floats and currencies.

Why has it come to make this setting is the main question here. Why a working system was interfered without any proper knowledge? Will we face such issues in the future?

toofun666 commented 4 years ago

Basically there were two places where you could set the formatting in currency and global. For whatever reason the currency setting was not implemented and was reported as a bug. When we fixed this "bug", it overrode the system settings which was the design. What "we" (maintainers, not frappe) finally decided was to reintroduce the bug and ignore the currency settings, since this was clearly not optimal. The problem was that no one was accurately able to identify the issue and it kept boiling over in the forum. If someone had put it clearly and made a suggestion, rather than fixing the blame, it could have been fixed much faster. On 09-Dec-2019, at 3:56 PM, Martin Seibert @.***> wrote: I do not expect Frappe to fix this here or that Frappe supports the German market. I think, that German users and service providers should be capable of maintaining and customizing the software to their needs. But when @rmehta https://github.com/rmehta writes: "We had a discussion on this and have decided to drop the formatting from Currency and use System Settings." I get the feeling, that Frappe wants to rule how the platform shall work. That's irritating for me. There should be a place for basic German needs in an ERP software in ERPNext. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#19255?email_source=notifications&email_token=AABCGLGDQG3GQ5IKL4OFB7LQXYMOLA5CNFSM4I5WX7T2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGITXCA#issuecomment-563166088>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABCGLBZOG2DVPBQDKKPGDDQXYMOLANCNFSM4I5WX7TQ.

Solution is so simple: Why was this change made? Anyone remembers? Why were there no issues and complaints before? Revert all changes to the code and if needed develop a fix which does not require anyone to make changes to their systems. Advising manual changes to such a core issue is just unstable and unconvincing.

nextchamp-saqib commented 4 years ago

Closing the issue as all the changes have been reverted.