Created attachment 42690 [details] Image of freezing Holds tab display When trying to display the holds for a user, if an item has no holdingbranch, it causes the svc/holds service to crash and the display to freeze. Tentative steps to reproduce on any DB: 1) put 1 or more items on hold for a given user. 2) go into the database and set the item.holdingbranch to NULL 3) In the user's page, in checkouts (circulation.pl), click the tab that should be written 1 Hold(s) 4) It will be "Processing" indefinitely. See attached image. The cause is the svc/holds code that crashes. The fix will validate that there's an holdingbranch before proceding with the code.
Created attachment 42691 [details] [review] Bug 14846 - Items with no holdingbranch causes svc/holds to crash When trying to display the holds for a user, if an item has no holdingbranch, it causes the svc/holds service to crash and the display to freeze. Tentative steps to reproduce on any DB: 1) put 1 or more items on hold for a given user. 2) go into the database and set the item.holdingbranch to NULL 3) In the user's page, in checkouts (circulation.pl), click the tab that should be written 1 Hold(s) 4) It will be "Processing" indefinitely. The cause is the svc/holds code that crashes. The fix will validate that there's an holdingbranch before proceding with the code.
This was done differently (fixed) by Kyle recently in master. The issue is just with 3.20, then. I suppose this makes this little patch useless.
I get this when applying on 3.20.05: Bug 14846 - Items with no holdingbranch causes user's holds display to freeze 42691 - Bug 14846 - Items with no holdingbranch causes svc/holds to crash Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 14846 - Items with no holdingbranch causes svc/holds to crash Using index info to reconstruct a base tree... M svc/holds Falling back to patching base and 3-way merge... Auto-merging svc/holds CONFLICT (content): Merge conflict in svc/holds Failed to merge in the changes. Patch failed at 0001 Bug 14846 - Items with no holdingbranch causes svc/holds to crash
Created attachment 44840 [details] [review] Bug 14846 - Items with no holdingbranch causes svc/holds to crash When trying to display the holds for a user, if an item has no holdingbranch, it causes the svc/holds service to crash and the display to freeze. Tentative steps to reproduce on any DB: 1) put 1 or more items on hold for a given user. 2) go into the database and set the item.holdingbranch to NULL 3) In the user's page, in checkouts (circulation.pl), click the tab that should be written 1 Hold(s) 4) It will be "Processing" indefinitely. The cause is the svc/holds code that crashes. The fix will validate that there's an holdingbranch before proceding with the code.
(In reply to Blou from comment #2) > This was done differently (fixed) by Kyle recently in master. > > The issue is just with 3.20, then. I suppose this makes this little patch > useless. How was it fixed in master? Couldn't it be backported?
I tried it, but it didn't apply. Before that patch (13853), there are at least two others that modify the structure of svc/holds. So maybe it would be the right solution. But it would certainly require more work, if it's only to patch on an older release. I know what we did locally. :)
(In reply to Blou from comment #6) > I tried it, but it didn't apply. Before that patch (13853), there are at > least two others that modify the structure of svc/holds. Thanks for the explanation. Since #13853 is an enhancement, it mustn't be backported, and so your 3.20 & 3.18 patch is valid, imho.
Created attachment 45493 [details] [review] Bug 14846 - Items with no holdingbranch causes svc/holds to crash When trying to display the holds for a user, if an item has no holdingbranch, it causes the svc/holds service to crash and the display to freeze. Tentative steps to reproduce on any DB: 1) put 1 or more items on hold for a given user. 2) go into the database and set the item.holdingbranch to NULL 3) In the user's page, in checkouts (circulation.pl), click the tab that should be written 1 Hold(s) 4) It will be "Processing" indefinitely. The cause is the svc/holds code that crashes. The fix will validate that there's an holdingbranch before proceding with the code. Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
The bug exists on master, and the patch fixes the described issue. Could you please explain how you can have a holdingbranch value set to NULL?
I asked Francois, and he has no idea how we got to that. But we had TWO clients with the situation, one of which it couldn't be blamed on a migration issue. The clerk put the item on Reserve to put them aside when they would come back. We thought he had no branch associated, but then he probably could not do much, and anyway we confirmed it was not the case.
Created attachment 45517 [details] [review] Bug 14846 - Items with no holdingbranch causes svc/holds to crash When trying to display the holds for a user, if an item has no holdingbranch, it causes the svc/holds service to crash and the display to freeze. Tentative steps to reproduce on any DB: 1) put 1 or more items on hold for a given user. 2) go into the database and set the item.holdingbranch to NULL 3) In the user's page, in checkouts (circulation.pl), click the tab that should be written 1 Hold(s) 4) It will be "Processing" indefinitely. The cause is the svc/holds code that crashes. The fix will validate that there's an holdingbranch before proceding with the code. Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I don't know if we should allow NULL in items.holdingbranch and items.homebranch. But since it's possible at the moment, we should take it into account.
Pushed to master! Thanks Philippe!
Patch pushed to 3.22.x, will be in 3.22.1
This patch has been pushed to 3.20.x, will be in 3.20.7.