Noticed when working on bug 31318, and discussed on IRC http://irc.koha-community.org/koha/2022-08-09#i_2443598 It seems that we adding/modifying the hold using the holdingbranch of the serial item published first. If no item is created (or no holdingbranch), we are using the subscription.branchcode Shouldn't we use the branchcode of the issue we previously created (if an item exist) instead of the first published one?
The fallback to subscription.branchcode makes sense. But as you wrote, I think using the first item makes no sense - what if you moved part of the holdings into a "closed stacks" library location or just to another branch. This could easily happen in academic libraries where the newest issues might be kept somewhere else than the older less requested items. If an item is linked to the serial via serialitems, we should use the info of this specific item.
Created attachment 138912 [details] [review] Bug 31330: Routing preview - Use the library from the selected issue
The code looked so wrong. I have the feeling that this patch is correcting a problem, and clean the code a bit, but I need someone familiar with this page to know if it's correct. The change is about the library used to generate the hold, should it be the one from the holding library (if an item has been created with the serial item)? The one from the first published serial item? If no items have been created then the library from the subscription is used.
code wise more fixes are needed here, especially the creation of methods for Koha::Serial::Item, but that's not in the scope.
> The change is about the library used to generate the hold, should it be the > one from the holding library (if an item has been created with the serial > item)? The one from the first published serial item? > If no items have been created then the library from the subscription is used. Hm, the hold feature has been broken for so long, that I have never seen it working :( Bug 2894 - Routing list holds are broken So you are probably right about the code not being correct. I think it if there is no item, you can't place a hold and the hold should be an item level hold (it needs to be as serial items represent a specific issue and are not identical). If you have created multiple items on receiving the issue (a recent feature given the age of this code) we might need to pick one, which makes it more complicate than it was in the past maybe. I thought this was a display question for the routing slip until now... should we move the hold stuff elsewhere or does it need to be solved?
Created attachment 166955 [details] [review] Bug 31330: Routing preview - Use the library from the selected issue
Created attachment 166956 [details] [review] Bug 31330: Don't pass serial_escaped What's the point of this? Am I missing something here?
Rebased and adjust. Can we get this moving again? I am a bit lost to be honest. What's needed? Happy to work on it if I get some guidance.
Hi Any news ? Is this enhancement or bug ?
(In reply to Fridolin Somers from comment #9) > Hi > > Any news ? Is this enhancement or bug ? Why are you asking? We need more eyes on this, so if someone at BibLibre could help it would be great. Look at this and bug 2894. What's the expected behaviour? Are the patches fixing the problem correctly?
A librarian asked about this saying it is a bug.
Can they test the patch and confirm it fixes the issue as they would expect?