[Bug 43604] New: commit_file.pl --revert dies: safe_to_delete calls can_edit_items_from on undef
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43604 Bug ID: 43604 Summary: commit_file.pl --revert dies: safe_to_delete calls can_edit_items_from on undef Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Tools Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org Target Milestone: --- This fixes a regression introduced by bug 33070, where Koha::Item->safe_to_delete assumed C4::Context->userenv->{number} always corresponds to a real Koha::Patron. misc/commit_file.pl sets a dummy CLI userenv with borrowernumber 0: C4::Context->set_userenv( 0, 'batch', 0, 'batch', 'batch', 'batch', 'batch' ); Running --revert on a batch calls C4::ImportBatch::BatchRevertItems, which calls Koha::Item->safe_delete on every item created by the batch. safe_delete calls safe_to_delete, which runs: !Koha::Patrons->find( C4::Context->userenv->{number} )->can_edit_items_from( $self->homebranch ) Since there is no patron with borrowernumber 0, Koha::Patrons->find returns undef, and calling ->can_edit_items_from on undef is a fatal error that aborts the whole script. Other call sites touched by bug 33070 already guard against a missing patron (e.g. Koha::UI::Table::Builder::Items->build_table: nomod => $patron ? !$patron->can_edit_items_from(...) : 0), but Koha::Item->safe_to_delete never got the same guard. Test plan: 1. Stage and commit a batch of records so items get created (e.g. via the Manage staged MARC records tool, or misc/commit_file.pl --batch-number NNN). 2. Run: misc/commit_file.pl --batch-number NNN --revert 3. Without this patch, the script dies with: Can't call method "can_edit_items_from" on an undefined value at Koha/Item.pm line 374. 4. Apply the patch. 5. Run: prove t/db_dependent/Koha/Item.t -- all tests pass, including the new regression test. 6. Repeat step 2 -- the batch reverts successfully. -- 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=43604 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 206586 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206586&action=edit Bug 43604: Fix Koha::Item->safe_to_delete crash when userenv has no matching patron Koha::Item->safe_to_delete assumed C4::Context->userenv->{number} always corresponds to a real Koha::Patron. misc/commit_file.pl sets a dummy CLI userenv with borrowernumber 0, so running --revert on a batch calls safe_delete/safe_to_delete on each item and dies: Can't call method "can_edit_items_from" on an undefined value at Koha/Item.pm line 374. Guard against Koha::Patrons->find returning undef, matching the pattern already used elsewhere (e.g. Koha::UI::Table::Builder::Items->build_table). Test plan: 1. Run misc/commit_file.pl --batch-number NNN --revert on a committed import batch -- it dies as above. 2. Apply this patch. 3. prove t/db_dependent/Koha/Item.t -- all tests pass. 4. Repeat step 1 -- the batch reverts successfully. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.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=43604 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk -- 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=43604 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |33070 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33070 [Bug 33070] Get rid of Koha::Patron->can_edit_item and use can_edit_item_from instead -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43604 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43604 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |26.11 Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43604 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43604 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43604 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #206586|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=43604 --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 207690 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=207690&action=edit Bug 43604: Fix Koha::Item->safe_to_delete crash when userenv has no matching patron Koha::Item->safe_to_delete assumed C4::Context->userenv->{number} always corresponds to a real Koha::Patron. misc/commit_file.pl sets a dummy CLI userenv with borrowernumber 0, so running --revert on a batch calls safe_delete/safe_to_delete on each item and dies: Can't call method "can_edit_items_from" on an undefined value at Koha/Item.pm line 374. Guard against Koha::Patrons->find returning undef, matching the pattern already used elsewhere (e.g. Koha::UI::Table::Builder::Items->build_table). Test plan: 1. Run misc/commit_file.pl --batch-number NNN --revert on a committed import batch -- it dies as above. 2. Apply this patch. 3. prove t/db_dependent/Koha/Item.t -- all tests pass. 4. Repeat step 1 -- the batch reverts successfully. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43604 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #3 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. I Saved Programming Perl (262) as MARC UTF8 standard (had issues when importing MARCXML file). 2. Deleted item and record for Programming Perl. 3. After step 1 of the test plan, I reset everything after the patch was applied - otherwise I got this after running the command - with the failure in step 1 it was still sitting as "Reverting" in the manage staged records: misc/commit_file.pl --batch-number 3 --revert misc/commit_file.pl: import batch 3 status is 'reverting', and therefore cannot be imported -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43604 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |andrew@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43604 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #207690|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=43604 --- Comment #4 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 207984 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=207984&action=edit Bug 43604: Fix Koha::Item->safe_to_delete crash when userenv has no matching patron Koha::Item->safe_to_delete assumed C4::Context->userenv->{number} always corresponds to a real Koha::Patron. misc/commit_file.pl sets a dummy CLI userenv with borrowernumber 0, so running --revert on a batch calls safe_delete/safe_to_delete on each item and dies: Can't call method "can_edit_items_from" on an undefined value at Koha/Item.pm line 374. Guard against Koha::Patrons->find returning undef, matching the pattern already used elsewhere (e.g. Koha::UI::Table::Builder::Items->build_table). Test plan: 1. Run misc/commit_file.pl --batch-number NNN --revert on a committed import batch -- it dies as above. 2. Apply this patch. 3. prove t/db_dependent/Koha/Item.t -- all tests pass. 4. Repeat step 1 -- the batch reverts successfully. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste Henry <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=43604 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.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=43604 --- Comment #5 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.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org