Bug 9593 - Prices not imported correctly from a staged file
Summary: Prices not imported correctly from a staged file
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Sophie MEYNIEUX
QA Contact:
URL:
Whiteboard:
Keywords: rel_3_16_candidate
: 7504 (view as bug list)
Depends on:
Blocks: 12146
  Show dependency treegraph
 
Reported: 2013-02-12 10:22 UTC by Koha Team Lyon 3
Modified: 2016-06-21 21:36 UTC (History)
14 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this: 2013-10-30 00:00:00
Who signed the patch off:
Text to go in the release notes:


Attachments
filetest.elc (33.26 KB, application/octet-stream)
2013-02-12 10:22 UTC, Koha Team Lyon 3
Details
Bug 9593 Prices not imported correctly from a staged file (2.59 KB, patch)
2013-02-12 10:27 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
some records to test (16.88 KB, application/octet-stream)
2013-02-20 17:43 UTC, mathieu saby
Details
Bug 9593 Prices not imported correctly from a staged file (2.80 KB, patch)
2013-02-21 14:53 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (2.82 KB, patch)
2013-02-22 11:21 UTC, mathieu saby
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (7.19 KB, patch)
2013-05-03 10:22 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (7.02 KB, patch)
2013-05-04 13:55 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (7.01 KB, patch)
2013-05-04 14:23 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Prices not imported correctly from a staged file (10.28 KB, patch)
2013-05-10 14:58 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
sample_multicurrencies (8.81 KB, application/x-extension-mrc)
2013-05-10 15:37 UTC, Koha Team Lyon 3
Details
Bug 9593 Prices not imported correctly (plus conversion) (12.18 KB, patch)
2013-05-21 10:53 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
19records-testSaby-20130425 (26.28 KB, application/octet-stream)
2013-05-25 23:37 UTC, mathieu saby
Details
Bug 9593 Prices not imported correctly from a staged file (10.27 KB, patch)
2013-06-04 14:15 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (10.02 KB, patch)
2013-09-20 13:27 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
patch signed off (10.11 KB, patch)
2013-09-26 09:28 UTC, Christophe Croullebois
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (9.98 KB, patch)
2013-10-21 08:05 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (9.95 KB, patch)
2013-10-30 07:46 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (9.74 KB, patch)
2013-11-08 14:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (11.72 KB, patch)
2013-12-16 17:57 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (11.73 KB, patch)
2014-01-09 14:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (11.61 KB, patch)
2014-03-10 11:49 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (11.63 KB, patch)
2014-03-11 16:12 UTC, Sophie MEYNIEUX
Details | Diff | Splinter Review
File used to sign-off patch (256.67 KB, text/plain)
2014-03-11 16:17 UTC, Sophie MEYNIEUX
Details
Bug 9593 Prices not imported correctly from a staged file (11.59 KB, patch)
2014-04-11 10:35 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 9593 - Additional Unit Tests (1.71 KB, patch)
2014-04-11 11:28 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (12.41 KB, patch)
2014-04-15 09:55 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (12.38 KB, patch)
2014-04-15 10:21 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 9593 Prices not imported correctly from a staged file (12.44 KB, patch)
2014-04-22 11:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9593 [QA Followup] (2.84 KB, patch)
2014-04-22 11:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 9593 Prices not imported correctly from a staged file (12.70 KB, patch)
2014-04-26 15:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9593 [QA Followup] (2.91 KB, patch)
2014-04-26 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Koha Team Lyon 3 2013-02-12 10:22:26 UTC
Created attachment 15269 [details]
filetest.elc

When importing a record from a staged file, sometimes, the price is set to 0 in the Vendor price field (Accounting details block) even if initially present. it occurs when the price is written without decimals :
example : 19 EUR.
In this case it's not captured by the regular expression in MungeMarcPrice routine (Biblio.pm) :
( $price ) = $price =~ m/([\d\,\.]+[[\,\.]\d\d]?)/;

Same thing if the currency symbol was used after the digits, the result would be the same because of this :
$price = $parts[1];

If I got it well, seems that the whole routine can be replaced by 2 regular expressions (may be we could even go up to a single one) integrated in GetMarcprice :
the first part (symbol treatment) is unusefull as it can be treated directly by the regular expression that's following it.

The part concerning groups of more than 2 digits ( if I got it, it 's come out to delete the eventual separator between thousands and hundreds) can be replaced by another regular expression.

Test Plan :
1) get on your pc, the joined example file (testfile.elc). That's the kind of file most currently used in France for our acquisitions. Most of the prices in it match the pattern '[digit][digit] EUR' except 2 titles out of them :
La méthodologie appliquée du commentaire de texte (14,20 EUR)
Le tournant artiste de la littérature française (54,80 EUR)

2) Go to Tools/Stage MARC records for import and stage it in koha.
3) Go to Acquisition module, and an open basket of your choice and click on 'From a staged file'
4) Choose testfile.elc and add it to the basket
5) Click on the first individual 'add order' link :
the price might beeing set to 00.00 in the 'Vendor price' field of Accounting details block.
6) Now go back with go back function of your browser :
7) Apply the patch 
8) Click again on the same link
9) The price is now present (I hope so)
10 Sign off (I hope so again)
Comment 1 Koha Team Lyon 3 2013-02-12 10:27:59 UTC Comment hidden (obsolete)
Comment 2 mathieu saby 2013-02-12 16:12:25 UTC
Hello Lyon 3
I noticed the problem last week.
I have worked on a solution of my own, and our vendor (Biblibre) has been working on it too...
So I set the bug In Discussion.

Mathieu Saby
Rennes 2 university
Comment 3 mathieu saby 2013-02-12 16:13:51 UTC
My solution was changing these lines in MungeMarcPrice routine:

- ( $price ) = $price =~ m/([\d\,\.]+[[\,\.]\d\d]?)/;
+ ( $price ) = $price =~ m/([\d\,\.]+[\,|\.\d\d]?)/;
     ## Split price into array on periods and commas
     my @parts = split(/[\,\.]/, $price);
     ## If the last grouping of digits is more than 2 characters, assume there is no decimal value and put it back.
     my $decimal = pop( @parts );
- if ( length( $decimal ) > 2 ) {
+ if ((scalar @parts == 0) or ( length( $decimal ) > 2 )) { 




M. Saby
Comment 4 Koha Team Lyon 3 2013-02-13 09:17:42 UTC
(En réponse au commentaire 3)
> My solution was changing these lines in MungeMarcPrice routine:
> 
> - ( $price ) = $price =~ m/([\d\,\.]+[[\,\.]\d\d]?)/;
> + ( $price ) = $price =~ m/([\d\,\.]+[\,|\.\d\d]?)/;
>      ## Split price into array on periods and commas
>      my @parts = split(/[\,\.]/, $price);
>      ## If the last grouping of digits is more than 2 characters, assume
> there is no decimal value and put it back.
>      my $decimal = pop( @parts );
> - if ( length( $decimal ) > 2 ) {
> + if ((scalar @parts == 0) or ( length( $decimal ) > 2 )) { 
> 
> 
> 
> 
> M. Saby

Bonjour Mathieu

Your solution will not work on prices with such pattern as :

1 000 EUR :
To get it work you must add a space in first alternative here
( $price ) = $price    =~ m/([\d\,\. ]+[\,|\.\d\d]?)/;
and there :
 my @parts = split(/[\,\. ]/, $price);

125 € :
You must delete the first part of the routine until the regular expression (may be it was implied in what you showed ?).
Otherwise, when the symbol had been entered in mysql symbol field as "active currency" and is typed after the digit(s), the variable $price in $price = $parts[1]  will be undef.

£ 10 :
I don't know if it could be frequent but in case, the result will be ".10". To avoid this, I add a mandatory digit at the beginning of regexp and replace + by * after the first alternative.

Olivier Crouzet
Comment 5 mathieu saby 2013-02-13 09:33:34 UTC
Hello
> 
> Your solution will not work on prices with such pattern as :
> 
> 1 000 EUR :
> To get it work you must add a space in first alternative here
> ( $price ) = $price    =~ m/([\d\,\. ]+[\,|\.\d\d]?)/;
> and there :
>  my @parts = split(/[\,\. ]/, $price);


You must be right!
I only tested on a few records (10 EUR ; 10,50 EUR ; 10.50 EUR)


> 
> 125 € :
> You must delete the first part of the routine until the regular expression
> (may be it was implied in what you showed ?).
> Otherwise, when the symbol had been entered in mysql symbol field as "active
> currency" and is typed after the digit(s), the variable $price in $price =
> $parts[1]  will be undef.
> 
> £ 10 :
> I don't know if it could be frequent but in case, the result will be ".10".
> To avoid this, I add a mandatory digit at the beginning of regexp and
> replace + by * after the first alternative.
> 

Yes, I did not think of symbols...

I think Biblibre is going to take part in this discussion.

Mathieu
Comment 6 Koha Team Lyon 3 2013-02-19 14:24:30 UTC
Hello,
I don't understand the status "in discussion" : if this patch correct the problem, why don't you sign it off ? It can be applied in the next version of koha and biblibre can apply this patch in your university ? 
I'm not sure that the status in discussion is made for waiting to another solution ?
Sonia (you can Recognize my perfect english..) 
I swith the status to "needs signoff"
Comment 7 mathieu saby 2013-02-19 16:03:09 UTC
Hello Sonia
Biblibre told me they were working on a patch, and that your solution was maybe not the best. But they don't seem to take part in the discussion...
I am going to ask them again. If they dont react, I will try to sign off tomorrow.

Mathieu
Comment 8 mathieu saby 2013-02-20 17:42:19 UTC
Désolé...
Failed QA

In your marc file : 

 Le tournant artiste de la littérature française / écrire avec la peinture au XIXe siècle by Bernard Vouilloux
price = 54,80 EUR

In my basket, after applying your patch : 
price = 54.00 EUR


I also join an other marc file for testing other price schemes, like 1 125.00 EUR.

M. Saby
Comment 9 mathieu saby 2013-02-20 17:43:14 UTC
Created attachment 15569 [details]
some records to test
Comment 10 Koha Team Lyon 3 2013-02-21 14:53:59 UTC Comment hidden (obsolete)
Comment 11 mathieu saby 2013-02-22 11:18:37 UTC
Tested with a marc file ('some records to test') containting some records with different paterns for prices
=> everything is OK in created basket

010    $a 978-2-251-32888-1 $b br. sous jaquette $d 75 EUR
200 1  $a L'affirmation de la puissance romaine en Judee (63 a. C.-136 p. C.) 
basket : 75.00

010    $a 972-8462-28-X $b br. $d 25.12 EUR
200 1  $a Les bandits 
basket : 25.12

010    $a 978-2-227-48271-5 $b br. $d 21,35 EUR
200 1  $a Chocolat, clown negre
basket : 21.35

010    $a 978-2-7010-1601-6 $b br. $d 49.00 EUR
200 1  $a Ephemera catholiques
basket : 49.00

010    $a 2-7082-2961-3 $b br. $d 20,00 EUR
200 1  $a L'Histoire de France autrement
basket : 20.00

010    $a 978-2-262-02799-5 $b br. $d 1125,52 EUR
200 1  $a Histoire de la Resistance, 1940-1945
basket : 1125.52

010    $a 978-2-07-044382-6 $b br. $d 1125 EUR
200 1  $a Ni droite ni gauche
basket : 1125.00

010    $a 978-2-330-00227-5 $b br. $d 1 125 EUR
200 1  $a Ni valise, ni cercueil
basket : 1125.00 

010    $a 978-2-84952-166-3 $b br. $d 1 125.50 EUR
200 1  $a Nivelle
basket : 1125.50


I sign off

M. Saby
Comment 12 mathieu saby 2013-02-22 11:21:47 UTC Comment hidden (obsolete)
Comment 13 Marcel de Rooy 2013-03-29 12:01:54 UTC
QA Comment:
Like the idea of simplification!
But I read in the deleted code:
> This allows us to prefer this native currency price over other currency 
> prices, if possible.

I understand that it so handles a thing like: $4, €3.
Your solution makes that no longer possible. It just picks the first.

Probably we still need the routine, and could you improve the code there?
Comment 14 Koha Team Lyon 3 2013-03-29 15:41:14 UTC
Hello,
I'm not sure to understand. This new patch actually handles prices like $2 and $€ : if you have a '$2' input price, it returns '2'. Is it not what is expected ? 
I add that the previous function MungeMarcPrice, from what I tested, did not handle prices like $2 !?!

Olivier Crouzet
Comment 15 mathieu saby 2013-04-18 16:25:14 UTC
This is a VERY annoying bug for us. So does something be done to pass QA, or not ?

M. Saby
Comment 16 mathieu saby 2013-04-21 18:04:39 UTC
I wanted to make some tests to see how your patch is dealing exactly with $ and €.
But I got a nasty error in master when I wanted to add orders : 'Can't use string ("") as a HASH ref while "strict refs" in use at /home/msaby/kohamaster/src/C4/Items.pm line 2712.'
I will create a new bug for that...

I think that the function you suppressed assumed the currency symbol is always before the price, while this is not the case in every country, and especially in France. In France, we write : 15 €, 12 $.
So I wonder if it was working well before...

See http://en.wikipedia.org/wiki/Currency_sign : "Many currencies, especially in Latin America and the English-speaking world, place it before the amount (e.g., R$50,00); many others place it after the amount (e.g., 50.00 S₣)"

Moreover, in France, according to cataloguing rules, we normally don't use those symbols, but 3 letters codes put after the price : 15 EUR, 12 USD, 10 GBP etc.
Those 3 letter symbols are defined in standard ISO 4217.
In the record we receive from our main providers (Electre, BNF). For Sudoc network (ABES), it is a recommandation, but symbols like $ are tolerated.

Mathieu
Comment 17 mathieu saby 2013-04-25 11:31:36 UTC
Hello Lyon 3
I made the following test to check the way Koha was handling multi-currency prices. 
I defined these currencies : 
EUR € 1.00 (active)
USD $ 0.50

Before applying your patch, I created a basket with staged records, selecting EUR currency in Accounting details (in addorderiso2709.pl).
I applied the patch, and created an other basket with the same file of stage records.

Here are the results.
It is not good before your patch, not good after, but in a different way :(

So managing multi-currency prices is a complex issue. I suggest you to rewrite your patch only to fix the initial bug (, instead of . in prices), so that we could push your fix as soon as possible. And create an other ticket for improving/fixing multi-currency prices support...

What do you think of that?

Detailed test : 

Record 1 : 
010$d : 75.00 €, 30.00 $
Without your patch => in basket : 30 euros
With your patch    => in basket : 75 euros

Record 2 : 
010$d : 25.00€, 32.00$
Without your patch => in basket : 32 euros 
With your patch    => in basket : 25 euros 

Record 3 : 
010$d : €22.00, $56.00
Without your patch => in basket : 22 euros
With your patch    => in basket : 22 euros

Record 4 : 
010$d : $42.00, £12.00
Without your patch => in basket : 0 (uncertain price)
With your patch    => in basket : 0 (uncertain price)

Record 5 : 
010$d : 20.40 EUR, 12.50 USD
Without your patch => in basket : 20.40 euros
With your patch    => in basket : 20.40 euros

Record 6 : 
010$d : 112.10 €, 110.15 $, 90.15 £
Without your patch => in basket : 110.15 euros
With your patch    => in basket : 112.10 euros

Record 7 : 
010$d : 1125.00 EUR
Without your patch => in basket : 1125 euros
With your patch    => in basket : 1125 euros

Record 8 : 
010$d : €125.00
Without your patch => in basket : 125 euros
With your patch    => in basket : 125 euros

Record 9 : 
010$d : 1125.50 USD
Without your patch => in basket : 1125.50 euros
With your patch    => in basket : 1125.50 euros

Record 10 : 
010$d : $1125.50
Without your patch => in basket : 0 (uncertain price)
With your patch    => in basket : 0 (uncertain price) 

Record 11 : 
010$d : $10.00, €12.00
Without your patch => in basket : 0 (uncertain price)
With your patch    => in basket : 0 (uncertain price) 

Record 12 : 
010$d : 10.00 $, 12.00 €
Without your patch => in basket : 10 euros
With your patch    => in basket : 10 euros

Record 13 : 
010$d : 10.00 USD, 12.00 EUR
Without your patch => in basket : 10 euros
With your patch    => in basket : 10 euros

Mathieu
Comment 18 Koha Team Lyon 3 2013-05-03 10:22:47 UTC Comment hidden (obsolete)
Comment 19 Kyle M Hall 2013-05-03 17:20:04 UTC
Comment on attachment 17923 [details] [review]
Bug 9593 Prices not imported correctly from a staged file

Review of attachment 17923 [details] [review]:
-----------------------------------------------------------------

::: installer/data/mysql/updatedatabase.pl
@@ +6785,5 @@
>  }
>  
> +$DBversion = "3.11.00.XXX";
> +if ( CheckVersion($DBversion) ) {
> +    $dbh->do("ALTER TABLE currncy ADD isocode VARCHAR(5) default NULL AFTER symbol;");

Typo!
Comment 20 Koha Team Lyon 3 2013-05-04 13:55:15 UTC Comment hidden (obsolete)
Comment 21 Koha Team Lyon 3 2013-05-04 14:23:48 UTC Comment hidden (obsolete)
Comment 22 mathieu saby 2013-05-04 14:48:28 UTC
Olivier, could you be more explicit in your commit message about what exactly your patch is supposed to do, and add a test plan ?

Mathieu
Comment 23 Koha Team Lyon 3 2013-05-10 14:58:00 UTC Comment hidden (obsolete)
Comment 24 Koha Team Lyon 3 2013-05-10 15:37:54 UTC
Created attachment 18075 [details]
sample_multicurrencies
Comment 25 mathieu saby 2013-05-17 12:51:07 UTC
My 1st test, just to check the new behavior of currency module
=> seems ok

My test plan:

1. $perl installer/data/mysql/updatedatabase.pl
Upgrade to 3.11.00.XXX done (Added isocode to the currency table)
2. going to admin/currency.pl page : there is a new column for iso code
=> ok
3. editing my two currencies (euro and dollar) to add iso codes (EUR and USD)
=> ok
4. adding a new currency (sterling pound) with symbol £ and iso code GBP
=> ok
5. suppressing a currency
=> ok
6. changing the active currency
=> ok


I am now testing the import of marc record

Mathieu
Comment 26 mathieu saby 2013-05-17 13:20:15 UTC
Following of my test : 

add currencies matching codes and symbols in your records

CANADA 	1.00000 	$ 	CAN
DOLLAR 	1.00000 	$ 	USD
EGYPT 	1.00000 	£E 	EGP
EUR 	1.00000 	€ 	EUR
KENYAN SHI 	1.00000 	KSh 	KES
LIVRE 	1.00000 	£ 	GBP
YEN 	1.00000 	¥ 	JPY

(CAN should be CAD I suppose, but this is cosmetic)
active currency : EUR

Checking 010$d fields of your records in with yaz-marcdump.

Staging records
Filling a basket with staged records, selecting currency : EUR.

Checking content of the basket : 

A hauteur d'homme / Rousseau et l'écriture de soi by Lionel Bourg
    <subfield code="d">75.40 EUR, 93.5 USD, 20 250 EGP</subfield>
RRP tax exc : 75.40

A la conquête du Graal by Alicia Bekhouche
    <subfield code="d">$44, 23 €, 64 Br,£30</subfield>
RRP tax exc : 23.00

Bernanos et les {copy}ages de la vie / actes du colloque d'Aix-en-Provence, 23, 24, 25 octobre 2008 by sous la direction de André Not
    <subfield code="d">25,5 EUR, $US32, $LD35</subfield>
RRP tax exc : 25.50

Chateaubriand et la gravité du comique by Fabio Vasarri
    <subfield code="d">38 Ksh, ¥300, $62</subfield>
RRP tax exc : 0 (uncertain)

Ecrire la sculpture, XIXe-XXe siècles / actes du colloque organisé à l'Ecole normale supérieure et à la Maison de la recherche du 16 au 18 juin 2011 by sous la direction d'Ivanne Rialland
    <subfield code="d">32 EUR, 45$ CAN, 46$ USD</subfield>
RRP tax exc : 32.00


=> problem with "Chateaubriand et la gravité du comique".
But I suppose we would never find such thing as "38 Ksh, ¥300, $62" in 010$d (more likely "Ksh38, ¥300, $62", or "38 Ksh, 300¥, 62$").

I will make other tests later

Mathieu
Comment 27 Koha Team Lyon 3 2013-05-17 13:55:16 UTC
Hey Mathieu,
My god, you are everywhere !
fallen in a magic potion barrel when a baby ?

As for Chateaubriand case, it's a normal result with euro as active currency because there's no euro currency in the string price '38 Ksh, ¥300, $62'
As I 'd mentionned :
Whenever active currency is enabled and neather the symbol nor the isocode of this currency is found in the price string, the resulting price is set to 0 (rather than picking the first one by default).
I did this knowingly to make clear that no price has been picked.
If first price would be picked, in your test context,38 would be the resulting
price and we could think it's in euro.

Olivier
Comment 28 mathieu saby 2013-05-17 14:43:27 UTC
I did not read all your description, I must confess ;-)
Not sure to agree with this behavior if it is a new one.

I think it should be : 
- in Koha, 3 currencies = euros €, dollar $, pound £ ; and active currency = euro

- in 010 : 40 €, 50 $
=> 40 €
- in 010 : 50 $
=> convert 50 $ in € using the exchange rate defined in Koha
- in 010 : 50 $, 60 ¥
=> convert 50 $ in € using the exchange rate defined in Koha
(¥ should be ignored, as ¥ is not defined in Koha list of currencies)
- in 010 : 60 ¥
=> 0 (undefined)
(¥ should be ignored, as ¥ is not defined in Koha list of currencies)
- in 010 : 50 $, 30 £
=> 2 solutions : 
- either 0 (undefined)
- or maybe convert the first price of the list in euros, using exchange rate

What's your opinion?

Mathieu
Comment 29 Koha Team Lyon 3 2013-05-21 10:53:40 UTC Comment hidden (obsolete)
Comment 30 mathieu saby 2013-05-21 11:24:24 UTC
I will take a look tomorrow.

we don't use it either in Rennes2, and I think no french library is using it.
Maybe you can ask the question on koha list.

Mathieu
Comment 31 mathieu saby 2013-05-25 16:51:09 UTC
Hello. 
I tested your 2d patch.
It needs to be rebased (conflict in updatedatabse, and need to remplace 3.11.XXX by 3.13.XXX)

The result is the same, except for

 Chateaubriand et la gravité du comique by Fabio Vasarri
=> 300.00

This is not good, but as I put before, this kind of data is not supposed to be found... 
    <subfield code="d">38 Ksh, ¥300, $62</subfield>

I will edit your sample batch of record, so it would be easier to test.

Mathieu
Comment 32 mathieu saby 2013-05-25 23:36:15 UTC
Olivier,
With your 2d patch, I made 4 tests, with a batch of 19 records (I join them)
tva: 0
discount : 5%

test A : Euro as active currency, all currency rates = 1 ; selecting "Euro" currency in addorderiso2709.pl (in Accounting details)
test B : Euro as active currency ; rates : 1 € = 10 $ = 4 £ ; selecting "Euro" currency in addorderiso2709.pl (in Accounting details)
test C : GB Pound as active currency ; rates : 1 £ = 10 $ = 5 € ; selecting "Pound" currency in addorderiso2709.pl (in Accounting details)
test D : GB Pound as active currency ; rates : 1 £ = 10 $ = 5 € ; selecting "Euro" currency in addorderiso2709.pl (in Accounting details)

A, B, C are OK
D seems not OK, but I am not sure to understand how the currency you can choose when importing the record is supposed to interact with the currency stored of the record.
What Koha is supposed to do if there is a price with £ symbol in the record, but if we select "euro" while importing?
Maybe the currency choosen when importing is made to be used if the prices have no currency symbol in the record??
I did not test this point without your patch. Perhaps it is broken... If it is the case, I would agree to sign it off, as as the problem it fixes has been noticed in january. And we can fix it in an other patch.
Could you give me your analysis?
If you are as lost as myself, you can ask the koha-devel list ;-)

Titre01
75.40 EUR, 93.5 USD, 20 250 GBP
test A and test B : OK
- should be : 75.40
- result : 75.40
test C : OK
- should be : RRP = 20250.00 ; Total tax inc = 19237.50
- result : RRP : 20250.00 ; Total tax inc. (POUND) : 19237.50
test D : KO 
- should be : don't know
- result : RRP : 4050.00 ; Total tax inc. (POUND) : 19237.50
=> it means Koha knows £ is the active currency, takes the 20250 pounds price,
BUT after that, the system considers it is 20250 euros, so it divides it by 5 (according to currency rate), to establish a RRP price of 4050£
AND rather strangely, the Total tax inc. price is calculated according to the raw value in the record (20250) and not to the wrongly calculated price of 4050£.

Titre02
75.40 EUR, 93.5 USD, 20,250 GBP
test A and test B : OK
- should be : 75.40
- result : 75.40
test C : OK
- should be : RRP = 20250.00 ; Total tax inc = 19237.50
- result : RRP : 20250.00 ; Total tax inc. (POUND) : 19237.50
test D : KO 
- should be : don't know
- result : RRP : 4050.00 ; Total tax inc. (POUND) : 19237.50

Titre03
75.40 EUR, 93.5 USD, 20,250.10 GBP
test A and test B : OK
- should be : 75.40
- result : 75.40
test C : OK
- should be : RRP = 20250.10 ; Total tax inc = 19237.60
- result : RRP : 20250.10 ; Total tax inc. (POUND) : 19237.60
test D : KO 
- should be : don't know
- result : RRP : 4050.02 ; Total tax inc. (POUND) : 19237.60

Titre04
€75.40, $93.5, £20,250.10
test A and test B : OK
- should be : 75.40
- result : 75.40
test C : OK
- should be : RRP = 20250.10 ; Total tax inc = 19237.60
- result : RRP : 20250.10 ; Total tax inc. (POUND) : 19237.60
test D : KO 
- should be : don't know
- result : RRP : 4050.02 ; Total tax inc. (POUND) : 19237.60

Titre10
32,10 EUR, 45,12 USD
test A and test B : OK
- should be : 32.10
- result : 32.10
test C : OK
- should be : RRP = 6.42 (= 32.10/5) ; Total tax inc = 6.10
- result : = 6.42 (= 32.10/5) ; Total tax inc = 6.10
test D : KO
- should be : don't know
- result : RRP : 1.28 ; Total tax inc. (POUND) : 6.10
=> not good : Koha took the first price (32.10), and considered it is euro. I don't know how, but for RRP price, it divided it by 5, and then again by 5 (32.10/5/5 = 1.28). For Total tax inc, it only took 32.10 /5 (minus the discount), that is correct.

Titre11
32,10 €, 45,12 $
test A and test B : OK
- should be : 32.10
- result : 32.10
test C : OK
- should be : RRP = 6.42 (= 32.10/5) ; Total tax inc = 6.10
- result : = 6.42 (= 32.10/5) ; Total tax inc = 6.10
test D : KO
- should be : don't know
- result : RRP : 1.28 ; Total tax inc. (POUND) : 6.10

Titre12
32.10, USD 45.12
test A and test B : OK
- should be : 32.10
- result : 32.10
test C : OK
- should be : RRP = 6.42 (= 32.10/5) ; Total tax inc = 6.10
- result : = 6.42 (= 32.10/5) ; Total tax inc = 6.10
test D : KO
- should be : don't know
- result : RRP : 1.28 ; Total tax inc. (POUND) : 6.10

Titre13
€32.10, $45.12
test A and test B : OK
- should be : 32.10
- result : 32.10
test C : OK
- should be : RRP = 6.42 (= 32.10/5) ; Total tax inc = 6.10
- result : = 6.42 (= 32.10/5) ; Total tax inc = 6.10
test D : KO
- should be : don't know
- result : RRP : 1.28 ; Total tax inc. (POUND) : 6.10

Titre20
300 ¥, 62.20 $
test A : OK
- should be : 62.20
- result : 62.20
test B : OK
- should be : 6.22
- result : 6.22
test C : OK
- should be : RRP = 6.22 (= 62.20/10) ; Total tax inc = 5.91
- result : RRP = 6.22 ; Total tax inc = 5.91
test D : KO
- should be : don't know
- result : RRP : 1.24 ; Total tax inc. (POUND) : 5.91
=> same as before : RRP = raw price / 25 ; total tax inc. = raw price / 5 minus discount

Titre22
¥300, $62.20
test A :
- should be : 62.20
- result : 62.20
test B : 
- should be : 6.22 (as 10 $ = 1 €)
- result : 6.22
test C : OK
- should be : RRP = 6.22 (= 62.20/10) ; Total tax inc = 5.91
- result : RRP = 6.22 ; Total tax inc = 5.91
test D : KO
- should be : don't know
- result : RRP : 1.24 ; Total tax inc. (POUND) : 5.91

Titre23
300 YEN, 62.20 USD
test A :
- should be : 62.20
- result : 62.20 => OK
test B : OK
- should be : 6.22
- result : 6.22
test C : OK
- should be : RRP = 6.22 (= 62.20/10) ; Total tax inc = 5.91
- result : RRP = 6.22 ; Total tax inc = 5.91
test D : KO
- should be : don't know
- result : RRP : 1.24 ; Total tax inc. (POUND) : 5.91

Titre24
YEN 300, USD 62.20
test A :
- should be : 62.20
- result : 62.20
test B : OK
- should be : 6.22
- result : 6.22
test D : OK
- should be : RRP = 6.22 (= 62.20/10) ; Total tax inc = 5.91
- result : RRP = 6.22 ; Total tax inc = 5.91
test D : KO
- should be : don't know
- result : RRP : 1.24 ; Total tax inc. (POUND) : 5.91

Titre30
75 EUR
test A and B : OK
- should be : 75.00
- result : 75.00
test C : OK
- should be : RRP = 15£ (=75€/5) ; Total tax inc. : 14.25£
- result : RRP : 15 ; Total tax inc. (POUND) : 14.25
test D :
- should be : don't know
- result : RRP : 3 ; Total tax inc. (POUND) : 14.25
=> same as before : RRP = raw price / 25 ; total tax inc. = raw price / 5 minus discount

Titre31
€75
test A and B : OK
- should be : 75.00
- result : 75.00
test C : OK
- should be : RRP = 15£ (=75€/5) ; Total tax inc. : 14.25£
- result : RRP : 15 ; Total tax inc. (POUND) : 14.25
test D :
- should be : don't know
- result : RRP : 3 ; Total tax inc. (POUND) : 14.25

Titre32
EUR 75
test A and B : OK
- should be : 75.00
- result : 75.00
test C : OK
- should be : RRP = 15£ (=75€/5) ; Total tax inc. : 14.25£
- result : RRP : 15 ; Total tax inc. (POUND) : 14.25
test D : KO
- should be : don't know
- result : RRP : 3 ; Total tax inc. (POUND) : 14.25

Titre33
75 €
test A and B : OK
- should be : 75.00
- result : 75.00 => OK
test C : OK
- should be : RRP = 15£ (=75€/5) ; Total tax inc. : 14.25£
- result : RRP : 15 ; Total tax inc. (POUND) : 14.25
test D : KO
- should be : don't know
- result : RRP : 3 ; Total tax inc. (POUND) : 14.25

Titre34
¥300
all tests : OK
- should be : 0
- result : 0 => OK

Titre35
300 YEN
all tests : OK
- should be : 0
- result : 0 => OK

Titre36
300 ¥
all tests : OK
- should be : 0
- result : 0 => OK


Mathieu
Comment 33 mathieu saby 2013-05-25 23:37:16 UTC
Created attachment 18405 [details]
19records-testSaby-20130425

the records I used for testing
Comment 34 mathieu saby 2013-05-26 10:23:10 UTC
I made again test D, but WITHOUT your patch.
GB Pound as active currency ; rates : 1 £ = 10 $ = 5 € ; selecting "Euro" currency in addorderiso2709.pl (in Accounting details)
I call it test E

Titre01, Titre02, Titre03, Titre30, Titre31, Titre32, Titre33 are broken, but that's logical.

Titre04 has the same values as test D
€75.40, $93.5, £20,250.10
test D & E - result : RRP : 4050.02 ; Total tax inc. (POUND) : 19237.60

Titre10, Titre11, Titre12, Titre13 as values different from test D
32,10 EUR, 45,12 USD
Test D - result : RRP : 1.28 ; Total tax inc. (POUND) : 6.10
Test E - result : RRP : 6.42 ; Total tax inc. (POUND) : 30.50

Titre20, Titre21, Titre22, Titre24 as values different from test D
Test D - result : RRP : 1.24 ; Total tax inc. (POUND) : 5.91
Test E - result : RRP : 12.44 ; Total tax inc. (POUND) : 59.09


So your patch seems to change Koha behavior on that point. Could you check it 
why?

Mathieu
Comment 35 Koha Team Lyon 3 2013-06-04 14:15:21 UTC Comment hidden (obsolete)
Comment 36 I'm just a bot 2013-09-18 11:11:40 UTC
Applying: Bug 9593 Prices not imported correctly from a staged file
Using index info to reconstruct a base tree...
M	C4/Biblio.pm
M	installer/data/mysql/kohastructure.sql
M	installer/data/mysql/updatedatabase.pl
Falling back to patching base and 3-way merge...
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging C4/Biblio.pm
Patch failed at 0001 Bug 9593 Prices not imported correctly from a staged file
The copy of the patch that failed is found in:
   /home/christopher/git/koha/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 37 Koha Team Lyon 3 2013-09-20 13:27:31 UTC Comment hidden (obsolete)
Comment 38 I'm just a bot 2013-09-24 05:42:22 UTC
Patch applied cleanly, go forth and signoff
Comment 39 Christophe Croullebois 2013-09-25 15:35:37 UTC
Hello Koha Team Lyon 3,
I have made tests with filetest.elc, some records to test, and  sample_multicurrencies.
First I have tested with currency in Euro => OK
After I have tested in Dollar
the case : 25,5 EUR, $US32, $LD35 is "Uncertain" for the others it's ok.
For me you have done a great job, with your patch many cases are taken into account.
Even if $US32 is not recognised, I think $US 32 will be, I am ok to sign of, because it's a good improvement and without the patch it's awful.
Tell me if you want to add something, or I'll sign of tomorrow.
Thx for the patch
Comment 40 Christophe Croullebois 2013-09-26 09:27:13 UTC
As I said, it's ok for me.
Signed off
Comment 41 Christophe Croullebois 2013-09-26 09:28:47 UTC Comment hidden (obsolete)
Comment 42 Koha Team Lyon 3 2013-09-26 09:41:25 UTC
Hey Christophe
I posted this one the same minute as yours and there was a collision !. I saw you just signed off but here is my comment nevertheless :

I tested the string you told of ('25.5 EUR, $US32, $LD35') with symbol='$US' in a separated perl script, and I get 32 as expected, same for $LD.
But I know that testing this import into koha may not be so easy. May be some cache issues caused by testing in a loop or test file with encoding errors ?
For me, the important thing is that if the string is not found the returned price must be 0. ( better not price at all than a false one)
Anyway, I could not swear it will cover whatever kind of price string but if it can deal with main part of not too screwy ones, you are welcome to sign it off.

Oliver C.
Comment 43 Paul Poulain 2013-10-21 00:46:35 UTC
QA comment: nice patch, but one question: why did you put 
+            ( $subfield_value ) = $subfield_value =~ m/(\d[\d\,\. ]*\d{0,2})/;
+            # remove comma,dot or space when used as separators from hundreds
+            $subfield_value =~s/[\,\. ](\d{3})/$1/g;
+            # convert comma to dot to ensure correct display of decimals if existing
+            $subfield_value =~s/,/./;

outside from MungeMarcPrice, it should be here as well, because it's related to discovering & retrieving the price from the MARC field
So I think it should go here

Otherwise :
 * great regexp description !
 * patch does not apply anymore due to updatedatabase changes

Failing QA, answer my question 1, provide an updated patch if my point is relevant and switch back to signed off
Comment 44 Koha Team Lyon 3 2013-10-21 08:05:33 UTC Comment hidden (obsolete)
Comment 45 I'm just a bot 2013-10-30 07:40:42 UTC
Applying: Bug 9593 Prices not imported correctly from a staged file
Using index info to reconstruct a base tree...
M	C4/Biblio.pm
M	installer/data/mysql/kohastructure.sql
M	installer/data/mysql/updatedatabase.pl
Falling back to patching base and 3-way merge...
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging C4/Biblio.pm
Patch failed at 0001 Bug 9593 Prices not imported correctly from a staged file
The copy of the patch that failed is found in:
   /home/christopher/git/koha/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 46 Chris Cormack 2013-10-30 07:46:49 UTC Comment hidden (obsolete)
Comment 47 mathieu saby 2013-11-03 11:53:56 UTC
The patch was signed off. I'm fixing the status.

Mathieu
Comment 48 Kyle M Hall 2013-11-08 14:01:57 UTC
The patch does not have a sign-off line. Could whoever meant to sign it please do so.

As the original author of MungeMarcPrice I fully endorse these changes. However, I'd really like to see some unit tests to test all these various price string situations. This sub should be one of the absolute easiest ones to write unit tests for since all it does is take in a price string and return a price string.
Comment 49 Kyle M Hall 2013-11-08 14:03:45 UTC Comment hidden (obsolete)
Comment 50 mathieu saby 2013-11-08 14:57:39 UTC
It was signed off the 26th september by C.Croullebois, in comment 41, (attachment 21477 [details] [review]), but a new patch was added the 21st october 'attachment.cgi?id=22151) without signoff line (see comment 44).

I think Lyon 3 deleted by mistake the signoff line.
Lyon 3, could you confirm that ?

Mathieu
Comment 51 Koha Team Lyon 3 2013-11-12 15:12:35 UTC
I know very few about unit test. Would something like that do the trick ?

#!/usr/bin/perl

use strict;
use warnings;
use C4::Biblio;
use Test::More tests=>1;

# start transaction
my $dbh = C4::Context->dbh;
$dbh->{AutoCommit} = 0;
$dbh->{RaiseError} = 1;

# set a test price string
my $PRICE_STRING='25.5 EUR, $32, $LD35';
my $expectedout='32';

# set active currency test data
my $CURRENCY = 'TEST';
my $SYMBOL = '$';
my $ISOCODE = 'USD';
my $RATE= 1;

# disables existing active currency if necessary.
my $active_currency = C4::Budgets->GetCurrency();
my $curr;
if ($active_currency) {
    $curr = $active_currency->{'currency'};
    $dbh->do("UPDATE currency set active = 0 where currency = '$curr'");
}

$dbh->do("INSERT INTO currency ( currency,symbol,isocode,rate,active ) 
          VALUES ('$CURRENCY','$SYMBOL','$ISOCODE','$RATE',1)");

my $mungemarcprice=MungeMarcPrice($PRICE_STRING);
ok ($mungemarcprice eq $expectedout, "MungeMarcPrice returned $expectedout as expected");

# Cleanup
$dbh->rollback;


Olivier Crouzet
Comment 52 mathieu saby 2013-12-12 10:13:56 UTC
Hello Olivier
I think it's the idea...
But the idea of UT is also to test different configuraton. So, you should not only test
my $PRICE_STRING='25.5 EUR, $32, $LD35';
my $expectedout='32';
but also made 3 or 4 other cases like '25,00 EUR' or '25 EUR' or '25 €' for example

Also, could you confirm that the original patch was signed off by C. Croullebois, and restore its sign-off line? So that it will be clear that we are at "QA" stage, and not at "Need sign off" ?

Mathieu
Comment 53 Koha Team Lyon 3 2013-12-16 17:57:05 UTC Comment hidden (obsolete)
Comment 54 Koha Team Lyon 3 2013-12-16 18:03:32 UTC
oups ! I meant "in order that any currency sign COULD BE captured"
Comment 55 mathieu saby 2013-12-25 18:11:27 UTC
*** Bug 7504 has been marked as a duplicate of this bug. ***
Comment 56 Jonathan Druart 2014-01-09 14:05:38 UTC Comment hidden (obsolete)
Comment 57 Jonathan Druart 2014-01-09 14:06:11 UTC
rebased patch.
Comment 58 Koha Team Lyon 3 2014-03-10 11:49:01 UTC Comment hidden (obsolete)
Comment 59 Sophie MEYNIEUX 2014-03-11 16:12:35 UTC Comment hidden (obsolete)
Comment 60 Sophie MEYNIEUX 2014-03-11 16:17:10 UTC
Created attachment 26127 [details]
File used to sign-off patch

Patch tested with file liste.elec

Without patch, all prices without decimals are replaced by 0 in order.
With the patch, all prices are correctly set in order
Comment 61 Katrin Fischer 2014-04-09 23:17:23 UTC
Patch still applies, but there is a complaint from the QA script:

 FAIL	t/db_dependent/MungeMarcPrice.t
   OK	  pod
   OK	  forbidden patterns
   OK	  valid
   FAIL	  critic
		I/O layer ":utf8" used at line 11, column 1. Use ":encoding(UTF-8)" to get strict validation.
		I/O layer ":utf8" used at line 12, column 1. Use ":encoding(UTF-8)" to get strict validation.

Could you take a look at switch back to 'signed off' please?
Comment 62 Koha Team Lyon 3 2014-04-11 10:35:37 UTC Comment hidden (obsolete)
Comment 63 Kyle M Hall 2014-04-11 11:28:00 UTC Comment hidden (obsolete)
Comment 64 Kyle M Hall 2014-04-11 11:31:40 UTC
I'm afraid it appears that this patch introduces some regressions. If you try the examples from the original bug report that added MungeMarcPrice, you'll see that some variations fail that previously would not. I've attached a followup that with tests that pass with the original version of the subroutine but fail with the new version.
Comment 65 Koha Team Lyon 3 2014-04-15 09:55:55 UTC Comment hidden (obsolete)
Comment 66 Koha Team Lyon 3 2014-04-15 10:21:23 UTC Comment hidden (obsolete)
Comment 67 Katrin Fischer 2014-04-20 14:26:52 UTC
Just 2 notes about this, leaving final QA to Kyle:

- I think the note on the currency page is misplaced. It would be better using a diferent styling (similar to other hints) below the table. 
- I am a bit torn about adding a new column for ISO code. The sample Koha currencies already have the ISO code as code and the overdue_notices.pl that prints the fine into the notices also assumes code to be the ISO code. Maybe we should just enforce the code to be the ISO code a bit more?
Comment 68 Kyle M Hall 2014-04-21 13:01:04 UTC
> - I think the note on the currency page is misplaced. It would be better
> using a diferent styling (similar to other hints) below the table. 

I agree with this. In addition, I think it should be worded more like:

When importing MARC files via the staging tools, the tool will attempt to find and use the price of the currently active currency. 

> - I am a bit torn about adding a new column for ISO code. The sample Koha
> currencies already have the ISO code as code and the overdue_notices.pl that
> prints the fine into the notices also assumes code to be the ISO code. Maybe
> we should just enforce the code to be the ISO code a bit more?

I agree, the default data convention is for the "Currency" field to be the ISO code. However, that doesn't mean that everyone is currently following this convention. I *don't* think this is a blocker. I could easily see the Currency being "Brazilian real" while the ISO code would be "BRL". Since not everyone memorizes ISO codes for currency, I think this may be an improvement anyway.
Comment 69 Katrin Fischer 2014-04-21 15:58:01 UTC
I wanted to note it - adding the column is ok with me.
Comment 70 Kyle M Hall 2014-04-22 11:23:29 UTC Comment hidden (obsolete)
Comment 71 Kyle M Hall 2014-04-22 11:23:38 UTC Comment hidden (obsolete)
Comment 72 Katrin Fischer 2014-04-26 15:36:47 UTC
Created attachment 27657 [details] [review]
[PASSED QA] Bug 9593 Prices not imported correctly from a staged file

Initial bug :
When there's a round price with no decimals after it,
or when the symbol is after the digits, the price is not captured
by regular expression in MungeMarcPrice routine and the variable
is not initialized.

Enhancement :
The MungeMarcPrice routine had been widely modified.
It 's still possible to priority pick the active currency but
unlike the previous mechanism that worked only for prices preceded
by the currency sign, it's now valid wherever the symbol is situated.
As symbol you may enter a pure currency sign as well as a string
including it like '$US'. Moreover, an 'isocode' column had been
added in currency table (editable in the pro interface from
Administration/Currencies and exchange rates). So the active
currency can be picked either through its symbol or through its iso
code.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests, especially t/db_dependent/MungeMarcPrice.t
Checked currencies can be added, edited and deleted.
Notes: new ISO code field is mandatory.
       Sample sql files need to be updated (bug 12146)
Comment 73 Katrin Fischer 2014-04-26 15:37:02 UTC
Created attachment 27658 [details] [review]
[PASSED QA] Bug 9593 [QA Followup]

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 74 Katrin Fischer 2014-04-26 15:42:08 UTC
I am still a bit torn about this patch and the new column. I think we need a follow up to clean up the sample files at least. 
Also the new field being mandatory means that you are forced to fill it in on editing now, but I see no way to get around that. Does it really need to be mandatory?
Comment 75 Galen Charlton 2014-04-30 17:56:42 UTC
(In reply to Katrin Fischer from comment #74)
> I am still a bit torn about this patch and the new column. I think we need a
> follow up to clean up the sample files at least. 
> Also the new field being mandatory means that you are forced to fill it in
> on editing now, but I see no way to get around that. Does it really need to
> be mandatory?

I don't think it needs to be mandatory, and I also think that if isocode is null, that it would be reasonable to fall back to checking for the main currency code/name (currency.currency) when parsing the prices.

I'm willing to deal with that as an RM follow-up, but I would appreciate confirmation from the patch submitter that there's no issue with my proposed change.
Comment 76 Galen Charlton 2014-04-30 18:00:25 UTC
(In reply to Kyle M Hall from comment #68)
> I could easily see the
> Currency being "Brazilian real" while the ISO code would be "BRL".

Note that currency.currency is only a varchar(10).
Comment 77 Galen Charlton 2014-05-04 22:24:51 UTC
Pushed to master, along with a follow-up implementing my suggestion to not make the ISO code required and to fall back to the currency code/name if the ISO code is not present.  Thanks, Olivier!
Comment 78 Fridolin SOMERS 2014-10-29 10:44:00 UTC
Patch too complex and big to backport to 3.14.x
Comment 79 Koha Team Lyon 3 2015-09-07 09:39:25 UTC
Sorry to be very late, I missed your question in comment 75.
The isocode had to furnished in currencies table in order the whole thing to work whenever the prices string includes the isocode form.
If you are sure that all your prices will only use the symbol form, it's ok but I doubt it could be always true. So I had made it required when entering data in table currencies.
It could be left optionnal, but at least 2 things would have to be added :
1) Resetting an isocode column in the currencies table : now, you have to click the edit button to be aware of the existence of an isocode parameter, I think this column must appear anyway.
2) Indicating clearly in the legend that the isocode data is required for prices string that include it.

Olivier Crouzet
Comment 80 Katrin Fischer 2015-09-07 09:55:16 UTC
I think it would be better to discuss any changes on a new bug report - as this is already in released versions.