Bugzilla – Attachment 111002 Details for
Bug 26581
Elasticsearch - Records can be indexed multiple times during returns
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 26581: Only reindex records once per checkin
Bug-26581-Only-reindex-records-once-per-checkin.patch (text/plain), 10.66 KB, created by
Nick Clemens (kidclamp)
on 2020-09-30 16:56:54 UTC
(
hide
)
Description:
Bug 26581: Only reindex records once per checkin
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2020-09-30 16:56:54 UTC
Size:
10.66 KB
patch
obsolete
>From c0b4795c631150f74e8172d6930ca1f95f0cbec8 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Wed, 30 Sep 2020 16:52:43 +0000 >Subject: [PATCH] Bug 26581: Only reindex records once per checkin > >This patch simply passes skip_record_index calls to Koha:Item:store >for all the changes done in AddReturn. > >Testing is really verifiying that items are still correctly indexed at the end > >For both search engines >To test: >1 - Find or create a record with multipel items >2 - Populate both: > UpdateItemLocationOnCheckin > UpdateNotForLoanStatusOnCheckin >3 - Confirm that checking in an item correctly updates the item status in search results >4 - Test with items issued, and items not issued >5 - Test when generating a transfer (checkin at different branch) >6 - Test when item was marked lost >7 - Test when filling transfer >--- > C4/Circulation.pm | 40 +++++++++++++++++++++++++--------------- > C4/Items.pm | 18 +++++++++++------- > 2 files changed, 36 insertions(+), 22 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index ed2b43f3d5..0603862e8e 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -58,6 +58,7 @@ use Koha::Config::SysPrefs; > use Koha::Charges::Fees; > use Koha::Util::SystemPreferences; > use Koha::Checkouts::ReturnClaims; >+use Koha::SearchEngine::Indexer; > use Carp; > use List::MoreUtils qw( uniq any ); > use Scalar::Util qw( looks_like_number ); >@@ -1887,7 +1888,7 @@ sub AddReturn { > . Dumper($issue->unblessed) . "\n"; > } else { > $messages->{'NotIssued'} = $barcode; >- $item->onloan(undef)->store if defined $item->onloan; >+ $item->onloan(undef)->store({skip_record_index=>1}) if defined $item->onloan; > > # even though item is not on loan, it may still be transferred; therefore, get current branch info > $doreturn = 0; >@@ -1915,9 +1916,9 @@ sub AddReturn { > if (defined $update_loc_rules->{_ALL_}) { > if ($update_loc_rules->{_ALL_} eq '_PERM_') { $update_loc_rules->{_ALL_} = $item->permanent_location; } > if ($update_loc_rules->{_ALL_} eq '_BLANK_') { $update_loc_rules->{_ALL_} = ''; } >- if ( $item->location ne $update_loc_rules->{_ALL_}) { >+ if ( defined $item->location && $item->location ne $update_loc_rules->{_ALL_}) { > $messages->{'ItemLocationUpdated'} = { from => $item->location, to => $update_loc_rules->{_ALL_} }; >- $item->location($update_loc_rules->{_ALL_})->store; >+ $item->location($update_loc_rules->{_ALL_})->store({skip_record_index=>1}); > } > } > else { >@@ -1926,7 +1927,7 @@ sub AddReturn { > if ( $update_loc_rules->{$key} eq '_BLANK_') { $update_loc_rules->{$key} = '' ;} > if ( ($item->location eq $key && $item->location ne $update_loc_rules->{$key}) || ($key eq '_BLANK_' && $item->location eq '' && $update_loc_rules->{$key} ne '') ) { > $messages->{'ItemLocationUpdated'} = { from => $item->location, to => $update_loc_rules->{$key} }; >- $item->location($update_loc_rules->{$key})->store; >+ $item->location($update_loc_rules->{$key})->store({skip_record_index=>1}); > last; > } > } >@@ -1945,7 +1946,7 @@ sub AddReturn { > foreach my $key ( keys %$rules ) { > if ( $item->notforloan eq $key ) { > $messages->{'NotForLoanStatusUpdated'} = { from => $item->notforloan, to => $rules->{$key} }; >- $item->notforloan($rules->{$key})->store({ log_action => 0 }); >+ $item->notforloan($rules->{$key})->store({ log_action => 0, skip_record_index => 1 }); > last; > } > } >@@ -1959,7 +1960,8 @@ sub AddReturn { > Wrongbranch => $branch, > Rightbranch => $message > }; >- $doreturn = 0; >+ my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX }); >+ $indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" ); > return ( $doreturn, $messages, $issue, $patron_unblessed); > } > >@@ -1979,7 +1981,7 @@ sub AddReturn { > > if ($patron) { > eval { >- MarkIssueReturned( $borrowernumber, $item->itemnumber, $return_date, $patron->privacy ); >+ MarkIssueReturned( $borrowernumber, $item->itemnumber, $return_date, $patron->privacy, { skip_record_index => 1} ); > }; > unless ( $@ ) { > if ( >@@ -1995,26 +1997,29 @@ sub AddReturn { > } else { > carp "The checkin for the following issue failed, Please go to the about page, section 'data corrupted' to know how to fix this problem ($@)" . Dumper( $issue->unblessed ); > >+ my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX }); >+ $indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" ); >+ > return ( 0, { WasReturned => 0, DataCorrupted => 1 }, $issue, $patron_unblessed ); > } > > # FIXME is the "= 1" right? This could be the borrower hash. > $messages->{'WasReturned'} = 1; > >+ } else { >+ $item->onloan(undef)->store({ log_action => 0 , skip_record_index => 1 }); > } >- >- $item->onloan(undef)->store({ log_action => 0 }); > } > > # the holdingbranch is updated if the document is returned to another location. > # this is always done regardless of whether the item was on loan or not > if ($item->holdingbranch ne $branch) { >- $item->holdingbranch($branch)->store; >+ $item->holdingbranch($branch)->store({ skip_record_index => 1 }); > } > > my $item_was_lost = $item->itemlost; > my $leave_item_lost = C4::Context->preference("BlockReturnOfLostItems") ? 1 : 0; >- my $updated_item = ModDateLastSeen( $item->itemnumber, $leave_item_lost ); # will unset itemlost if needed >+ my $updated_item = ModDateLastSeen( $item->itemnumber, $leave_item_lost, { skip_record_index => 1 } ); # will unset itemlost if needed > > # fix up the accounts..... > if ( $item_was_lost ) { >@@ -2139,7 +2144,7 @@ sub AddReturn { > )) { > $debug and warn sprintf "about to call ModItemTransfer(%s, %s, %s, %s)", $item->itemnumber,$branch, $returnbranch, $transfer_trigger; > $debug and warn "item: " . Dumper($item->unblessed); >- ModItemTransfer($item->itemnumber, $branch, $returnbranch, $transfer_trigger); >+ ModItemTransfer($item->itemnumber, $branch, $returnbranch, $transfer_trigger, { skip_record_index => 1 }); > $messages->{'WasTransfered'} = 1; > } else { > $messages->{'NeedsTransfer'} = $returnbranch; >@@ -2160,6 +2165,9 @@ sub AddReturn { > } > } > >+ my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX }); >+ $indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" ); >+ > if ( $doreturn and $issue ) { > my $checkin = Koha::Old::Checkouts->find($issue->id); > >@@ -2176,7 +2184,7 @@ sub AddReturn { > > =head2 MarkIssueReturned > >- MarkIssueReturned($borrowernumber, $itemnumber, $returndate, $privacy); >+ MarkIssueReturned($borrowernumber, $itemnumber, $returndate, $privacy, [$params] ); > > Unconditionally marks an issue as being returned by > moving the C<issues> row to C<old_issues> and >@@ -2192,10 +2200,12 @@ Ideally, this function would be internal to C<C4::Circulation>, > not exported, but it is currently used in misc/cronjobs/longoverdue.pl > and offline_circ/process_koc.pl. > >+The last optional parameter allos passing skip_record_index to the item store call. >+ > =cut > > sub MarkIssueReturned { >- my ( $borrowernumber, $itemnumber, $returndate, $privacy ) = @_; >+ my ( $borrowernumber, $itemnumber, $returndate, $privacy, $params ) = @_; > > # Retrieve the issue > my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return; >@@ -2241,7 +2251,7 @@ sub MarkIssueReturned { > # And finally delete the issue > $issue->delete; > >- $issue->item->onloan(undef)->store({ log_action => 0 }); >+ $issue->item->onloan(undef)->store({ log_action => 0, skip_record_index => $params->{skip_record_index} }); > > if ( C4::Context->preference('StoreLastBorrower') ) { > my $item = Koha::Items->find( $itemnumber ); >diff --git a/C4/Items.pm b/C4/Items.pm >index 9da7f61fc6..b1e8de7029 100644 >--- a/C4/Items.pm >+++ b/C4/Items.pm >@@ -334,14 +334,16 @@ sub ModItemFromMarc { > > =head2 ModItemTransfer > >- ModItemTransfer($itemnumber, $frombranch, $tobranch, $trigger); >+ ModItemTransfer($itemnumber, $frombranch, $tobranch, $trigger, [$params]); > > Marks an item as being transferred from one branch to another and records the trigger. > >+The last optional parameter allows for passing skip_record_index through to the items store call. >+ > =cut > > sub ModItemTransfer { >- my ( $itemnumber, $frombranch, $tobranch, $trigger ) = @_; >+ my ( $itemnumber, $frombranch, $tobranch, $trigger, $params ) = @_; > > my $dbh = C4::Context->dbh; > my $item = Koha::Items->find( $itemnumber ); >@@ -358,30 +360,32 @@ sub ModItemTransfer { > $sth->execute($itemnumber, $frombranch, $tobranch, $trigger); > > # FIXME we are fetching the item twice in the 2 next statements! >- Koha::Items->find($itemnumber)->holdingbranch($frombranch)->store({ log_action => 0 }); >- ModDateLastSeen($itemnumber); >+ Koha::Items->find($itemnumber)->holdingbranch($frombranch)->store({ log_action => 0, skip_record_index => $params->{skip_record_index} }); >+ ModDateLastSeen($itemnumber, undef, { skip_record_index => $params->{skip_record_index} }); > return; > } > > =head2 ModDateLastSeen > >-ModDateLastSeen( $itemnumber, $leave_item_lost ); >+ModDateLastSeen( $itemnumber, $leave_item_lost, $params ); > > Mark item as seen. Is called when an item is issued, returned or manually marked during inventory/stocktaking. > C<$itemnumber> is the item number > C<$leave_item_lost> determines if a lost item will be found or remain lost > >+The last optional parameter allows for passing skip_record_index through to the items store call. >+ > =cut > > sub ModDateLastSeen { >- my ( $itemnumber, $leave_item_lost ) = @_; >+ my ( $itemnumber, $leave_item_lost, $params ) = @_; > > my $today = output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }); > > my $item = Koha::Items->find($itemnumber); > $item->datelastseen($today); > $item->itemlost(0) unless $leave_item_lost; >- $item->store({ log_action => 0 }); >+ $item->store({ log_action => 0, skip_record_index => $params->{skip_record_index} }); > } > > =head2 CheckItemPreSave >-- >2.11.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 26581
:
111001
|
111002
|
111596
|
111597
|
111878
|
111879
|
112563
|
112578
|
112829