is not an acceptable design I guess? That is also why I cannot get InWords in Turkish. A code walkthrough is due on these issues.
Maybe I’m beeing dumb, but… Are you change currency? I’m using in portuguese, and, without the fix posted here (for that, thanks a lot!!!), its happen…
This can be edited in
/desk#Form/Currency/EUR
and
/desk#Form/System Settings
both methods do not work in de locale language/regional setting.
This unfortunatelly makes it unusable for the german market for the moment.
3 Likes
Hi all,
posting on this again, this is very unfortunate, any country that uses a comma in their currency as decimal separator will not be able to enter any decimal currency value!
Please urgently revert this!
Only workaround is to use the “# ###.##”, but still they keyboard with , on the numeric field will give issues…
This is the code that breaks all: fix: Incorrect currency value while pasting from excel to grid and other UX issues by nextchamp-saqib · Pull Request #8497 · frappe/frappe · GitHub the eval function does not work in a comma-separated currency (e.g. DE, AT).
5 Likes
We are losing code quality fast! @rushikesherp We need to do something. A code walkthrough and action plan is needed.
1 Like
Adding insult to injury, you need to reverse this to get it functional:
https://github.com/frappe/frappe/pull/8563#issue-325630695
I don’t think this is tested at all!
With the newest update, if I type the decimals, they are working. But if it is imported from a file, it still does not.
I’m basically stuck at stock reconciliation. When I uploaded a csv file containing item with a price that has decimals in it, the input is fine. But when I click ‘save’, the decimals get shifted again, as if it’s multiplied by 100.
As a disclaimer, I haven’t even tried using or studying it in depth yet, since I can’t even start to import my item data into it.
I don’t think anyone cares or affected by this…
1 Like
My understanding is rather, that the people affected by this do not have the energy to properly escalate it. I do not blame Frappe for not prioritizing regional challenges in the app that do not affect their core customers. Who in the European community could offer a Pull Request that fixes the problem in Github? We do not have that yet, right?
1 Like
That should be a reversal rather than a Pull Request with new development. There are at least my remarks on the committed request on which the developer did not even bother to respond.
As commercial users, possible partners and pioneers of our countries we cannot vouch for careless developments.
1 Like
It is actually less about making a new pull request at this point but reverting the “new feature” which breaks this (and als its fix, which makes matters worse)…
I.e. please revert https://github.com/frappe/erpnext/issues/19255, it does not work and should not be in the stable branch.
Do you know, who we can ping, who could revert that change? Who is the maintainer of this module? Understanding and commenting on this is over my head.
I think the issue is already fixed. By the way this is a perfect reason to @ tag an admin or raise an alert in the Telegram group.
We can either blame or help. I see more blame than help. Specially when we are talking about “free software”. Thank you very much for your kindness!
Edit: this is the perfect reason i stay away from discuss. It burns me out.
3 Likes
We only want to help here. But the indicated resolution does not fix the problem, as posted above.
A value “5.000,00” in some localisations is valid “5000.00”. So cropping the comma will only work for some currencies but is highly dangerous… There should be a test case which enters values in each localisation and checks the value that gets to the db. In a calm minute, I’ll try to think of something…
Sorry for messing things up and the worst part not having a clue about it.
https://github.com/frappe/frappe/pull/8719
Please try changes of above PR until it gets merged in stable version on 20th Nov.
Also please do let me know what are your issues after this PR stating your global number format and default currency’s number format.
P.S: Another related issue which will clarify things a bit
1 Like
Precisely - the takeaway learning here is the need to write a test for say one or two locales. (Whoever cares to extend to their locale can do so.)
With the problem identified, the test would illustrate and reproduce the exact issue. Once the code is fixed, the test would validate the fix.
If that test ever fails again - say some proposed change breaks this - it would help identify and resolve the problem. And that would be cause to not commit that code, revisit whether the test is valid and so on.
Moreover tests show how code APIs work, capture business rule context, are change detectors, are key to enable code refactoring - that is essential to manage technical debt.
1 Like
Dears,
not all who contribute or discuss here are coding savy. Some are business owners, experts, etc.
@Saqib_Ansari a rule of thumb while dealing with an international system especially ERP involving financials is not to mess with it without proper design document or knowledge of the domain.
Same testing issue arises in the Currency Doctype. The remedy is already submitted in GitHub and still not committed. The design and tests are not in accordance so basically the Currency Doctype is not compliant with any business rules. fix: Currency Exchange for_selling and for_buying on the same day by toofun666 · Pull Request #19339 · frappe/erpnext · GitHub Just noticed my fix was held up due to space/tab issues. Correcting and submitting again.
I could not agree more. We need unit test, UI and integration tests. The more we can get, the better it will be for the overall quality of the system. And I agree, that it is the duty of our region to provide tests for such “comma issues” as discussed here.
I have created a separate thread on the UI and integration tests here so that this thread does not need to be highjacked:
I applied those changes but nothing changes for me here.
My number formats:
Global 00.000,00
Currency 00.000,00
Please revert all your changes on the 5 files.