https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25273
Julian Maurice <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #12 from Julian Maurice <[email protected]> --- Hi Nick, Thanks for the explanation, it is much more clear to me now. The change makes sense and the patch works as expected. However I would like to see some changes in the patch before validating it: 1) - ModZebra( $authid, 'specialUpdate', 'authorityserver', $record ); + ModZebra( $authid, 'specialUpdate', 'authorityserver', { record => $record, authtypecode => $authtypecode } ); This change is confusing. In ModZebra we now have a $record variable which is a hash that contain a 'record' key. Even with this simple patch I had to ask myself several times « what's this $record variable I'm looking at ? The hash or the MARC::Record ? ». Look at this line for instance: + $record = $record->{record}; At some point in the subroutine, $record was a hasref, now it's a MARC::Record. This is the kind of things that make code hard to read, and make it easier for bugs to appear. And I think it is not needed to pass the authtypecode to ModZebra, since it can be obtained from the MARC::Record. 2) - unless ($record) { + if ($record) { + $indexer->update_index_background( [$biblionumber], [$record] ); + } else { $record = GetMarcBiblio({ biblionumber => $biblionumber, embed_items => 1 }); + $indexer->update_index_background( [$biblionumber], [{ record => $record }] ); } - my $records = [$record]; - $indexer->update_index_background( [$biblionumber], [$record] ); I think it was easier to read before : unless there is a record, fetch it; in all cases call update_index_background Now it's : if there is a record, call update_index_background, otherwise fetch the record and call update_index_background. This change was not needed, so why ? :) 3) - my $id = $record->id // $record->authid; + my $id = $record->id; Again this change is not needed, but this time it causes a bug. Try this : misc/search_tools/rebuild_elasticsearch.pl -a -ai X (replace X by an existing authid) -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
