[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

Julian Maurice <julian.maurice at biblibre.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Signed Off                  |Failed QA

--- Comment #12 from Julian Maurice <julian.maurice at biblibre.com> ---
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.