[Koha-bugs] [Bug 25273] Elasticsearch Authority matching is returning too many results

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25273

--- Comment #14 from Nick Clemens <nick at bywatersolutions.com> ---
(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.