Bug 23986

Summary: Batch Record Deletion does not remove records from Elasticsearch search index
Product: Koha Reporter: Andrew Fuerste-Henry <andrewfh>
Component: Searching - ElasticsearchAssignee: Nick Clemens (kidclamp) <nick>
Status: CLOSED FIXED QA Contact:
Severity: major    
Priority: P5 - low CC: alex.arnaud, ere.maijala, fridolin.somers, glasklas, jonathan.druart, joonas.kylmala, julian.maurice, m.de.rooy, martin.renvoize, nick, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25342
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24823
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.05
Attachments: Bug 23986: Remove sort from batch bib deletion
Bug 23986: Pass strings to Catmandu for deletion
Bug 23986: Pass strings to Catmandu for deletion
Bug 23986: Pass strings to Catmandu for deletion

Description Andrew Fuerste-Henry 2019-11-07 13:30:48 UTC
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
Comment 1 Nick Clemens (kidclamp) 2019-11-07 16:40:06 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens (kidclamp) 2019-11-07 16:41:43 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens (kidclamp) 2019-11-08 03:00:46 UTC
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
Comment 4 Jonathan Druart 2019-11-08 08:34:31 UTC
Now it makes sense! :)
Comment 5 Jonathan Druart 2019-11-08 09:45:23 UTC
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...
Comment 6 Jonathan Druart 2019-11-08 09:49:02 UTC
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>
Comment 7 Martin Renvoize 2019-11-08 15:03:32 UTC
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>
Comment 8 Martin Renvoize 2019-11-08 15:04:25 UTC
Man, that's silly.

Passing QA
Comment 9 Martin Renvoize 2019-11-11 10:33:13 UTC
Nice work!

Pushed to master for 19.11.00
Comment 10 Fridolin Somers 2019-11-15 10:25:53 UTC
Pushed to 19.05.x for 19.05.05
Comment 11 David Gustafsson 2020-05-04 11:55:07 UTC
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.