Bug 17980 - Fix wrong calls to ModZebra in AuthorityMarc
Summary: Fix wrong calls to ModZebra in AuthorityMarc
Status: CLOSED DUPLICATE of bug 18014
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Authority data support (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 17908
  Show dependency treegraph
 
Reported: 2017-01-24 09:39 UTC by Marcel de Rooy
Modified: 2019-06-27 09:24 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 17980: Fix wrong call to ModZebra in AddAuthority (affects Elastic Search users) (2.24 KB, patch)
2017-01-24 10:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17980: Corresponding fix in DelAuthority (1.18 KB, patch)
2017-01-24 10:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17980: Corresponding fix in DelAuthority (1.24 KB, patch)
2017-01-24 10:23 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2017-01-24 09:39:30 UTC

    
Comment 1 Marcel de Rooy 2017-01-24 10:10:04 UTC
Created attachment 59490 [details] [review]
Bug 17980: Fix wrong call to ModZebra in AddAuthority (affects Elastic Search users)

The ModZebra call in AddAuthority incorrectly passes five parameters
to ModZebra including $oldRecord.
This makes the last parameter (the new record) being ignored !!
A closer inspection of ModZebra reveals that this only affects installs
using Elastic Search. The record parameter is ignored when you still
use Zebra.

Note: Keep in mind that AddAuthority is being used in adding as well as
modifying authority records. This patch is part of a larger effort to
make improvements in this area and will not be last one. Please help to
get this further (see bug 17908).

Trivial fix.

Test plan:
[1] If you use Elastic Search, verify that the new authority record is
    being indexed, not the old one.
[2] If you do not use Elastic Search, add or modify an authority and
    search for this change in Authority (Search entire record).
Comment 2 Marcel de Rooy 2017-01-24 10:11:38 UTC
Chris or Nick: could one of you please confirm the Elastic Search side? Thanks.
Comment 3 Marcel de Rooy 2017-01-24 10:17:00 UTC
Actually, the Elastic Search problem is larger than this:
Look at ModZebra itself:
        if ( $op eq 'specialUpdate' ) {
            unless ($record) {
                $record = GetMarcBiblio($biblionumber, 1);
            }
            my $records = [$record];
            $indexer->update_index_background( [$biblionumber], [$record] );
        }

This code is wrong in case you call ModZebra for an authority !
Comment 4 Marcel de Rooy 2017-01-24 10:18:03 UTC
(In reply to Marcel de Rooy from comment #3)
> Actually, the Elastic Search problem is larger than this:
> Look at ModZebra itself:
>         if ( $op eq 'specialUpdate' ) {
>             unless ($record) {
>                 $record = GetMarcBiblio($biblionumber, 1);
>             }
>             my $records = [$record];
>             $indexer->update_index_background( [$biblionumber], [$record] );
>         }
> 
> This code is wrong in case you call ModZebra for an authority !

At least if you pass no record and call GetMarcBiblio :)
Comment 5 Marcel de Rooy 2017-01-24 10:21:08 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2017-01-24 10:23:41 UTC
Created attachment 59492 [details] [review]
Bug 17980: Corresponding fix in DelAuthority

The call to ModZebra here does not need more than three parameters.
Same for Zebra and Elastic Search.

Note: On omnibus bug 17908 a few more fixes are coming on this subject of
deleting authorities.

Test plan:
[1] Delete an authority.
[2] Search for it in the Authority module.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2017-02-18 14:57:14 UTC
Closed in favor of the combination of bugs 18014 (pushed) and 18070 (SO).

*** This bug has been marked as a duplicate of bug 18014 ***