This method should be moved to the Koha:: namespace.
Created attachment 182924 [details] [review] Bug 40058: Add Koha::Hold->revert_waiting() This patch adds a new method to the `Koha::Hold` class. The method follows the original implementation in C4::Reserves. Tests are added to cover all the code paths. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Hold.t => SUCCESS: Tests pass :-D 3. Sign off :-D
Created attachment 182925 [details] [review] Bug 40058: Remove RevertWaitingStatus()
Created attachment 182926 [details] [review] Bug 40058: Adjust existing tests
Created attachment 182941 [details] [review] Bug 40058: Add Koha::Hold->revert_waiting() This patch adds a new method to the `Koha::Hold` class. The method follows the original implementation in C4::Reserves. Tests are added to cover all the code paths. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Hold.t => SUCCESS: Tests pass :-D 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com>
Created attachment 182942 [details] [review] Bug 40058: Remove RevertWaitingStatus() Signed-off-by: David Nind <david@davidnind.com>
Created attachment 182943 [details] [review] Bug 40058: Adjust existing tests Signed-off-by: David Nind <david@davidnind.com>
Created attachment 184980 [details] [review] Bug 40058: Add Koha::Hold->revert_waiting() This patch adds a new method to the `Koha::Hold` class. The method follows the original implementation in C4::Reserves. Tests are added to cover all the code paths. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Hold.t => SUCCESS: Tests pass :-D 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Created attachment 184981 [details] [review] Bug 40058: Remove RevertWaitingStatus() Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Created attachment 184982 [details] [review] Bug 40058: Adjust existing tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Nice work everyone! Pushed to main for 25.11
I'm afraid this has introduced a few new bugs - there are a few places in the code where RevertWaitingStatus was being used to revert other found statuses, which now throw an error 500: - in request.tt, the "Revert waiting status" button's text changes to "Revert transit status" if the hold is in-transit, but under the hood it's the same code that calls revert_waiting either way - when checking out an item with a found hold, if the user chooses to revert the status of the hold and check out the item, revert_waiting gets called regardless of which found status the item has Any reason not to name the method revert_found and explicitly allow it to revert any found status?
(In reply to Emily Lamancusa (emlam) from comment #11) > I'm afraid this has introduced a few new bugs - there are a few places in > the code where RevertWaitingStatus was being used to revert other found > statuses, which now throw an error 500: > > - in request.tt, the "Revert waiting status" button's text changes to > "Revert transit status" if the hold is in-transit, but under the hood it's > the same code that calls revert_waiting either way > > - when checking out an item with a found hold, if the user chooses to revert > the status of the hold and check out the item, revert_waiting gets called > regardless of which found status the item has > > Any reason not to name the method revert_found and explicitly allow it to > revert any found status? Good catch. This is why we need 100% tests coverage in the code. I'll provide a follow-up in a separate bug.