Added middleware for formatting currency

[cchoe1]

Middleware might be overboard but I figured it could be reused in case there are other sections that also require server-side logic to compute values. The middleware receives the request, replaces commas with periods using regular expressions, and passes it next() which is Items\Items@totalItem where it can be used normally. totalItem() wants to type cast the value as double but when the string contains commas as decimal points, then it messes up.

If necessary, I can reimplement it without middleware.

Referencing issue #185

[carvallegro]

I think this operation should be in the front-end. A reusable InputPrice component would make more sense. I feel like having the backend handle this would result end up a bit bloated.

[cchoe1]

Ok I'll cook something up on the front-end in a few hours, if its not fixed by then. It might be a little messy though because the AJAX request is sent on keypress so it will basically just have to automatically replace all commas as you type before the request gets sent to the server otherwise the wrong number gets read. There could be other options too, maybe I haven't thought of them yet.

[carvallegro]

I'll look at the front-end as well and if I come up with an idea I'll let you know

[cchoe1]

Ok I thought app.js was doing some magic and handling all these requests but I found some JS code near the bottom of create.blade.php. I'm not sure if there is a way to cleanly handle the data with Javascript since the AJAX request will serialize the data into the URL header and immediately pass it to the server, so we'd have to bind an event listener that triggers before the itemTotal() function that will just reformat the user input completely before the AJAX is called. I could do that but I wanted to see your input on doing something like that.

[carvallegro]

Which create.blade.php are we talking about ? There's one in each folders.

I think the first thing to do is to determine what's the expected behavior. From what you said in in #185 we should first determine what's the expected format depending on the user's locale. This'll allow us to create a price input field with :

• A validator
• Some kind of convertor from the locale format to the dev standard (123.456...).
  There even be a JS lib that does it all!

This way we would have everything sorted on the frontend and we wouldn't have to worry about the format anywhere else.

I'm not a Laravel expert or a PHP dev, but we should investigate to see if Laravel has any number regex for each locale. Then it's just a matter of sending this to the front-end.

@denisdulici any suggestions?

[cchoe1]

Oh sorry, I was referring to the create.blade.php file inside views/Invoices specifically but I'm not sure how many pages actually use the server to calculate subtotals & subtotal + tax.

I did find a solution on stackoverflow that shows a way to convert any format of numbers into a proper format that can be used correctly when (double) is applied to it (https://stackoverflow.com/questions/4325363/converting-a-number-with-comma-as-decimal-point-to-float -- this example is in PHP but could be easily translated into javascript).

I did think of one thing earlier regarding currency codes: it might not be the best idea to format depending on the currency selected since there might be a multinational (ebay seller, perhaps) company using this software and they might want to retain the ability to use their preferred currency format even if dealing with foreign currencies (instead of forcing one or the other). This might be a really obscure situation though.

---

Edit: Meant to respond to the last part of your post... There is a number_format() function in PHP but applying it by itself might cause some bugs if a user enters in a number with commas separating thousands digits. If we go that route, then we do need to validate locale. Otherwise a regex expression might perform better. This could be handled pretty shortly on the server, with or without middleware, as well since it's already being passed data but I initially thought to use something modular like middleware so it could be reused.

---

I'm not too sure about how Denis would like to split this functionality so I'm open to hearing what he might have to say about this. On one hand, I feel like it would be best to assign this job (as in, calculating subtotals for invoicing/other currency related inputs) fully to either client or server only, but I'm sure there are compelling reasons for both cases.

[carvallegro]

I'm not a big fan of the SO solution you found as it manage only one special case. What about the time where decimals are space separated ? Or using ' ?

You're right, we shouldn't base the format on the currency but rather on the locale. I think we got confused around that. As a french person, I expect every amount of money to be written with the same format, regardless of the currency. It is then easier to manage numbers that way as the regex will be associated to the locale and will fallback to the default en-EN number format if necessary.

I understand that you thought it'd would be a good idea to convert the numbers alongside the conversion but it is does not respect the separation of concern.
It is the job of the backend to do the conversion as it contains all of the business code and it is the job of the front-end to manage number conversion as its job is to handle user interfacing.
Plus, having this in the frontend as an input feature would allow us to use it for any number input, not just the tax calculation.

Thinking about it, Is this issue anywhere else in the app? If not it must be that number format is already handled. If that's the case then we should do the same way to stay homogenous.

[cchoe1]

I didn't know that apostrophes were also used commonly so I suppose that single Regex won't suffice. Your explanation of the separation of concerns actually makes a lot of sense and sounds like a better idea than mine.

I'm not too familiar with the way the Javascript is being handled and how Denis and others want to design it. If it's not too big of an issue or concern, I can mess around sometime and see if there is anything I can think of but I'm not entirely too well read in creating Javascript components minus classes (if that can be considered a 'component' or 'module').

I'm not too sure if this issue exists in other areas, I can dig around the PHP code and see if anything else calls totalItem() on the server-side (not the JS function) and it might reveal something. I'll report back afterwards.

[carvallegro]

Glad you agree :) SOC is a great way of coding, you can read this if interested.

I think that at this point we need a feedback from the team to see if there's an existing mechanism in place. Maybe you can dig around while waiting ;)

Regarding the JS component, looking at the stack I reckon it'll be more something a bit jQuery-esque. A simple listener on input[type=text].price that operates magic (magic TBD).

[cchoe1]

I've read the basics like 50 times but I feel like every time I do, I learn something new or see something in a new way. That was a good read, thanks!

I have a little bit of free time before going to bed so I'll take a look down the rabbit hole and see if I can find anything and report back and hopefully someone else can chip in also. I have some time tomorrow too so I'll resume then, as well. Thanks for your input, it was actually very helpful!

Edit: I'll leave this request open for a while but I actually messed up quite a few things with the PR like using master branch and not rebasing so I will go ahead and do that first before submitting another one but I'll leave this up in case anyone else has something to input regarding this issue.

[denisdulici]

Thanks guys for all your inputs. Here are my points:

1. Calculations are always done in the back-end, it's more economical.
2. The solution seems to fix only the first item of invoice, the second etc ones?
3. The problem is not limited to Invoices but available with Revenues, Bills, and Payments too so a more generic solution is required.
4. It doesn't seem to be directly related to currency but price input so the naming could be optimized.

[carvallegro]

Thanks Denis for the feedback!

So the suggestion is to create a input field that will validate input based on the locale. This way a standard number will be sent over to the backend to do a clean calculation.

This input will be used for any price input and avoid any issue.

[cchoe1]

Denis, your point # 2 is actually something I overlooked badly. Glad I could get all your inputs, those points are very helpful. If you don't have an immediate solution for this, carvallegro and I could possibly cook up some ideas for this and report back.

---

As for any ideas I currently have, I'm torn between a few ideas to start off. I typically add a CSS class identifier to an element to select it and I'm thinking we create a self-contained class (object) and have a method that attaches an event handler to all instances of a certain CSS class.

The only issue I can see with this is that we have to specifically remember what the CSS class name is whenever we want to implement more input boxes that needs to rely on this price formatting component. Would anyone possibly have a better idea for this?

So my brief outline:

1. Event listener listens for 'change' or 'input' or 'keydown/up/press' event within input box.
2. Self contained component accepts all events and triggers the following functions:

• initial AJAX request to server to query 'xxx_settings' database for specified locale (sidenote: mine defaulted to GB but I don't remember if I was ever prompted to set it so this might be something to consider)
• receive locale info, use format function to format any style to a single style that the server can recognize when type casting to double.

3. Server handles data normally, as it does now

I'm unsure of what route we should bind to this database query for the locale. Should we just make a completely new controller that handles this? Or is there an existing controller that could be called by the views for Revenues, Invoicing, Expenses, etc. that could handle this?

Will update if I find out anything new.

[carvallegro]

Hey @cchoe1 I'm pretty happy with what you suggested, that's what I had in mind as well. I think the self-contained class is the best option we have as of now since we're not using any component library (i.e, React, Vue or Angular).

As of remembering it, that's what documentation is for! :)

To answer your outline:

1. Yes
2. Yes but:

• I think the AJAX request to get the locale is overkill as you can load this value during the page generation on the server side.
• Yes, but to save our formatted value the component will need to create a hidden input that will store the formatted value.

3. Yes

I'm sure that there's a parent controller that handle all of the i18n matter.

[cchoe1]

@carvallegro Thanks for your reply, I actually really like that idea on loading it on screen load since we only need the information once. Ah, I see what you mean by point 2: we just move the value of the input box to the hidden field and send off that value instead of the actual input box.

---

Looking for all instances on the server that receive user input for money(not saying we need to edit these controllers but the views that these controllers control):

• Controllers/Expenses/:
  Bills@store()
Bills@update()
Bills@import() (possibly)
  Bills@edit()
Bills@payment()
Payments@create()
Payments@update()
Payments@import() (possibly)
• Controllers/Incomes/:
  Invoices@store()
Invoices@update()
Invoices@payment()
Revenues@store()
Revenues@update()
Revenues@import() (possibly)
• Controllers/Items/:
  Items@store()
Items@update()
Items@totalItem()
Items@convertPrice() (possibly)
  Items@import() (possibly)
  (if I left off any, feel free to let me know)

Import methods have (possibly) next to them because I'm not sure how the Importer works exactly but I'm guessing it'll just read the numbers straight off something and if it has European styling, then it won't work correctly either.

---

I actually noticed that when setting up Currencies in the 'Settings' page, you can set your preferred thousands separator + the decimal point and it gets recognized correctly on the front end, however the back-end still doesn't save the number properly (if trying to use anything except periods for decimal points). This could be problematic especially for people who see the total summing up correctly but the number isn't actually being saved properly in the DB (it drops decimal points).

Edit: pics kinda clogged things up, but the idea was that high volume + this issue = potential for bad.

Sorry for the epicly long post, I just want to be thorough so this issue gets fixed, it seems like it could be a nasty one in the right circumstances! Also, most of this is just mental notes so don't feel obligated to reply to all of this haha. I think I have a good idea on where to begin now.

[cchoe1]

So I wanted to ask something about the second part of my previous post. I found that going into the Settings, you can specify the format of currency. Doing so actually results in the front-end working but it doesn't actually fix the back end from saving the wrong numbers.

If we can ignore the front-end aspect, then the fix should be a little bit easier. I'm finding it tricky to basically mirror the contents of each input box, especially when there is a list of them rather than just 1 within hidden input fields since a request goes out on each keypress. If I only had to fix it for the actual form submission, then I think a fix could be easier and we could just notify users to set their preferred formatting in the Settings page for it to work properly.

Also I'm not sure how non-invasive of a solution I can find but at best I think I would still need to make changes to the existing POST request that gets sent on form submission to include the new input boxes rather than the ones that might contain the locale-specific format.

If these don't sound too bad, I think I can probably have something done soon. Sorry I meant to post this earlier but didn't have time.

[denisdulici]

Unfortunately, MySQL accepts only period for double format so the solution needs to format the value before being stored in database. The list (index) pages are OK, the problem is with form (create, edit) pages.

[denisdulici]

Hello guys,

What if we just use the number field type?

Regards

[fellek]

Hi there,
This issue still exists in Version 1.3.2. (PHP 7.2.11, Ubuntu 16.04) by setting prices for items. It is not possible to set a floating price seperated by comma.

Steps to reproduce:

1. Choose EUR as currency.
2. Choos Decimal Mark: , (Comma)
3. Add new item: Sale Price and Puchase Price = 3,9
4. Save
5. Overview shows a price of €3,00
6. Click Edit item: Price Inputfield: 3 without any trailing comma or dot.
7. Change Sale Price = 3.9 and Save
8. Overview shows €3,90

Is it a local issue? (Germany, EURO)

Nevertheless, thanks for this great tool!