Bug 11243 - Supplier list counts all items, even canceled ones
Summary: Supplier list counts all items, even canceled ones
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-13 13:59 UTC by Jonathan Druart
Modified: 2015-06-04 23:30 UTC (History)
4 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
Bug 11243: Supplier list should distinct canceled items (2.60 KB, patch)
2013-11-13 14:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11243: Supplier list should distinct canceled items (2.69 KB, patch)
2013-11-14 18:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 11243: Display count of items and biblios cancelled (3.26 KB, patch)
2013-11-20 09:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11243: UT: unit tests for GetBasketsInfosByBookseller (4.21 KB, patch)
2013-11-20 09:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11243: Supplier list should distinct canceled items (2.70 KB, patch)
2013-12-29 03:08 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 11243: Display count of items and biblios cancelled (3.32 KB, patch)
2013-12-29 03:08 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 11243: UT: unit tests for GetBasketsInfosByBookseller (4.26 KB, patch)
2013-12-29 03:09 UTC, Chris Cormack
Details | Diff | Splinter Review
0001-PASSED-QA-Bug-11243-Supplier-list-should-distinct-ca.patch (2.91 KB, patch)
2014-04-20 08:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
0002-PASSED-QA-Bug-11243-Display-count-of-items-and-bibli.patch (3.39 KB, patch)
2014-04-20 08:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
0003-PASSED-QA-Bug-11243-UT-unit-tests-for-GetBasketsInfo.patch (4.34 KB, patch)
2014-04-20 08:44 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 Jonathan Druart 2013-11-13 13:59:46 UTC

    
Comment 1 Jonathan Druart 2013-11-13 14:04:48 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2013-11-14 18:59:13 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2013-11-19 22:07:08 UTC
This includes a string change, so it can't go in immediately.

And I think I also found a problem:

With the patch, the items column will display a total, followed by the number of items cancelled. Example:

Item count:
3 (2 cancelled) 
So 2 out of 3 are cancelled.

In this case, it is a basket with 2 order lines. Of those 2 order lines, one with 2 items ordered has been cancelled.

The biblio count is: 1

This makes it a bit confusing. I see 2 options:

Don't count the cancelled items and order lines, then it would be:

Item count: 1
Biblio count: 1

Or we do count the cancelled items and order lines in both columns:

Item count: 3 (2 cancelled)
Bibio count: 1 (1 cancelled)

What do you think?
Comment 4 Jonathan Druart 2013-11-20 09:14:01 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2013-11-20 09:14:06 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2013-11-20 09:16:26 UTC
Katrin, I did not want to add a new sql query for the count of cancelled biblios.
But I agree it is the more consistent to do.

Back to needs signoff
Comment 7 Katrin Fischer 2013-11-20 21:49:04 UTC
Thx Jonathan - I will wait until the follow-ups are signed off
Comment 8 Chris Cormack 2013-12-29 03:08:21 UTC Comment hidden (obsolete)
Comment 9 Chris Cormack 2013-12-29 03:08:41 UTC Comment hidden (obsolete)
Comment 10 Chris Cormack 2013-12-29 03:09:00 UTC Comment hidden (obsolete)
Comment 11 Katrin Fischer 2013-12-29 10:29:11 UTC
Hi Jonathan, I retested this and think I spotted a problem:

Order: 2 orders
Cancel one order, delete bibliographic record
Item cound is correct, but biblio count doesn't show the cancelled biblio.
Comment 12 Jonathan Druart 2014-01-06 17:43:42 UTC
(In reply to Katrin Fischer from comment #11)
> Hi Jonathan, I retested this and think I spotted a problem:
> 
> Order: 2 orders
> Cancel one order, delete bibliographic record
> Item cound is correct, but biblio count doesn't show the cancelled biblio.

Since the aqorders.biblionumber is set to null on deleting a biblio I don't know how to get the correct count.
I could do something like: 
  COUNT(DISTINCT(COALESCE(aqorders.biblionumber, timestamp))) AS total_biblios
in order to get the total number of biblio for a basket. But if a same biblio is ordered in several orders, the value will be wrong.
Comment 13 Jonathan Druart 2014-03-04 15:19:19 UTC
Switch back to Signed off, I need help from QA.
Comment 14 Katrin Fischer 2014-04-20 08:35:53 UTC
I think maybe the problem here is that the biblionumber should not be nulled - I think Mathieu was working on something in that area. I think we can move this to another bug.
Comment 15 Katrin Fischer 2014-04-20 08:42:46 UTC
Bug 10758 looks like a likely candidate to improve the situation here in the future.
Comment 16 Katrin Fischer 2014-04-20 08:44:01 UTC
Created attachment 27318 [details] [review]
0001-PASSED-QA-Bug-11243-Supplier-list-should-distinct-ca.patch
Comment 17 Katrin Fischer 2014-04-20 08:44:27 UTC
Created attachment 27319 [details] [review]
0002-PASSED-QA-Bug-11243-Display-count-of-items-and-bibli.patch
Comment 18 Katrin Fischer 2014-04-20 08:44:44 UTC
Created attachment 27320 [details] [review]
0003-PASSED-QA-Bug-11243-UT-unit-tests-for-GetBasketsInfo.patch
Comment 19 Galen Charlton 2014-04-20 15:21:17 UTC
Pushed to master.  Thanks, Jonathan!
Comment 20 Fridolin Somers 2014-08-13 07:54:31 UTC
Pushed to 3.14.x, will be in 3.14.10