[Bug 40671] New: Rename `revert_waiting` => `revert_found` in Koha::Hold
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 Bug ID: 40671 Summary: Rename `revert_waiting` => `revert_found` in Koha::Hold Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org The original method -RevertWaitingStatus- was actually used to revert any `found` status, not just waiting. The `Koha::Hold->revert_waiting()` method implemented on bug 40058 throws an exception if the hold is not waiting. This causes errors in a couple places (reverting in transit holds, reverting in processing holds, etc). We need to document things better and cover all the use cases with tests to avoid this. Will do my best. -- 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=40671 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Severity|normal |major Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.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=40671 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |blocker -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emily.lamancusa@montgomeryc | |ountymd.gov, | |martin.renvoize@openfifth.c | |o.uk Depends on| |40058 Status|NEW |ASSIGNED Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40058 [Bug 40058] Move RevertWaitingStatus to Koha::Hold->revert_waiting() -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Rename `revert_waiting` => |Expand |`revert_found` in |Koha::Hold->revert_waiting |Koha::Hold |to handle all found | |statuses -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch 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=40671 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 185541 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185541&action=edit Bug 40671: Refactor Koha::Hold->revert_waiting into revert_found This patch refactors the revert_waiting method to handle all found statuses instead of just waiting holds, making it more versatile and consistent. Changes: * Renamed method from revert_waiting() to revert_found() * Expanded functionality to handle all found statuses (W, T, P) * Updated exception logic to check is_found instead of is_waiting * Changed exception message from hold_not_waiting to hold_not_found * Added comprehensive tests for all found statuses * Updated POD documentation to reflect new behavior The method now reverts any hold with a found status back to a regular hold with priority 1, regardless of whether it was waiting, in transit, or in processing. To test: 1. Apply the patches 2. Run tests: $ ktd --shell k$ cd /kohadevbox/koha k$ prove t/db_dependent/Koha/Hold.t => SUCCESS: All tests pass including new subtests for T and P statuses 3. Verify all hold test suites pass: k$ prove t/db_dependent/Koha/Hold*.t => SUCCESS: All hold-related tests pass 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 185542 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185542&action=edit Bug 40671: Update method calls from revert_waiting to revert_found This patch updates all existing calls to the renamed method throughout the codebase to maintain compatibility. Changes: * Updated C4::Circulation calls to use revert_found() * Updated C4::Reserves calls to use revert_found() * Updated test files to use revert_found() * Maintained all existing functionality and behavior All existing functionality is preserved - this is purely a method name change to reflect the expanded capability. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |40672 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40672 [Bug 40672] `desk_id` not cleared when `revert_found()` called -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #185541|0 |1 is obsolete| | --- Comment #3 from Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 185562 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185562&action=edit Bug 40671: Refactor Koha::Hold->revert_waiting into revert_found This patch refactors the revert_waiting method to handle all found statuses instead of just waiting holds, making it more versatile and consistent. Changes: * Renamed method from revert_waiting() to revert_found() * Expanded functionality to handle all found statuses (W, T, P) * Updated exception logic to check is_found instead of is_waiting * Changed exception message from hold_not_waiting to hold_not_found * Added comprehensive tests for all found statuses * Updated POD documentation to reflect new behavior The method now reverts any hold with a found status back to a regular hold with priority 1, regardless of whether it was waiting, in transit, or in processing. To test: 1. Apply the patches 2. Run tests: $ ktd --shell k$ cd /kohadevbox/koha k$ prove t/db_dependent/Koha/Hold.t => SUCCESS: All tests pass including new subtests for T and P statuses 3. Verify all hold test suites pass: k$ prove t/db_dependent/Koha/Hold*.t => SUCCESS: All hold-related tests pass 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #185542|0 |1 is obsolete| | --- Comment #4 from Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 185563 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185563&action=edit Bug 40671: Update method calls from revert_waiting to revert_found This patch updates all existing calls to the renamed method throughout the codebase to maintain compatibility. Changes: * Updated C4::Circulation calls to use revert_found() * Updated C4::Reserves calls to use revert_found() * Updated test files to use revert_found() * Maintained all existing functionality and behavior All existing functionality is preserved - this is purely a method name change to reflect the expanded capability. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> 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=40671 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |21572 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21572 [Bug 21572] Improve flexibility and consistency of AllowItemsOnHoldCheckoutSIP and AllowItemsOnHoldCheckoutSCO -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |BLOCKED QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- * Proving /usr/share/koha/t/db_dependent/SIP/Transaction.t FAIL # Failed test 'The fine is displayed on checkin when show_outstanding_amount is enabled' # at /usr/share/koha/t/db_dependent/SIP/Transaction.t line 706. # got: 'You owe 12.00yqsA for this item.' # expected: 'You owe $12.00 for this item.' # Looks like you failed 1 test of 2. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Will fix that one here now. Stumbled over it somewhere else recently too.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #185562|0 |1 is obsolete| | --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 185883 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185883&action=edit Bug 40671: Refactor Koha::Hold->revert_waiting into revert_found This patch refactors the revert_waiting method to handle all found statuses instead of just waiting holds, making it more versatile and consistent. Changes: * Renamed method from revert_waiting() to revert_found() * Expanded functionality to handle all found statuses (W, T, P) * Updated exception logic to check is_found instead of is_waiting * Changed exception message from hold_not_waiting to hold_not_found * Added comprehensive tests for all found statuses * Updated POD documentation to reflect new behavior The method now reverts any hold with a found status back to a regular hold with priority 1, regardless of whether it was waiting, in transit, or in processing. To test: 1. Apply the patches 2. Run tests: $ ktd --shell k$ cd /kohadevbox/koha k$ prove t/db_dependent/Koha/Hold.t => SUCCESS: All tests pass including new subtests for T and P statuses 3. Verify all hold test suites pass: k$ prove t/db_dependent/Koha/Hold*.t => SUCCESS: All hold-related tests pass 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> 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=40671 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #185563|0 |1 is obsolete| | --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 185884 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185884&action=edit Bug 40671: Update method calls from revert_waiting to revert_found This patch updates all existing calls to the renamed method throughout the codebase to maintain compatibility. Changes: * Updated C4::Circulation calls to use revert_found() * Updated C4::Reserves calls to use revert_found() * Updated test files to use revert_found() * Maintained all existing functionality and behavior All existing functionality is preserved - this is purely a method name change to reflect the expanded capability. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> 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=40671 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 185885 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185885&action=edit Bug 40671: (QA follow-up) Fix hardcoded currency symbol It could be before or after the amount. Assuming that it does not contain digits itself ;) 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=40671 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |25.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 --- Comment #11 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 25.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|25.11.00 |25.11.00,25.05.04 released in| | Status|Pushed to main |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 --- Comment #12 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 25.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Needs documenting CC| |fridolin.somers@biblibre.co | |m --- Comment #13 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 40058 not in 24.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40671 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Needs documenting |RESOLVED CC| |david@davidnind.com --- Comment #14 from David Nind <david@davidnind.com> --- No changes to the manual required. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org