Bug 11471

Summary:	Basket groups display 0 0
Product:	Koha	Reporter:	Mark Tompsett <mtompset>
Component:	Acquisitions	Assignee:	Mark Tompsett <mtompset>
Status:	CLOSED FIXED	QA Contact:	Testopia <testopia>
Severity:	minor
Priority:	P5 - low	CC:	fridolin.somers, gmcharlt, jonathan.druart, kyle, veron
Version:	Main
Hardware:	All
OS:	All
See Also:	http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9654 http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11820
Change sponsored?:	---	Patch complexity:	Small patch
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Attachments:	Bug 11471 - Basket groups display 0 0 Bug 11471 - Basket groups display 0 0 [Signed-off] Bug 11471 - Basket groups display 0 0 [ALT] Bug 11471: Display the currency for baskets in a basketgroup [SIGNED OFF] Bug 11471: Display the currency for baskets in a basketgroup Bug 11471: Display the currency for baskets in a basketgroup

Description Mark Tompsett 2014-01-02 19:02:42 UTC

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9654#c9
I was looking into the function GetBasketsByBasketgroup, and realized that invoiceprice, from aqbooksellers, is not included in the SQL query. Also, invoiceprice should really be invoicingcurrency. But let's deal with the functional problems, before other problems. :)

Comment 1 Mark Tompsett 2014-01-02 22:23:11 UTC Comment hidden (obsolete)

Created attachment 23897 [details] [review]
Bug 11471 -  Basket groups display 0 0

While testing Bug 9654, I clicked "Edit" on the basket group,
and got "0 0" displayed on the "Baskets in this group" text area. It had displayed correctly ("0 USD") when I had clicked "Save".

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 'Test Basket' 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) Click 'Save'
21) Click 'Edit'
22) Notice it displays incorrectly. (e.g. Total: 00)
23) Apply the patch (git bz apply 11471)
24) Refresh the page.
25) Notice it displays less incorrectly. (e.g. Total: 0USD)
    NOTE: The space issue is fixed in Bug 9654.
          This can be applied separately from that bug.
26) Run the Koha QA Tool: (~/qa-test-tools/koha-qa.pl -v 2 -c 1)

Comment 2 Mark Tompsett 2014-01-02 22:47:50 UTC Comment hidden (obsolete)

Created attachment 23898 [details] [review]
Bug 11471 - Basket groups display 0 0

While testing Bug 9654, I clicked "Edit" on the basket group,
and got "0 0" displayed on the "Baskets in this group" text area. It had displayed correctly ("0 USD") when I had clicked "Save".

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 'Test Basket' 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) Drag and drop the entry into the 'Baskets in this group'
     text area.
21) Click 'Save'
22) Click 'Edit'
23) Notice it displays incorrectly. (e.g. Total: 00)
24) Apply the patch (git bz apply 11471)
25) Refresh the page.
26) Notice it displays less incorrectly. (e.g. Total: 0USD)
    NOTE: The space issue is fixed in Bug 9654.
          This can be applied separately from that bug.
27) Run the Koha QA Tool: (~/qa-test-tools/koha-qa.pl -v 2 -c 1)

Comment 3 Marc Véron 2014-02-17 19:50:13 UTC Comment hidden (obsolete)

Created attachment 25352 [details] [review]
[Signed-off] Bug 11471 - Basket groups display 0 0

While testing Bug 9654, I clicked "Edit" on the basket group,
and got "0 0" displayed on the "Baskets in this group" text area. It had displayed correctly ("0 USD") when I had clicked "Save".

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 'Test Basket' 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) Drag and drop the entry into the 'Baskets in this group'
     text area.
21) Click 'Save'
22) Click 'Edit'
23) Notice it displays incorrectly. (e.g. Total: 00)
24) Apply the patch (git bz apply 11471)
25) Refresh the page.
26) Notice it displays less incorrectly. (e.g. Total: 0USD)
    NOTE: The space issue is fixed in Bug 9654.
          This can be applied separately from that bug.
27) Run the Koha QA Tool: (~/qa-test-tools/koha-qa.pl -v 2 -c 1)

Followed test plan 1-26. Patch behaves as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Comment 4 Jonathan Druart 2014-02-18 14:10:55 UTC Comment hidden (obsolete)

Created attachment 25372 [details] [review]
[ALT] Bug 11471: Display the currency for baskets in a basketgroup

On editing a basketgroup, the currency for baskets in a basketgroup is
always '0'.

With this patch, the currency is correctly displayed.

Test plan:
Edit a basket group with baskets and verify the currenty is correct.

Comment 5 Jonathan Druart 2014-02-18 14:13:06 UTC

Hello Marc,
Your patch modifies a core routine without providing UT, so it can't be pushed as it.
However I think my patch fixes the issue by correcting a bug in the code: The second parameter for BasketTotal is $bookseller, not a budget.

Comment 6 Mark Tompsett 2014-02-18 16:10:26 UTC

(In reply to Jonathan Druart from comment #5)
> Your patch modifies a core routine without providing UT, so it can't be
> pushed as [is].

You are correct. I suppose I should fix up a t/db_dependent test for it.

Comment 7 Mark Tompsett 2014-02-18 16:20:49 UTC

(In reply to Jonathan Druart from comment #5)
> However I think my patch fixes the issue by correcting a bug in the code:
> The second parameter for BasketTotal is $bookseller, not a budget.

Having read through the code further, you are correct. My fix fixes it as a side effect. Yours fixes it properly.

Comment 8 Mark Tompsett 2014-02-18 17:04:26 UTC Comment hidden (obsolete)

Created attachment 25388 [details] [review]
[SIGNED OFF] Bug 11471: Display the currency for baskets in a basketgroup

On editing a basketgroup, the currency for baskets in a basketgroup is
always '0'.

With this patch, the currency is correctly displayed.

TEST PLAN
=========
 1) Log into staff client
 2) Acquisitions
 3) Click 'Search' in the 'Manage orders' box
 4) Click '+ New basket' because a vendor name
 5) Type 'Test Basket' 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 the basket'
16) Click 'Yes, close (Y)' without checking attach to a
     basket group
17) Click the 'Basket groups' tab
18) Click '+ New basket group'
19) Notice the listing in the 'Ungrouped baskets'.
20) Drag and drop the entry into the 'Baskets in this group'
     text area
21) Click 'Save'
22) Click 'Edit'
23) Notice it displays incorrectly. (e.g. Total: 0 0)
24) Apply the patch (git bz apply 11471)
25) Refresh the page
26) Notice it displays the currency correctly. (e.g. Total: 0 USD)
    NOTE: If there is a space issue, see Bug 9654.
          This can be applied separately from that bug.
27) Run the Koha QA Tool: (~/qa-test-tools/koha-qa.pl -v 2 -c 1)

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Comment 9 Kyle M Hall (khall) 2014-02-21 13:45:17 UTC

Created attachment 25532 [details] [review]
Bug 11471: Display the currency for baskets in a basketgroup

On editing a basketgroup, the currency for baskets in a basketgroup is
always '0'.

With this patch, the currency is correctly displayed.

TEST PLAN
=========
 1) Log into staff client
 2) Acquisitions
 3) Click 'Search' in the 'Manage orders' box
 4) Click '+ New basket' because a vendor name
 5) Type 'Test Basket' 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 the basket'
16) Click 'Yes, close (Y)' without checking attach to a
     basket group
17) Click the 'Basket groups' tab
18) Click '+ New basket group'
19) Notice the listing in the 'Ungrouped baskets'.
20) Drag and drop the entry into the 'Baskets in this group'
     text area
21) Click 'Save'
22) Click 'Edit'
23) Notice it displays incorrectly. (e.g. Total: 0 0)
24) Apply the patch (git bz apply 11471)
25) Refresh the page
26) Notice it displays the currency correctly. (e.g. Total: 0 USD)
    NOTE: If there is a space issue, see Bug 9654.
          This can be applied separately from that bug.
27) 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: Kyle M Hall <kyle@bywatersolutions.com>

Comment 10 Galen Charlton 2014-02-21 19:32:04 UTC

Comment on attachment 25532 [details] [review]
Bug 11471: Display the currency for baskets in a basketgroup

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

::: acqui/basketgroup.pl
@@ +254,1 @@
>      if(! $booksellerid){

Ordinarily, fetching the vendor record before seeing if a booksellerid was passed would be a mistake.  In this case, however, I don't see any evidence that this script is every called with op eq 'add' but no booksellerid.

Can that whole conditional branch be removed, perhaps?

Comment 11 Galen Charlton 2014-02-21 19:46:04 UTC

Pushed to master.  Thanks, Jonathan!

Comment 12 Galen Charlton 2014-02-21 20:12:20 UTC

(In reply to Galen Charlton from comment #10)
> Ordinarily, fetching the vendor record before seeing if a booksellerid was
> passed would be a mistake.  In this case, however, I don't see any evidence
> that this script is every called with op eq 'add' but no booksellerid.
> 
> Can that whole conditional branch be removed, perhaps?

See bug 11820.

Comment 13 Fridolin Somers 2014-05-21 09:03:52 UTC

Pushed to 3.14.x, will be in 3.14.07

Comment 14 Kyle M Hall (khall) 2014-06-06 13:55:49 UTC

Pushed to 3.12.x will be in 3.12.13