Bug 26611 - Required match checks don't work for authority records
Summary: Required match checks don't work for authority records
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Aleisha Amohia
QA Contact: Nick Clemens
URL:
Keywords: rel_22_05_candidate
: 35485 (view as bug list)
Depends on: 31794
Blocks: 35343
  Show dependency treegraph
 
Reported: 2020-10-05 18:18 UTC by Nick Clemens
Modified: 2023-12-06 04:18 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes match checking for authorities when importing records, so that the required match checks are correctly applied. Previously, match checks for authority records did not work.
Version(s) released in:
23.05.00,22.11.07,22.05.17


Attachments
Bug 26611: Make authority record matching use required match checks (2.68 KB, patch)
2023-05-23 03:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 26611: Make authority record matching use required match checks (2.72 KB, patch)
2023-05-23 23:22 UTC, David Nind
Details | Diff | Splinter Review
Bug 26611: Make authority record matching use required match checks (2.78 KB, patch)
2023-05-24 10:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26611: [22.05] Make authority record matching use required match checks (2.82 KB, patch)
2023-07-10 03:12 UTC, Alex Buckley
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-10-05 18:18:06 UTC
The code specifically ignores these for authorities, I am not sure why?

While we could easily disable the feature for authority matching to follow current behaviour, I believe we need this functionality to be able to match correctly.

Match points check only the first 20 records (see bug 21972) so can only target unique or unique-ish values

Match checks allow you to compare specific fields and can verify that matches above are correct. For example:
Importing records and checking the 010a - which matches the LC-card-number index can have false matches because the index also contains the 010z - the match check would allow us to compare the specific fields in the records as well.
Comment 1 Aleisha Amohia 2023-05-23 03:43:23 UTC
Created attachment 151549 [details] [review]
Bug 26611: Make authority record matching use required match checks

This patch fixes record matching for authorities to correctly apply required match checks.

To test:

1) Create a record matching rule:

Threshold: 100
Record type: Authority record

Match point 1
Search index: subject-topical
Score: 100
Tag: 150
Subfields: a

Match check 1
Both source and target record check fields:
Tag: 040
Subfields: a

This rule says we want to match on the topical term heading, then confirm the match with 040$a.

2) Create two topical term authority records

Authority 1:
150$a: Test
040$a: A

Authority 2:
150$a: Test
040$a: B

3) Export authority 2 and save

4) Go to Cataloging -> Stage records for import

5) Upload your downloaded authority 2 file. Change the record type to Authority. Choose your new record matching rule.

6) Stage for import.

7) Confirm that with the record matching rule applied, both authorities 1 and 2 show as possible matches, even though only authority 2 has a matching 040$a.

8) Apply the patch and restart services

9) Change the matching rule to "Do not look for matching records" and apply. This is to essentially refresh the page.

10) Change the matching rule to your new matching rule and apply. Confirm only the matching authority 2 shows and is selected.

Sponsored-by: Waikato Institute of Technology
Comment 2 David Nind 2023-05-23 23:22:10 UTC
Created attachment 151608 [details] [review]
Bug 26611: Make authority record matching use required match checks

This patch fixes record matching for authorities to correctly apply required match checks.

To test:

1) Create a record matching rule:

Threshold: 100
Record type: Authority record

Match point 1
Search index: subject-topical
Score: 100
Tag: 150
Subfields: a

Match check 1
Both source and target record check fields:
Tag: 040
Subfields: a

This rule says we want to match on the topical term heading, then confirm the match with 040$a.

2) Create two topical term authority records

Authority 1:
150$a: Test
040$a: A

Authority 2:
150$a: Test
040$a: B

3) Export authority 2 and save

4) Go to Cataloging -> Stage records for import

5) Upload your downloaded authority 2 file. Change the record type to Authority. Choose your new record matching rule.

6) Stage for import.

7) Confirm that with the record matching rule applied, both authorities 1 and 2 show as possible matches, even though only authority 2 has a matching 040$a.

8) Apply the patch and restart services

9) Change the matching rule to "Do not look for matching records" and apply. This is to essentially refresh the page.

10) Change the matching rule to your new matching rule and apply. Confirm only the matching authority 2 shows and is selected.

Sponsored-by: Waikato Institute of Technology
Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Nick Clemens 2023-05-24 10:46:50 UTC
Created attachment 151622 [details] [review]
Bug 26611: Make authority record matching use required match checks

This patch fixes record matching for authorities to correctly apply required match checks.

To test:

1) Create a record matching rule:

Threshold: 100
Record type: Authority record

Match point 1
Search index: subject-topical
Score: 100
Tag: 150
Subfields: a

Match check 1
Both source and target record check fields:
Tag: 040
Subfields: a

This rule says we want to match on the topical term heading, then confirm the match with 040$a.

2) Create two topical term authority records

Authority 1:
150$a: Test
040$a: A

Authority 2:
150$a: Test
040$a: B

3) Export authority 2 and save

4) Go to Cataloging -> Stage records for import

5) Upload your downloaded authority 2 file. Change the record type to Authority. Choose your new record matching rule.

6) Stage for import.

7) Confirm that with the record matching rule applied, both authorities 1 and 2 show as possible matches, even though only authority 2 has a matching 040$a.

8) Apply the patch and restart services

9) Change the matching rule to "Do not look for matching records" and apply. This is to essentially refresh the page.

10) Change the matching rule to your new matching rule and apply. Confirm only the matching authority 2 shows and is selected.

Sponsored-by: Waikato Institute of Technology
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 4 Nick Clemens 2023-05-24 10:47:58 UTC
Note: I had to change 'subject-topical' to 'Heading-Main' for the matching to work

This code could really use unit tests, but this patch is mostly a copy from the existing biblio code, and fixes a long standing bug - PQA
Comment 5 Tomás Cohen Arazi 2023-05-24 14:22:25 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 6 Matt Blenkinsop 2023-06-08 17:15:44 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 7 Alex Buckley 2023-07-10 03:12:25 UTC
Created attachment 153236 [details] [review]
Bug 26611: [22.05] Make authority record matching use required match checks

This patch fixes record matching for authorities to correctly apply required match checks.

To test on Koha 22.05:

1) Create a record matching rule:

Threshold: 100
Record type: Authority record

Match point 1
Search index: subject-topical
Score: 100
Tag: 150
Subfields: a

Match check 1
Both source and target record check fields:
Tag: 040
Subfields: a

This rule says we want to match on the topical term heading, then confirm the match with 040$a.

2) Create two topical term authority records

Authority 1:
150$a: Test
040$a: A

Authority 2:
150$a: Test
040$a: B

3) Export authority 2 and save

4) Go to Cataloging -> Stage records for import

5) Upload your downloaded authority 2 file. Change the record type to Authority. Choose your new record matching rule.

6) Stage for import.

7) Confirm that with the record matching rule applied, both authorities 1 and 2 show as possible matches, even though only authority 2 has a matching 040$a.

8) Apply the patch and restart services

9) Change the matching rule to "Do not look for matching records" and apply. This is to essentially refresh the page.

10) Change the matching rule to your new matching rule and apply. Confirm only the matching authority 2 shows and is selected.

Sponsored-by: Waikato Institute of Technology
Comment 8 Alex Buckley 2023-07-10 03:15:48 UTC
The client who sponsored this fix is currently running Koha 22.05.

As Koha 22.05 cannot use 'Koha::Authorities->find( $id )->record;' we have just attached another patch using C4::AuthoritiesMarc->GetAuthorities() instead so this fix can be backported to 22.05
Comment 9 Lucas Gass 2023-11-07 14:28:51 UTC
Backported to 22.05.x for upcoming 22.05.17 release
Comment 10 Martin Renvoize 2023-11-15 15:59:39 UTC
As highlighed above, this is also an issue in 22.11.x.. this patch inadvertantly depends on bug 31794 which adds the record accessor into Koha::Authority
Comment 11 ryszard.d79 2023-11-28 23:59:42 UTC
Hello.
Solution to the problem in version 22.11 (it worked for me)

/C4/Matcher.pm

line ~~33

     remove_spaces
     upper_case
 );
+use C4::AuthoritiesMarc qw( GetAuthority );
 
 =head1 NAME

Line ~~703
 
foreach my $result (@$authresults) {
             my $id = $result->{authid};
- my $target_record = Koha::Authorities->find( $id )->record;
+ my $target_record = GetAuthority($id);
             $matches->{$id}->{score} += $matchpoint->{'score'};
             $matches->{$id}->{record} = $target_record;
            }

my first comment, sorry for the syntax
Comment 12 Katrin Fischer 2023-11-30 21:54:50 UTC
(In reply to ryszard.d79 from comment #11)
> Hello.
> Solution to the problem in version 22.11 (it worked for me)
> 
> /C4/Matcher.pm
> 
> line ~~33
> 
>      remove_spaces
>      upper_case
>  );
> +use C4::AuthoritiesMarc qw( GetAuthority );
>  
>  =head1 NAME
> 
> Line ~~703
>  
> foreach my $result (@$authresults) {
>              my $id = $result->{authid};
> - my $target_record = Koha::Authorities->find( $id )->record;
> + my $target_record = GetAuthority($id);
>              $matches->{$id}->{score} += $matchpoint->{'score'};
>              $matches->{$id}->{record} = $target_record;
>             }
> 
> my first comment, sorry for the syntax

Thanks for sharing. The bug will be fixed with the next release in 22.11 with the patches from 
Bug 35343 - record method, required for bug 26611, missing from Koha::Authority
Comment 13 Aleisha Amohia 2023-12-06 04:18:27 UTC
*** Bug 35485 has been marked as a duplicate of this bug. ***