[Bug 11336] New: Priority is not updated on deleting holds
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Bug ID: 11336 Summary: Priority is not updated on deleting holds Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com There are a lot of places where a hold deletion is possible. But I just found 1 place where it works! To reproduce: - select or create 2 users U1 and U2 - select or create an holdable item - place on hold for both U1 and U2. U1 has priority 1 and U2 has priority 2. - delete the hold for U1 - go on circ/circulation.pl?borrowernumber=XXXX for U2 (or in the DB directly) and verify the priority has not been set to 1 The issue is repeatable (at least) on these 2 pages: * circ/circulation.pl?borrowernumber=XXXX (tab 'Holds', select "yes" in the dropdown list and submit the form) * reserve/request.pl?biblionumber=XXXX (clic on the red cross) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |jonathan.druart@biblibre.co |ity.org |m -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 23286 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23286&action=edit Bug 11336: Priority is not updated on deleting holds There are a lot of places where a hold deletion is possible. But I just found 1 place where it works! To reproduce: - select or create 2 users U1 and U2 - select or create an holdable item - place on hold for both U1 and U2. U1 has priority 1 and U2 has priority 2. - delete the hold for U1 - go on circ/circulation.pl?borrowernumber=XXXX for U2 (or in the DB directly) and verify the priority has not been set to 1 The issue is repeatable (at least) on these 2 pages: * circ/circulation.pl?borrowernumber=XXXX (tab 'Holds', select "yes" in the dropdown list and submit the form) * reserve/request.pl?biblionumber=XXXX (clic on the red cross) The CancelReserve calls _FixPriority with a reserveid. _FixPriority try to retrieve the reserve with this reserveid but it does not exist anymore... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #2 from Jonathan Druart <jonathan.druart@biblibre.com> --- The fix I propose is quite dirty but I did not find another fix without rewriting a big part of code. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Christopher Brannon <cbrannon@cdalibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |cbrannon@cdalibrary.org --- Comment #3 from Christopher Brannon <cbrannon@cdalibrary.org> --- Looks like it does what is expected. Thanks for addressing this issue. Christopher -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |sandboxes@biblibre.com --- Comment #4 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Patch tested with a sandbox, by Christopher Brannon <cbrannon@cdalibrary.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23286|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #5 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Created attachment 23384 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23384&action=edit Bug 11336: Priority is not updated on deleting holds There are a lot of places where a hold deletion is possible. But I just found 1 place where it works! To reproduce: - select or create 2 users U1 and U2 - select or create an holdable item - place on hold for both U1 and U2. U1 has priority 1 and U2 has priority 2. - delete the hold for U1 - go on circ/circulation.pl?borrowernumber=XXXX for U2 (or in the DB directly) and verify the priority has not been set to 1 The issue is repeatable (at least) on these 2 pages: * circ/circulation.pl?borrowernumber=XXXX (tab 'Holds', select "yes" in the dropdown list and submit the form) * reserve/request.pl?biblionumber=XXXX (clic on the red cross) The CancelReserve calls _FixPriority with a reserveid. _FixPriority try to retrieve the reserve with this reserveid but it does not exist anymore... Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion CC| |kyle@bywatersolutions.com --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- I agree this is a bit of a dirty fix. Wouldn't it be better to modify GetReserve to look for the reserve in reserves first, and then old_reserves if it doesn't find it there? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #7 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Kyle M Hall from comment #6)
I agree this is a bit of a dirty fix. Wouldn't it be better to modify GetReserve to look for the reserve in reserves first, and then old_reserves if it doesn't find it there?
Hum... It could work. But how to be sure this won't introduce a regression? For ex. a logic like: if GetReserve returns undef, the reserve is deleted. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23384|0 |1 is obsolete| | --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 23522 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23522&action=edit Bug 11336: Priority is not updated on deleting holds There are a lot of places where a hold deletion is possible. But I just found 1 place where it works! To reproduce: - select or create 2 users U1 and U2 - select or create an holdable item - place on hold for both U1 and U2. U1 has priority 1 and U2 has priority 2. - delete the hold for U1 - go on circ/circulation.pl?borrowernumber=XXXX for U2 (or in the DB directly) and verify the priority has not been set to 1 The issue is repeatable (at least) on these 2 pages: * circ/circulation.pl?borrowernumber=XXXX (tab 'Holds', select "yes" in the dropdown list and submit the form) * reserve/request.pl?biblionumber=XXXX (clic on the red cross) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- It looks good. I add regression test. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23522|0 |1 is obsolete| | --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 23531 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23531&action=edit Bug 11336: Priority is not updated on deleting holds There are a lot of places where a hold deletion is possible. But I just found 1 place where it works! To reproduce: - select or create 2 users U1 and U2 - select or create an holdable item - place on hold for both U1 and U2. U1 has priority 1 and U2 has priority 2. - delete the hold for U1 - go on circ/circulation.pl?borrowernumber=XXXX for U2 (or in the DB directly) and verify the priority has not been set to 1 The issue is repeatable (at least) on these 2 pages: * circ/circulation.pl?borrowernumber=XXXX (tab 'Holds', select "yes" in the dropdown list and submit the form) * reserve/request.pl?biblionumber=XXXX (clic on the red cross) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 23532 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23532&action=edit Bug 11336: [QA Followup] Priority is not updated on deleting holds Patch does not fix issue when canceling hold from moremember.pl. This followup fixes that issue. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de QA Contact|testopia@bugs.koha-communit |katrin.fischer@bsz-bw.de |y.org | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Jonathan, could you check the follow-up so I can do QA? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #13 from Jonathan Druart <jonathan.druart@biblibre.com> --- Kyle, I disagree with your patch. The call stack could be weird: If ModReserve is called for deleting an hold, _FixPriority will be called with $rank=del which will call CancelReserve which will call again _FixPriority. Since the code in ModReserve (when call with rank=del) cancels a reserve, maybe the best is to call CancelReserve :) I am submitting a new patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23532|0 |1 is obsolete| | --- Comment #14 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 23567 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23567&action=edit Bug 11336: (follow-up) Priority is not updated on deleting holds - moremember Patch does not fix issue when canceling hold from moremember.pl. This followup fixes that issue. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23567|0 |1 is obsolete| | --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 23629 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23629&action=edit Bug 11336: (follow-up) Priority is not updated on deleting holds - moremember Patch does not fix issue when canceling hold from moremember.pl. This followup fixes that issue. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Medium patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23531|0 |1 is obsolete| | Attachment #23629|0 |1 is obsolete| | --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 23706 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23706&action=edit [PASSED QA] Bug 11336: Priority is not updated on deleting holds There are a lot of places where a hold deletion is possible. But I just found 1 place where it works! To reproduce: - select or create 2 users U1 and U2 - select or create an holdable item - place on hold for both U1 and U2. U1 has priority 1 and U2 has priority 2. - delete the hold for U1 - go on circ/circulation.pl?borrowernumber=XXXX for U2 (or in the DB directly) and verify the priority has not been set to 1 The issue is repeatable (at least) on these 2 pages: * circ/circulation.pl?borrowernumber=XXXX (tab 'Holds', select "yes" in the dropdown list and submit the form) * reserve/request.pl?biblionumber=XXXX (clic on the red cross) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Testing notes on second patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 23707 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23707&action=edit [PASSED QA] Bug 11336: (follow-up) Priority is not updated on deleting holds - moremember Patch does not fix issue when canceling hold from moremember.pl. This followup fixes that issue. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Preparations: - Create holds for different patrons on a record: * 1st - title level hold * 2nd - item level hold * 3rd - title level hold * 4th - title level hold - AllowOnShelfHolds = On/Allow (items were not checked out) Tests: Deleted holds from various pages, confirming bugs first, then testing with applied patches. Reloading database after each test. 1) Cancel holds from OPAC patron account /cgi-bin/koha/opac-user.pl#opac-user-holds - Cancel 4th - ok, before and after applying the patch - Cancel 2nd - ok, after applying the patch 2) Cancel hold from holds tab on staff detail page /cgi-bin/koha/reserve/request.pl?biblionumber=7 a) Setting priority to 'del', submitting with 'Update holds' - Cancel first (1st) - ok, before and after - Cancel hold in the middle (was 3rd) - ok, before and after - Cancel last (was 4th) -ok, before and after b) Using red X - Repeating tests from a) - before the patch is applied holds get totally 'out of order' - after applying the patch, it works correctly Additional tests done on this page: - Change priority using up, down, to top, to bottom icons - Change priority with 'toggle to lowest' 3) Cancel hold from the patron's account a) Check out tab - Delete? Yes, 'Cancel marked holds' /cgi-bin/koha/circ/circulation.pl?borrowernumber=X - Cancel first (1st) - ok, after applying the patch - Cancel hold in the middle (was 3rd) - ok, after applying the patch - Cancel last (was 4th) - ok, after applying the patch b) Details tab - Delete? yes, 'Cancel marked holds' /cgi-bin/koha/members/moremember.pl?borrowernumber=X - Cancel first (1st) - ok, after applying the patch - Cancel hold in the middle (was 3rd) - ok, after applying the patch - Cancel last (was 4th) - ok, after applying the patch Without the patch, holds priorities get out of order. Additional tests done: - Check in one item to trigger first hold - Check in one item to trigger second hold - Check out first item Priorities are kept while the item is waiting, when it's checked out, priorities of remaining holds get reset correctly. Conclusion: Big improvement, no regressions found. Passes all tests in t, xt and QA script. Also: t/db_dependent/Holds.t t/db_dependent/HoldsQueue.t t/db_dependent/Reserves.t -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |In Discussion --- Comment #18 from Galen Charlton <gmcharlt@gmail.com> --- (In reply to Jonathan Druart from comment #7)
(In reply to Kyle M Hall from comment #6)
I agree this is a bit of a dirty fix. Wouldn't it be better to modify GetReserve to look for the reserve in reserves first, and then old_reserves if it doesn't find it there?
Hum... It could work. But how to be sure this won't introduce a regression? For ex. a logic like: if GetReserve returns undef, the reserve is deleted.
Upon review, I prefer Jonathan's original approach. Yes, it's a bit "dirty", but that's mostly because _FixPriority() does not have all that great an interface: if it's passed a single $reserve_id, it's really meant to fix the priority for all holds on that hold's bib; if it's passed a rank and a $reserve_id, it touches that specific hold first. No other accessor routine currently falls back to looking in the old_* or deleted* table if it can't find a row in the main table, and I don't see a good reason to make GetReserve() different when all that is needed is to pass the biblionumber to _FixPriority. Also, making hold priority updates after cancellation contingent on there being a row in old_reserves opens up the possibility (admittedly, a remote one) of a race condition if anybody ever writes a cronjob that purges old_reserves. I suggest going back to Jonathan's original approach -- most of the test cases and the follow-up can be used as is, I suspect. For extra credit, it might be a good time to make _FixPriority accept a hashref, so that you can say: _FixPriority({ biblionumber => $foo}); vs. _FixPriority({ reserve_id => $foo, rank => 'del' }) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #19 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 23721 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23721&action=edit Bug 11336: WIP add GetOldReserve -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #20 from Jonathan Druart <jonathan.druart@biblibre.com> --- Is this patch serves your purpose? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #21 from Jonathan Druart <jonathan.druart@biblibre.com> --- No, I read too quickly. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23706|0 |1 is obsolete| | Attachment #23707|0 |1 is obsolete| | Attachment #23721|0 |1 is obsolete| | --- Comment #22 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 23787 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23787&action=edit Bug 11336: Priority is not updated on deleting holds There are a lot of places where a hold deletion is possible. But I just found 1 place where it works! To reproduce: - select or create 2 users U1 and U2 - select or create an holdable item - place on hold for both U1 and U2. U1 has priority 1 and U2 has priority 2. - delete the hold for U1 - go on circ/circulation.pl?borrowernumber=XXXX for U2 (or in the DB directly) and verify the priority has not been set to 1 The issue is repeatable (at least) on these 2 pages: * circ/circulation.pl?borrowernumber=XXXX (tab 'Holds', select "yes" in the dropdown list and submit the form) * reserve/request.pl?biblionumber=XXXX (clic on the red cross) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #23 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Patch tested with a sandbox, by Christopher Brannon <cbrannon@cdalibrary.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23787|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #24 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Created attachment 23831 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23831&action=edit Bug 11336: Priority is not updated on deleting holds There are a lot of places where a hold deletion is possible. But I just found 1 place where it works! To reproduce: - select or create 2 users U1 and U2 - select or create an holdable item - place on hold for both U1 and U2. U1 has priority 1 and U2 has priority 2. - delete the hold for U1 - go on circ/circulation.pl?borrowernumber=XXXX for U2 (or in the DB directly) and verify the priority has not been set to 1 The issue is repeatable (at least) on these 2 pages: * circ/circulation.pl?borrowernumber=XXXX (tab 'Holds', select "yes" in the dropdown list and submit the form) * reserve/request.pl?biblionumber=XXXX (clic on the red cross) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #25 from Christopher Brannon <cbrannon@cdalibrary.org> --- Works as defined. Christopher -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #26 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Working on this now... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #27 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Comment on attachment 23831 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23831 Bug 11336: Priority is not updated on deleting holds Review of attachment 23831: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=11336&attachment=23831) ----------------------------------------------------------------- ::: C4/Reserves.pm @@ +252,4 @@
$res = GetReserve( $reserve_id );
+ Return the current reserve or the old reserve.
This POD change no longer applies - please fix in a follow-up. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23831|0 |1 is obsolete| | --- Comment #28 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 23862 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23862&action=edit [PASSED QA] Bug 11336: Priority is not updated on deleting holds There are a lot of places where a hold deletion is possible. But I just found 1 place where it works! To reproduce: - select or create 2 users U1 and U2 - select or create an holdable item - place on hold for both U1 and U2. U1 has priority 1 and U2 has priority 2. - delete the hold for U1 - go on circ/circulation.pl?borrowernumber=XXXX for U2 (or in the DB directly) and verify the priority has not been set to 1 The issue is repeatable (at least) on these 2 pages: * circ/circulation.pl?borrowernumber=XXXX (tab 'Holds', select "yes" in the dropdown list and submit the form) * reserve/request.pl?biblionumber=XXXX (click on the red cross) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Reran my tests: Preparations: - Create holds for different patrons on a record: * 1st - title level hold * 2nd - item level hold * 3rd - title level hold * 4th - title level hold - AllowOnShelfHolds = On/Allow (items were not checked out) Tests: Deleted holds from various pages, confirming bugs first, then testing with applied patches. Reloading database after each test. 1) Cancel holds from OPAC patron account /cgi-bin/koha/opac-user.pl#opac-user-holds - Cancel 4th - ok, before and after applying the patch - Cancel 2nd - ok, after applying the patch 2) Cancel hold from holds tab on staff detail page /cgi-bin/koha/reserve/request.pl?biblionumber=7 a) Setting priority to 'del', submitting with 'Update holds' - Cancel first (1st) - ok, before and after - Cancel hold in the middle (was 3rd) - ok, before and after - Cancel last (was 4th) -ok, before and after b) Using red X - Repeating tests from a) - before the patch is applied holds get totally 'out of order' - after applying the patch, it works correctly Additional tests done on this page: - Change priority using up, down, to top, to bottom icons - Change priority with 'toggle to lowest' 3) Cancel hold from the patron's account a) Check out tab - Delete? Yes, 'Cancel marked holds' /cgi-bin/koha/circ/circulation.pl?borrowernumber=X - Cancel first (1st) - ok, after applying the patch - Cancel hold in the middle (was 3rd) - ok, after applying the patch - Cancel last (was 4th) - ok, after applying the patch b) Details tab - Delete? yes, 'Cancel marked holds' /cgi-bin/koha/members/moremember.pl?borrowernumber=X - Cancel first (1st) - ok, after applying the patch - Cancel hold in the middle (was 3rd) - ok, after applying the patch - Cancel last (was 4th) - ok, after applying the patch Without the patch, holds priorities get out of order. Additional tests done: - Check in one item to trigger first hold - Check in one item to trigger second hold - Check out first item Priorities are kept while the item is waiting, when it's checked out, priorities of remaining holds get reset correctly. Conclusion: Big improvement, no regressions found. Passes all tests in t, xt and QA script. Also: t/db_dependent/Holds.t t/db_dependent/HoldsQueue.t t/db_dependent/Reserves.t -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #29 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 23863 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23863&action=edit Bug 11336: Follow-up - fixing capitalization in templates Fixes capitalization on the holds and check in page. Changes are easy to spot using: git diff HEAD^ --color-words=. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #30 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Jonathan! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #31 from Jonathan Druart <jonathan.druart@biblibre.com> --- Galen, commit 543e1dc6731d4a26b777223920bfe4037e7546dd introduces a call to a nonexistent routine: =head2 ModReserve @@ -1184,7 +1181,7 @@ sub ModReserveFill { # now fix the priority on the others (if the priority wasn't # already sorted!).... unless ( $priority == 0 ) { - _FixPriority({ reserve_id => $reserve_id }); + FixPriority({ reserve_id => $reserve_id }); } } The FixPriority routine does not exist. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 --- Comment #32 from Jonathan Druart <jonathan.druart@biblibre.com> --- Fixed by commit 7af64ff7bd26eaa0d8cdb688f0db1526e438f087 Bug 11336: (follow-up) fix typo in previous follow-up -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11336 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |fridolyn.somers@biblibre.co | |m --- Comment #33 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Patches pushed to 3.14.x, will be in 3.14.4. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org