WIP disallow entering numbers with commas
Resolves #3373
Description
Change the input type of the line item quantity from number to string
Before this change, if the user added a comma to the number, it was considered invalid. The value was not sent to the backend, the save failed silently.
Now, with the string input type, the value is sent to the backend, and there is always an explicit error.
Note that this affects multiple places where the issue also occurred, for example when creating a distribution.
WIP because some barcode tests are still failing.
Type of change
- Bug fix (non-breaking change which fixes an issue)
How Has This Been Tested?
Hey @fchatterji -- I notice that you say it's WIP because some barcode tests are still failing. If they fail sometimes, but not always, that's a known issue we have here.
If that's so, and it's the only reason you're holding it in WIP, let's remove the WIP and see if we can get this reviewed and moved forward!
hi @cielf , no unfortunately its tests regularly failing, because of changes made here. i'll try to debug in the next few days
Hey @fchatterji -- Just checking in because it's been a couple of weeks -- how is this coming along?
Note: I rebased onto main due to an issue with the sidebar's entry expanders that I was seeing with this branch locally but not on main.
Hmm. Unfortunately this seems to break the barcode workflow (tested on Firefox on macOs with a USB barcode scanner).
I'll try to do a screen recording of the working version on main vs on this branch and upload those.
Here are the recordings:
main:
https://user-images.githubusercontent.com/934707/232325689-bab5663e-f84b-4418-8fe6-583f40d21bc7.mov
this PR:
https://user-images.githubusercontent.com/934707/232325698-e6ea6e00-6dd3-4419-84f9-8c5175511bbb.mov
Ok. So a bunch of barcode stuff is depending on that field input type being integer, then, it would seem. The question then becomes do we a) back off from changing the field type to string and find a different way to trap the comma if entered, or b) find all the places that are depending on it being integer and address them one by one. Could be an excellent opportunity to learn more about how we handle barcodes for someone...(or c) some other option I haven't expressed.)
Seems like it would be good to truly fix it, but I'd understand if that's more than @fchatterji had time for. If they aren't sure how to continue from here, it might make sense to tweak/regroom the issue with the additional information?
To a certain extent, the barcode stuff is -- well, we don't currently have an expert on it. So it will be a learning experience for whoever takes it on.
We have had this issue come up again in the wild, it doesn't happen often, but when it does it is very confusing.
Closing due to inactivity.