|
Description
Martha Fuerst
2020-11-30 17:09:31 UTC
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. |