[Bug 19260] New: Reservations / holds marked as problems being seen as expired ones and deleted wrongly.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Bug ID: 19260 Summary: Reservations / holds marked as problems being seen as expired ones and deleted wrongly. Change sponsored?: --- Product: Koha Version: 17.05 Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: r.delahunty@arts.ac.uk QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com We have the ReservesMaxPickupDelay set to 7 and the ExpireReservesMaxPickupDelay system parameter set to Don’t Allow. Our workflow is that staff use the report generated by waitingreserves.pl to identify titles to retrieve from the Reservations / holds awaiting pickup shelf, and delete the reservation and return the items to the shelf, or have that action trigger reservation satisfaction for the next person in the queue. Since our upgrade for 16.05 to 17.05 we are finding that when we run the cancel_expired_holds.pl job at 01:00 daily, reservations marked as problem ones are being deleted in addition to expired ones (expiration date set by user has been reached, or the default 100 days has been reached). Koha 17.05 appears to be disregarding the ExpireReservesMaxPickupDelay system parameter. (http://translate.koha-community.org/manual/master/en/html/17_cron_jobs.html#...). We have suspended this cron job in the meantime, and will write a query to identify expired ones we need to delete. Ideally we want problem reservations NOT to be seen as expired ones. Ray Delahunty -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@libriotech.no --- Comment #1 from Magnus Enger <magnus@libriotech.no> --- As far as I can tell, this is a regression introduced by bug 12063. Attachment 63091 on that bug moves the code that takes care of ExpireReservesMaxPickupDelay from C4/Reserves.pm (sub CancelExpiredReserves): - if ( C4::Context->preference("ExpireReservesMaxPickUpDelay") ) { - my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); - my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); to Koha/Hold.pm (sub set_waiting): + if ( C4::Context->preference("ExpireReservesMaxPickUpDelay") ) { + my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); + my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); + my $calendar = Koha::Calendar->new( branchcode => $self->branchcode ); Then attachment 63095 removes the check on ExpireReservesMaxPickUpDelay: - if ( C4::Context->preference("ExpireReservesMaxPickUpDelay") ) { - my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); - my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); - my $calendar = Koha::Calendar->new( branchcode => $self->branchcode ); ... + my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); + my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); + my $calendar = Koha::Calendar->new( branchcode => $self->branchcode ); But it is not clear from the commit message that this is intentional... Perhaps the fix is as easy as putting the if on ExpireReservesMaxPickUpDelay back in? I will investigate this further, if noone beats me to it. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #2 from Magnus Enger <magnus@libriotech.no> --- Created attachment 67428 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67428&action=edit Bug 19260 - Restore ExpireReservesMaxPickupDelay It looks like bug 12063 accidentally removed the functionality of ExpireReservesMaxPickupDelay. This patch aims to restore it. To test: Before applying the patch: - Make sure ExpireReservesMaxPickUpDelay = "Don't allow" - Check out a book to a user - Add a reservastion on it for another user - Check the hold in the database. You should have: found = W, waitingdate = today, expirationdate = today + the value of ReservesMaxPickUpDelay The problem here is the expirationdate, it should be empty when ExpireReservesMaxPickUpDelay = "Don't allow". After applying the patch: - Repeat the steps above - expirationdate should now be NULL Please be creative when testing this. I am not sure I have seen all the possible concequences. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|17.05 |unspecified Assignee|koha-bugs@lists.koha-commun |magnus@libriotech.no |ity.org | Severity|enhancement |major CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |12063 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12063 [Bug 12063] Change date calculation for reserve expiration to skip all holidays -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- We will need tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67428|0 |1 is obsolete| | --- Comment #4 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Created attachment 67489 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67489&action=edit Bug 19260 - Restore ExpireReservesMaxPickupDelay It looks like bug 12063 accidentally removed the functionality of ExpireReservesMaxPickupDelay. This patch aims to restore it. To test: Before applying the patch: - Make sure ExpireReservesMaxPickUpDelay = "Don't allow" - Check out a book to a user - Add a reservastion on it for another user - Check the hold in the database. You should have: found = W, waitingdate = today, expirationdate = today + the value of ReservesMaxPickUpDelay The problem here is the expirationdate, it should be empty when ExpireReservesMaxPickUpDelay = "Don't allow". After applying the patch: - Repeat the steps above - expirationdate should now be NULL Please be creative when testing this. I am not sure I have seen all the possible concequences. Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |caroline.cyr-la-rose@inlibr | |o.com --- Comment #5 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Works as advertised. There was a step missing from the test plan, however, you have to return the reserved item for the db to show W in "found" field and (today's) date in "waiting date". Once I figured that out, everything worked as you said. :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67489|0 |1 is obsolete| | Attachment #67489|1 |0 is patch| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |ASSIGNED CC| |josef.moravec@gmail.com --- Comment #6 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Magnus Enger from comment #2)
Created attachment 67428 [details] [review] Bug 19260 - Restore ExpireReservesMaxPickupDelay
It looks like bug 12063 accidentally removed the functionality of ExpireReservesMaxPickupDelay. This patch aims to restore it.
To test:
Before applying the patch: - Make sure ExpireReservesMaxPickUpDelay = "Don't allow" - Check out a book to a user - Add a reservastion on it for another user - Check the hold in the database. You should have: found = W, waitingdate = today, expirationdate = today + the value of ReservesMaxPickUpDelay
The problem here is the expirationdate, it should be empty when ExpireReservesMaxPickUpDelay = "Don't allow".
After applying the patch: - Repeat the steps above - expirationdate should now be NULL
Please be creative when testing this. I am not sure I have seen all the possible concequences.
I don't think, that is the right place to fix it... after 12063, we really need a date in expirationdate I think... ExpireReservesMaxPickUpDelay just has to say, if cancel also waiting reserves or not... Attachment 63095 Bug 12063 - Fix QA failures added this to C4::Reserves.pm + sub CancelExpiredReserves { + return unless C4::Context->preference("ExpireReservesMaxPickUpDelay"); After this Marcel's comment: sub CancelExpiredReserves { + return unless C4::Context->preference("ExpireReservesMaxPickUpDelay"); This may have a unwanted side-effect. If we do not use the pickup delay, but still want to cancel expired reserves. It is no longer possible. (Note that patrons may have entered expiration dates too.) Not marking this as a blocker, since this is probably exceptional. Do you have an easy fix or can you open a new report for it? The return line was removed... The comment is right, when ExpireReservesMaxPickUpDelay is false, then no reserve is canceled - even that which should be (not waiting bud after expiration date) But the solution should be more complex (and yes, we will need tests for this) The CancelExpiredReserved has to deal with ExpireReservesMaxPickUpDelay - nad use the different SQL for this two cases. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Jessie Zairo <jzairo@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jzairo@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #7 from Magnus Enger <magnus@libriotech.no> --- (In reply to Josef Moravec from comment #6)
But the solution should be more complex (and yes, we will need tests for this)
The CancelExpiredReserved has to deal with ExpireReservesMaxPickUpDelay - nad use the different SQL for this two cases.
Could you make an alternative patch for that? I will sign off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #8 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 67653 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67653&action=edit Bug 19260: Add test for CancelExpiredReserves -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #9 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 67654 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67654&action=edit Bug 19260: Holds marked as problems being seen as expired ones and deleted wrongly Test plan: 0) Apply just the first patch - the one with test 1) Run t/db_dependent/Reserves.t - test for CancelExpiredReserves should fail 2) Apply the second patch 3) t/db_dependent/Reserves.t should pass now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|magnus@libriotech.no |josef.moravec@gmail.com --- Comment #10 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Magnus Enger from comment #7)
(In reply to Josef Moravec from comment #6)
But the solution should be more complex (and yes, we will need tests for this)
The CancelExpiredReserved has to deal with ExpireReservesMaxPickUpDelay - nad use the different SQL for this two cases.
Could you make an alternative patch for that? I will sign off.
I did try ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67653|0 |1 is obsolete| | --- Comment #11 from Alex Buckley <alexbuckley@catalyst.net.nz> --- Created attachment 67659 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67659&action=edit Bug 19260: Add test for CancelExpiredReserves Passes QA test tool Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67654|0 |1 is obsolete| | --- Comment #12 from Alex Buckley <alexbuckley@catalyst.net.nz> --- Created attachment 67660 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67660&action=edit Bug 19260: Holds marked as problems being seen as expired ones and deleted wrongly Test plan: 0) Apply just the first patch - the one with test 1) Run t/db_dependent/Reserves.t - test for CancelExpiredReserves should fail 2) Apply the second patch 3) t/db_dependent/Reserves.t should pass now Followed test plan, patch worked as described. Passes QA test tool Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alexbuckley@catalyst.net.nz --- Comment #13 from Alex Buckley <alexbuckley@catalyst.net.nz> --- Oh Sorry Magnus I just saw your comment that you would test and sign off on these patches, apologies that I tested and found that everything worked and passed the QA test tool so I signed off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #14 from Magnus Enger <magnus@libriotech.no> --- (In reply to Alex Buckley from comment #13)
Oh Sorry Magnus I just saw your comment that you would test and sign off on these patches, apologies that I tested and found that everything worked and passed the QA test tool so I signed off
Hehe, absolutely no need to apologize for that! :-) Just very happy to see this moving along. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |master QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67659|0 |1 is obsolete| | --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 67677 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67677&action=edit Bug 19260: Add test for CancelExpiredReserves Passes QA test tool Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67660|0 |1 is obsolete| | --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 67678 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67678&action=edit Bug 19260: Holds marked as problems being seen as expired ones and deleted wrongly Test plan: 0) Apply just the first patch - the one with test 1) Run t/db_dependent/Reserves.t - test for CancelExpiredReserves should fail 2) Apply the second patch 3) t/db_dependent/Reserves.t should pass now Followed test plan, patch worked as described. Passes QA test tool Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 67679 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67679&action=edit Bug 19260: [QA Follow-up] Rearranging tests The first patch adds CancelExpiredReserves tests to Reserves.t. But note that we already have some tests in Holds/CancelReserves.t. This patch does: Renames Holds/CancelReserves.t to Reserves/CancelExpiredReserves.t. Rearranges modules there. Moves its existing tests into a first subtest. Moves the new subtest from Reserves.t to CancelExpiredReserves.t. Replaces $dbh->do('DELETE FROM reserves'). Adds some TestBuilder statements for missing data (by the move): adding biblio, item, borrower (removing slow AddMember call). Test plan: Run Reserves.t and Reserves/CancelExpiredReserves.t. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 67680 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67680&action=edit Bug 19260: [QA Follow-up] Remove obsolete $dbh The variable is no longer used. Removed a few empty lines on the way. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Kelly McElligott <kelly@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kelly@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67679|0 |1 is obsolete| | --- Comment #20 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Comment on attachment 67679 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67679 Bug 19260: [QA Follow-up] Rearranging tests I prefer to see this change done on a separate bug report. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #21 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 17.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #22 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Two tests are failing, please fix ASAP t/db_dependent/Holds/CancelReserves.t .. 1/5 # Failed test 'reserve 2 should be canceled.' # at t/db_dependent/Holds/CancelReserves.t line 61. # got: 'Koha::Hold=HASH(0x9fbec48)' # expected: undef # Failed test 'Reserve 3 should be canceled.' # at t/db_dependent/Holds/CancelReserves.t line 96. # got: 'Koha::Hold=HASH(0x9f3a750)' # expected: undef # Looks like you failed 2 tests of 5. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67679|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67679|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #23 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #18)
Created attachment 67679 [details] [review] Bug 19260: [QA Follow-up] Rearranging tests
Tests fail even with this patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #24 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 67733 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67733&action=edit Bug 19260: (followup) Fix CancelReserves.t test Test plan: Run t/db_dependent/Holds/CancelReserves.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #25 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Thanks Josef, patch push to master! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #26 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #23)
(In reply to Marcel de Rooy from comment #18)
Created attachment 67679 [details] [review] [review] Bug 19260: [QA Follow-up] Rearranging tests
Tests fail even with this patch.
Yes, as Joseph mentioned, ExpireReservesMaxPickUpDelay should be Allow (as was my case already). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |19437 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19437 [Bug 19437] Orphaned QA follow-up: Rearrange CancelExpiredReserves tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #27 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #20)
Comment on attachment 67679 [details] [review] Bug 19260: [QA Follow-up] Rearranging tests
I prefer to see this change done on a separate bug report.
See bug 19437 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #28 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Patch "Holds marked as problems being seen as expired ones and deleted wrongly" does not apply easily to 17.05.x. Please provide a rebased patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #29 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 67992 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67992&action=edit [17.05] Bug 19260: Add test for CancelExpiredReserves Passes QA test tool Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #30 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 67993 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67993&action=edit [17.05] Bug 19260: Holds marked as problems being seen as expired ones and deleted wrongly Test plan: 0) Apply just the first patch - the one with test 1) Run t/db_dependent/Reserves.t - test for CancelExpiredReserves should fail 2) Apply the second patch 3) t/db_dependent/Reserves.t should pass now Followed test plan, patch worked as described. Passes QA test tool Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #31 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 67994 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67994&action=edit [17.05] Bug 19260: [QA Follow-up] Remove obsolete $dbh The variable is no longer used. Removed a few empty lines on the way. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #32 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 67995 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67995&action=edit [17.05] Bug 19260: (followup) Fix CancelReserves.t test Test plan: Run t/db_dependent/Holds/CancelReserves.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 --- Comment #33 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Fridolin SOMERS from comment #28)
Patch "Holds marked as problems being seen as expired ones and deleted wrongly" does not apply easily to 17.05.x. Please provide a rebased patch.
I rebased whole patchset, qa tools passes, test passes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #34 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Pushed to 17.05.x, will be in 17.05.05. UT fails without correction and success with it. Thanks a lot Josef. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #35 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Depends on bug 12063 that is not in 16.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19260 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to Stable |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org