Bug 7129 - actual cost showing too many zeros
Summary: actual cost showing too many zeros
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low major (vote)
Assignee: Christophe Croullebois
QA Contact: Bugs List
URL: /cgi-bin/koha/acqui/orderreceive.pl?o...
Keywords:
Depends on: 9923
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-01 19:34 UTC by Nicole C. Engard
Modified: 2013-12-05 20:04 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
actual cost on receipt page (22.01 KB, image/png)
2011-11-01 19:34 UTC, Nicole C. Engard
Details
Added a sprintf to format the actual cost. (1.14 KB, patch)
2012-01-24 00:38 UTC, Aleksa Vujicic
Details | Diff | Splinter Review
Bug 7129: Added a sprintf to format the actual cost. (1.20 KB, patch)
2012-06-09 14:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7129: Added a sprintf to format the actual cost. (1.26 KB, patch)
2012-06-11 14:28 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 7129: Added a sprintf to format the actual cost. (1.40 KB, patch)
2012-06-15 15:15 UTC, Ian Walls
Details | Diff | Splinter Review
Bug 7129 [3.8.x] actual cost showing too many zeros (1.22 KB, patch)
2012-11-28 15:05 UTC, Owen Leonard
Details | Diff | Splinter Review
transitional patch (1.57 KB, patch)
2013-02-15 10:36 UTC, Christophe Croullebois
Details | Diff | Splinter Review
[SIGNED-OFF] transitional patch (1.64 KB, patch)
2013-03-08 20:45 UTC, mathieu saby
Details | Diff | Splinter Review
(MT #11060) correction of a bug due to bz 7129 (1.59 KB, patch)
2013-03-18 17:48 UTC, Paul Poulain
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2011-11-01 19:34:15 UTC Comment hidden (obsolete)
Comment 1 Aleksa Vujicic 2012-01-24 00:38:49 UTC Comment hidden (obsolete)
Comment 2 Ian Walls 2012-02-07 20:22:16 UTC
Simple change to add string formatting to numeric values.  Marking Signed off and Passed QA in one go.
Comment 3 Paul Poulain 2012-02-10 14:40:02 UTC
patch pushed, but this kind of display is a good candidate for : http://wiki.koha-community.org/wiki/Generalise_the_use_Template_Toolkit_plugins !
Comment 4 Jared Camins-Esakov 2012-05-23 23:11:46 UTC
Resetting to Passed QA (by way of ASSIGNED, Needs Signoff, and Signed Off), since this patch does not appear to have been pushed.
Comment 5 Paul Poulain 2012-05-24 16:00:35 UTC
This patch does not apply anymore (since bug 7175)
Comment 6 Paul Poulain 2012-05-24 16:00:52 UTC
oups, mistake in the status...
Comment 7 Katrin Fischer 2012-06-09 14:27:14 UTC Comment hidden (obsolete)
Comment 8 Mirko Tietgen 2012-06-11 14:05:08 UTC
Tried to test this patch in a sandbox but could not put any items to the basket because a mandatory field was set to readonly and could not be edited. Should have nothing to do with this patch but can't signoff because can't test it.
Comment 9 Katrin Fischer 2012-06-11 14:07:52 UTC
Hi Mirko,

try clicking the "Add link" at the end of the item form. Quantity will go up if you do that.
Comment 10 Mirko Tietgen 2012-06-11 14:15:56 UTC
My bad, it works now and fixes the problem, thanks Katrin.
Comment 11 Kyle M Hall 2012-06-11 14:28:30 UTC Comment hidden (obsolete)
Comment 12 Ian Walls 2012-06-15 15:15:47 UTC Comment hidden (obsolete)
Comment 13 Ian Walls 2012-06-15 15:16:37 UTC
Same work that passed QA the first time, just updated to reflect new variable names.
Comment 14 Paul Poulain 2012-06-20 08:12:39 UTC
Patch pushed, but, in the long term, I'd love to see a "currency display" T::T plugin (I would be surprised if it does not exist already, as it's a very common need)
Comment 15 Chris Cormack 2012-06-20 09:33:53 UTC
Does not apply for 3.8.x please reformat for 3.8.x
Comment 16 Owen Leonard 2012-11-28 15:05:00 UTC Comment hidden (obsolete)
Comment 17 Colin Campbell 2012-11-28 16:05:08 UTC
Heres an approach using TT to format money amounts. At the top of the template declare
[% USE money=format('%.2f') -%]
then in the body of the template when you want to display e.g. price use
[% money(price) %]

It takes the display logic out of the the cgi scripts ( a good thing) and you've got an easier to maintain template
Comment 18 Owen Leonard 2012-11-28 16:18:17 UTC
There are many existing examples in master of doing it this way:

[% total | format('%.2f') %]

...which is a little more verbose if you have a lot of instances. It might be something we should have a guideline for.

Since the patch I just submitted was a reworking for 3.8.x, I didn't want to re-do it in a different way. I can resubmit if the TT version is preferable.
Comment 19 Katrin Fischer 2012-11-28 22:03:09 UTC
I think it would be awesome if we had a plugin doing that, because it would allow us to extend the formatting options for different currencies later on.
Comment 20 Chris Cormack 2012-11-29 03:11:35 UTC
Yeah a custom filter plugin would be good, since quite a few currencies format differently to 2dp, the plugin could do the right thing following the syspref, like KohaDates does.
Comment 21 Chris Cormack 2012-12-08 18:51:24 UTC
Pushed to 3.8.x will be in 3.8.8
Comment 22 Christophe Croullebois 2013-02-15 10:31:26 UTC
We must revert this patch, it introduces a bug if the unitprice is 0.0000.
Instead of showing in this case the 'ecost' if there is not 'unitprice', it shows 0.00 and the 'Actual cost' must be manually entered.
The line :
if ( @$results[0]->{'unitprice'} == 0 ) {
        @$results[0]->{'unitprice'} = '';
was wrote in this perspective.
But sprintf ( "%.2f", with '' or 0 or any string will return 0.00
and then, in the .tt 'unitprice' exists so we have the bad result.
I have made a quick patch to just resolve this problem, but I think that the Colin's idea is nice, I will create a new patch to work in this way in the .tt page.
I join the quick patch, but I think it would be better to make one with the Colin's idea.
I am waiting for advises.
Comment 23 Christophe Croullebois 2013-02-15 10:36:07 UTC Comment hidden (obsolete)
Comment 24 mathieu saby 2013-03-06 17:06:43 UTC
Hello
We have just discovered the problem, and I agree with Christophe Croullebois, it must be solved. I will test transitional patch tomorrow.

M. Saby
Rennes 2 university
Comment 25 mathieu saby 2013-03-08 20:43:35 UTC
Your corrective patch works : 
- before your patch :
If unitprice = 0 in database, in receipt page, the value of "Actual cost" is 0

- before your patch :
If unitprice = 0 in database, in receipt page, the value of "Actual cost" is ecost

I sign it off.

M. Saby
Comment 26 mathieu saby 2013-03-08 20:45:20 UTC Comment hidden (obsolete)
Comment 27 Paul Poulain 2013-03-18 17:48:08 UTC
Created attachment 16300 [details] [review]
(MT #11060) correction of a bug due to bz 7129

See comments in the patch

Signed-off-by: Mathieu Saby <mathieu.saby@univ-rennes2.fr>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Comment 28 Paul Poulain 2013-03-18 17:48:54 UTC
QA comment:
 * passes koha-qa.pl
 * small patch, I made some changes in the comment, for a better readability

passed QA
Comment 29 Jared Camins-Esakov 2013-03-20 02:03:07 UTC
What is this patch? If this is fixing a bug caused by 7129 it should be moved to a new bug, which can be set straight to "Passed QA."
Comment 30 Jared Camins-Esakov 2013-03-20 19:17:25 UTC
Setting to In Discussion for review and probable patch movement by the author.
Comment 31 Christophe Croullebois 2013-03-25 16:01:32 UTC
Ok Jared,
I have created bz 9923 in "passed QA"
Thx
Comment 32 Paul Poulain 2013-03-25 18:40:02 UTC
follow-up in bug 9923