To test: - create a bib record - perform a search that returns a list of results including your record - delete record via batch record delete tool - repeat your search, observe that record still appears in results - click deleted title in search results, get "The record you requested does not exist" error
Created attachment 95171 [details] [review] Bug 23986: Remove sort from batch bib deletion After lots and lots fo digging, I removed the sort from this loop. Somehow, this allows the ES records to be deleted. I do not understand this patch, but it works. Possibly we need to sort the error arrays after the fact To test: 1 - Have or create a bib 2 - Go to Tools->Batch record deletion 3 - Click 'Enter a list of record numbers' 4 - Enter the biblionumber' 5 - Confirm deletion 6 - Search for the record, it still appears 7 - Apply patch, restart all the things 8 - Repeat 1-6 9 - This time the record is deleted
I could use a hand understanding what is happening here
Created attachment 95177 [details] [review] Bug 23986: Pass strings to Catmandu for deletion The deletion subroutine expects to get a string, this is fine in most places, but when we sort our records for batch deletion they are cast to numbers and the delete is not processed. This patch quotes the biblionumbers as strings before passing them to Catmandu To aid in detecting orphaned records you can use the patches on bug 22831 To test: 1 - Have or create a bib 2 - Go to Tools->Batch record deletion 3 - Click 'Enter a list of record numbers' 4 - Enter the biblionumber' 5 - Confirm deletion 6 - Search for the record, it still appears 7 - Apply patch, restart all the things 8 - Repeat 1-6 9 - This time the record is deleted 10 - Test other methods of record deletion to confirm the record is removed
Now it makes sense! :)
I guess we could have replaced <=> with cmp, but it seems wrong (we want a numeric sort). It's weird to see the delete request ignored by Catmandu. What about tests? I do not think it should be required for this patch as it's a quite critical bugfix, but we really should start cover that area a bit more...
Created attachment 95188 [details] [review] Bug 23986: Pass strings to Catmandu for deletion The deletion subroutine expects to get a string, this is fine in most places, but when we sort our records for batch deletion they are cast to numbers and the delete is not processed. This patch quotes the biblionumbers as strings before passing them to Catmandu To aid in detecting orphaned records you can use the patches on bug 22831 To test: 1 - Have or create a bib 2 - Go to Tools->Batch record deletion 3 - Click 'Enter a list of record numbers' 4 - Enter the biblionumber' 5 - Confirm deletion 6 - Search for the record, it still appears 7 - Apply patch, restart all the things 8 - Repeat 1-6 9 - This time the record is deleted 10 - Test other methods of record deletion to confirm the record is removed Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 95229 [details] [review] Bug 23986: Pass strings to Catmandu for deletion The deletion subroutine expects to get a string, this is fine in most places, but when we sort our records for batch deletion they are cast to numbers and the delete is not processed. This patch quotes the biblionumbers as strings before passing them to Catmandu To aid in detecting orphaned records you can use the patches on bug 22831 To test: 1 - Have or create a bib 2 - Go to Tools->Batch record deletion 3 - Click 'Enter a list of record numbers' 4 - Enter the biblionumber' 5 - Confirm deletion 6 - Search for the record, it still appears 7 - Apply patch, restart all the things 8 - Repeat 1-6 9 - This time the record is deleted 10 - Test other methods of record deletion to confirm the record is removed Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Man, that's silly. Passing QA
Nice work! Pushed to master for 19.11.00
Pushed to 19.05.x for 19.05.05
Good catch. I guess this fix can be reverted once https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24823 is merged since this seams to be a catmandu-issue.