Bug 25273 - Elasticsearch Authority matching is returning too many results
Summary: Elasticsearch Authority matching is returning too many results
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Authority data support (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Julian Maurice
URL:
Keywords:
Depends on: 24269
Blocks:
  Show dependency treegraph
 
Reported: 2020-04-24 10:54 UTC by Nick Clemens
Modified: 2020-10-20 19:38 UTC (History)
14 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Manual merge request:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.05, 19.11.11


Attachments
Bug 25273: WIP (14.30 KB, patch)
2020-04-27 18:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25273: Make match-heading rely on authority type configuration (15.64 KB, patch)
2020-04-28 12:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25273: Make match-heading rely on authority type configuration (16.09 KB, patch)
2020-05-05 13:05 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 25273: Make match-heading rely on authority type configuration (15.92 KB, patch)
2020-05-29 11:53 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25273: (follow-up) (7.52 KB, patch)
2020-07-09 18:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25273: (follow-up) (7.21 KB, patch)
2020-07-27 15:00 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25273: (follow-up) Don't die on unknown authtype (1.95 KB, patch)
2020-08-24 14:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25273: Make match-heading rely on authority type configuration (15.97 KB, patch)
2020-08-27 20:49 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25273: (follow-up) (7.26 KB, patch)
2020-08-27 20:49 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25273: (follow-up) Don't die on unknown authtype (2.01 KB, patch)
2020-08-27 20:49 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2020-04-24 10:54:36 UTC
Bug 24269 attempted to improve the authority matching process in ES, however, it does not accomplish the full extent of what is needed.

Match-heading is used as the matching field for records, and I added C4::Heading to be used to generate the correct form including subfields, however, the subfields are still indexed at normally too. This is problematic as the index for a heading like:
$aCats$vFiction$zVermont

Is indexed into match-heading as:
["Cats","Cats genresubdiv Fiction geosubdiv Vermont","Vermont"]

Thus if a record has a heading of:
Vermont

It returns the above record.

We need to index for the linker into a field that only gets the correct search form for heading and no other data. I propose to do this by retrieving the fields set to be copied from Administration->Authority types and indexing them into a field that can be used only for matching/
Comment 1 Nick Clemens 2020-04-27 18:45:37 UTC
Created attachment 103801 [details] [review]
Bug 25273: WIP

This patch moves the code for indexing the match-heading field into its own special section
only used for authorities

Rather than allowing the user to map this field, we create it on our own and add to the indexe documents.

Currently, it doesn't work. I think the issue is that match-heading is not being added to the index so is not searchable
Comment 2 Nick Clemens 2020-04-28 12:28:40 UTC
Created attachment 103854 [details] [review]
Bug 25273: Make match-heading rely on authority type configuration

The match-heading field is a special field used only by the linker, not accessible
to staff or patrons via the interface. This field is used to store the constructed
'search form' used for matching bib headings to authority fields.

In bug 24269 I attempted to use the mappings defined in the inferface and also inject the search term.
This did not work as too many subfields were indexed on their own and leading to false matches.
In this bug we remove the mappings for this field, and create it ourselves during
the indexing process. The C4::Headings module is still used to generate the correct form,
however, the mappings are set based on the authority types in the system. This gives the user
the ability to add new typoes, but prevents mapping changes from breaking linker functionality

To test:
 1 - Start form a sample database
 2 - Download via Z39.50 2 authorities, one of which is a narrower heading of the other, e.g.:
    Waterworks
    Waterworks - Costs
 3 - Place a heading for the broader term in a record
 4 - Make sure linker is set to default
 5 - Attempt to link the records
 6 - Linking fails
 7 - Apply patch
 8 - Refresh index settings (if using a custom file, remove 'match-heading')
 9 - Reindex ES
10 - Try to link again
11 - It succeeds!
Comment 3 Victor Grousset/tuxayo 2020-05-05 10:11:59 UTC
>  4 - Make sure linker is set to default

LinkerModule syspref right?
Comment 4 Victor Grousset/tuxayo 2020-05-05 11:44:33 UTC
I might have done a step wrong, it seems I'm not getting a well linked auth in the end.

Note that my ES setup was only the two steps listed here:
https://wiki.koha-community.org/wiki/User:Victor_Grousset_-_tuxayo:Setup_Koha_development_environment_(koha-testing-docker)#Use_Elasticsearch

Don't know if that's enough.
And also: ES 5 (default of koha-testing-docker)

>  2 - Download via Z39.50 2 authorities, one of which is a narrower heading of the other, e.g.:

Same auths as the example.

>  3 - Place a heading for the broader term in a record

syspref BiblioAddsAuthorities => allow

Go to a record; edit record; go to 650
Replace the existing 650$a by "Waterworks"
and 650$x by "Costs"

> 4 - Make sure linker is set to default

C4::Linker::Default

>  5 - Attempt to link the records

misc/link_bibs_to_authorities.pl

>  6 - Linking fails

In the record's page the link is:

http://172.30.0.6:8081/cgi-bin/koha/catalogue/search.pl?q=su:%22%20Waterworks%22

It's wrong right? (and expected on this step)

>  7 - Apply patch

And after that: restart_all

>  8 - Refresh index settings (if using a custom file, remove 'match-heading')
>  9 - Reindex ES

misc/search_tools/rebuild_elasticsearch.pl -v -d -r
Comment 5 Victor Grousset/tuxayo 2020-05-05 13:05:14 UTC
Created attachment 104363 [details] [review]
Bug 25273: Make match-heading rely on authority type configuration

The match-heading field is a special field used only by the linker, not accessible
to staff or patrons via the interface. This field is used to store the constructed
'search form' used for matching bib headings to authority fields.

In bug 24269 I attempted to use the mappings defined in the inferface and also inject the search term.
This did not work as too many subfields were indexed on their own and leading to false matches.
In this bug we remove the mappings for this field, and create it ourselves during
the indexing process. The C4::Headings module is still used to generate the correct form,
however, the mappings are set based on the authority types in the system. This gives the user
the ability to add new typoes, but prevents mapping changes from breaking linker functionality

To test:
 1 - Start form a sample database with ElasticSearch working
 2 - Download via Z39.50 2 authorities, one of which is a narrower heading of the other, e.g.:
    Waterworks
    Waterworks - Costs
 3 - Place a heading for the broader term in a record. e.g. Waterworks
       In 650$a, without the cataloguing authority plugin. We don't want
       the link created now.
       You need syspref BiblioAddsAuthorities => allow
 4 - Make sure linker is set to default
 5 - Attempt to link the records
       misc/link_bibs_to_authorities.pl
 6 - Linking fails
 7 - Apply patch
 8 - refresh index settings (if using a custom file, remove 'match-heading')
       You can reset mappings in the UI or run this:
       misc/search_tools/rebuild_elasticsearch.pl -v -d -r
 9 - Reindex ES
10 - Try to link again
11 - It succeeds!
12 - Run the tests
     prove t/db_dependent/Koha/SearchEngine/Elasticsearch.t

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 6 Victor Grousset/tuxayo 2020-05-05 13:06:30 UTC
Changes to the test plan:


 1 - Start form a sample database
↓
 1 - Start form a sample database with ElasticSearch working

--

 3 - Place a heading for the broader term in a record
↓
 3 - Place a heading for the broader term in a record. e.g. Waterworks
       In 650$a, without the cataloguing authority plugin. We don't want
       the link created now.
       You need syspref BiblioAddsAuthorities => allow

--

 5 - Attempt to link the records
↓
 5 - Attempt to link the records
       misc/link_bibs_to_authorities.pl

--

 8 - refresh index settings (if using a custom file, remove 'match-heading')
↓
 8 - refresh index settings (if using a custom file, remove 'match-heading')
       You can reset mappings in the UI or run this:
       misc/search_tools/rebuild_elasticsearch.pl -v -d -r

--

Added step 12
12 - Run the tests
     prove t/db_dependent/Koha/SearchEngine/Elasticsearch.t
Comment 7 Nick Clemens 2020-05-29 11:53:42 UTC
Created attachment 105435 [details] [review]
Bug 25273: Make match-heading rely on authority type configuration

The match-heading field is a special field used only by the linker, not accessible
to staff or patrons via the interface. This field is used to store the constructed
'search form' used for matching bib headings to authority fields.

In bug 24269 I attempted to use the mappings defined in the inferface and also inject the search term.
This did not work as too many subfields were indexed on their own and leading to false matches.
In this bug we remove the mappings for this field, and create it ourselves during
the indexing process. The C4::Headings module is still used to generate the correct form,
however, the mappings are set based on the authority types in the system. This gives the user
the ability to add new typoes, but prevents mapping changes from breaking linker functionality

To test:
 1 - Start form a sample database with ElasticSearch working
 2 - Download via Z39.50 2 authorities, one of which is a narrower heading of the other, e.g.:
    Waterworks
    Waterworks - Costs
 3 - Place a heading for the broader term in a record. e.g. Waterworks
       In 650$a, without the cataloguing authority plugin. We don't want
       the link created now.
       You need syspref BiblioAddsAuthorities => allow
 4 - Make sure linker is set to default
 5 - Attempt to link the records
       misc/link_bibs_to_authorities.pl
 6 - Linking fails
 7 - Apply patch
 8 - refresh index settings (if using a custom file, remove 'match-heading')
       You can reset mappings in the UI or run this:
       misc/search_tools/rebuild_elasticsearch.pl -v -d -r
 9 - Reindex ES
10 - Try to link again
11 - It succeeds!
12 - Run the tests
     prove t/db_dependent/Koha/SearchEngine/Elasticsearch.t

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 8 Alex Arnaud 2020-05-29 14:45:01 UTC
Functional test works perfectly with default (hardcoded) authority types.

What about custom authority types ?

Wouldn't it be better to had all Match-Heading (those harcoded in C4::Heading::Marc21... for marc21 part) in the mappings.yaml ?

Without this patch applied:
 - I just added the following mapping for authorities:
     Match-Heading => 150(abgvxyz)

 - Imported an authority: $aWaterworks

 - Imported another authority: $aWaterworks $xCosts

 - Added a 650$aWaterworks in a biblio

 - Added a 650$aWaterworks $xCosts in the same biblio

 - reindex and misc/link_bibs_to_authorities.pl

=> 1st 650 is linked to the authority with the broader term. 2nd 650 is linked to the other authority.
Comment 9 Nick Clemens 2020-05-29 15:55:10 UTC
(In reply to Alex Arnaud from comment #8)
> What about custom authority types ?

It should work as before - all we do here is form the field in the index the same way as we generate the search_form that we will search later

> Wouldn't it be better to had all Match-Heading (those harcoded in
> C4::Heading::Marc21... for marc21 part) in the mappings.yaml ?
I don't see a benefit here, as the search_form is hardcoded when linking. We should ensure that we generate the data in the index the same way that we will search for it. 

Authority searching is a different thing than biblio searching - in this case anyways it is strictly used to match the two records. The user has no control over the search_form, so allowing control over the index form can only lead to confusion

 
> Without this patch applied:
>  - I just added the following mapping for authorities:
>      Match-Heading => 150(abgvxyz)

This may work in this simpler case, however, specifying the fields generates them in a fixed order, and authorities order can differ and has different meanings

Also consider:
$aScience$vFiction
$aScience fiction

Which will match each other under the current code if all subfields mapped
Comment 10 Julian Maurice 2020-06-05 07:47:21 UTC
Hi Nick,

(In reply to Nick Clemens from comment #9)
> This may work in this simpler case, however, specifying the fields generates
> them in a fixed order, and authorities order can differ and has different
> meanings

Can you provide a test plan with a complex case, which cannot be solved using configuration only ?
Comment 11 Nick Clemens 2020-06-05 14:58:39 UTC
(In reply to Julian Maurice from comment #10)
> Hi Nick,
> 
> (In reply to Nick Clemens from comment #9)
> > This may work in this simpler case, however, specifying the fields generates
> > them in a fixed order, and authorities order can differ and has different
> > meanings
> 
> Can you provide a test plan with a complex case, which cannot be solved
> using configuration only ?

I don't have a specific example of an authority that doesn't work, but if we look at the example Alex provides:

Before the patches, with mapping 150(abgvxyz) the two records are indexed with match-heading as:
['Waterworks']
['Waterworks Costs','Waterworks generalsubdiv Costs']

After the patch:
['Waterworks']
['Waterworks generalsubdiv Costs']

In both cases when linking we perform a search for:
'Waterworks generalsubdiv Costs'

The matching works before only because we are already generating the heading search form and storing it in the index, the mappings don't affect the terms used for matching.

Custom added authority types still use the hard coded hashes in C4/Heading/{marcflavour} to generate the heading search form, so will only work if they use a field defined there with or without the patches

Adding the user defined fields only adds the possibility for mismatches, it doesn't add functionality. There are subdivisions that can be reordered (which mappings don't handle) and terms like '$aScience$vFiction' and '$aScience fiction' which can end up mismatching

Since we are always using the standardized C4::Heading->search_form when performing the search we should also only store that search_form in the index
Comment 12 Julian Maurice 2020-06-19 11:34:04 UTC
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)
Comment 13 Nick Clemens 2020-07-09 18:41:40 UTC
Created attachment 106759 [details] [review]
Bug 25273: (follow-up)
Comment 14 Nick Clemens 2020-07-09 18:43:04 UTC
(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
Comment 15 Julian Maurice 2020-07-24 12:11:23 UTC
(In reply to Nick Clemens from comment #14)
> 1 - Yes, was trying to save a call to GuessAuthType, reverted
GuessAuthTypeCode only checks in its hardcoded list of authtypecodes, it doesn't even check if the auth type code exist in database. If someone use custom authority types, this will probably break.

There is get_auth_type_location but it needs the authtypecode... so we cannot use it to find the authtypecode in the MARC record.

It seems that the only way to get the correct information is to get it directly from database using the authid (SELECT authtypecode WHERE authid = ?)
Or maybe it would be better to pass a Koha::Object (Koha::Authority or Koha::Biblio) to the indexer, so we can retrieve easily the authtypecode and the MARC::Record but that would probably require rewriting some parts of the indexation code.

I'm not sure where to go on this one. It clearly shows that passing the MARC::Record alone to the indexer is not enough.
Any thoughts ?
Comment 16 Julian Maurice 2020-07-24 12:47:48 UTC
Also QA script says:

 FAIL   C4/Biblio.pm
   FAIL   forbidden patterns
                forbidden pattern: trailing space char (line 2548)

And there's a warn remaining in misc/search_tools/rebuild_elasticsearch.pl

+            warn "we called;";
Comment 17 Nick Clemens 2020-07-27 15:00:14 UTC
Created attachment 107434 [details] [review]
Bug 25273: (follow-up)
Comment 18 Nick Clemens 2020-07-27 15:00:32 UTC
(In reply to Julian Maurice from comment #16)
> Also QA script says:
> 
>  FAIL   C4/Biblio.pm
>    FAIL   forbidden patterns
>                 forbidden pattern: trailing space char (line 2548)
> 
> And there's a warn remaining in misc/search_tools/rebuild_elasticsearch.pl
> 
> +            warn "we called;";

Warn removed, I don't get the other
Comment 19 Nick Clemens 2020-07-27 15:02:16 UTC
(In reply to Julian Maurice from comment #15)
> (In reply to Nick Clemens from comment #14)
> > 1 - Yes, was trying to save a call to GuessAuthType, reverted
> GuessAuthTypeCode only checks in its hardcoded list of authtypecodes, it
> doesn't even check if the auth type code exist in database. If someone use
> custom authority types, this will probably break.
> 
> There is get_auth_type_location but it needs the authtypecode... so we
> cannot use it to find the authtypecode in the MARC record.
> 
> It seems that the only way to get the correct information is to get it
> directly from database using the authid (SELECT authtypecode WHERE authid =
> ?)
> Or maybe it would be better to pass a Koha::Object (Koha::Authority or
> Koha::Biblio) to the indexer, so we can retrieve easily the authtypecode and
> the MARC::Record but that would probably require rewriting some parts of the
> indexation code.
> 
> I'm not sure where to go on this one. It clearly shows that passing the
> MARC::Record alone to the indexer is not enough.
> Any thoughts ?

Custom authorites don't work in C4/Headings.pm either - the lists of subfields are hardcoded - I think that should be its own bug.

This issue affects stable branches and we should fix it for standard authorities then tackle custom I believe as it will be a bigger job. We should keep these small for backporting
Comment 20 Katrin Fischer 2020-08-23 21:46:25 UTC
When I run the reindex as suggested, I have a warn that I haven't without the patches:

$ misc/search_tools/rebuild_elasticsearch.pl -v -d -r

Use of uninitialized value $authtypecode in hash element at /home/vagrant/kohaclone/Koha/SearchEngine/Elasticsearch.pm line 542.
Can't call method "tag" on an undefined value at /home/vagrant/kohaclone/C4/Heading.pm line 71.

I tried to follow the test plan, but the linking on my "Waterworks" record didn't happen :(
Comment 21 Nick Clemens 2020-08-24 14:28:02 UTC
Created attachment 108989 [details] [review]
Bug 25273: (follow-up) Don't die on unknown authtype

We are guessing authtype code and inserting the heading built accorindg to C4::Heading

If we can't identify the auth type, we can format the heading. There is a record in the koha test
data that is missing the heading field so type cannot be idenfitied

This prevents us from dying n a record where we cannot identify the type.

Note: This code will also be triggered for custom authority types, higlighting
that they won't link because of hardcoded mappings in C4::Heading. We must tackle this
on a new bug
Comment 22 Katrin Fischer 2020-08-27 20:49:49 UTC
Created attachment 109261 [details] [review]
Bug 25273: Make match-heading rely on authority type configuration

The match-heading field is a special field used only by the linker, not accessible
to staff or patrons via the interface. This field is used to store the constructed
'search form' used for matching bib headings to authority fields.

In bug 24269 I attempted to use the mappings defined in the inferface and also inject the search term.
This did not work as too many subfields were indexed on their own and leading to false matches.
In this bug we remove the mappings for this field, and create it ourselves during
the indexing process. The C4::Headings module is still used to generate the correct form,
however, the mappings are set based on the authority types in the system. This gives the user
the ability to add new typoes, but prevents mapping changes from breaking linker functionality

To test:
 1 - Start form a sample database with ElasticSearch working
 2 - Download via Z39.50 2 authorities, one of which is a narrower heading of the other, e.g.:
    Waterworks
    Waterworks - Costs
 3 - Place a heading for the broader term in a record. e.g. Waterworks
       In 650$a, without the cataloguing authority plugin. We don't want
       the link created now.
       You need syspref BiblioAddsAuthorities => allow
 4 - Make sure linker is set to default
 5 - Attempt to link the records
       misc/link_bibs_to_authorities.pl
 6 - Linking fails
 7 - Apply patch
 8 - refresh index settings (if using a custom file, remove 'match-heading')
       You can reset mappings in the UI or run this:
       misc/search_tools/rebuild_elasticsearch.pl -v -d -r
 9 - Reindex ES
10 - Try to link again
11 - It succeeds!
12 - Run the tests
     prove t/db_dependent/Koha/SearchEngine/Elasticsearch.t

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 23 Katrin Fischer 2020-08-27 20:49:54 UTC
Created attachment 109262 [details] [review]
Bug 25273: (follow-up)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Katrin Fischer 2020-08-27 20:49:59 UTC
Created attachment 109263 [details] [review]
Bug 25273: (follow-up) Don't die on unknown authtype

We are guessing authtype code and inserting the heading built accorindg to C4::Heading

If we can't identify the auth type, we can format the heading. There is a record in the koha test
data that is missing the heading field so type cannot be idenfitied

This prevents us from dying n a record where we cannot identify the type.

Note: This code will also be triggered for custom authority types, higlighting
that they won't link because of hardcoded mappings in C4::Heading. We must tackle this
on a new bug

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Katrin Fischer 2020-08-27 20:50:45 UTC
This time around I managed to link the authority and record perfectly.
Comment 26 Jonathan Druart 2020-08-31 14:20:43 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 27 Lucas Gass 2020-09-04 20:56:26 UTC
There are 3 patches here but only 2 pushed to master. Do we need all 3?
Comment 28 Jonathan Druart 2020-09-07 07:35:04 UTC
(In reply to Lucas Gass from comment #27)
> There are 3 patches here but only 2 pushed to master. Do we need all 3?

First 2 patches have been squashed.
Comment 29 Lucas Gass 2020-09-28 15:31:43 UTC
backported to 20.05.x for 20.05.05
Comment 30 Aleisha Amohia 2020-10-20 01:48:10 UTC
backported to 19.11.x for 19.11.11
Comment 31 Victor Grousset/tuxayo 2020-10-20 18:42:12 UTC
Can't backport to 19.05.x got a few conflicts that I can't safely resolve:

https://copycat.drycat.fr/?714af2fc72b434a4#DsBAssdpNbFnqgbP2crhStRfrTXhB8rAnfVeTaGebjPL
Comment 32 Victor Grousset/tuxayo 2020-10-20 19:38:06 UTC
Okay, it's due to the dependency on bug 24269