| Summary: | Move RevertWaitingStatus to Koha::Hold->revert_waiting() | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Component: | Architecture, internals, and plumbing | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Status: | Pushed to stable --- | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | emily.lamancusa, kyle, martin.renvoize, tomascohen |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40062 | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Crowdfunding goal: | 0 |
| Patch complexity: | --- | Documentation contact: | |
| Documentation submission: | Text to go in the release notes: |
This development refactors and existing function into a class method. The idea is that the code gets simplified, and also improve performance as the method acts on the already instantiated object instead of fetching it from the DB, internally.
|
|
| Version(s) released in: |
25.11.00,25.05.04
|
Circulation function: | |
| Bug Depends on: | 40055 | ||
| Bug Blocks: | 17728, 40671 | ||
| Attachments: |
Bug 40058: Add Koha::Hold->revert_waiting()
Bug 40058: Remove RevertWaitingStatus() Bug 40058: Adjust existing tests Bug 40058: Add Koha::Hold->revert_waiting() Bug 40058: Remove RevertWaitingStatus() Bug 40058: Adjust existing tests Bug 40058: Add Koha::Hold->revert_waiting() Bug 40058: Remove RevertWaitingStatus() Bug 40058: Adjust existing tests |
||
|
Description
Tomás Cohen Arazi (tcohen)
2025-06-03 13:01:30 UTC
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. Nice work everyone! Pushed to 25.05.x |