Bug 30467

Summary: BatchDeleteItem task does not deal with indexation correctly
Product: Koha Reporter: Jonathan Druart <jonathan.druart+koha>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart+koha>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: normal    
Priority: P5 - low CC: dcook, fridolin.somers, julian.maurice, kyle, martin.renvoize, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00
Bug Depends on: 27344    
Bug Blocks: 27341, 30710    
Attachments: Bug 30467: Don't send the delete request for each item
Bug 30467: Don't send the delete request for each item
Bug 30467: Don't send the delete request for each item
Bug 30467: (QA follow-up) Restore indexing for item only deletion
Bug 30467: (QA follow-up) Restore indexing for item only deletion
Bug 30467: (QA follow-up) Fix copy paste error to update updated_biblionumbers
Bug 30467: (QA follow-up) Fix copy paste error to update updated_biblionumbers

Description Jonathan Druart 2022-04-06 12:04:15 UTC

    
Comment 1 Jonathan Druart 2022-04-06 12:14:59 UTC
Created attachment 132992 [details] [review]
Bug 30467: Don't send the delete request for each item

We are sending the request when all the items have been deleted.
Comment 2 Tomás Cohen Arazi 2022-04-29 17:13:17 UTC
Created attachment 134398 [details] [review]
Bug 30467: Don't send the delete request for each item

We are sending the request when all the items have been deleted.
Comment 3 Martin Renvoize 2022-05-05 14:01:17 UTC
Created attachment 134640 [details] [review]
Bug 30467: Don't send the delete request for each item

We are sending the request when all the items have been deleted.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2022-05-05 14:02:12 UTC
Yup, confirmed...

All works and this prevents a double indexing process.

Passing QA
Comment 5 Fridolin Somers 2022-05-05 20:46:35 UTC
Sorry does not work for me.

In Koha::BackgroundJob::BatchDeleteItem :

I see there is indexing for deleted biblio records :
                    if (@deleted_biblionumbers) {
                        my $indexer = Koha::SearchEngine::Indexer->new(
                            { index => $Koha::SearchEngine::BIBLIOS_INDEX } );

                        $indexer->index_records( \@deleted_biblionumbers,
                            'recordDelete', "biblioserver", undef );
                    }

But I see not indexing for deleted items.

I see batch items update uses Koha::Items->batch_update() with :
    if (@modified_itemnumbers) {
        my @biblionumbers = uniq(
            Koha::Items->search( { itemnumber => \@modified_itemnumbers } )
                       ->get_column('biblionumber'));

        if ( @biblionumbers ) {
            my $indexer = Koha::SearchEngine::Indexer->new(
                { index => $Koha::SearchEngine::BIBLIOS_INDEX } );

            $indexer->update_index( \@biblionumbers );
        }
    }

Should we also create Koha::Items->batch_delete() ?
Maybe in another bug report.
Comment 6 Martin Renvoize 2022-05-06 06:50:41 UTC
Item updates result in biblio re-indexes and these item deletes are all attached to the biblio's we're deleting.. so the re-index is caught by the biblio re-index call you spotted.

Or am I missing something more involved?
Comment 7 Martin Renvoize 2022-05-06 07:12:10 UTC
Created attachment 134672 [details] [review]
Bug 30467: (QA follow-up) Restore indexing for item only deletion

This patch restores indexing for item only bulk deletions.
Comment 8 Martin Renvoize 2022-05-06 07:12:52 UTC
I was totally missing something, good catch.

Follow-up attached, but I'd love to get a QA stamp from someone to check my work on it
Comment 9 Jonathan Druart 2022-05-06 07:26:38 UTC
Created attachment 134675 [details] [review]
Bug 30467: (QA follow-up) Restore indexing for item only deletion

This patch restores indexing for item only bulk deletions.

JD amended patch:
* remove trailing whitespaces
* Move if inside first condition

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Nick Clemens 2022-05-06 14:36:25 UTC
Created attachment 134741 [details] [review]
Bug 30467: (QA follow-up) Fix copy paste error to update updated_biblionumbers
Comment 11 Martin Renvoize 2022-05-06 15:07:07 UTC
Created attachment 134744 [details] [review]
Bug 30467: (QA follow-up) Fix copy paste error to update updated_biblionumbers

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Fridolin Somers 2022-05-06 19:16:00 UTC
Works perfect, thanks for your fast and great work
Comment 13 Fridolin Somers 2022-05-06 20:34:25 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄