When placing multiple holds from the result list, the pick-up location is empty now, which makes it a more tedious process than it needs to be. When placing a single hold the home library of the patron is used a default, same should apply if you place multiple holds. Could this be a side effect of bug 28273?
Hm testing again - it's actually probably the library currently logged in at? Not the homebranch... in any case: same logic should apply for single hold and multi-holds.
Created attachment 124419 [details] [review] Bug 28919: Select logged in library as pickup for multi holds When placing multiple holds from the result list, the pick-up location is empty now, which makes it a more tedious process than it needs to be. Logged in library must be selected to match the single hold logic.
The very happy with the patch but it's working.
Created attachment 125221 [details] [review] Bug 28919: Select logged in library as pickup for multi holds When placing multiple holds from the result list, the pick-up location is empty now, which makes it a more tedious process than it needs to be. Logged in library must be selected to match the single hold logic. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
I am sorry, I tried "restart_all" multiple times, but the pull downs remain empty. I am using the admin user (42), logged in at Centerville. When placing a single hold, Centerville is selected.
(In reply to Katrin Fischer from comment #5) > I am sorry, I tried "restart_all" multiple times, but the pull downs remain > empty. > > I am using the admin user (42), logged in at Centerville. > > When placing a single hold, Centerville is selected. The pull down from the top, the "Pickup at:"? https://snipboard.io/pRHAMn.jpg
> > The pull down from the top, the "Pickup at:"? > > https://snipboard.io/pRHAMn.jpg Oh! I never noticed the one on top - will try to retry :)
Created attachment 125658 [details] [review] Bug 28919: Select logged in library as pickup for multi holds When placing multiple holds from the result list, the pick-up location is empty now, which makes it a more tedious process than it needs to be. Logged in library must be selected to match the single hold logic. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Ok, this works as I think it should, pre-selecting the right branch. BUT: When you try to save the holds, it complains: Please make sure all selected titles have a pickup location set If you switch the pick-up location to another, they will all be set below as well. So I think we do not only need to pre-select for the one on top, but for all holds displayed.
Well, what's the point of the first/main select then? Tomas?
Created attachment 128144 [details] [review] Bug 28919: [Alternative] Select logged in library as pickup for multi holds Default pickup location to the logged in library for staff placed multiholds. Holds with a blank pickup location will cause 500 errors in the Reserves tab of the circulation screen, so we want to make sure the pickup location does not default to blank.
Attaching an alternative patch here, although I think Jonathan's is better overall (just needs to be applies to the "Pickup location" as well). This is causing frequent problems for a library of mine, so I'm keen to get a fix implemented ASAP. Personally, I think a lot of holds logic and error-handling needs work, but I don't mind just putting out fires for now.
(In reply to Jonathan Druart from comment #10) > Well, what's the point of the first/main select then? Tomas? This form had a problem, orginally: it allowed you to place holds with invalid pickup locations, and didn't even require AllowHoldPolicyOverride. The idea was to make it so you weren't offered bad pickup locations. Asking different libraries, it turned out this wasn't a bad workflow: - You select several biblios and click place hold - A form is presented (similar to the one before - There's a dropdown that lets you choose the union of the valid pickup locations from all selected titles - Only those for which the chosen one is a valid one are set - The rest require manually setting them - If you manually set one, changing the top-level 'Pickup at' one should not change it - It needs to honour the UI decision we made about overriden pickup locations, having them show a notice about them being an override to rules So that's it. Reading the bug description, my guess is that: - The 'Pickup at' dropdown might need some better explanation in the UI (a hint, a visible text) - I read a comment from dcook somewhere else, and it felt like he found a bug that made the form send empty pickup locations, that actually hit the DB, that'd be a bug, not a usability discussion.
(In reply to Tomás Cohen Arazi from comment #13) > (In reply to Jonathan Druart from comment #10) > > Well, what's the point of the first/main select then? Tomas? > > This form had a problem, orginally: it allowed you to place holds with > invalid pickup locations, and didn't even require AllowHoldPolicyOverride. > The idea was to make it so you weren't offered bad pickup locations. > > Asking different libraries, it turned out this wasn't a bad workflow: > - You select several biblios and click place hold > - A form is presented (similar to the one before > - There's a dropdown that lets you choose the union of the valid pickup > locations from all selected titles > - Only those for which the chosen one is a valid one are set > - The rest require manually setting them > - If you manually set one, changing the top-level 'Pickup at' one should not > change it > - It needs to honour the UI decision we made about overriden pickup > locations, having them show a notice about them being an override to rules > > So that's it. Reading the bug description, my guess is that: > - The 'Pickup at' dropdown might need some better explanation in the UI (a > hint, a visible text) > - I read a comment from dcook somewhere else, and it felt like he found a > bug that made the form send empty pickup locations, that actually hit the > DB, that'd be a bug, not a usability discussion. And making a default selection is good too.
(In reply to Tomás Cohen Arazi from comment #13) > - I read a comment from dcook somewhere else, and it felt like he found a > bug that made the form send empty pickup locations, that actually hit the > DB, that'd be a bug, not a usability discussion. Yeah I was thinking the same thing. We could throw an exception client-side or server-side (or both). (In reply to Tomás Cohen Arazi from comment #14) > And making a default selection is good too. I think this would be a good first step, since it requires only a small change and brings multiholds in line with require holds I think.
I like Jonathan's patch here, it does what libraries have indicated they want. Though I agree that on initial page load all dropdowns (the overall dropdown at the top and the individual dropdowns on each title) should default to the logged in branch if that's a valid pickup location. But if the logged-in branch is not a valid pickup location for one of the titles, then that title's dropdown should be blank to start and require one to select a valid pickup location before the form can be submitted.
(In reply to Andrew Fuerste-Henry from comment #16) > I like Jonathan's patch here, it does what libraries have indicated they > want. Though I agree that on initial page load all dropdowns (the overall > dropdown at the top and the individual dropdowns on each title) should > default to the logged in branch if that's a valid pickup location. > > But if the logged-in branch is not a valid pickup location for one of the > titles, then that title's dropdown should be blank to start and require one > to select a valid pickup location before the form can be submitted. +1
(In reply to Andrew Fuerste-Henry from comment #16) > I like Jonathan's patch here, it does what libraries have indicated they > want. Though I agree that on initial page load all dropdowns (the overall > dropdown at the top and the individual dropdowns on each title) should > default to the logged in branch if that's a valid pickup location. > > But if the logged-in branch is not a valid pickup location for one of the > titles, then that title's dropdown should be blank to start and require one > to select a valid pickup location before the form can be submitted. +1 I think this would be behavior that is consistent with that Tomas did in Bug 29349
Tomas, I think you are the best placed to finish the work here.
I'm having libraries still experiencing this issue. At the moment, the build_holds_queue.pl is now sending them emails with error messages every 15 minutes...