Bug 26057 added a 'cancellationdate' column to branchtransfers The code in pendingreserves.pl checks against transfers like: 'itembib.itemnumber' => { -not_in => \'SELECT itemnumber FROM branchtransfers WHERE datearrived IS NULL' } This needs to be updated to: 'itembib.itemnumber' => { -not_in => \'SELECT itemnumber FROM branchtransfers WHERE datearrived IS NULL AND datecancelled IS NULL' }
Created attachment 132935 [details] [review] Bug 30447: Check if transfers arrived or cancelled TO test: 1 - Find a bib with a single item 2 - Browse to Circulation->Transfer 3 - Transfer the item to another branch 4 - Check the item in at current branch and cancel the transfer on the dialog 5 - Place a hold on the biblio for a patron at current branch 6 - Browse to Circulation->Holds to pull 7 - Item does not show 8 - Apply patch, restart all 9 - Browse to Circulation->Holds to pull 10 - Item shows
Created attachment 132941 [details] [review] Bug 30447: Check if transfers arrived or cancelled TO test: 1 - Find a bib with a single item 2 - Browse to Circulation->Transfer 3 - Transfer the item to another branch 4 - Check the item in at current branch and cancel the transfer on the dialog 5 - Place a hold on the biblio for a patron at current branch 6 - Browse to Circulation->Holds to pull 7 - Item does not show 8 - Apply patch, restart all 9 - Browse to Circulation->Holds to pull 10 - Item shows Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Can you provide a test please?
Created attachment 136184 [details] [review] Bug 30447: Check if transfers arrived or cancelled TO test: 1 - Find a bib with a single item 2 - Browse to Circulation->Transfer 3 - Transfer the item to another branch 4 - Check the item in at current branch and cancel the transfer on the dialog 5 - Place a hold on the biblio for a patron at current branch 6 - Browse to Circulation->Holds to pull 7 - Item does not show 8 - Apply patch, restart all 9 - Browse to Circulation->Holds to pull 10 - Item shows Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 136185 [details] [review] Added test to ensure that Koha::Holds->get_items_that_can_fill returns items with datecancelled and datearrived
Created attachment 136186 [details] [review] Added test to ensure that Koha::Holds->get_items_that_can_fill returns items with datecancelled and datearrived
Created attachment 136187 [details] [review] Added test to ensure that Koha::Holds->get_items_that_can_fill returns items with either datecancelled or datearrived (inclusive). I made an error with my understanding of my boolean logic. Also cleaned up the variables in the test by removing unblessed and using the variable base forms.
Created attachment 136198 [details] [review] Continued from previous patch: Added test for correct number of items for each call of "get_items_that_fill"
Created attachment 136199 [details] [review] Bug 30447: Check if transfers arrived or cancelled TO test: 1 - Find a bib with a single item 2 - Browse to Circulation->Transfer 3 - Transfer the item to another branch 4 - Check the item in at current branch and cancel the transfer on the dialog 5 - Place a hold on the biblio for a patron at current branch 6 - Browse to Circulation->Holds to pull 7 - Item does not show 8 - Apply patch, restart all 9 - Browse to Circulation->Holds to pull 10 - Item shows Test in Holds.t Added test to ensure that Koha::Holds->get_items_that_can_fill returns items with datecancelled and datearrived Also cleaned up the variables in the test by removing unblessed and using the variable base forms.
Created attachment 136200 [details] [review] Bug 30447: Check if transfers arrived or cancelled TO test: 1 - Find a bib with a single item 2 - Browse to Circulation->Transfer 3 - Transfer the item to another branch 4 - Check the item in at current branch and cancel the transfer on the dialog 5 - Place a hold on the biblio for a patron at current branch 6 - Browse to Circulation->Holds to pull 7 - Item does not show 8 - Apply patch, restart all 9 - Browse to Circulation->Holds to pull 10 - Item shows Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 136201 [details] [review] Bug 30447: Unit tests Added tests to ensure that Koha::Holds->get_items_that_can_fill returns items with datecancelled and datearrived Added test for correct number of items for each call of "get_items_that_fill"
Shouldn't we want both using the same code (a filter_by method in Koha::Item::Transfers)?
(In reply to Jonathan Druart from comment #12) > Shouldn't we want both using the same code (a filter_by method in > Koha::Item::Transfers)? Yes, that would be a good goal. I will move that patch forward when you write it ;-) This one fixes the bug and backports nicely for now, can it move forward?
Created attachment 137704 [details] [review] Bug 30447: Check if transfers arrived or cancelled TO test: 1 - Find a bib with a single item 2 - Browse to Circulation->Transfer 3 - Transfer the item to another branch 4 - Check the item in at current branch and cancel the transfer on the dialog 5 - Place a hold on the biblio for a patron at current branch 6 - Browse to Circulation->Holds to pull 7 - Item does not show 8 - Apply patch, restart all 9 - Browse to Circulation->Holds to pull 10 - Item shows Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 137705 [details] [review] Bug 30447: Unit tests Added tests to ensure that Koha::Holds->get_items_that_can_fill returns items with datecancelled and datearrived Added test for correct number of items for each call of "get_items_that_fill" Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Good catch. I've still got a second phase of transfers cleanup I'll get back to some time soon and will make sure we add filter methods there so I don't think we need to hold back this simple to backport version. Works as expected and has tests. Passing QA
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.05
thx pushed to 21.11.x for 21.11.12
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document, marking resolved.