This enhancement would allow patrons to change the pickup location for any hold that is not already waiting ( e.g. it would be allowable for in transit holds ). For non-found holds the feature will not change much behavior. For in-transit holds the feature would trigger a completion of the current transfer and generate a new transfer from the previous target library to the new target library. The feature would be controlled via a system preference.
My staff is asking for this option. We have a branch closed for remodeling and it would have been great to allow patrons to redirect their unfilled holds to another location. Redirecting after it is in-transit isn't as important. NancyK
Just migrated over to Koha and our staff and users are asking for this option too.
We are trying to understand how it makes sense to allow changing location for in transit holds. Could someone comment on this?
Hm it might be nice to control 'not pulled yet' and 'in transit' separately. But I guess patrons could realize they made a wrong setting after placing the hold and it could be useful then?
I think we could have a multiple select in the system preference to set which statuses to allow it for. Something like: OpacAllowPickupLocationChange Enable allowing patrons to change pickup location for these statuses: <multiple select element> Then each library can decide which statuses to allow it for at the same time as activating it.
Should it be possible to change pick up location for suspended holds?
(In reply to Timothy Alexis Vass from comment #6) > Should it be possible to change pick up location for suspended holds? I'd say so. You suspend them before they have been filled, so this should be ok.
Created attachment 118189 [details] [review] Allow users to change pickup location for hold from the opac
Timothy gave this bug to me so here's a patch. :) Bug 14783: Change pickup location from opac. This patch allows users to change the pickup location for their holds from the opac. A syspref (OPACAllowUserToChangeBranch) controls at what stage a hold can be changed. To test: 1. Set up holds with diffrent statuses for a patron (pending, waiting, in transit, suspended) 2. No pickup locations for any hold should be able to be changed. 3. Turn on and off the different options under OPACAllowUserToChangeBranch. Make sure that only the corresponding holds can be changed from the opac. Check eg in the staff client that pickup location has changed. 4. The available pickup locations should respect any transfer restrictions. (Same as the holds list for a biblio in staff client) 5. For an in-transit hold: Check in the item at the original pickup location. 6. Note that it will be redirected to the new location. Sponsored-by: Lunds Universitetsbibliotek
Created attachment 118241 [details] [review] Allow users to change pickup location for hold from the opac Doh! Updated patch! Previous patch included some bits that wasn't supposed to be there. Same test case.
Created attachment 123199 [details] [review] Bug 14783: Change pickup location from opac. This patch allows users to change the pickup location for their holds from the opac. A syspref (OPACAllowUserToChangeBranch) controls at what stage a hold can be changed. To test: 1. Set up holds with diffrent statuses for a patron (pending, waiting, in transit, suspended) 2. No pickup locations for any hold should be able to be changed. 3. Turn on and off the different options under OPACAllowUserToChangeBranch. Make sure that only the corresponding holds can be changed from the opac. Check eg in the staff client that pickup location has changed. 4. The available pickup locations should respect any transfer restrictions. (Same as the holds list for a biblio in staff client) 5. For an in-transit hold: Check in the item at the original pickup location. 6. Note that it will be redirected to the new location. Sponsored-by: Lunds Universitetsbibliotek Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 123271 [details] [review] Bug 14783: (follow-up) Move pickup library form into modal This patch attemps to simplify the list of holds by moving the form for changing a hold's pickup library into a modal dialog. This patch also makes minor tweaks to the CSS for the "X" close button shown in modal in windows. Instances where modal markup used "x" have been updated to use the Bootstrap 4 standard "×" To test, apply the patch and rebuild the OPAC SCSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) - Turn on the OPACAllowUserToChangeBranch system preference. - Log in to the OPAC as a user with holds. - In the view of your holds, confirm that each hold which can be modified shows a "Change" link. - Clicking the link should show a modal window with the title of the item on hold shown in the header. - Confirm that changing the pickup location works correctly.
Created attachment 123708 [details] [review] Bug 14783: Change pickup location from opac. This patch allows users to change the pickup location for their holds from the opac. A syspref (OPACAllowUserToChangeBranch) controls at what stage a hold can be changed. To test: 1. Set up holds with diffrent statuses for a patron (pending, waiting, in transit, suspended) 2. No pickup locations for any hold should be able to be changed. 3. Turn on and off the different options under OPACAllowUserToChangeBranch. Make sure that only the corresponding holds can be changed from the opac. Check eg in the staff client that pickup location has changed. 4. The available pickup locations should respect any transfer restrictions. (Same as the holds list for a biblio in staff client) 5. For an in-transit hold: Check in the item at the original pickup location. 6. Note that it will be redirected to the new location. Sponsored-by: Lunds Universitetsbibliotek Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Created attachment 123709 [details] [review] Bug 14783: (follow-up) Move pickup library form into modal This patch attemps to simplify the list of holds by moving the form for changing a hold's pickup library into a modal dialog. This patch also makes minor tweaks to the CSS for the "X" close button shown in modal in windows. Instances where modal markup used "x" have been updated to use the Bootstrap 4 standard "×" To test, apply the patch and rebuild the OPAC SCSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) - Turn on the OPACAllowUserToChangeBranch system preference. - Log in to the OPAC as a user with holds. - In the view of your holds, confirm that each hold which can be modified shows a "Change" link. - Clicking the link should show a modal window with the title of the item on hold shown in the header. - Confirm that changing the pickup location works correctly. Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Took a first look here and I think we will need unit tests for the new sub in Holds.pm: can_change_branch_opac
Taking another look at this - I had been hoping to see some unit tests here by now :( 1) There are some changes that appear unrelated to the patch sets intent - maybe local changes that slipped in? To me it looks like they should be moved to a separate bug: - <span aria-hidden="true">x</span> + <span aria-hidden="true">×</span> 2) For any change to a .pm we require unit tests. 3) Being a really nit-picky librarian (and because I could not stand being told I had missed it) (not blocker) ('OPACAllowUserToChooseBranch','1','1','Allow the user to choose the branch they want to pickup their hold from','YesNo'), +('OPACAllowUserToChangeBranch','','Pending, In-Transit, Suspended','Allow users to change the library to pick up a hold for these statuses:','multiple'), Change should be before Choose :) 4) Table column has "pick up", but modal has "pickup" - what's correct? This is a fail for 2) mostly - others I would have fixed myself. Please come back with this patch set, it's would be a great addition.
(In reply to Katrin Fischer from comment #16) > - <span aria-hidden="true">x</span> > + <span aria-hidden="true">×</span> This change is correct, so if it can be slipped in here I would approve.
Haha because I did it. Of course I would approve. I have no memory of this patch!
(In reply to Owen Leonard from comment #18) > Haha because I did it. Of course I would approve. I have no memory of this > patch! I didn't notice either when asking you to have look... what does that say about us?
Created attachment 127488 [details] [review] (QA followup) Unit tests Got around to doing unit tests (a bit of a noob at this still)....
Created attachment 127490 [details] [review] Bug 14783: (follow-up revised) Move pickup library form into modal Replacing the modal-followup, removing the change of x -> × as it is already done in a recent patch.
Created attachment 127492 [details] [review] (QA followup, suggestion) Change "Pick up location" to "Pickup location" Adding a followup patch to change "Pick up location" to "Pickup location" in holds-table.inc I belive that's used elsewhere. As for the syspref, from what I see the wording and syspref is already as suggested?
Setting back to signed off, for another round of QA.
Thx, Björn!
Hi Björn, there is a small conflict when I try to apply these patches that I had hoped I had fixed, but there is also this: FAIL t/db_dependent/Hold.t FAIL valid "my" variable $dt masks earlier declaration in same scope "my" variable $dt masks earlier declaration in same scope Could you please have a look? Also we have some guidelines of how the patch subject line should be formatted (always starting with Bug xxxx: optionally followed by (QA follow-up) or (Follow-up). Let me know if I can help with fixing those: Processing additional checks * Commit title does not contain 'follow-up' correctly spelt - 19baf3584b * Commit title does not contain '(QA follow-up)' correctly spelt - 19baf3584b * Commit title does not contain 'follow-up' correctly spelt - 65676a3ba0 * Commit title does not contain '(QA follow-up)' correctly spelt - 65676a3ba0
Created attachment 130344 [details] [review] Bug 14783: Change pickup location from opac. This patch allows users to change the pickup location for their holds from the opac. A syspref (OPACAllowUserToChangeBranch) controls at what stage a hold can be changed. To test: 1. Set up holds with diffrent statuses for a patron (pending, waiting, in transit, suspended) 2. No pickup locations for any hold should be able to be changed. 3. Turn on and off the different options under OPACAllowUserToChangeBranch. Make sure that only the corresponding holds can be changed from the opac. Check eg in the staff client that pickup location has changed. 4. The available pickup locations should respect any transfer restrictions. (Same as the holds list for a biblio in staff client) 5. For an in-transit hold: Check in the item at the original pickup location. 6. Note that it will be redirected to the new location. Sponsored-by: Lunds Universitetsbibliotek Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Created attachment 130345 [details] [review] Bug 14783: (QA follow-up) Add unit tests. This commit adds unit tests for Bug 14783.
Created attachment 130346 [details] [review] Bug 14783: (follow-up) Move pickup library form into modal This patch attemps to simplify the list of holds by moving the form for changing a hold's pickup library into a modal dialog. This patch also makes minor tweaks to the CSS for the "X" close button shown in modal in windows. Instances where modal markup used "x" have been updated to use the Bootstrap 4 standard "×" To test, apply the patch and rebuild the OPAC SCSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) - Turn on the OPACAllowUserToChangeBranch system preference. - Log in to the OPAC as a user with holds. - In the view of your holds, confirm that each hold which can be modified shows a "Change" link. - Clicking the link should show a modal window with the title of the item on hold shown in the header. - Confirm that changing the pickup location works correctly. Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Created attachment 130347 [details] [review] Bug 14783: (QA follow-up) Change "Pick up location" to "Pickup location" in table.
Got around to this. Rebased the patchset onto current master as there was some conflicts. Fixed the warning in the tests and commit-titles. Test and qa-tool passes for me now. Setting back to Signed off as well.
This works well, I do wonder if there should be a public holds API to allow the user to change their own holds, but the modal works nicely. Two issues: The test should be in their own subtest as they are covering a new subroutine The opac-modrequest.pl script needs to check that the hold belongs to the signed in user, it is possible with these patches to alter another users hold via URL manipulation
Hi Björn, will you have time to work on the fixes Nick asked for? I'd really like to see this one in :)
Created attachment 141499 [details] [review] Bug 14783: Change pickup location from opac.
Created attachment 141500 [details] [review] Bug 14783: (follow-up) Move pickup library form into modal
Created attachment 141501 [details] [review] (QA follow-up) Change "Pick up location" to "Pickup location" in table.
Created attachment 141502 [details] [review] (QA follow-up) Add unit tests.
Had forgotten about this. Rebased on to master as well. Bug 30500 already did part of this patch (in regards to intransit holds) so added to that code in some parts and replaces the syspref with this one. Moved the tests to a subtest. User is checked in opac-modrequest.pl (from 30500) Setting back to signed off for more qa.
Created attachment 141727 [details] [review] Bug 14783: Change pickup location from opac. This patch allows users to change the pickup location for their holds from the opac. A syspref (OPACAllowUserToChangeBranch) controls at what stage a hold can be changed. To test: 1. Set up holds with diffrent statuses for a patron (pending, waiting, in transit, suspended) 2. No pickup locations for any hold should be able to be changed. 3. Turn on and off the different options under OPACAllowUserToChangeBranch. Make sure that only the corresponding holds can be changed from the opac. Check eg in the staff client that pickup location has changed. 4. The available pickup locations should respect any transfer restrictions. (Same as the holds list for a biblio in staff client) 5. For an in-transit hold: Check in the item at the original pickup location. 6. Note that it will be redirected to the new location. Sponsored-by: Lunds Universitetsbibliotek Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 141728 [details] [review] Bug 14783: (follow-up) Move pickup library form into modal This patch attemps to simplify the list of holds by moving the form for changing a hold's pickup library into a modal dialog. This patch also makes minor tweaks to the CSS for the "X" close button shown in modal in windows. Instances where modal markup used "x" have been updated to use the Bootstrap 4 standard "×" To test, apply the patch and rebuild the OPAC SCSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) - Turn on the OPACAllowUserToChangeBranch system preference. - Log in to the OPAC as a user with holds. - In the view of your holds, confirm that each hold which can be modified shows a "Change" link. - Clicking the link should show a modal window with the title of the item on hold shown in the header. - Confirm that changing the pickup location works correctly. Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 141729 [details] [review] Bug 14783: (QA follow-up) Change "Pick up location" to "Pickup location" in table. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 141730 [details] [review] Bug 14783: (QA follow-up) Add unit tests. This commit adds unit tests for Bug 14783. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Thanks, Nick!
Created attachment 142009 [details] [review] Bug 14783: Change pickup location from opac. This patch allows users to change the pickup location for their holds from the opac. A syspref (OPACAllowUserToChangeBranch) controls at what stage a hold can be changed. To test: 1. Set up holds with diffrent statuses for a patron (pending, waiting, in transit, suspended) 2. No pickup locations for any hold should be able to be changed. 3. Turn on and off the different options under OPACAllowUserToChangeBranch. Make sure that only the corresponding holds can be changed from the opac. Check eg in the staff client that pickup location has changed. 4. The available pickup locations should respect any transfer restrictions. (Same as the holds list for a biblio in staff client) 5. For an in-transit hold: Check in the item at the original pickup location. 6. Note that it will be redirected to the new location. Sponsored-by: Lunds Universitetsbibliotek Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 142010 [details] [review] Bug 14783: (follow-up) Move pickup library form into modal This patch attemps to simplify the list of holds by moving the form for changing a hold's pickup library into a modal dialog. This patch also makes minor tweaks to the CSS for the "X" close button shown in modal in windows. Instances where modal markup used "x" have been updated to use the Bootstrap 4 standard "×" To test, apply the patch and rebuild the OPAC SCSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) - Turn on the OPACAllowUserToChangeBranch system preference. - Log in to the OPAC as a user with holds. - In the view of your holds, confirm that each hold which can be modified shows a "Change" link. - Clicking the link should show a modal window with the title of the item on hold shown in the header. - Confirm that changing the pickup location works correctly. Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 142011 [details] [review] Bug 14783: (QA follow-up) Change "Pick up location" to "Pickup location" in table. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 142012 [details] [review] Bug 14783: (QA follow-up) Add unit tests. This commit adds unit tests for Bug 14783. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 142013 [details] [review] Bug 14783: (QA follow-up) Rename method and move tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 142014 [details] [review] Bug 14783: Compiled CSS Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 142015 [details] [review] Bug 14783: Update atomicupdate syntax Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Pushed to master for 22.11. Nice work everyone, thanks!
This was a very BAD dbrev. It clears all preferences !
(In reply to Marcel de Rooy from comment #51) > This was a very BAD dbrev. It clears all preferences ! Sorry about that one... I'm certain I tested the sql before. :blush:
(In reply to Björn Nylén from comment #52) > (In reply to Marcel de Rooy from comment #51) > > This was a very BAD dbrev. It clears all preferences ! > > Sorry about that one... I'm certain I tested the sql before. :blush: Don't worry about it, we all broke something before.
Created attachment 143025 [details] [review] Bug 14783: (follow-up) MySQL doesn't like selecting from a table it is updating I have a problem with the db_rev. MySQL doesn't allow selecting from a table you are updating (which has tripped me up before too). MySQL error message: ERROR 1093 (HY000): You can't specify target table 'systempreferences' for update in FROM clause The value needs to be selected separately.
Thank you for the release notes, Björn!
(In reply to Michael Hafen from comment #54) > Created attachment 143025 [details] [review] [review] > Bug 14783: (follow-up) MySQL doesn't like selecting from a table it is > updating > > I have a problem with the db_rev. MySQL doesn't allow selecting from a > table you are updating (which has tripped me up before too). MySQL error > message: > > ERROR 1093 (HY000): You can't specify target table 'systempreferences' for > update in FROM clause > > > The value needs to be selected separately. It looks like the follow-up might not completely fix the issue. Maybe the tables need to be named differently? s1 and s2? See report on the mailing list: https://lists.katipo.co.nz/pipermail/koha/2022-December/058794.html
Is the Koha code from that email using the patch from my follow-up? I don't think the follow-up has been applied to the release yet (maybe I should have changed the status when I posted that patch).
(In reply to Michael Hafen from comment #57) > Is the Koha code from that email using the patch from my follow-up? I don't > think the follow-up has been applied to the release yet (maybe I should have > changed the status when I posted that patch). It was pushed, but no comment was left here: https://git.koha-community.org/Koha-community/Koha/commit/ab1e0a5d6ad2eb31e4322ec11a1e971386144f5d
Interesting. But that's a different follow-up that isn't in the patch list here. That's not my follow-up to make MySQL work.
You are correct, so we might have the fix on oour hands here then!
The only way for a late follow-up to be noticed, is by adding the 'additional_work_needed' keyword. In this case, we noticed the breakage and talked about it on IRC, so I added an RM follow-up. But missed the follow-up added a few days later.
(In reply to Michael Hafen from comment #57) > Is the Koha code from that email using the patch from my follow-up? I don't > think the follow-up has been applied to the release yet (maybe I should have > changed the status when I posted that patch). Michael, can you please post your follow-up on a new bug that depends on this one? So it has the required QA review? It doesn't apply right now. Thanks!
We just got stung by this DB update too.. I'll schedule a bit more of a deep dive for Monday
Hello, Is there a workaround for this issue? I ran into this issue when updating the structure of the database via the web installer. It's not letting me complete the installation. Victor
(In reply to Victor Zuniga from comment #64) > Hello, > > Is there a workaround for this issue? I ran into this issue when updating > the structure of the database via the web installer. It's not letting me > complete the installation. > > Victor Hi Victor, see bug 32470. The fix is available in the new release of 21.11.x but the debian packages are not ready yet. They should be published very soon!
Hi Jonathan, Thanks so much for the update! I just upgraded and was able to complete the DB schema upgrade. We are up and running. Best, Victor (In reply to Jonathan Druart from comment #65) > (In reply to Victor Zuniga from comment #64) > > Hello, > > > > Is there a workaround for this issue? I ran into this issue when updating > > the structure of the database via the web installer. It's not letting me > > complete the installation. > > > > Victor > > Hi Victor, see bug 32470. The fix is available in the new release of 21.11.x > but the debian packages are not ready yet. They should be published very > soon!
I add dependancy with Bug 30500 because it deletes the preference 'OPACInTransitHoldPickupLocationChange' this bug added