Created attachment 114060 [details] screenshot of error when trying to open the drop-down menu for pick-up location on record details page This is perhaps related to Bug 18729. Prior to 20.05.05, staff without the modify_holds_priority permission were able to update the pick-up location for a hold from the record details page, but not the patron's details or checkouts pages. Staff now see an error (see attached screenshot) when they open the drop-down menu, and when they click "close", the dialog box keeps popping up. I counted 30 times before I was able to (I think?) click away so that it didn't regenerate. This needs to be fixed, and ultimately, Bug 18729 really needs to be addressed.
Created attachment 114063 [details] [review] Bug 27117 : Only place_holds permission is needed to adjust pickup locations The GET /pickup_locations route is requesting the whole reserveforothers permission whereas only the subpermission place_holds is needed. Test plan: 0. Don't apply this patch 1. Set the subpermission place_holds but modify_holds_priority 2. Edit a hold and click the pickup library dropdown list 3. You get a JS alert and log displays GET /api/v1/app.pl/api/v1/holds/5/pickup_locations 403 Forbidden 4. Apply this patch 5. Reload the page, click the dropdown list, modify the pickup location and save => Success!
Created attachment 114064 [details] [review] Bug 27117 : Only place_holds permission is needed to adjust pickup locations The GET /pickup_locations route is requesting the whole reserveforothers permission whereas only the subpermission place_holds is needed. Test plan: 0. Don't apply this patch 1. Set the subpermission place_holds but modify_holds_priority 2. Edit a hold and click the pickup library dropdown list 3. You get a JS alert and log displays GET /api/v1/app.pl/api/v1/holds/5/pickup_locations 403 Forbidden 4. Apply this patch 5. Reload the page, click the dropdown list, modify the pickup location and save => Success!
I agree with the solution and also with the follow-up bug on 18729. This is giving a weird error if the user doesn't have permissions. The right thing would be to completely disable the dropdown IMHO.
Created attachment 114310 [details] [review] Bug 27117: Only place_holds permission is needed to adjust pickup locations The GET /pickup_locations route is requesting the whole reserveforothers permission whereas only the subpermission place_holds is needed. Test plan: 0. Don't apply this patch 1. Set the subpermission place_holds but modify_holds_priority 2. Edit a hold and click the pickup library dropdown list 3. You get a JS alert and log displays GET /api/v1/app.pl/api/v1/holds/5/pickup_locations 403 Forbidden 4. Apply this patch 5. Reload the page, click the dropdown list, modify the pickup location and save => Success! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This works as expected (today), not sure what mistake I made yesterday. The QA script is happy, and the changes to the tests make sense (and they pass). Thanks Jonathan!
Created attachment 114451 [details] [review] Bug 27117: Only place_holds permission is needed to adjust pickup locations The GET /pickup_locations route is requesting the whole reserveforothers permission whereas only the subpermission place_holds is needed. Test plan: 0. Don't apply this patch 1. Set the subpermission place_holds but modify_holds_priority 2. Edit a hold and click the pickup library dropdown list 3. You get a JS alert and log displays GET /api/v1/app.pl/api/v1/holds/5/pickup_locations 403 Forbidden 4. Apply this patch 5. Reload the page, click the dropdown list, modify the pickup location and save => Success! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.01
Pushed to 20.05.x for 20.05.07
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.