Bug 4078

Summary: Add the ability to customize and display the symbol for a currency
Product: Koha Reporter: Owen Leonard <oleonard>
Component: TemplatesAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Josef Moravec <josef.moravec>
Severity: enhancement    
Priority: P5 - low CC: chris, claire_gravely, J.P.Knight, jonathan.druart, josef.moravec, katrin.fischer, koha.sekjal, nick, paul.poulain, veron
Version: master   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15374
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18002
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16940
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18888
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 12979    
Bug Blocks: 16940, 20352, 20353, 20354, 19188, 19479, 20466, 20625, 20922    
Attachments: Added a sub that places currency symbol. Added to most prices.
Bug 4078 - Display active currency symbol on currency output and input
Bug 4078 - Display active currency symbol on currency output and input
Bug 4078 - Display active currency symbol on currency output and input
Bug 4078 - Display active currency symbol on currency output and input
Bug 4078: DBIC Schema changes
Bug 4078: Update DB entry - add new column currency.p_sep_by_space
Bug 4078: Add the checkbox to the UI
Bug 4078: Pass with_symbol to module
Bug 4078: Add the ability to display and configure the symbol for prices
Bug 4078: Add a new test
Bug 4078: DBIC Schema changes
Bug 4078: Update DB entry - add new column currency.p_sep_by_space
Bug 4078: Add the checkbox to the UI
Bug 4078: Pass with_symbol to module
Bug 4078: Add the ability to display and configure the symbol for prices
Bug 4078: Add a new test
Bug 4078: DBIC Schema changes
Bug 4078: Update DB entry - add new column currency.p_sep_by_space
Bug 4078: Add the checkbox to the UI
Bug 4078: Pass with_symbol to module
Bug 4078: Add the ability to display and configure the symbol for prices
Bug 4078: Add a new test
Bug 4078: Fix test when fr_FR.UTF-8 is installed

Description Chris Cormack 2010-05-21 01:22:54 UTC


---- Reported by oleonard@myacpl.org 2010-01-29 16:12:23 ----

There is now a way to specify a particular currency as "active," making it possible now for the template to display the correct currency symbol on pages where a currency amount is displayed or input.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:22 UTC  ---

This bug was previously known as _bug_ 4078 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=4078

Actual time not defined. Setting to 0.0

Comment 1 Aleksa Vujicic 2012-01-24 01:24:57 UTC Comment hidden (obsolete)
Comment 2 Ian Walls 2012-02-07 15:52:32 UTC
It seems to me that currency formatting is much the same as date formatting, and as such should be handled by a T:T plugin.  Koha passes the raw numeric values to the template, and the template formats the currency as appropriate.
Comment 3 Paul Poulain 2012-02-21 13:31:05 UTC
Hi Aleksa,

The more I look at your patch, the more I think it's the *perfect* candidate for Template::Toolkit plugins.
Something similar to what has been made for displaying dates:
see : http://wiki.koha-community.org/wiki/Coding_Guidelines#Displaying_dates

Look at Koha/Template/Plugin/KohaDates/KohaDates.pm, create a KohaCurrencies.pm that could be (untested)

use Template::Plugin::Filter;
use base qw( Template::Plugin::Filter );

use C4::Dates;

sub filter {
    my ($self,$amount) = @_;
	my $curr_symbol = GetCurrency()->{symbol};
	if ($curr_symbol && $amount) {
		if ($amount =~ /^\-/) {
			$amount =~ s/^\-/\-$curr_symbol/;
		} else {
			$amount = $curr_symbol.$amount;
		}
	}
	return $amount;
}

and just update all template to display currencies:
[% USE KohaCurrencies %]

and
[% totspent | $KohaCurrencies %] 
...

thanks for your feedback on this
1;
Comment 4 Marc Véron 2012-03-06 10:10:01 UTC
I propose to expand this functionality to the formatting of currency values as well.

Up to now, formatting is handled via the preference "CurrencyFormat". User can choose between 360,000.00 (US) and 360 000,00 (FR). The preference is evaluated in a hard coded way in several .pl and .pm files as
acqui-home.pl
aqbudgetperiods.pl
layout2pages.pm
aqplan.pl
aqbudgetperiods.pl
layout3pages.pm
layout2pages.pm
(maybe more)

The code reads:
---Snip---
my $cur_format = C4::Context->preference("CurrencyFormat");
my $num;

if ( $cur_format eq 'US' ) {
    $num = new Number::Format(
        'int_curr_symbol'   => '',
        'mon_thousands_sep' => ',',
        'mon_decimal_point' => '.'
    );
} elsif ( $cur_format eq 'FR' ) {
    $num = new Number::Format(
        'decimal_fill'      => '2',
        'decimal_point'     => ',',
        'int_curr_symbol'   => '',
        'mon_thousands_sep' => ' ',
        'thousands_sep'     => ' ',
        'mon_decimal_point' => ','
    );
}
---Snip---


To make things more configurable I propose to do the following:

1) 
Expand table currency (http://schema.koha-community.org/tables/currency.html) with an additional column "format"

This column could be pre-filled by system_preferences.sql, and it should be editable (/admin/currency.pl). 
Maybe some input validation is necessary to get well formed strings.

2)
Remove preference "CurrencyFormat" (not longer used)

3)
The sub GetCurrency()(located in Budgets.pm) will return the currency format string along with the currency symbol. No chnges needed (Select *....)

Use GetCurrency()->{format} in sub filter and format the number as appropriate.
Comment 5 Katrin Fischer 2012-03-06 10:26:58 UTC
Hi Marc,
I agree with 1) and 2) - not sure I get 3). I would really like to see a new TT plugin doing the formatting on template level. Then we could start stepping through the code and making it show correctly everywhere. 
Do we need to distinguish between active currency and other currency display? I think later is only needed in acquisitions.
Comment 6 Marc Véron 2012-03-06 12:09:05 UTC
Hi Katrin,

I expect that GetCurrency() would be used in a context like proposed by Paul Poluain in Comment 3, i.e. it would be the template that does the formatting.

GetCurency() returns an aray resulting form a select * statement, you simply could use $active_currency->{format} along with the existing $active_currency->{currency}

The same with the result of GetCurrencies()
(but it seems that it is currently only used to get the rates).
Comment 7 Paul Poulain 2012-03-12 09:56:50 UTC
(In reply to comment #4)
> I propose to expand this functionality to the formatting of currency values
> as well.
Marc, when you say that you "propose" does it mean that you'll submit a patch ? Or you propose to go this way (and I agree), and hope someone else will write a patch.

Aleksa = we have no feedback from you !
Comment 8 Chris Cormack 2012-03-12 10:02:11 UTC
Aleksa was a high school student doing some work on the school holidays. He may time and/or the desire to work on this again. But to expect him to leap in and make a template::toolkit plugin is a pretty big ask for a high school kid.
Comment 9 Marc Véron 2012-03-12 11:38:54 UTC
Paul,

If everybody agrees with my proposition I can try to submit a patch within the next weeks.

However I would like to try first with some smaller issue, just to find out how to patch.

Marc
Comment 10 Paul Poulain 2012-03-12 14:37:14 UTC
(In reply to comment #9)
> Paul,
> 
> If everybody agrees with my proposition I can try to submit a patch within
> the next weeks.
I don't understand why we need a column in currencies table. There is only one currency displayed, it's the one that has "active=1".
When you select a different currency when you enter an amount, it's transformed, with the rate, to become a "currency that has active=1".
So I think the CurrencyFormat should be switched to a formatting display, but we shouldn't need a column.

> However I would like to try first with some smaller issue, just to find out
> how to patch.
Just in case:

Marc, I see you're from switzerland. Do you know that next week, BibLibre organize a hackfest in Marseille (France), there will be mbalmer, from Switzerland too, you're welcomed if you want & can join us (
http://drupal.biblibre.com/en/blog/entry/2012-hackfest-in-europe)

The hackfest would be the perfect place to write your first patches !
just drop me a mail or catch me on IRC for more information if you want
Comment 11 Jonathan Druart 2015-02-20 16:04:26 UTC
Should be done on top of bug 12979, see Koha::Number::Price.
Comment 12 Marc Véron 2015-10-10 23:44:31 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2015-10-12 07:13:28 UTC
Note: the symbol should not be displayed by default.
(I have not tested it, but it's a personal note for later).
Comment 14 Marc Véron 2015-12-01 15:06:53 UTC
Still applies.
Removed 12979 as blocking (12979 is pushed to Master).
Comment 15 Jonathan Druart 2015-12-01 15:36:47 UTC
(In reply to Marc Véron from comment #14)
> Still applies.
> Removed 12979 as blocking (12979 is pushed to Master).

I think it's a good thing to keep the dependencies, even when the patches are pushed.
Comment 16 Marc Véron 2015-12-08 19:10:54 UTC
Still applies.
Comment 17 Marc Véron 2015-12-21 08:26:10 UTC Comment hidden (obsolete)
Comment 18 Marc Véron 2015-12-21 08:27:24 UTC Comment hidden (obsolete)
Comment 19 Aleisha Amohia 2015-12-21 20:20:44 UTC
Created attachment 45888 [details] [review]
Bug 4078 - Display active currency symbol on currency output and input

Display active currency using international formatting patterns and active
currency symbol withplugin price.

To test:

- Apply patch
- Make sure that you have defined an active currency in
  Home > Administration > Currency & Exchange rates
- Search for Syspref 'Currency Format'
- Select a formatting pattern from the drop down list
- Go to a page that uses Price formatting and verify
  that currency symbol displays wheere price formatting
  is applied (git grep '| $Price')

(Amended patch to remove example opac-user.tt. To test with
OPAC see bug 15374)

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>
Comment 20 Jonathan Druart 2015-12-23 09:59:45 UTC
Comment on attachment 45888 [details] [review]
Bug 4078 - Display active currency symbol on currency output and input

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

You will have to write tests.

I don't think it's the good approach. What about the other cases?
For instance : "-£127.54" vs "-127,54 €", you will have to add 3 values.
I think that what you are trying to add with this patch should be done at the currency level (currency table), not the syspref.
Actually the syspref should be removed.

::: Koha/Number/Price.pm
@@ +79,4 @@
>  
>  sub _format_params {
>      my ( $self, $params ) = @_;
> +    my $with_symbol = $params->{with_symbol};

You do you remove this default value, it does not look mandatory.

@@ +96,5 @@
> +        $p_cs_precedes = 1 unless ( defined $p_cs_precedes );
> +    }
> +
> +    if ( $currency_format =~ m/(_TRAIL)/ ) {
> +        $p_cs_precedes = 0 unless ( defined $p_cs_precedes );

You are mixing ternary operator in the conditions, it does not make the code easy to read.
Comment 21 Jonathan Druart 2015-12-23 10:00:06 UTC
(In reply to Jonathan Druart from comment #20)
> You do you remove this default value, it does not look mandatory.

s/You/Why
Comment 22 Marc Véron 2015-12-23 12:31:41 UTC
(In reply to Jonathan Druart from comment #20)
> Comment on attachment 45888 [details] [review] [review]

> (...) should be done at the currency level (currency table), not the syspref.
> Actually the syspref should be removed.
> (...)

Hi Jonathan, your approach will be better, for sure. 

I remove assignement from this long outstanding bug and will be happy to sign-off your solution. I simply hope that we soon have the possibility to display currency symbols and more currency formatting than the US and French ones only, e.g. 360'000.00 for Switzerland.
Comment 23 Jonathan Druart 2018-02-01 17:22:05 UTC
I am stealing this bug to add a new column to the currency table.
It will allow to display or not a space between the symbol and the value.

The problem we also have is that the with_symbol flag is not correctly handled in template (not passed from Koha::Template::Plugin::Price to Koha::Format::Price)
Comment 24 Jonathan Druart 2018-02-01 17:43:06 UTC
Created attachment 71118 [details] [review]
Bug 4078: DBIC Schema changes
Comment 25 Jonathan Druart 2018-02-01 17:43:10 UTC
Created attachment 71119 [details] [review]
Bug 4078: Update DB entry - add new column currency.p_sep_by_space
Comment 26 Jonathan Druart 2018-02-01 17:43:15 UTC
Created attachment 71120 [details] [review]
Bug 4078: Add the checkbox to the UI
Comment 27 Jonathan Druart 2018-02-01 17:43:19 UTC
Created attachment 71121 [details] [review]
Bug 4078: Pass with_symbol to module

The template plugin did not pass the with_symbol flag to the module and
so was not taken into account
Comment 28 Jonathan Druart 2018-02-01 17:43:23 UTC
Created attachment 71122 [details] [review]
Bug 4078: Add the ability to display and configure the symbol for prices

The symbol of currencies are not displayed (or not correctly) so far.
This patch set adds the ability to configure the display of the symbol
(with or without a whitespace between the symbol and the price).

Test plan:
1. Execute the update DB entry, go to the currency admin page and tick the
new "Space separation between symbol and value" checkbox
2. Add a fine to a patron and use their credentials to login at the OPAC
3. You should see the "$ 42 due fines and charges" info in the
dashboard
4. Untick the new checkbox to remove the space and reload the OPAC main
page.
=> The space should not be longer displayed ($42)
Comment 29 Jonathan Druart 2018-02-01 17:43:27 UTC
Created attachment 71123 [details] [review]
Bug 4078: Add a new test
Comment 30 Claire Gravely 2018-02-15 07:15:42 UTC
Hi,

I applied the patch, did an updatedatabase, restart_all and emptied cache. I am not seeing the checkbox on the currency admin page or any currency symbols in the OPAC. Am I missing a step still?
Comment 31 Claire Gravely 2018-02-15 07:40:51 UTC
(In reply to Claire Gravely from comment #30)
> Hi,
> 
> I applied the patch, did an updatedatabase, restart_all and emptied cache. I
> am not seeing the checkbox on the currency admin page or any currency
> symbols in the OPAC. Am I missing a step still?

sorry, I do see the currency symbol in the OPAC, just not the checkbox to add a space.
Comment 32 Jonathan Druart 2018-02-15 15:01:35 UTC
It's named "Space separation between symbol and value:" on cgi-bin/koha/admin/currency.pl?op=add_form&currency_code=USD (when you edit a currency).

Tested right now and I confirm it is displayed.
Comment 33 Kyle M Hall 2018-03-02 17:45:59 UTC
Created attachment 72378 [details] [review]
Bug 4078: DBIC Schema changes

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 34 Kyle M Hall 2018-03-02 17:46:08 UTC
Created attachment 72379 [details] [review]
Bug 4078: Update DB entry - add new column currency.p_sep_by_space

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 35 Kyle M Hall 2018-03-02 17:46:11 UTC
Created attachment 72380 [details] [review]
Bug 4078: Add the checkbox to the UI

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 36 Kyle M Hall 2018-03-02 17:46:16 UTC
Created attachment 72381 [details] [review]
Bug 4078: Pass with_symbol to module

The template plugin did not pass the with_symbol flag to the module and
so was not taken into account

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 37 Kyle M Hall 2018-03-02 17:46:19 UTC
Created attachment 72382 [details] [review]
Bug 4078: Add the ability to display and configure the symbol for prices

The symbol of currencies are not displayed (or not correctly) so far.
This patch set adds the ability to configure the display of the symbol
(with or without a whitespace between the symbol and the price).

Test plan:
1. Execute the update DB entry, go to the currency admin page and tick the
new "Space separation between symbol and value" checkbox
2. Add a fine to a patron and use their credentials to login at the OPAC
3. You should see the "$ 42 due fines and charges" info in the
dashboard
4. Untick the new checkbox to remove the space and reload the OPAC main
page.
=> The space should not be longer displayed ($42)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 38 Kyle M Hall 2018-03-02 17:46:23 UTC
Created attachment 72383 [details] [review]
Bug 4078: Add a new test

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 39 Josef Moravec 2018-03-07 16:34:26 UTC
Created attachment 72511 [details] [review]
Bug 4078: DBIC Schema changes

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 40 Josef Moravec 2018-03-07 16:34:31 UTC
Created attachment 72512 [details] [review]
Bug 4078: Update DB entry - add new column currency.p_sep_by_space

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 41 Josef Moravec 2018-03-07 16:34:35 UTC
Created attachment 72513 [details] [review]
Bug 4078: Add the checkbox to the UI

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 42 Josef Moravec 2018-03-07 16:34:39 UTC
Created attachment 72514 [details] [review]
Bug 4078: Pass with_symbol to module

The template plugin did not pass the with_symbol flag to the module and
so was not taken into account

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 43 Josef Moravec 2018-03-07 16:34:43 UTC
Created attachment 72515 [details] [review]
Bug 4078: Add the ability to display and configure the symbol for prices

The symbol of currencies are not displayed (or not correctly) so far.
This patch set adds the ability to configure the display of the symbol
(with or without a whitespace between the symbol and the price).

Test plan:
1. Execute the update DB entry, go to the currency admin page and tick the
new "Space separation between symbol and value" checkbox
2. Add a fine to a patron and use their credentials to login at the OPAC
3. You should see the "$ 42 due fines and charges" info in the
dashboard
4. Untick the new checkbox to remove the space and reload the OPAC main
page.
=> The space should not be longer displayed ($42)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 44 Josef Moravec 2018-03-07 16:34:47 UTC
Created attachment 72516 [details] [review]
Bug 4078: Add a new test

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 45 Josef Moravec 2018-03-07 16:36:02 UTC
Passed QA. This area really need more work, but it is out of the scope of this bug report I think.
Comment 46 Jonathan Druart 2018-03-23 15:36:58 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 47 Jonathan Druart 2018-03-23 16:14:21 UTC
Created attachment 73196 [details] [review]
Bug 4078: Fix test when fr_FR.UTF-8 is installed
Comment 48 Jonathan Druart 2018-03-23 16:24:54 UTC
(In reply to Jonathan Druart from comment #47)
> Created attachment 73196 [details] [review] [review]
> Bug 4078: Fix test when fr_FR.UTF-8 is installed

Pushed to master.
Comment 49 Nick Clemens 2018-03-26 12:00:27 UTC
Enhancement, not backported for 17.11