In acqui/parcel.pl both the "Pending orders" and "Already received" tables show how many holds there are for the given record. However, the count of holds in the "Pending orders" table confuses librarians because it only lists holds for the particular items in the orderline. Due to that, the holds column may show 0 holds even if there are a dozen record level holds for that bib! This is not what librarians seem to expect, instead it seems that the same total holds in the "Pending orders" table would be preferred.
Created attachment 69799 [details] [review] Bug 19812 - Holds count in "Already received" table has confusing and unexpected values In acqui/parcel.pl both the "Pending orders" and "Already received" tables show how many holds there are for the given record. However, the count of holds in the "Pending orders" table confuses librarians because it only lists holds for the particular items in the orderline. Due to that, the holds column may show 0 holds even if there are a dozen record level holds for that bib! This is not what librarians seem to expect, instead it seems that the same total holds in the "Pending orders" table would be preferred. Test Plan: 1) Find an invoice with an item in the "Already received" table 2) Add one or more record level holds to the record 3) Note the holds column does not count those holds 4) Apply this patch 5) Note the holds column now shows total holds and holds for just those ordered items
*** Bug 19741 has been marked as a duplicate of this bug. ***
Created attachment 69829 [details] [review] Bug 19812 - Holds count in "Already received" table has confusing and unexpected values In acqui/parcel.pl both the "Pending orders" and "Already received" tables show how many holds there are for the given record. However, the count of holds in the "Pending orders" table confuses librarians because it only lists holds for the particular items in the orderline. Due to that, the holds column may show 0 holds even if there are a dozen record level holds for that bib! This is not what librarians seem to expect, instead it seems that the same total holds in the "Pending orders" table would be preferred. Test Plan: 1) Find an invoice with an item in the "Already received" table 2) Add one or more record level holds to the record 3) Note the holds column does not count those holds 4) Apply this patch 5) Note the holds column now shows total holds and holds for just those ordered items Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Nancy Keener <nkeener@washoecounty.us>
*** Bug 19846 has been marked as a duplicate of this bug. ***
Created attachment 70115 [details] [review] Bug 19812 - Holds count in "Already received" table has confusing and unexpected values In acqui/parcel.pl both the "Pending orders" and "Already received" tables show how many holds there are for the given record. However, the count of holds in the "Pending orders" table confuses librarians because it only lists holds for the particular items in the orderline. Due to that, the holds column may show 0 holds even if there are a dozen record level holds for that bib! This is not what librarians seem to expect, instead it seems that the same total holds in the "Pending orders" table would be preferred. Test Plan: 1) Find an invoice with an item in the "Already received" table 2) Add one or more record level holds to the record 3) Note the holds column does not count those holds 4) Apply this patch 5) Note the holds column now shows total holds and holds for just those ordered items Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Nancy Keener <nkeener@washoecounty.us> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
If there are no holds, should not we display "0" instead of "0 / 0"?
"Holds count in "Already received" table has confusing and unexpected values" Does not describe what the patch does.
There is an access to the removed variable "holds" in the template loop_order.holds That means we now see "Can't delete catalog record, see constraints below", but without anything below...
(In reply to Jonathan Druart from comment #8) > There is an access to the removed variable "holds" in the template > loop_order.holds > > That means we now see "Can't delete catalog record, see constraints below", > but without anything below... The code you are referencing is in loop_orders, but this patch only alters loop_received which does not have that message. I'm not sure how this patch could be causing that. If you can give me some line numbers I'd be happy to take another look!
(In reply to Kyle M Hall from comment #9) > (In reply to Jonathan Druart from comment #8) > > There is an access to the removed variable "holds" in the template > > loop_order.holds > > > > That means we now see "Can't delete catalog record, see constraints below", > > but without anything below... > > The code you are referencing is in loop_orders, but this patch only alters > loop_received which does not have that message. I'm not sure how this patch > could be causing that. If you can give me some line numbers I'd be happy to > take another look! Indeed, sounds like I mixed the 2 tables reading the code. Something else: 1. What about comment 6? 2. "1 / 0" reads wrong, but I have no idea how it could be better. 3. line 141 $line{holds} = 0; should be removed, right?
I've tested it and I think it's still a little confusing. Right now it is: total of holds / holds on on order items Which can lead to the 1 / 0 that Jonathan pointed out. Maybe it would make more sense to switch the values? Or just show the total number of holds = record level + item level on on order items?
(In reply to Katrin Fischer from comment #11) > I've tested it and I think it's still a little confusing. > > Right now it is: total of holds / holds on on order items > > Which can lead to the 1 / 0 that Jonathan pointed out. Maybe it would make > more sense to switch the values? Or just show the total number of holds = > record level + item level on on order items? I like the idea of switching the values around. I'll get a followup done asap!
Created attachment 73606 [details] [review] Bug 19812 [QA Followup] - Swap sides for total and item holds
Created attachment 73607 [details] [review] Bug 19812 [QA Followup] - If 0 holds show '0' not '0 / 0'
Created attachment 73608 [details] [review] Bug 19812 [QA Followup] - Remove unnecessary line
Patches will be squashed for readability.
Pushed to master for 18.05, thanks to everybody involved!
Pushed to stable for 17.11.06 Awesome work all!
I prefer not to push to 17.05.x, its minor