Bug 19135

Summary: AllowHoldsOnPatronsPossessions is not working
Product: Koha Reporter: Kyle M Hall <kyle>
Component: Hold requestsAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: major    
Priority: P5 - low CC: clackman, fridolin.somers, gmcharlt, jdeca, jonathan.druart, katrin.fischer, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13374
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 14695    
Bug Blocks:    
Attachments: Bug 19135 - AllowHoldsOnPatronsPossessions not working
[SIGNED OFF] Bug 19135 - AllowHoldsOnPatronsPossessions not working
Bug 19135 - AllowHoldsOnPatronsPossessions not working
Bug 19135: Restore AllowHoldsOnPatronsPossessions behaviour

Description Kyle M Hall 2017-08-17 15:49:45 UTC

    
Comment 1 Kyle M Hall 2017-08-17 15:54:11 UTC
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>
Comment 2 Christopher Brannon 2017-08-17 17:02:31 UTC
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>
Comment 3 Jonathan Druart 2017-08-31 14:30:02 UTC
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.
Comment 4 Jonathan Druart 2017-10-02 15:50:51 UTC
Upping severity, it impacts last 2 stable releases (17.05.x and 16.11.x).
Comment 5 Katrin Fischer 2017-10-03 00:28:50 UTC
Hi Jonathan, commit message includes a test plan, what did you mean?
Comment 6 Jonathan Druart 2017-10-03 14:14:32 UTC
(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.
Comment 7 Jonathan Druart 2017-10-03 17:49:45 UTC
My remarks should not block QA, I can edit the commit message later if the author of the patch does not do it.
Comment 8 Marcel de Rooy 2017-10-06 09:45:36 UTC
QA: Looking here now
Comment 9 Marcel de Rooy 2017-10-06 09:52:33 UTC
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>
Comment 10 Marcel de Rooy 2017-10-06 09:54:32 UTC
Hmm two signoffs on my name. Where did this patch come from?
Comment 11 Jonathan Druart 2017-10-06 14:34:07 UTC
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.
Comment 12 Jonathan Druart 2017-10-06 14:39:24 UTC
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>
Comment 13 Jonathan Druart 2017-10-06 15:17:45 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 14 Nick Clemens 2017-10-13 14:58:39 UTC
*** Bug 13374 has been marked as a duplicate of this bug. ***
Comment 15 Fridolin Somers 2017-10-25 08:31:46 UTC
Pushed to 17.05.x, will be in 17.05.06.

Lines are impacted by Bug 17829 but rebase was easy.
Comment 16 Katrin Fischer 2017-10-29 18:14:42 UTC
This patch has been pushed to 16.11.x and will be in 16.11.14.