Bug 37511 - Add option to place the currency symbol before or after the amount
Summary: Add option to place the currency symbol before or after the amount
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Janusz Kaczmarek
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
: 20352 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-07-29 11:47 UTC by Janusz Kaczmarek
Modified: 2024-11-22 21:43 UTC (History)
4 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Adds the option in currencies to define whether the currency symbol should appear before or after the amount in displays.
Version(s) released in:
24.11.00
Circulation function:


Attachments
Bug 37511: DB changes (2.18 KB, patch)
2024-10-01 13:35 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 37511: DBIC schema changes for bug 37511 (1.38 KB, patch)
2024-10-01 13:35 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 37511: Location of the currency symbol (before or *after*) the amount (4.71 KB, patch)
2024-10-01 13:35 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 37511: Unit tests (1.61 KB, patch)
2024-10-01 13:35 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 37511: DB changes (2.23 KB, patch)
2024-10-01 19:24 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 37511: DBIC schema changes for bug 37511 (1.44 KB, patch)
2024-10-01 19:24 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 37511: Location of the currency symbol (before or *after*) the amount (4.76 KB, patch)
2024-10-01 19:24 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 37511: Unit tests (1.66 KB, patch)
2024-10-01 19:24 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 37511: DB changes (2.29 KB, patch)
2024-10-28 15:10 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37511: DBIC schema changes for bug 37511 (1.50 KB, patch)
2024-10-28 15:10 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37511: Location of the currency symbol (before or *after*) the amount (4.83 KB, patch)
2024-10-28 15:10 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37511: Unit tests (1.73 KB, patch)
2024-10-28 15:10 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37511: (QA follow-up) Add boolean hint (897 bytes, patch)
2024-10-28 15:10 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Janusz Kaczmarek 2024-07-29 11:47:08 UTC
It is customary in non-English-speaking European countries to write the currency symbol after the amount and separated from it by a space (e.g. 123 €, 54 zł, 38 Kč).  This custom should be taken into account in Koha::Number::Price::_format_params (by adding the p_cs_precedes parameter). 

This parameter could either be directly associated with the use of spaces (p_sep_by_space), or given as a separate data when configuring the currency.
Comment 1 Janusz Kaczmarek 2024-10-01 13:35:30 UTC
Created attachment 172264 [details] [review]
Bug 37511: DB changes
Comment 2 Janusz Kaczmarek 2024-10-01 13:35:32 UTC
Created attachment 172265 [details] [review]
Bug 37511: DBIC schema changes for bug 37511
Comment 3 Janusz Kaczmarek 2024-10-01 13:35:34 UTC
Created attachment 172266 [details] [review]
Bug 37511: Location of the currency symbol (before or *after*) the amount

It is customary in non-English-speaking European countries to write the
currency symbol after the amount and separated from it by a space (e.g.
123 €, 54 zł, 38 Kč).  This custom should be taken into account
in Koha::Number::Price::_format_params (by proper use the p_cs_precedes
parameter).

The correct display of the currency is particularly important when using
OPACShowSavings system preference.

Test plan:
==========
1. Enable OPACShowSavings, have some items with defined 'v - Cost,
   replacement price'.  Check out the items to a patron.
2. Log in to OPAC as the patron.  See the amount of 'total savings' on
   the main page.  Note that the currency symbol precedes the amount.
   There is no way to configure the position of the currency symbol.
3. Apply the patch ; updatedatabase.pl ; restart_all.
4. Go to  Administration > Currencies and exchange rates.  Modify the
   active currency unchecking the 'Currency symbol precedes value'
   checkbox.
5. Refresh the OPAC page.  See that the position of the currency symbol
   has been changed.

Sponsored-by: Ignatianum University in Cracow
Comment 4 Janusz Kaczmarek 2024-10-01 13:35:36 UTC
Created attachment 172267 [details] [review]
Bug 37511: Unit tests
Comment 5 Roman Dolny 2024-10-01 19:24:24 UTC
Created attachment 172286 [details] [review]
Bug 37511: DB changes

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 6 Roman Dolny 2024-10-01 19:24:26 UTC
Created attachment 172287 [details] [review]
Bug 37511: DBIC schema changes for bug 37511

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 7 Roman Dolny 2024-10-01 19:24:28 UTC
Created attachment 172288 [details] [review]
Bug 37511: Location of the currency symbol (before or *after*) the amount

It is customary in non-English-speaking European countries to write the
currency symbol after the amount and separated from it by a space (e.g.
123 €, 54 zł, 38 Kč).  This custom should be taken into account
in Koha::Number::Price::_format_params (by proper use the p_cs_precedes
parameter).

The correct display of the currency is particularly important when using
OPACShowSavings system preference.

Test plan:
==========
1. Enable OPACShowSavings, have some items with defined 'v - Cost,
   replacement price'.  Check out the items to a patron.
2. Log in to OPAC as the patron.  See the amount of 'total savings' on
   the main page.  Note that the currency symbol precedes the amount.
   There is no way to configure the position of the currency symbol.
3. Apply the patch ; updatedatabase.pl ; restart_all.
4. Go to  Administration > Currencies and exchange rates.  Modify the
   active currency unchecking the 'Currency symbol precedes value'
   checkbox.
5. Refresh the OPAC page.  See that the position of the currency symbol
   has been changed.

Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 8 Roman Dolny 2024-10-01 19:24:31 UTC
Created attachment 172289 [details] [review]
Bug 37511: Unit tests

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 9 Martin Renvoize (ashimema) 2024-10-28 15:10:30 UTC
Created attachment 173551 [details] [review]
Bug 37511: DB changes

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize (ashimema) 2024-10-28 15:10:32 UTC
Created attachment 173552 [details] [review]
Bug 37511: DBIC schema changes for bug 37511

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize (ashimema) 2024-10-28 15:10:34 UTC
Created attachment 173553 [details] [review]
Bug 37511: Location of the currency symbol (before or *after*) the amount

It is customary in non-English-speaking European countries to write the
currency symbol after the amount and separated from it by a space (e.g.
123 €, 54 zł, 38 Kč).  This custom should be taken into account
in Koha::Number::Price::_format_params (by proper use the p_cs_precedes
parameter).

The correct display of the currency is particularly important when using
OPACShowSavings system preference.

Test plan:
==========
1. Enable OPACShowSavings, have some items with defined 'v - Cost,
   replacement price'.  Check out the items to a patron.
2. Log in to OPAC as the patron.  See the amount of 'total savings' on
   the main page.  Note that the currency symbol precedes the amount.
   There is no way to configure the position of the currency symbol.
3. Apply the patch ; updatedatabase.pl ; restart_all.
4. Go to  Administration > Currencies and exchange rates.  Modify the
   active currency unchecking the 'Currency symbol precedes value'
   checkbox.
5. Refresh the OPAC page.  See that the position of the currency symbol
   has been changed.

Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize (ashimema) 2024-10-28 15:10:37 UTC
Created attachment 173554 [details] [review]
Bug 37511: Unit tests

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize (ashimema) 2024-10-28 15:10:39 UTC
Created attachment 173555 [details] [review]
Bug 37511: (QA follow-up) Add boolean hint

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Katrin Fischer 2024-10-30 08:13:57 UTC
1) Database update

Please use FIRST/AFTER to make sure the column ends in the same spot as defined by kohastructure.sql. Fixed on push.
Comment 15 Katrin Fischer 2024-10-30 09:57:46 UTC
*** Bug 20352 has been marked as a duplicate of this bug. ***
Comment 16 Katrin Fischer 2024-10-30 10:59:54 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 17 Katrin Fischer 2024-10-30 17:24:25 UTC
Something here is not quite right, please check:

I was trying to clean up the DBIC schema update I missed and I get this:

   data_type: 'tinyint'
   default_value: 1
-  is_nullable: 0
+  is_nullable: 1
 
 =cut
 
@@ -103,7 +103,7 @@ __PACKAGE__->add_columns(
   "p_sep_by_space",
   { data_type => "tinyint", default_value => 0, is_nullable => 1 },
   "p_cs_precedes",
-  { data_type => "tinyint", default_value => 1, is_nullable => 0 },
+  { data_type => "tinyint", default_value => 1, is_nullable => 1 },


The second patch here set is_nullable: 0, now the dbic run after running the database update sets is_nullable: 1. Something is not in sync.
Comment 18 Jonathan Druart 2024-11-07 09:26:02 UTC
We should not try catch without rethrow. Please adjust the db rev: Simply remove the try-catch block.

See bug 37895 for more details.
Comment 19 Jonathan Druart 2024-11-07 14:32:56 UTC
(In reply to Jonathan Druart from comment #18)
> We should not try catch without rethrow. Please adjust the db rev: Simply
> remove the try-catch block.
> 
> See bug 37895 for more details.

See bug 38394.
Comment 20 Jonathan Druart 2024-11-07 14:33:16 UTC
(In reply to Katrin Fischer from comment #17)
> Something here is not quite right, please check:
> 
> I was trying to clean up the DBIC schema update I missed and I get this:
> 
>    data_type: 'tinyint'
>    default_value: 1
> -  is_nullable: 0
> +  is_nullable: 1
>  
>  =cut
>  
> @@ -103,7 +103,7 @@ __PACKAGE__->add_columns(
>    "p_sep_by_space",
>    { data_type => "tinyint", default_value => 0, is_nullable => 1 },
>    "p_cs_precedes",
> -  { data_type => "tinyint", default_value => 1, is_nullable => 0 },
> +  { data_type => "tinyint", default_value => 1, is_nullable => 1 },
> 
> 
> The second patch here set is_nullable: 0, now the dbic run after running the
> database update sets is_nullable: 1. Something is not in sync.

ping?
Comment 21 Katrin Fischer 2024-11-07 15:43:35 UTC
Janusz or Martin please?
Comment 22 Janusz Kaczmarek 2024-11-07 22:33:56 UTC
(In reply to Jonathan Druart from comment #19)
> (In reply to Jonathan Druart from comment #18)
> > We should not try catch without rethrow. Please adjust the db rev: Simply
> > remove the try-catch block.
> > 
> > See bug 37895 for more details.
> 
> See bug 38394.

I'm sorry but I got a little bit lost.  The whole patch (with its db rev. = 240600053.pl) is already in main since 30th October.  If I trace it correctly you have corrected the aforesaid db rev. (by removing try/catch) with Bug 38394.  Is there anything that I could/should do?
Comment 23 Katrin Fischer 2024-11-08 07:47:17 UTC
Hi Janusz, 

to explain a bit more:

In your DBIC schema changes file, that I dropped when pushing (as we usually do), you have is_nullable = 0 (second patch)
 
But when I regenerated the DBIC file running dbic after the database update, it came out with is_nullable = 1

So: which one is correct?
If we want is_nullable = 0, we will probably need another database update and kohastructure.sql change.
Comment 24 Katrin Fischer 2024-11-08 08:12:41 UTC
(In reply to Janusz Kaczmarek from comment #22)
> (In reply to Jonathan Druart from comment #19)
> > (In reply to Jonathan Druart from comment #18)
> > > We should not try catch without rethrow. Please adjust the db rev: Simply
> > > remove the try-catch block.
> > > 
> > > See bug 37895 for more details.
> > 
> > See bug 38394.
> 
> I'm sorry but I got a little bit lost.  The whole patch (with its db rev. =
> 240600053.pl) is already in main since 30th October.  If I trace it
> correctly you have corrected the aforesaid db rev. (by removing try/catch)
> with Bug 38394.  Is there anything that I could/should do?

You don't need to fix the dbrev for the try/catch yourself - we took care of it yesterday already.
Comment 25 Janusz Kaczmarek 2024-11-08 21:14:25 UTC
(In reply to Katrin Fischer from comment #23)
> Hi Janusz, 
> 
> to explain a bit more:
> 
> In your DBIC schema changes file, that I dropped when pushing (as we usually
> do), you have is_nullable = 0 (second patch)
>  
> But when I regenerated the DBIC file running dbic after the database update,
> it came out with is_nullable = 1
> 
> So: which one is correct?
> If we want is_nullable = 0, we will probably need another database update
> and kohastructure.sql change.

Thank you for your explanation, Katrin.  I have tried to retrace the process of preparing of my original schema changes and this time I got is_nullable = 1.  No idea why in the original patch it was = 0...  And I am pretty sure I did not correct the schema file by hand.  As it is now (is_nullable = 1) seems totally correct and coherent with the table description in mysql.

I am sorry for the confusion.  And thank you for your patience.
Comment 26 Katrin Fischer 2024-11-11 17:15:47 UTC
Questions resolved, removing keyword :)
Comment 27 Lucas Gass (lukeg) 2024-11-22 21:43:21 UTC
Enhancement, no 24.05.x backport.