Created attachment 22906 [details] [review] Bug 11243: Supplier list should distinct canceled items On the supplier result list, the "Item count" columns contain the sum of all items ordered for a basket. But if an order is canceled, the item count is not really meaningful. This patch just adds, in parenthesis, the number of items canceled. Test plan: - create a basket and 3 orders with different number of items - cancel 1 order - verify on the supplier list that the number of items is correct and the number of canceled items is correct too.
Created attachment 22946 [details] [review] [SIGNED-OFF] Bug 11243: Supplier list should distinct canceled items On the supplier result list, the "Item count" columns contain the sum of all items ordered for a basket. But if an order is canceled, the item count is not really meaningful. This patch just adds, in parenthesis, the number of items canceled. Test plan: - create a basket and 3 orders with different number of items - cancel 1 order - verify on the supplier list that the number of items is correct and the number of canceled items is correct too. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
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?
Created attachment 23050 [details] [review] Bug 11243: Display count of items and biblios cancelled Same as previous patch but for the biblios count.
Created attachment 23051 [details] [review] Bug 11243: UT: unit tests for GetBasketsInfosByBookseller Verify prove t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t returns green.
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
Thx Jonathan - I will wait until the follow-ups are signed off
Created attachment 23844 [details] [review] Bug 11243: Supplier list should distinct canceled items On the supplier result list, the "Item count" columns contain the sum of all items ordered for a basket. But if an order is canceled, the item count is not really meaningful. This patch just adds, in parenthesis, the number of items canceled. Test plan: - create a basket and 3 orders with different number of items - cancel 1 order - verify on the supplier list that the number of items is correct and the number of canceled items is correct too. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 23845 [details] [review] Bug 11243: Display count of items and biblios cancelled Same as previous patch but for the biblios count. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 23846 [details] [review] Bug 11243: UT: unit tests for GetBasketsInfosByBookseller Verify prove t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t returns green. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
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.
(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.
Switch back to Signed off, I need help from QA.
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.
Bug 10758 looks like a likely candidate to improve the situation here in the future.
Created attachment 27318 [details] [review] 0001-PASSED-QA-Bug-11243-Supplier-list-should-distinct-ca.patch
Created attachment 27319 [details] [review] 0002-PASSED-QA-Bug-11243-Display-count-of-items-and-bibli.patch
Created attachment 27320 [details] [review] 0003-PASSED-QA-Bug-11243-UT-unit-tests-for-GetBasketsInfo.patch
Pushed to master. Thanks, Jonathan!
Pushed to 3.14.x, will be in 3.14.10