Bug 19288 - Holds placed on a specific item after a next available hold will show varied results
Summary: Holds placed on a specific item after a next available hold will show varied ...
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 20948
Blocks:
  Show dependency treegraph
 
Reported: 2017-09-11 15:07 UTC by Kelly McElligott
Modified: 2020-04-21 10:28 UTC (History)
13 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.05, 19.05.10


Attachments
Bug 19288: Only display holds if next in queue or waiting (1.52 KB, patch)
2018-06-15 13:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19288: [Alternate] Only show existence of item level hold unless waiting (2.57 KB, patch)
2018-06-15 13:38 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19288: [Alternate] Only show existence of item level hold unless waiting (2.62 KB, patch)
2019-04-30 00:42 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 19288: Don't show patron info for item level holds on details.pl unless hold is waiting (3.01 KB, patch)
2019-12-02 11:48 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19288: Don't show patron info for item level holds on details.pl unless hold is waiting (3.15 KB, patch)
2020-03-09 09:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19288: Don't show patron info for item level holds on details.pl unless hold is waiting (3.22 KB, patch)
2020-03-12 13:26 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19288: Don't show patron info for item level holds on details.pl unless hold is waiting (3.39 KB, patch)
2020-03-12 13:27 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kelly McElligott 2017-09-11 15:07:35 UTC
When a hold is placed on an item as "next available" and then a hold is placed on same item, but it is placed on a specific copy, Koha will show the specific copy hold on the status screen of the item incorrectly and sometimes give the hold to that specific copy before next available. 

On 16.11.07
Steps:  
1. Place a "next available" hold on an item with only one copy.  
2. Place a "specific item" hold this same record.
3. Looking at the holds- they are in the correct order
4. Look at the status of the item in the holdings table  you can see that the hold for the "specific copy" is next- not the next available. 
5. Check this item in and see which hold gets filled first.
6. Results vary on the process of who gets the hold when checking in this item .  Sometimes the specific item will get filled before the hold that was placed as next available even though the "Next available" hold was placed first.
Comment 1 Katrin Fischer 2017-10-22 09:55:11 UTC
Could settings like preferring local holds play into that?
Comment 2 Kelly McElligott 2017-10-23 14:32:16 UTC
Katrin,
This library that felt the process did not work is a single branch library- so local holds don't really come into play with her process.

Kelly
Comment 3 Katrin Fischer 2017-10-23 19:42:55 UTC
I was wondering because you said it's not consistent, if there was another factor.
Comment 4 Nick Clemens 2018-06-15 13:14:09 UTC Comment hidden (obsolete)
Comment 5 Nick Clemens 2018-06-15 13:17:05 UTC
This patch will only show the hold if it is next in the priority list, however, localholds priority and other settings may mean the hold triggered at checkin is not the one listed. Would it be reasonable to only show "there is a hold on this item" unless the hold is waiting?
Comment 6 Nick Clemens 2018-06-15 13:38:26 UTC Comment hidden (obsolete)
Comment 7 Michal Denar 2018-12-02 19:23:44 UTC
Please rebase on master.
Comment 8 Nick Clemens 2018-12-03 16:53:25 UTC
(In reply to Michal Denar from comment #7)
> Please rebase on master.

Either patch seems to apply, you should not apply both, they are alternate options

I suggest the second patch, but want opinions
Comment 9 Liz Rea 2019-04-30 00:42:43 UTC
Created attachment 89059 [details] [review]
Bug 19288: [Alternate] Only show existence of item level hold unless waiting

To test:
1 - Please a biblio level hold for one patron, and an item level hold
for another on same record
2 - Note that you patron name for item lvel hold
3 - Apply patch
4 - You should now see 'There is a hold on this item' with no patron
detail
5 - Confirm hold, patron detail should be visible

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
I like the 2nd approach better. :)
Comment 10 Katrin Fischer 2019-05-11 11:56:48 UTC
I am not sure here if less information is better. Maybe the original message should just be phrased a bit better?

We switch from:

Item-level hold (placed 05/11/2019) for delivery at Centerville. Hold for: katrin koha (42) 

To:

There is a hold on this item 

I am also not sure how this related to the initial bug report, where Kelly reported that sometimes the item level hold would be filled first, even if not first in the queue. Can you explain?

While the first message is very 'talkative', maybe we can find some middle ground? Something like:

Item-level hold for .... (priority x) (similar to how we display the priority in the OPAC)

And I would leave out the hold placed and pickup location.
Comment 11 Nick Clemens 2019-10-17 13:17:32 UTC
(In reply to Katrin Fischer from comment #10)
> I am not sure here if less information is better. Maybe the original message
> should just be phrased a bit better?
> 
> We switch from:
> 
> Item-level hold (placed 05/11/2019) for delivery at Centerville. Hold for:
> katrin koha (42) 
> 
> To:
> 
> There is a hold on this item 
> 
> I am also not sure how this related to the initial bug report, where Kelly
> reported that sometimes the item level hold would be filled first, even if
> not first in the queue. Can you explain?
> 
> While the first message is very 'talkative', maybe we can find some middle
> ground? Something like:
> 
> Item-level hold for .... (priority x) (similar to how we display the
> priority in the OPAC)
> 
> And I would leave out the hold placed and pickup location.

As I understand the main issue, the problem is that Koha shows one hold on the details page, and triggers another at checkin.

Different factors can affect which hold will be triggered at checkin, and we don't want to do too many calculations to show the item details, so we are getting a simple 'what's in the queue' and showing the wrong name.

Trying to show the correct name is the issue. Until the hold is actually waiting it can fluctuate. I think this solution gives the library the info they need "Check this item in to trigger a hold" without adding info that may be wrong
Comment 12 Katrin Fischer 2019-10-18 06:29:41 UTC
Can you explain why the displayed patron would not be the one receiving the next item? I have thought about it, but fail to come up with a use case. And if it's not something that depends on environment (current library location or something) we should be able to fix it.

Our libraries are mostly single branch and I think for them this works well and they would not like it being removed for some less informative text.
Comment 13 Katrin Fischer 2019-10-18 06:31:27 UTC
We should not make such a change without fully understanding the why and how here. That's why I pick 'in discussion'. If we end up doing it, I want to be able to explain to the libraries why.
Comment 14 Nick Clemens 2019-10-18 11:36:09 UTC
(In reply to Katrin Fischer from comment #13)
> We should not make such a change without fully understanding the why and how
> here. That's why I pick 'in discussion'. If we end up doing it, I want to be
> able to explain to the libraries why.

Imagine Patron A has a 'next available' hold and Patron B has an item level hold on Item A

To find the holds on an item we call Koha::Item::current_holds - this will returnholds on the specific item, it will never find 'next available' holds because they are not on a specific item - We will display Patron B

At checkin we actually check the holds, and we check hold_fill_targets at some level. The effect being that if you have not run the holds queue the item will fill the higher priority hold for Patron A, however, if you have built the holds queue it could do a few things:
If there are multiple items available then Patron A may get assigned Item B for the queue and Patron B will get Item A - for this scenario Item A will trigger for patron B
If there is only one available item then Patron A will be assigned Item A and Item A will trigger Patron A

This scenario seems reasonable, the holds queue is going to try and fill the holds so that both patrons can get the item they want if it can. For display though we would need to check the DB for both hold_fill_targets and specific holds. 

I don't think this information of who gets the hold is so important on details, since we will resolve it at checkout, I think simply letting the librarian know this item is held should suffice. I will ask around internally for more opinions
Comment 15 Andrew Fuerste-Henry 2019-10-18 12:28:51 UTC
First thought: the inconsistent capturing behavior is the more pressing issue here. Let's not lose that while addressing the wording on the item details.

Second: I like changing the item details message to just "there is a hold on this item."

The greater context of who the hold is for, where it will be picked up, and what priority it's at is all just a click away on the Holds tab, where it is presented in a clearer, more contextualized fashion. Making the note in item details try to give all that information is trying to shove a lot of detail into a small box, which sets us up for misunderstandings when folks don't read that information carefully.
Comment 16 Marcel de Rooy 2019-11-29 08:55:34 UTC
Bug 20948 has hit PQA in favor of this one, sorry ;)
Please reopen and explain if this cannot be closed.

*** This bug has been marked as a duplicate of bug 20948 ***
Comment 17 Nick Clemens 2019-12-02 11:48:36 UTC
Created attachment 95898 [details] [review]
Bug 19288: Don't show patron info for item level holds on details.pl unless hold is waiting

This persists after bug 20948 - we can still display patron information even if this patron is not the one
who will receive the item upon checkin. The holds tab can give an overview of holds and information - I think
on the details page the only info we need is whether there might be a item level hold, not who it is for

To test:
1 - Place an item level hold for delivery at a branch not matching home branch of item
2 - Place a record level hold for delivery at homebranch of item placed on hold above
3 - Enable LocalHoldsPriority with 'pickup library' matches the item's 'home library'
4 - View the details for the page, it says the item has a hold for the patron with an item level hold
5 - Check in the item, the hold triggered is for the next available patron, ignore the hold
6 - Apply patch
7 - Check details page, 'There is an item-level hold (priority=1)' with no patron info
8 - Check in, next available hold is still the one triggered
Comment 18 Marcel de Rooy 2019-12-02 12:55:10 UTC
Nick, does it depend on 20948 now ?
Comment 19 Nick Clemens 2019-12-02 12:57:03 UTC
(In reply to Marcel de Rooy from comment #18)
> Nick, does it depend on 20948 now ?

Yes :-)
Comment 20 Marcel de Rooy 2019-12-06 07:16:48 UTC
 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
   FAIL   filters
                missing_filter at line 367 (                                There is an item level hold on this item (priority = [% hold.priority %]).)
Comment 21 Martin Renvoize 2020-03-09 09:43:47 UTC
Created attachment 100328 [details] [review]
Bug 19288: Don't show patron info for item level holds on details.pl unless hold is waiting

This persists after bug 20948 - we can still display patron information even if this patron is not the one
who will receive the item upon checkin. The holds tab can give an overview of holds and information - I think
on the details page the only info we need is whether there might be a item level hold, not who it is for

To test:
1 - Place an item level hold for delivery at a branch not matching home branch of item
2 - Place a record level hold for delivery at homebranch of item placed on hold above
3 - Enable LocalHoldsPriority with 'pickup library' matches the item's 'home library'
4 - View the details for the page, it says the item has a hold for the patron with an item level hold
5 - Check in the item, the hold triggered is for the next available patron, ignore the hold
6 - Apply patch
7 - Check details page, 'There is an item-level hold (priority=1)' with no patron info
8 - Check in, next available hold is still the one triggered

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 22 Martin Renvoize 2020-03-09 09:44:19 UTC
Had to rebase this to apply it.. works as expected.. Signing off
Comment 23 Martin Renvoize 2020-03-09 09:46:46 UTC
Signed off as it does what's described.. would be interested in an update from Andrew or Kelly regarding the requirements however prior to a QA run.
Comment 24 Jonathan Druart 2020-03-12 13:26:01 UTC
Created attachment 100632 [details] [review]
Bug 19288: Don't show patron info for item level holds on details.pl unless hold is waiting

This persists after bug 20948 - we can still display patron information even if this patron is not the one
who will receive the item upon checkin. The holds tab can give an overview of holds and information - I think
on the details page the only info we need is whether there might be a item level hold, not who it is for

To test:
1 - Place an item level hold for delivery at a branch not matching home branch of item
2 - Place a record level hold for delivery at homebranch of item placed on hold above
3 - Enable LocalHoldsPriority with 'pickup library' matches the item's 'home library'
4 - View the details for the page, it says the item has a hold for the patron with an item level hold
5 - Check in the item, the hold triggered is for the next available patron, ignore the hold
6 - Apply patch
7 - Check details page, 'There is an item-level hold (priority=1)' with no patron info
8 - Check in, next available hold is still the one triggered

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 25 Jonathan Druart 2020-03-12 13:27:47 UTC
Created attachment 100633 [details] [review]
Bug 19288: Don't show patron info for item level holds on details.pl unless hold is waiting

This persists after bug 20948 - we can still display patron information even if this patron is not the one
who will receive the item upon checkin. The holds tab can give an overview of holds and information - I think
on the details page the only info we need is whether there might be a item level hold, not who it is for

To test:
1 - Place an item level hold for delivery at a branch not matching home branch of item
2 - Place a record level hold for delivery at homebranch of item placed on hold above
3 - Enable LocalHoldsPriority with 'pickup library' matches the item's 'home library'
4 - View the details for the page, it says the item has a hold for the patron with an item level hold
5 - Check in the item, the hold triggered is for the next available patron, ignore the hold
6 - Apply patch
7 - Check details page, 'There is an item-level hold (priority=1)' with no patron info
8 - Check in, next available hold is still the one triggered

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Amended patch: fix indentation
Comment 26 Martin Renvoize 2020-03-16 11:03:03 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 27 Joy Nelson 2020-04-03 17:25:03 UTC
Backported to 19.11.x branch for 19.11.05
Comment 28 Lucas Gass 2020-04-10 15:50:16 UTC
backported to 19.05.x for 19.05.10
Comment 29 Hayley Mapley 2020-04-14 23:46:47 UTC
Minor patch will not be backported to 18.11.1x series