Bug 27819 - Spurious errors when running delete_records_via_leader.pl
Summary: Spurious errors when running delete_records_via_leader.pl
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 23463
Blocks:
  Show dependency treegraph
 
Reported: 2021-02-27 17:24 UTC by Nick Clemens
Modified: 2021-05-24 19:22 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.06,20.05.12


Attachments
Bug 27819: Spurious item deletion errors from delete_records_via_leader.pl (2.99 KB, patch)
2021-02-27 17:49 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27819: Fix Object POD (707 bytes, patch)
2021-02-27 17:49 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27819: Spurious item deletion errors from delete_records_via_leader.pl (3.04 KB, patch)
2021-04-15 14:37 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 27819: Fix Object POD (757 bytes, patch)
2021-04-15 14:37 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 27819: Spurious item deletion errors from delete_records_via_leader.pl (3.10 KB, patch)
2021-04-27 13:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27819: Fix Object POD (822 bytes, patch)
2021-04-27 13:54 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-02-27 17:24:50 UTC
The job reports errors when deleting items.

The issue seems to be that Koha::Object->delete claims in the POD to return -1, 0, or 1 as a result, but it in fact returns the Object itself on a successful deletion

The errors are reported as:
ERROR DELETING ITEM 501740: Koha::Item=HASH(0x55ce407a1a78)

To recreate:
1 - Find or create a record with some items
2 - Ensure those  items can be deleted (not on loan, etc.)
3 - Edit the record leader and set position 5 to 'd'
4 - perl misc/cronjobs/delete_records_via_leader.pl -i -v --confirm
5 - Deletion succeeds, but reports failure on items
Comment 1 Nick Clemens 2021-02-27 17:49:17 UTC
Created attachment 117406 [details] [review]
Bug 27819: Spurious item deletion errors from delete_records_via_leader.pl

The job reports errors when deleting items.

The issue seems to be that Koha::Object->delete claims in the POD to return -1, 0, or 1 as a result, but it in fact returns the Object itself on a successful deletion

The errors are reported as:
ERROR DELETING ITEM 501740: Koha::Item=HASH(0x55ce407a1a78)

To recreate:
 1 - Find or create a record with some items
 2 - Ensure those  items can be deleted (not on loan, etc.)
 3 - Edit the record leader and set position 5 to 'd'
 4 - perl misc/cronjobs/delete_records_via_leader.pl -i -v --confirm
 5 - Deletion succeeds, but reports failure on items
 6 - Apply patch
 7 - Find or create a new record as above, but this time add an additional item and check it out to a patron
 8 - perl misc/cronjobs/delete_records_via_leader.pl -i -v
 9 - Test mode should report 1 item to be deleted, one with error
10 - perl misc/cronjobs/delete_records_via_leader.pl -i -v --confirm
11 - One item should be deleted, one item not, record not deleted
12 - check the item in
13 - perl misc/cronjobs/delete_records_via_leader.pl -i -v --confirm
14 - Successful deletion with no error reported
Comment 2 Nick Clemens 2021-02-27 17:49:20 UTC
Created attachment 117407 [details] [review]
Bug 27819: Fix Object POD
Comment 3 Owen Leonard 2021-04-15 14:37:05 UTC
Created attachment 119631 [details] [review]
Bug 27819: Spurious item deletion errors from delete_records_via_leader.pl

The job reports errors when deleting items.

The issue seems to be that Koha::Object->delete claims in the POD to
return -1, 0, or 1 as a result, but it in fact returns the Object
itself on a successful deletion

The errors are reported as:
ERROR DELETING ITEM 501740: Koha::Item=HASH(0x55ce407a1a78)

To recreate:
 1 - Find or create a record with some items
 2 - Ensure those  items can be deleted (not on loan, etc.)
 3 - Edit the record leader and set position 5 to 'd'
 4 - perl misc/cronjobs/delete_records_via_leader.pl -i -v --confirm
 5 - Deletion succeeds, but reports failure on items
 6 - Apply patch
 7 - Find or create a new record as above, but this time add an
     additional item and check it out to a patron
 8 - perl misc/cronjobs/delete_records_via_leader.pl -i -v
 9 - Test mode should report 1 item to be deleted, one with error
10 - perl misc/cronjobs/delete_records_via_leader.pl -i -v --confirm
11 - One item should be deleted, one item not, record not deleted
12 - check the item in
13 - perl misc/cronjobs/delete_records_via_leader.pl -i -v --confirm
14 - Successful deletion with no error reported

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Owen Leonard 2021-04-15 14:37:09 UTC
Created attachment 119632 [details] [review]
Bug 27819: Fix Object POD

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 5 Martin Renvoize 2021-04-27 13:54:28 UTC
Created attachment 120232 [details] [review]
Bug 27819: Spurious item deletion errors from delete_records_via_leader.pl

The job reports errors when deleting items.

The issue seems to be that Koha::Object->delete claims in the POD to
return -1, 0, or 1 as a result, but it in fact returns the Object
itself on a successful deletion

The errors are reported as:
ERROR DELETING ITEM 501740: Koha::Item=HASH(0x55ce407a1a78)

To recreate:
 1 - Find or create a record with some items
 2 - Ensure those  items can be deleted (not on loan, etc.)
 3 - Edit the record leader and set position 5 to 'd'
 4 - perl misc/cronjobs/delete_records_via_leader.pl -i -v --confirm
 5 - Deletion succeeds, but reports failure on items
 6 - Apply patch
 7 - Find or create a new record as above, but this time add an
     additional item and check it out to a patron
 8 - perl misc/cronjobs/delete_records_via_leader.pl -i -v
 9 - Test mode should report 1 item to be deleted, one with error
10 - perl misc/cronjobs/delete_records_via_leader.pl -i -v --confirm
11 - One item should be deleted, one item not, record not deleted
12 - check the item in
13 - perl misc/cronjobs/delete_records_via_leader.pl -i -v --confirm
14 - Successful deletion with no error reported

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2021-04-27 13:54:33 UTC
Created attachment 120233 [details] [review]
Bug 27819: Fix Object POD

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2021-04-27 13:55:04 UTC
All works as expected.. always nice to quieten things down bit.

Passing QA
Comment 8 Jonathan Druart 2021-04-28 09:11:27 UTC
(In reply to Martin Renvoize from comment #6)
> Created attachment 120233 [details] [review] [review]
> Bug 27819: Fix Object POD
> 
> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

For the record, this is tested correctly in t/db_dependent/Koha/Objects.t, subtest 'Koha::Object->delete'
Comment 9 Jonathan Druart 2021-04-28 09:16:48 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 10 Fridolin Somers 2021-04-30 09:23:36 UTC
Pushed to 20.11.x for 20.11.06
Comment 11 Andrew Fuerste-Henry 2021-05-24 19:18:50 UTC
Pushed to 20.05.x for 20.05.12
Comment 12 Victor Grousset/tuxayo 2021-05-24 19:22:20 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.