Bug 23986 - Batch Record Deletion does not remove records from Elasticsearch search index
Summary: Batch Record Deletion does not remove records from Elasticsearch search index
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-11-07 13:30 UTC by Andrew Fuerste-Henry
Modified: 2019-11-15 10:25 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
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 (1.34 KB, patch)
2019-11-07 16:40 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23986: Pass strings to Catmandu for deletion (1.49 KB, patch)
2019-11-08 03:00 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23986: Pass strings to Catmandu for deletion (1.56 KB, patch)
2019-11-08 09:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23986: Pass strings to Catmandu for deletion (1.62 KB, patch)
2019-11-08 15:03 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 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 2019-11-07 16:40:06 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens 2019-11-07 16:41:43 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 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