Bug 9654 - Missing space between amount and currency symbol in basket groups
Summary: Missing space between amount and currency symbol in basket groups
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Bugs List
QA Contact:
URL:
Keywords:
Depends on: 9807
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-17 17:58 UTC by Katrin Fischer
Modified: 2014-12-07 20:07 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
screenshot (29.45 KB, image/png)
2013-02-17 17:58 UTC, Katrin Fischer
Details
Proposed bug fix for 9654 (734 bytes, patch)
2013-12-30 01:47 UTC, Samuel Crosby
Details | Diff | Splinter Review
Bug 9654: Missing space between amount and currency symbol in basket group (963 bytes, patch)
2013-12-30 01:56 UTC, Chris Cormack
Details | Diff | Splinter Review
Fixed again (775 bytes, patch)
2013-12-30 04:01 UTC, Samuel Crosby
Details | Diff | Splinter Review
[SIGNED OFF] Bug 9654: Missing space between amount and currency symbol in basket group (1.93 KB, patch)
2013-12-30 04:21 UTC, Mark Tompsett
Details | Diff | Splinter Review
[PASSED QA] Bug 9654: Missing space between amount and currency symbol in basket group (2.02 KB, patch)
2014-01-02 06:56 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 Katrin Fischer 2013-02-17 17:58:28 UTC
Created attachment 15479 [details]
screenshot

To reproduce:

1) Create some orders, don't automatically add them to a basket group
2) Open a new basket group from the basket group page of the vendor
3) Check display of currencies for the orders shown
Comment 1 Samuel Crosby 2013-12-30 01:47:04 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2013-12-30 01:56:18 UTC Comment hidden (obsolete)
Comment 3 Mark Tompsett 2013-12-30 02:26:52 UTC
Working on "I've never used basket groups before" test plan, but this works as advertised. It even passes the qa test tool.
Comment 4 Mark Tompsett 2013-12-30 03:25:07 UTC
However, I did further thinking, and there's a problem. :(
--- BEGIN SAMPLE CODE ---
#!/usr/bin/perl -Tw

my $val = undef;

my $total = 'q';
$total .= " " . ($val // 0);
print "$total\n";

$total = 'q';
$total .= " " . $val // 0;
print "$total\n";
--- END SAMPLE CODE ---

Before the space was added, a precedent issue wasn't as big an issue. I think the patch should be:
    $total .= " " . ($bookseller->{invoiceprice} // 0);
This will avoid an noisy warning, and better reflect the intent of the original code, since I don't think (" " . $val) // 0; is the thing being aimed for.

Here is my revised test plan:
1) Log into staff client
2) Acquisitions
3) Click 'Search' in the 'Manage orders' box.
4) Click '+ New basket' beside a vendor name.
5) Type 'Bug 9654 Test 1' into basket name.
6) Click 'Save'
7) Click 'Add to basket'
8) Click 'From an external source'
9) Type 'Green Eggs and Ham' into the Title text box.
10) Click 'Search'
11) Click 'Order' on any one of the results.
12) Click 'Add Item' in the 'Item' box.
13) Select a Fund from the dropdown in the 'Accounting details' box.
14) Click 'Save'
15) Click 'Close this basket'
16) Click 'Yes, close (Y)' without checking the attach to a basket group.
17) Click the 'Basket groups' tab.
18) Click '+ New basket group'
19) Notice the listing in 'Ungrouped baskets' lacks a space between the number and the currency. (e.g. Total: 0USD)
20) Apply patch (git bz apply 9654)
21) Refresh the page
22) Notice there is now a space. (e.g. Total: 0 USD)
23) Run the Koha QA Tool: (~/qa-test-tools/koha-qa.pl -v 2 -c 1)
Comment 5 Samuel Crosby 2013-12-30 04:01:31 UTC Comment hidden (obsolete)
Comment 6 Mark Tompsett 2013-12-30 04:21:28 UTC Comment hidden (obsolete)
Comment 7 Mark Tompsett 2013-12-30 04:23:22 UTC
Congratulations on your first signed off bug fix. :)
Comment 8 Katrin Fischer 2014-01-02 06:56:39 UTC
Created attachment 23889 [details] [review]
[PASSED QA] Bug 9654: Missing space between amount and currency symbol in basket group

Changed:
    $total .= $bookseller->{invoiceprice} // 0;
Into:
    $total .= " " . ($bookseller->{invoiceprice} // 0);
in order to add a space between the total and currency in
the basket group.

Revised test plan:
 1) Log into staff client
 2) Acquisitions
 3) Click 'Search' in the 'Manage orders' box.
 4) Click '+ New basket' beside a vendor name.
 5) Type 'Bug 9654 Test 1' into basket name.
 6) Click 'Save'
 7) Click 'Add to basket'
 8) Click 'From an external source'
 9) Type 'Green Eggs and Ham' into the Title text box.
10) Click 'Search'
11) Click 'Order' on any one of the results.
12) Click 'Add Item' in the 'Item' box.
13) Select a Fund from the dropdown in the
     'Accounting details' box.
14) Click 'Save'
15) Click 'Close this basket'
16) Click 'Yes, close (Y)' without checking the attach to a
     basket group.
17) Click the 'Basket groups' tab.
18) Click '+ New basket group'
19) Notice the listing in 'Ungrouped baskets' lacks a space
     between the number and the currency. (e.g. Total: 0USD)
20) Apply patch (git bz apply 9654)
21) Refresh the page
22) Notice there is now a space. (e.g. Total: 0 USD)
23) Run the Koha QA Tool: (~/qa-test-tools/koha-qa.pl -v 2 -c 1)

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes all tests.
Comment 9 Katrin Fischer 2014-01-02 07:08:56 UTC
There is still a small problem. When all order lines of an order in the basket group are cancelled, it displays: 0 0. As it was 00 before the patch I think it's still an improvement :)

That said... I think this is Samuel's first patch - welcome! :)
Comment 10 Galen Charlton 2014-01-02 15:47:57 UTC
Pushed to master.  Thanks, Samuel, and congratulations!
Comment 11 Fridolin Somers 2014-01-27 14:03:34 UTC
Depends on Bug 9807 patch :
231f26b Bug 9807: (follow-up) quell warning if invoice price not set