While attempting to create a basket from a staged MARC record import, If 020$c has two prices - ie, $5.99 ($7.75 CAN) this will cause the basket creation to fail with the following message: Only one decimal separator permitted at /home/koha/kohaclone/acqui/addorderiso2709.pl line 210
Created attachment 6087 [details] Sample MARC file Here is a record for testing purposes that should cause the order add to fail from a staged import
After further testing it appears that the period (.) is the offending character here, since this also occurs on single prices in the 020$c: 18.95 (U.S.)
Created attachment 7512 [details] [review] Bug 7112 - Having two prices in 020$c causes basket creation to fail from staged marc import The root problem here is that the price is being pulled from the MARC record and is then run through Number::Format::unformat_number. This routine is really being misused, and should only be used to reverse the effects of Number::Format on a number string. We are apparently using it to strip out currency characters and the like. Number::Format::unformat_number will choke if there is more than one period (.) in the price field. MARC standards do not limit this field to a single period, so unless there is only one period, we should skip number unformatting. Examples of that break unformat_number include '18.95 (U.S.)', and '$5.99 ($7.75 CAN)', both of which are perfectly valid. This commit tests to see of $price contains more than one period, if it does have multiple periods, it will skip running the string though number_unformat. The variable $price may fail to have an actual price, in which case the price then defauls to '0.00', which would be rarely if ever the correct price. To combat this, I have added highlighting to any price in the Order Details table that begins with 0 ( i.e. '0.00' ). Also, fixed the incomplete table footer, adding a new td with a span of 3 to fill in the nonexistant cells.
As an aside, perhaps GetMarcPrice should do some regex magic to pull the first valid looking number from the price field. For example, if the price field contains "$5.99 ($7.75 CAN)", it should return "5.99". If the field contained "18.95 (U.S.)", then it would "18.95".
I was wondering if it works for prices with decimal commas? I should test that :)
That's a good question? I didn't even think of that! I suppose a first question would be do other currency fields in Koha handle decimal commas? If yes, then the question should be asked here as well. (In reply to comment #5) > I was wondering if it works for prices with decimal commas? I should test that > :)
Other fields do currently not handle commas. But I think this is a different problem - you can control what you enter in Koha and train staff accordingly. This bug is about making use of records from a vendor or third party, so it might be interesting to make it recognize other currency/number formatting as well and convert to a form that the acquisitions module can work with. Perhaps it's worth keeping this bug open with a note, if it's currently not working.
(In reply to comment #7) I see what you mean. I feel that maybe we should have a custom subroutine that does its best to make sense of any give price field and attempt to get a single number out of it. Then GetMarcPrice would run the price through this before passing back returning the price value. I think I will make a separate bug report for it so we can close this one once it has been signed off and pushed to master. > Other fields do currently not handle commas. But I think this is a different > problem - you can control what you enter in Koha and train staff accordingly. > This bug is about making use of records from a vendor or third party, so it > might be interesting to make it recognize other currency/number formatting as > well and convert to a form that the acquisitions module can work with. > Perhaps it's worth keeping this bug open with a note, if it's currently not > working.
*** Bug 7516 has been marked as a duplicate of this bug. ***
Created attachment 7539 [details] [review] Bug 7112 - Having two prices in 020$c causes basket creation to fail from staged marc import The root problem here is that the price is being pulled from the MARC record and is then run through Number::Format::unformat_number. This routine is really being misused, and should only be used to reverse the effects of Number::Format on a number string. We are apparently using it to strip out currency characters and the like. Number::Format::unformat_number will choke if there is more than one period (.) in the price field. MARC standards do not limit this field to a single period, so unless there is only one period, we should skip number unformatting. Examples of that break unformat_number include '18.95 (U.S.)', and '$5.99 ($7.75 CAN)', both of which are perfectly valid. This commit adds the function MungeMarcPrice that will better handle find a real price value in a given price field. It does a very good job at finding a price in any currency format, and attempts to find a price in whichever currency is active before falling back to the first valid price it can find. The variable $price may fail to have an actual price, in which case the price then defaults to '0.00', which would be rarely if ever the correct price. To combat this, I have added highlighting to any price in the Order Details table that begins with 0 ( i.e. '0.00' ). Also, fixed the incomplete table footer, adding a new td with a span of 3 to fill in the nonexistant cells. Bug 7516 - Create Subroutine For GetMarcPrice that will discern the actual price from a given price field value.
Created attachment 7540 [details] [review] Bug 7112 - Having two prices in 020$c causes basket creation to fail from staged marc import
Created attachment 7541 [details] [review] Bug 7112 - Having two prices in 020$c causes basket creation to fail from staged marc import
I think this one is a keeper. I marked bug 7516 that I opened for MungeMarcPrice as a duplicate and merged it into this patch. I did so because it will actually fix this problem, rather than only fixing one special case ( having more than one period in the price field ).
Created attachment 8250 [details] [review] Bug 7112 - Having two prices in 020$c causes basket creation to fail from staged marc import The root problem here is that the price is being pulled from the MARC record and is then run through Number::Format::unformat_number. This routine is really being misused, and should only be used to reverse the effects of Number::Format on a number string. We are apparently using it to strip out currency characters and the like. Number::Format::unformat_number will choke if there is more than one period (.) in the price field. MARC standards do not limit this field to a single period, so unless there is only one period, we should skip number unformatting. Examples of that break unformat_number include '18.95 (U.S.)', and '$5.99 ($7.75 CAN)', both of which are perfectly valid. This commit adds the function MungeMarcPrice that will better handle find a real price value in a given price field. It does a very good job at finding a price in any currency format, and attempts to find a price in whichever currency is active before falling back to the first valid price it can find. The variable $price may fail to have an actual price, in which case the price then defaults to '0.00', which would be rarely if ever the correct price. To combat this, I have added highlighting to any price in the Order Details table that begins with 0 ( i.e. '0.00' ). Also, fixed the incomplete table footer, adding a new td with a span of 3 to fill in the nonexistant cells. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Cleverly picks the best price based on active currency. Minimal changes to C4/Biblio, little chance of regression. Fixes template table issue. Marking as Passed QA.
patch pushed (i haven't tested it myself, but it's a small patch, so trusting others)
This does not appear to have been pushed. Resetting to Passed QA.
Bugfix, pushed to stable will be in 3.8.2
Very cosmetic problem introduced by this patch : In a basket, if some prices are between 0 and 1, they are displayed in red, with class = "number error". For example, a VAT of 0.89 euros is displayed in red. In don't think it is normal. I have made a new bug : http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9374 M. Saby