[Bug 12586] New: Record matching rules - Required match checks doesn't work with MARCXML
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 Bug ID: 12586 Summary: Record matching rules - Required match checks doesn't work with MARCXML Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Cataloging Assignee: gmcharlt@gmail.com Reporter: olli-antti.kivilahti@jns.fi QA Contact: testopia@bugs.koha-community.org CC: m.de.rooy@rijksmuseum.nl When trying to Match MARCXML-records from Zebra using Required match checks, C4::Matcher::_passes_required_checks() tries to make a MARC::Record out of MARCXML while thinking it is USMARC. This leads to matching records getting not-matched, and subsequently importing the same records again and again. Very hard to debug or create a test case. I accidentally noticed this while building an automatic acquisitions record overlay mechanism. There are no big errors, simply no match is found, even though a match exists. --------- TEST PLAN --------- 1. Create a Matcher with "Required match checks" in addition to normal checks. 2. Find matches using the C4::Matcher->get_matches() and notice that you don't find matches. 3. Somebody who is actually using this Matcher could help me out here with defining a test plan. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 --- Comment #1 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 29758 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29758&action=edit le patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |olli-antti.kivilahti@jns.fi -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 --- Comment #2 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Also refactored the Matcher a bit to prevent excessive MARC::Record creation for each "Required match check" for each search results. This probably would cripple any reasonable batch Matching operations. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #3 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Setting this to needs signoff, because there is no "needs help" -status -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 --- Comment #4 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 29948 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29948&action=edit Bug 12586 - Record matching rules - Required match checks doesn't work with MARCXML When trying to Match MARCXML-records from Zebra using Required match checks, C4::Matcher::_passes_required_checks() tries to make a MARC::Record out of MARCXML while thinking it is USMARC. This leads to matching records getting not-matched, and subsequently importing the same records again and again. Very hard to debug or create a test case. I accidentally noticed this while building an automatic acquisitions record overlay mechanism. There are no big errors, simply no match is found, even though a match exists. Also refactored the Matcher a bit to prevent excessive MARC::Record creation for each "Required match check" for each search results. This probably would cripple any reasonably sized batch Matching operations. --------- TEST PLAN --------- 1. Create a Matcher with "Required match checks" in addition to normal checks. 2. Find matches using the C4::Matcher->get_matches() and notice that you don't find matches. 3. Somebody who is actually using this Matcher could help me out here with defining a test plan. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29758|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 Zeno Tajoli <z.tajoli@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 29948 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29948 Bug 12586 - Record matching rules - Required match checks doesn't work with MARCXML Review of attachment 29948: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=12586&attachment=29948) ----------------------------------------------------------------- I ran into this problem today and this is pretty much exactly what I had in mind for a fix. I've just added a couple comments about things to change. Suggested test plan: 1) Create a bibliographic record with a control number of 123456 2) Create a matching rule with the following: a) Search index "Control-number" b) Score 100 c) Matchpoint components tag 001 d) Match check Source Tag 001 e) Match check Target Tag 001 3) Download the bibliographic from Step 1 as ISO MARC 4) Upload it via Stage Marc for Import, and use the record matching rule from Step 2 Before applying patch: 5) Note that the no records match with your rule Apply the patch: 6) Repeat Step 4 (ie upload using the matching rule from step 2) 7) Note that the record DOES match using the matching rule ::: C4/Matcher.pm @@ +689,5 @@
+ foreach my $matched ( @{$searchresults} ) { + my $record = C4::Search::new_record_from_zebra( 'biblioserver', $matched ); + $matches{$record}->{score} += $matchpoint->{'score'}; #Using $record HASH string representation as the key :) + $matches{$record}->{record} = $record; + }
This block all looks good. @@ +704,1 @@
keys %matches unless ($self->{'record_type'} eq 'auth');
There's no such record_type as 'auth'... this should be 'authority'. I know it's not related to this bug per se, but may as well change it now. @@ +711,4 @@
my $record_number; my $result = C4::Biblio::TransformMarcToKoha(C4::Context->dbh, $target_record, ''); $record_number = $result->{'biblionumber'}; + push @results, { 'record_id' => $record_number, 'score' => $matches{$hashkey}->{score}, 'target_record' => $target_record, 'target_biblio' => $result };
Why have you included "target_record" and "target_biblio" here? This seems unnecessary and is an addition to the original code. If it's being used elsewhere, I think that should be separated into a separate patch. @@ +760,4 @@
}
sub _passes_required_checks { + my ($source_record, $target_record, $matchchecks) = @_;
Looks good. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29948|0 |1 is obsolete| | --- Comment #6 from David Cook <dcook@prosentient.com.au> --- Created attachment 46380 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46380&action=edit Bug 12586 - Record matching rules - Required match checks doesn't work with MARCXML When trying to Match MARCXML-records from Zebra using Required match checks, C4::Matcher::_passes_required_checks() tries to make a MARC::Record out of MARCXML while thinking it is USMARC. This leads to matching records getting not-matched, and subsequently importing the same records again and again. Very hard to debug or create a test case. I accidentally noticed this while building an automatic acquisitions record overlay mechanism. There are no big errors, simply no match is found, even though a match exists. Also refactored the Matcher a bit to prevent excessive MARC::Record creation for each "Required match check" for each search results. This probably would cripple any reasonably sized batch Matching operations. --------- TEST PLAN --------- 1. Create a Matcher with "Required match checks" in addition to normal checks. 2. Find matches using the C4::Matcher->get_matches() and notice that you don't find matches. 3. Somebody who is actually using this Matcher could help me out here with defining a test plan. Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- Created attachment 46381 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46381&action=edit Bug 12586 - Record matching rules - Required match checks doesn't work with MARCXML (follow-up) The follow-up fixes a typo ('auth' to 'authority') and removes 'target_result' and 'target_biblio' which were introduced, seemingly unnecessarily, by the first patch in the set. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- So I've signed off Olli's original fix, and I added a follow-up to fix a typo in a related bit of code, and to remove some seemingly superfluous data added by Olli's fix. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 --- Comment #9 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 46380 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46380 Bug 12586 - Record matching rules - Required match checks doesn't work with MARCXML Review of attachment 46380: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=12586&attachment=46380) ----------------------------------------------------------------- ::: C4/Matcher.pm @@ +703,5 @@
+ foreach my $matched ( @{$searchresults} ) { + my $record = C4::Search::new_record_from_zebra( 'biblioserver', $matched ); + $matches{$record}->{score} += $matchpoint->{'score'}; #Using $record HASH string representation as the key :) + $matches{$record}->{record} = $record; + }
Actually, looking at this again, there's no point repeating the loop... it would've made more sense to have 1 foreach loop and done the record_type checks within it... Not a big deal though really -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- Maybe also worth mentioning that this fix ONLY fixes match checks for bibliographic records... Match checks don't work for authority records. More refactoring could be done to fix that... but that's outside the scope of this bug I think. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12586 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org