[Bug 43234] New: Move ILL request purging logic to Koha::ILL::Requests
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43234 Bug ID: 43234 Summary: Move ILL request purging logic to Koha::ILL::Requests Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: ILL Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org CC: lisette@bywatersolutions.com, pedro.amorim@openfifth.co.uk, tomascohen@gmail.com Depends on: 28740 Target Milestone: --- Follow-up from Bug 28740. Bug 28740 added ILL request purging to cleanup_database.pl using raw SQL in a PurgeIllRequests subroutine. This should be moved to a proper method on the Koha::ILL::Requests collection class. Proposed approach: - Add a filter method (e.g. filter_by_purgeable) that returns a filtered resultset - The script then calls ->count for dry-run or ->delete for actual purge - This follows the pattern used by other Koha::Objects subclasses Example usage in cleanup_database.pl: my $to_purge = Koha::ILL::Requests->filter_by_purgeable( { days => $days, statuses => \\@statuses } ); say sprintf('%d ILL requests would be removed', $to_purge->count) unless $confirm; $to_purge->delete if $confirm; Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28740 [Bug 28740] Delete old ILL requests with cleanup_database -- 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=43234 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> 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=43234 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203207 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203207&action=edit Bug 43234: Add Koha::ILL::Requests->filter_by_purgeable This patch adds a filter_by_purgeable() method to Koha::ILL::Requests that returns a filtered resultset of ILL requests eligible for purging based on status and age. The method accepts: - days: number of days since last update - statuses: arrayref of status codes to match The caller can use ->count for reporting or ->delete to purge, following the standard Koha::Objects pattern. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/ILL/Requests.t => SUCCESS: Tests pass 3. Tests cover: - Single status filter (COMP older than 30 days) - Multiple statuses (COMP + KILL) - Recent requests excluded by age threshold - Throws MissingParameter without days - Throws MissingParameter without statuses - Throws MissingParameter with empty statuses array 4. Sign off :-D -- 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=43234 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203208 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203208&action=edit Bug 43234: (follow-up) Refactor cleanup_database.pl to use filter_by_purgeable This patch refactors the ILL request purging in cleanup_database.pl to use the new Koha::ILL::Requests->filter_by_purgeable method instead of the raw SQL PurgeIllRequests subroutine. Changes: - Replace PurgeIllRequests() call with filter_by_purgeable + count/delete - Remove the PurgeIllRequests subroutine (raw SQL) - Add use Koha::ILL::Requests import The behavior is identical: dry-run reports count, --confirm deletes. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ perl misc/cronjobs/cleanup_database.pl --illrequests-days 30 \ --illrequests-status COMP -v => SUCCESS: Reports count of requests that would be removed 3. Run with --confirm: k$ perl misc/cronjobs/cleanup_database.pl --illrequests-days 30 \ --illrequests-status COMP --confirm -v => SUCCESS: Requests are deleted, count reported 4. Run unit tests: k$ prove t/db_dependent/Koha/ILL/Requests.t => SUCCESS: Tests pass 5. Sign off :-D -- 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=43234 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43234 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- 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=43234 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #203207|0 |1 is obsolete| | Attachment #203208|0 |1 is obsolete| | -- 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=43234 --- Comment #3 from David Nind <david@davidnind.com> --- Created attachment 203379 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203379&action=edit Bug 43234: Add Koha::ILL::Requests->filter_by_purgeable This patch adds a filter_by_purgeable() method to Koha::ILL::Requests that returns a filtered resultset of ILL requests eligible for purging based on status and age. The method accepts: - days: number of days since last update - statuses: arrayref of status codes to match The caller can use ->count for reporting or ->delete to purge, following the standard Koha::Objects pattern. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/ILL/Requests.t => SUCCESS: Tests pass 3. Tests cover: - Single status filter (COMP older than 30 days) - Multiple statuses (COMP + KILL) - Recent requests excluded by age threshold - Throws MissingParameter without days - Throws MissingParameter without statuses - Throws MissingParameter with empty statuses array 4. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- 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=43234 --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 203380 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203380&action=edit Bug 43234: (follow-up) Refactor cleanup_database.pl to use filter_by_purgeable This patch refactors the ILL request purging in cleanup_database.pl to use the new Koha::ILL::Requests->filter_by_purgeable method instead of the raw SQL PurgeIllRequests subroutine. Changes: - Replace PurgeIllRequests() call with filter_by_purgeable + count/delete - Remove the PurgeIllRequests subroutine (raw SQL) - Add use Koha::ILL::Requests import The behavior is identical: dry-run reports count, --confirm deletes. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ perl misc/cronjobs/cleanup_database.pl --illrequests-days 30 \ --illrequests-status COMP -v => SUCCESS: Reports count of requests that would be removed 3. Run with --confirm: k$ perl misc/cronjobs/cleanup_database.pl --illrequests-days 30 \ --illrequests-status COMP --confirm -v => SUCCESS: Requests are deleted, count reported 4. Run unit tests: k$ prove t/db_dependent/Koha/ILL/Requests.t => SUCCESS: Tests pass 5. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- 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=43234 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #5 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Enable ILL - set ILLModule system preference = Enable 2. Create several requests (I created 4) 3. Apply patches (including dependent bug 28740) 4. Restart everything: restart_all 5. Update the requests in the database so that the requests you created are completed and over 30 days old: - koha-mysql kohadev - update illrequests set updated = "2026-06-01 00:00:00"; - update illrequests set status = "COMP"; 6. Output from running the clean up script: perl misc/cronjobs/cleanup_database.pl --illrequests-days 30 --illrequests-status COMP -v Confirm flag not passed, running in dry-run mode... Purging unaccepted list share invites older than 14 days. Done with purging unaccepted list share invites. Start lock unsubscribed, anonymize and delete 0 patrons would have been locked 0 patrons would have been anonymized 0 patrons would have been deleted Purging ILL requests with statuses COMP older than 30 days. 4 ILL requests would have been removed 7. Run the script with --confirm: perl misc/cronjobs/cleanup_database.pl --illrequests-days 30 --illrequests-status COMP --confirm -v Purging unaccepted list share invites older than 14 days. Done with purging unaccepted list share invites. Start lock unsubscribed, anonymize and delete Locked 0 patrons Anonymized 0 patrons Deleted 0 patrons Purging ILL requests with statuses COMP older than 30 days. Done with purging 4 ILL requests. 8. Tests pass: prove t/db_dependent/Koha/ILL/Requests.t -- 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=43234 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- 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=43234 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |lisette@bywatersolutions.co |y.org |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43234 Lisette Scheer <lisette@bywatersolutions.com> 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=43234 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #203379|0 |1 is obsolete| | Attachment #203380|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=43234 --- Comment #6 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 205366 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205366&action=edit Bug 43234: Add Koha::ILL::Requests->filter_by_purgeable This patch adds a filter_by_purgeable() method to Koha::ILL::Requests that returns a filtered resultset of ILL requests eligible for purging based on status and age. The method accepts: - days: number of days since last update - statuses: arrayref of status codes to match The caller can use ->count for reporting or ->delete to purge, following the standard Koha::Objects pattern. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/ILL/Requests.t => SUCCESS: Tests pass 3. Tests cover: - Single status filter (COMP older than 30 days) - Multiple statuses (COMP + KILL) - Recent requests excluded by age threshold - Throws MissingParameter without days - Throws MissingParameter without statuses - Throws MissingParameter with empty statuses array 4. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43234 --- Comment #7 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 205367 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205367&action=edit Bug 43234: (follow-up) Refactor cleanup_database.pl to use filter_by_purgeable This patch refactors the ILL request purging in cleanup_database.pl to use the new Koha::ILL::Requests->filter_by_purgeable method instead of the raw SQL PurgeIllRequests subroutine. Changes: - Replace PurgeIllRequests() call with filter_by_purgeable + count/delete - Remove the PurgeIllRequests subroutine (raw SQL) - Add use Koha::ILL::Requests import The behavior is identical: dry-run reports count, --confirm deletes. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ perl misc/cronjobs/cleanup_database.pl --illrequests-days 30 \ --illrequests-status COMP -v => SUCCESS: Reports count of requests that would be removed 3. Run with --confirm: k$ perl misc/cronjobs/cleanup_database.pl --illrequests-days 30 \ --illrequests-status COMP --confirm -v => SUCCESS: Requests are deleted, count reported 4. Run unit tests: k$ prove t/db_dependent/Koha/ILL/Requests.t => SUCCESS: Tests pass 5. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43234 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |26.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43234 --- Comment #8 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Thanks everyone! Pushed to main for 26.11! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43234 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Pushed to main |RESOLVED Resolution|--- |FIXED --- Comment #9 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Enhancement, no 26.05.x backport -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org