Bug 9834

Summary: Reverting a waiting hold should lead to the former hold type (item or biblio level)
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: Hold requestsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Nick Clemens (kidclamp) <nick>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, george, gmcharlt, gwilliams, hughr, jonathan.druart, kelly, kyle.m.hall, liz, martin.renvoize, mirko, sally.healey, tomascohen, veron
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23265
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00
Circulation function:
Bug Depends on:    
Bug Blocks: 20844, 23070, 23133, 23267    
Attachments: Bug 9834: Add DB changes
Bug 9834: Add tests
Bug 9834: Preserve biblio level hold preference on reverting waiting status
Bug 9834: Add DB changes
Bug 9834: Add tests
Bug 9834: Preserve biblio level hold preference on reverting waiting status
Bug 9834: Add DB changes
Bug 9834: Add tests
Bug 9834: Preserve biblio level hold preference on reverting waiting status
Bug 9834: (QA follow-up) API fixes

Description Marcel de Rooy 2013-03-18 10:42:20 UTC
When you confirm a hold, the item number is saved with the reserve.
In case you revert the waiting status for some reason (probably exceptional), the hold becomes an item level hold.
Unfortunately, you cannot know anymore if that was the case when the hold was placed. I would argue that it is more likely that it was not a item level hold. So why not remove the item number by default?
Could we add a checkbox when doing the revert allowing the librarian to actively say "keep the item number" ?
Comment 1 Kyle M Hall 2013-04-10 17:03:58 UTC
The itemnumber is kept as a 'better safe than sorry' approach. That is, it is better to make sure the patron gets exactly the item they wanted rather than risk giving the patron an item he or she didn't want.

I think the solution is to just add a new field 'hold_level' to the reserves table that is marked as 'bib' or 'item' when a reserve is first placed. That way we can know at the time of reversion if we need to remove the itemnumber or not.
Comment 2 Liz Rea 2013-11-22 01:32:33 UTC
My feeling is that if it started as an "any available" it should go back to that. But yea, we'd have to know if it was placed that way in order to put it back. 

Practically, the current way might mean that a borrower waits an extra circ period for their reserve, which is bad service by the library. 

Liz
Comment 3 Marc Véron 2015-12-20 17:27:27 UTC
Still valid?
Comment 4 Sally 2016-08-18 09:14:05 UTC
Still valid.

Being able to revert an item level hold to a global hold would be excellent.

We only allow patrons to make global holds, so patrons waiting for specific items (which are now on loan) when they could've had one of the other copies is not ideal.

It also impacts the patron who checked out the item, as they have to return it after one circ period, and cannot renew.
Comment 5 Hugh Rundle 2017-12-28 23:59:06 UTC
Definitely needed for all the reasons mentioned: if holds are 'next available' (which is nearly always the case for us) then turning it into an item-level hold on allocation isn't ideal. Also, usually if you're reverting the waiting status it's because the item has gone missing, so the only way to fix this is actually to cancel the hold and then place a new hold and push the borrower to the top of the list again, which is ...suboptimal.
Comment 6 Kelly McElligott 2019-06-05 13:52:10 UTC
This is still valid in 18.11.05.
Comment 7 Jonathan Druart 2019-06-06 19:53:20 UTC
Created attachment 90397 [details] [review]
Bug 9834: Add DB changes
Comment 8 Jonathan Druart 2019-06-06 19:53:25 UTC
Created attachment 90398 [details] [review]
Bug 9834: Add tests
Comment 9 Jonathan Druart 2019-06-06 19:53:29 UTC
Created attachment 90399 [details] [review]
Bug 9834: Preserve biblio level hold preference on reverting waiting status

When the waiting status of a hold is revert we want to preserve the
level hold preference of the user.

Test plan:
Place a hold at biblio level
Confirm the hold
Revert the waiting hold

=> The hold must still be considered as a biblio level hold ("next
available item")
Comment 10 Liz Rea 2019-06-08 03:34:23 UTC
Created attachment 90436 [details] [review]
Bug 9834: Add DB changes

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 11 Liz Rea 2019-06-08 03:34:32 UTC
Created attachment 90437 [details] [review]
Bug 9834: Add tests

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 12 Liz Rea 2019-06-08 03:34:36 UTC
Created attachment 90438 [details] [review]
Bug 9834: Preserve biblio level hold preference on reverting waiting status

When the waiting status of a hold is revert we want to preserve the
level hold preference of the user.

Test plan:
Place a hold at biblio level
Confirm the hold
Revert the waiting hold

=> The hold must still be considered as a biblio level hold ("next
available item")

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 13 Nick Clemens (kidclamp) 2019-06-14 17:49:40 UTC
Created attachment 90611 [details] [review]
Bug 9834: Add DB changes

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 14 Nick Clemens (kidclamp) 2019-06-14 17:49:44 UTC
Created attachment 90612 [details] [review]
Bug 9834: Add tests

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 15 Nick Clemens (kidclamp) 2019-06-14 17:49:47 UTC
Created attachment 90613 [details] [review]
Bug 9834: Preserve biblio level hold preference on reverting waiting status

When the waiting status of a hold is revert we want to preserve the
level hold preference of the user.

Test plan:
Place a hold at biblio level
Confirm the hold
Revert the waiting hold

=> The hold must still be considered as a biblio level hold ("next
available item")

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 16 Martin Renvoize (ashimema) 2019-06-15 07:02:39 UTC
Nice work!

Pushed to master for 19.11.00
Comment 17 Tomás Cohen Arazi (tcohen) 2019-06-17 14:38:04 UTC
Comment on attachment 90611 [details] [review]
Bug 9834: Add DB changes

>+  `item_level_hold` BOOLEAN NOT NULL DEFAULT 0, -- Is the hpld placed at item level

Please see https://wiki.koha-community.org/wiki/Coding_Guidelines#SQL12:_Booleans

This is missing a tweak to hold.json and to the schema files.
Comment 18 Tomás Cohen Arazi (tcohen) 2019-06-17 15:17:13 UTC
Created attachment 90677 [details] [review]
Bug 9834: (QA follow-up) API fixes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 19 Jonathan Druart 2019-06-19 03:02:23 UTC
(In reply to Tomás Cohen Arazi from comment #17)
> Comment on attachment 90611 [details] [review] [review]
> Bug 9834: Add DB changes
> 
> >+  `item_level_hold` BOOLEAN NOT NULL DEFAULT 0, -- Is the hpld placed at item level
> 
> Please see
> https://wiki.koha-community.org/wiki/Coding_Guidelines#SQL12:_Booleans
> 
> This is missing a tweak to hold.json and to the schema files.

How do we catch the next ones?
Comment 20 Martin Renvoize (ashimema) 2019-06-20 16:15:19 UTC
Pushed followup to master
Comment 21 Mirko Tietgen 2019-07-04 10:06:12 UTC
I assume this breaks upgrading the Koha DB schema at DB revision 16.12.00.032

>[…]
>Upgrade to 16.12.00.030 done (Bug 16344 - Add a circ rule to limit the auto renewals given a specific date)
>Upgrade to 16.12.00.031 done (Bug 15108: OAI-PMH provider improvements)
>DBD::mysql::st execute failed: Unknown column 'me.item_level_hold' in 'field list' [for Statement "SELECT `me`.`reserve_id`, `me`.`borrowernumber`, `me`.`reservedate`, `me`.`biblionumber`, `me`.`branchcode`, `me`.`notificationdate`, `me`.`reminderdate`, `me`.`cancellationdate`, `me`.`reservenotes`, `me`.`priority`, `me`.`found`, `me`.`timestamp`, `me`.`itemnumber`, `me`.`waitingdate`, `me`.`expirationdate`, `me`.`lowestPriority`, `me`.`suspend`, `me`.`suspend_until`, `me`.`itemtype`, `me`.`item_level_hold` FROM `reserves` `me` WHERE ( ( `found` = ? AND `priority` = ? ) )" with ParamValues: 0='W', 1=0] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836.
>DBIx::Class::Storage::DBI::_dbh_execute(): Unknown column 'me.item_level_hold' in 'field list' at /usr/share/koha/lib/Koha/Objects.pm line 209
Comment 22 Katrin Fischer 2019-07-05 06:01:32 UTC
(In reply to Mirko Tietgen from comment #21)
> I assume this breaks upgrading the Koha DB schema at DB revision 16.12.00.032
> 
> >[…]
> >Upgrade to 16.12.00.030 done (Bug 16344 - Add a circ rule to limit the auto renewals given a specific date)
> >Upgrade to 16.12.00.031 done (Bug 15108: OAI-PMH provider improvements)
> >DBD::mysql::st execute failed: Unknown column 'me.item_level_hold' in 'field list' [for Statement "SELECT `me`.`reserve_id`, `me`.`borrowernumber`, `me`.`reservedate`, `me`.`biblionumber`, `me`.`branchcode`, `me`.`notificationdate`, `me`.`reminderdate`, `me`.`cancellationdate`, `me`.`reservenotes`, `me`.`priority`, `me`.`found`, `me`.`timestamp`, `me`.`itemnumber`, `me`.`waitingdate`, `me`.`expirationdate`, `me`.`lowestPriority`, `me`.`suspend`, `me`.`suspend_until`, `me`.`itemtype`, `me`.`item_level_hold` FROM `reserves` `me` WHERE ( ( `found` = ? AND `priority` = ? ) )" with ParamValues: 0='W', 1=0] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836.
> >DBIx::Class::Storage::DBI::_dbh_execute(): Unknown column 'me.item_level_hold' in 'field list' at /usr/share/koha/lib/Koha/Objects.pm line 209

Filed a new bug for this: bug 23267
Comment 23 Alex Buckley 2019-07-26 00:07:54 UTC
*** Bug 15679 has been marked as a duplicate of this bug. ***