https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25273
--- Comment #14 from Nick Clemens <[email protected]> --- (In reply to Julian Maurice from comment #12) > 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) 1 - Yes, was trying to save a call to GuessAuthType, reverted 2 - This was a result of above, turning record into a hash if needed, undone 3 - It made sense at the time, but yes, it breaks, removed -- 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/
