Created attachment 66148 [details] [review] Bug 19135 - AllowHoldsOnPatronsPossessions not working Test Plan: 1) Set AllowHoldsOnPatronsPossessions to "Don't" 2) Check out an item to a patron 3) Place a hold on that item for the same patron 4) Note you are allowed to with no alert 5) Delete the hold 6) Apply this patch 7) Place a hold on that item for the same patron 8) Note you recieve an alert now Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 66149 [details] [review] [SIGNED OFF] Bug 19135 - AllowHoldsOnPatronsPossessions not working Test Plan: 1) Set AllowHoldsOnPatronsPossessions to "Don't" 2) Check out an item to a patron 3) Place a hold on that item for the same patron 4) Note you are allowed to with no alert 5) Delete the hold 6) Apply this patch 7) Place a hold on that item for the same patron 8) Note you recieve an alert now Works as expected. Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Please update the commit message with something more meaningful (what the patch does?) Why do not you just remove the superfluous line? I will make the diff easier to read and backport.
Upping severity, it impacts last 2 stable releases (17.05.x and 16.11.x).
Hi Jonathan, commit message includes a test plan, what did you mean?
(In reply to Katrin Fischer from comment #5) > Hi Jonathan, commit message includes a test plan, what did you mean? "AllowHoldsOnPatronsPossessions not working" is not a correct and explicit commit title.
My remarks should not block QA, I can edit the commit message later if the author of the patch does not do it.
QA: Looking here now
Created attachment 67684 [details] [review] Bug 19135 - AllowHoldsOnPatronsPossessions not working Test Plan: 1) Set AllowHoldsOnPatronsPossessions to "Don't" 2) Check out an item to a patron 3) Place a hold on that item for the same patron 4) Note you are allowed to with no alert 5) Delete the hold 6) Apply this patch 7) Place a hold on that item for the same patron 8) Note you recieve an alert now Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Hmm two signoffs on my name. Where did this patch come from?
Bug caused by commit bc39f0392bbebaad4c083f81308f652a325be042 Bug 14695 - Add ability to place multiple item holds on a given record per patron (In reply to Jonathan Druart from comment #3) > Why do not you just remove the superfluous line? I will make the diff easier > to read and backport. I am going to amend the patch to only remove the problematic line in order to make the change easier to read.
Created attachment 67718 [details] [review] Bug 19135: Restore AllowHoldsOnPatronsPossessions behaviour Bug caused by commit bc39f0392bbebaad4c083f81308f652a325be042 Bug 14695 - Add ability to place multiple item holds on a given record per patron Test Plan: 1) Set AllowHoldsOnPatronsPossessions to "Don't" 2) Check out an item to a patron 3) Place a hold on that item for the same patron 4) Note you are allowed to with no alert 5) Delete the hold 6) Apply this patch 7) Place a hold on that item for the same patron 8) Note you recieve an alert now Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 17.11, thanks to everybody involved!
*** Bug 13374 has been marked as a duplicate of this bug. ***
Pushed to 17.05.x, will be in 17.05.06. Lines are impacted by Bug 17829 but rebase was easy.
This patch has been pushed to 16.11.x and will be in 16.11.14.