Bugzilla – Attachment 166441 Details for
Bug 34788
Add the ability to import KBART files to ERM
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 34788: (QA follow-up)
Bug-34788-QA-follow-up.patch (text/plain), 18.67 KB, created by
Nick Clemens (kidclamp)
on 2024-05-08 19:36:52 UTC
(
hide
)
Description:
Bug 34788: (QA follow-up)
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2024-05-08 19:36:52 UTC
Size:
18.67 KB
patch
obsolete
>From 69c7b1308083954d1f4e0102d2de410c7163c395 Mon Sep 17 00:00:00 2001 >From: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> >Date: Wed, 8 May 2024 14:29:36 +0000 >Subject: [PATCH] Bug 34788: (QA follow-up) > >This patch fixes a few things: >1) The blue dialog box now clears when navigating away from the page >2) The background job now uses skip_record_index to avoid queuing indexing jobs for every new biblio and instead queues one job at the end >3) Large files that get chunked now successfully create linked biblios if requested >4) Title matching rules have been expanded to check the package ID so that we can have duplicate titles in different packages >5) A link to the package is now included on the job report page > >Signed-off-by: Nick Clemens <nick@bywatersolutions.com> >--- > Koha/BackgroundJob/ImportKBARTFile.pm | 30 ++++++++--- > Koha/ERM/EHoldings/Title.pm | 4 +- > Koha/REST/V1/ERM/EHoldings/Titles/Local.pm | 14 ++---- > .../import_from_kbart_file.inc | 1 + > .../ERM/EHoldingsLocalTitlesKBARTImport.vue | 13 +++-- > .../Koha/BackgroundJob/ImportKBARTFile.t | 50 +++++++++++-------- > 6 files changed, 68 insertions(+), 44 deletions(-) > >diff --git a/Koha/BackgroundJob/ImportKBARTFile.pm b/Koha/BackgroundJob/ImportKBARTFile.pm >index 398476e3f54..de9d9606bae 100644 >--- a/Koha/BackgroundJob/ImportKBARTFile.pm >+++ b/Koha/BackgroundJob/ImportKBARTFile.pm >@@ -24,6 +24,7 @@ use POSIX qw( floor ); > use C4::Context; > > use Koha::ERM::EHoldings::Titles; >+use Koha::SearchEngine::Indexer; > > use base 'Koha::BackgroundJob'; > >@@ -94,10 +95,12 @@ sub process { > $self->size( scalar( @{$rows} ) )->store; > $total_rows = scalar( @{$rows} ); > >+ my @biblio_ids; >+ > foreach my $row ( @{$rows} ) { > next if !$row; > my $new_title = create_title_hash_from_line_data( $row, $column_headers, $invalid_columns ); >- my $title_match = check_for_matching_title($new_title); >+ my $title_match = check_for_matching_title( $new_title, $package_id ); > > if ($title_match) { > $duplicate_titles++; >@@ -122,6 +125,7 @@ sub process { > } else { > my $imported_title = Koha::ERM::EHoldings::Title->new($formatted_title) > ->store( { create_linked_biblio => $create_linked_biblio } ); >+ push( @biblio_ids, $imported_title->biblio_id ) if $create_linked_biblio; > create_linked_resource( > { > title => $imported_title, >@@ -146,10 +150,16 @@ sub process { > $self->step; > } > >+ if ( scalar(@biblio_ids) > 0 ) { >+ my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } ); >+ $indexer->index_records( \@biblio_ids, "specialUpdate", "biblioserver" ); >+ } >+ > $report->{duplicates_found} = $duplicate_titles; > $report->{titles_imported} = $titles_imported; > $report->{total_rows} = $total_rows; > $report->{failed_imports} = $failed_imports; >+ $report->{package_id} = $package_id; > > my $data = $self->decoded_data; > $data->{messages} = \@messages; >@@ -266,7 +276,7 @@ sub create_title_hash_from_line_data { > } > > # Remove any additional columns >- foreach my $invalid_column ( @$invalid_columns ) { >+ foreach my $invalid_column (@$invalid_columns) { > delete $new_title{$invalid_column}; > } > >@@ -280,7 +290,7 @@ Checks whether this title already exists to avoid duplicates > =cut > > sub check_for_matching_title { >- my ($title) = @_; >+ my ( $title, $package_id ) = @_; > > my $match_parameters = {}; > $match_parameters->{print_identifier} = $title->{print_identifier} if $title->{print_identifier}; >@@ -290,14 +300,20 @@ sub check_for_matching_title { > $match_parameters->{external_id} = $title->{title_id} if $title->{title_id}; > > # We should also check the date_first_issue_online for serial publications >- $match_parameters->{date_first_issue_online} = $title->{date_first_issue_online} if $title->{date_first_issue_online}; >+ $match_parameters->{date_first_issue_online} = $title->{date_first_issue_online} >+ if $title->{date_first_issue_online}; > > # If no match parameters are provided in the file we should add the new title > return 0 if !%$match_parameters; > >- my $title_match = Koha::ERM::EHoldings::Titles->search($match_parameters)->count; >+ my $matching_title_found; >+ my @title_matches = Koha::ERM::EHoldings::Titles->search($match_parameters)->as_list; >+ foreach my $title_match (@title_matches) { >+ my $resource = Koha::ERM::EHoldings::Resources->find( { title_id => $title_match->title_id } ); >+ $matching_title_found = 1 if $resource->package_id == $package_id; >+ } > >- return $title_match; >+ return $matching_title_found; > } > > =head3 create_linked_resource >@@ -437,7 +453,7 @@ sub is_file_too_large { > > =head3 rescue_EBSCO_files > >-EBSCO have an incorrect spelling for "preceding_publication_title_id" in all of their KBART files ("preceeding" instead of "preceding"). >+EBSCO have an incorrect spelling for "preceding_publication_title_id" in all of their KBART files (preceding is spelled with a double 'e'). > This means all of their KBART files fail to import using the current methodology. > There is no simple way of finding out who the vendor is before importing so all KBART files from any vendor are going to have to be checked for this spelling and corrected. > >diff --git a/Koha/ERM/EHoldings/Title.pm b/Koha/ERM/EHoldings/Title.pm >index e11c06abf5f..ea62ecb3602 100644 >--- a/Koha/ERM/EHoldings/Title.pm >+++ b/Koha/ERM/EHoldings/Title.pm >@@ -55,7 +55,7 @@ sub store { > my $record = $biblio->metadata->record(); > my $title_field = $record->field($title_tag); > $title_field->update( $title_subfield => $self->publication_title ); >- C4::Biblio::ModBiblio( $record, $self->biblio_id, '' ); >+ C4::Biblio::ModBiblio( $record, $self->biblio_id, '', { skip_record_index => 1 } ); > } else { > > # If it's not linked, we create a simple biblio and save the biblio id to the 'title' >@@ -64,7 +64,7 @@ sub store { > 'biblio.title' => $self->publication_title, > } > ); >- my ($biblio_id) = C4::Biblio::AddBiblio( $marc_record, '' ); >+ my ($biblio_id) = C4::Biblio::AddBiblio( $marc_record, '', { skip_record_index => 1 } ); > $self->biblio_id($biblio_id); > } > } >diff --git a/Koha/REST/V1/ERM/EHoldings/Titles/Local.pm b/Koha/REST/V1/ERM/EHoldings/Titles/Local.pm >index 5e363bc05bf..f2cf4c33700 100644 >--- a/Koha/REST/V1/ERM/EHoldings/Titles/Local.pm >+++ b/Koha/REST/V1/ERM/EHoldings/Titles/Local.pm >@@ -24,9 +24,9 @@ use Koha::BackgroundJob::CreateEHoldingsFromBiblios; > use Koha::BackgroundJob::ImportKBARTFile; > > use Scalar::Util qw( blessed ); >-use Try::Tiny qw( catch try ); >+use Try::Tiny qw( catch try ); > use MIME::Base64 qw( decode_base64 encode_base64 ); >-use POSIX qw( floor ); >+use POSIX qw( floor ); > use Text::CSV_XS; > > =head1 API >@@ -294,7 +294,7 @@ sub import_from_kbart_file { > > # Check that the column headers in the file match the standardised KBART phase II columns > # If not, return a warning >- my $warnings = {}; >+ my $warnings = {}; > my @valid_headers = Koha::BackgroundJob::ImportKBARTFile::get_valid_headers(); > foreach my $header (@$column_headers) { > if ( !grep { $_ eq $header } @valid_headers ) { >@@ -324,13 +324,7 @@ sub import_from_kbart_file { > my @chunked_files; > push @chunked_files, [ splice @$rows, 0, $max_number_of_rows ] while @$rows; > foreach my $chunk (@chunked_files) { >- my $params = { >- column_headers => $column_headers, >- rows => $chunk, >- package_id => $package_id, >- file_name => $file->{filename} >- }; >- >+ $params->{rows} = $chunk; > my $chunked_job_id = Koha::BackgroundJob::ImportKBARTFile->new->enqueue($params); > push @job_ids, $chunked_job_id; > } >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/background_jobs/import_from_kbart_file.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/background_jobs/import_from_kbart_file.inc >index 28d0dfb5f32..04032e0f432 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/background_jobs/import_from_kbart_file.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/background_jobs/import_from_kbart_file.inc >@@ -28,6 +28,7 @@ > <td>[% report.failed_imports | html %]</td> > </tr> > </table> >+ <a href="/cgi-bin/koha/erm/eholdings/local/packages/[% report.package_id | $raw %]">Click here to see the package</a> > [% ELSIF job.status == 'started' %] > <p id="jobactionlabel"></p> > [% END %] >diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/EHoldingsLocalTitlesKBARTImport.vue b/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/EHoldingsLocalTitlesKBARTImport.vue >index 6a75b2118bf..7cdfc0aef93 100644 >--- a/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/EHoldingsLocalTitlesKBARTImport.vue >+++ b/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/EHoldingsLocalTitlesKBARTImport.vue >@@ -80,10 +80,15 @@ > <script> > import ButtonSubmit from "../ButtonSubmit.vue" > import { APIClient } from "../../fetch/api-client.js" >-import { setMessage, setWarning } from "../../messages" >-import { ref } from "vue" >+import { ref, inject } from "vue" > > export default { >+ setup() { >+ const { setMessage } = inject("mainStore") >+ return { >+ setMessage, >+ } >+ }, > data() { > const fileLoader = ref() > return { >@@ -143,7 +148,7 @@ export default { > '<li>Job %s for uploaded file has been queued, <a href="/cgi-bin/koha/admin/background_jobs.pl?op=view&id=%s" target="_blank">click here</a> to check its progress.</li>' > ).format(i + 1, job) > }) >- setMessage(message, true) >+ this.setMessage(message, true) > } > if (success.warnings.invalid_columns) { > message += this.$__( >@@ -158,7 +163,7 @@ export default { > message += this.$__( > "<p style='margin-top: 0.1em;'>The data in these columns will not be imported.</p>" > ) >- setMessage(message) >+ this.setMessage(message, true) > } > if (success.invalid_filetype) { > message += this.$__( >diff --git a/t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t b/t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t >index a3486a7ed0b..0893ab1492e 100755 >--- a/t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t >+++ b/t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t >@@ -79,11 +79,11 @@ subtest 'read_file' => sub { > plan tests => 6; > > my $file = { >- filename => 'Test_file.tsv', >+ filename => 'Test_file.csv', > file_content => encode_base64( >- 'publication_title print_identifier online_identifier date_first_issue_online num_first_vol_online num_first_issue_online date_last_issue_online num_last_vol_online num_last_issue_online title_url first_author title_id embargo_info coverage_depth coverage_notes publisher_name publication_type date_monograph_published_print date_monograph_published_online monograph_volume monograph_edition first_editor parent_publication_title_id preceding_publication_title_id access_type >-Nature Plants 2055-0278 2015-01 1 1 https://www.nature.com/nplants 4aaa7 fulltext Hybrid (Open Choice) Nature Publishing Group UK serial P >-Nature Astronomy 2397-3366 2017-01 1 1 https://www.nature.com/natastron 4bbb0 fulltext Hybrid (Open Choice) Nature Publishing Group UK serial P' >+ 'publication_title,print_identifier,online_identifier,date_first_issue_online,num_first_vol_online,num_first_issue_online,date_last_issue_online,num_last_vol_online,num_last_issue_online,title_url,first_author,title_id,embargo_info,coverage_depth,coverage_notes,publisher_name,publication_type,date_monograph_published_print,date_monograph_published_online,monograph_volume,monograph_edition,first_editor,parent_publication_title_id,preceding_publication_title_id,access_type >+Nature Plants,,2055-0278,2015-01,1,1,,,,https://www.nature.com/nplants,,4aaa7,,fulltext,Hybrid (Open Choice),Nature Publishing Group UK,serial,,,,,,,,P >+Nature Astronomy,,2397-3366,2017-01,1,1,,,,https://www.nature.com/natastron,,4bbb0,,fulltext,Hybrid (Open Choice),Nature Publishing Group UK,serial,,,,,,,,P' > ) > }; > >@@ -119,21 +119,25 @@ subtest 'create_title_hash_from_line_data' => sub { > plan tests => 2; > > my $file = { >- filename => 'Test_file.tsv', >+ filename => 'Test_file.csv', > file_content => encode_base64( >- 'publication_title print_identifier online_identifier date_first_issue_online num_first_vol_online num_first_issue_online date_last_issue_online num_last_vol_online num_last_issue_online title_url first_author title_id embargo_info coverage_depth coverage_notes publisher_name publication_type date_monograph_published_print date_monograph_published_online monograph_volume monograph_edition first_editor parent_publication_title_id preceding_publication_title_id access_type >-Nature Plants 2055-0278 2015-01 1 1 https://www.nature.com/nplants 4aaa7 fulltext Hybrid (Open Choice) Nature Publishing Group UK serial P >-Nature Astronomy 2397-3366 2017-01 1 1 https://www.nature.com/natastron 4bbb0 fulltext Hybrid (Open Choice) Nature Publishing Group UK serial P' >+ 'publication_title,print_identifier,online_identifier,date_first_issue_online,num_first_vol_online,num_first_issue_online,date_last_issue_online,num_last_vol_online,num_last_issue_online,title_url,first_author,title_id,embargo_info,coverage_depth,coverage_notes,publisher_name,publication_type,date_monograph_published_print,date_monograph_published_online,monograph_volume,monograph_edition,first_editor,parent_publication_title_id,preceding_publication_title_id,access_type >+Nature Plants,,2055-0278,2015-01,1,1,,,,https://www.nature.com/nplants,,4aaa7,,fulltext,Hybrid (Open Choice),Nature Publishing Group UK,serial,,,,,,,,P >+Nature Astronomy,,2397-3366,2017-01,1,1,,,,https://www.nature.com/natastron,,4bbb0,,fulltext,Hybrid (Open Choice),Nature Publishing Group UK,serial,,,,,,,,P' > ) > }; > > my ( $column_headers, $lines ) = Koha::BackgroundJob::ImportKBARTFile::read_file($file); > my @invalid_columns; > >- my $title_from_line1 = >- Koha::BackgroundJob::ImportKBARTFile::create_title_hash_from_line_data( @{$lines}[0], $column_headers, \@invalid_columns ); >- my $title_from_line2 = >- Koha::BackgroundJob::ImportKBARTFile::create_title_hash_from_line_data( @{$lines}[1], $column_headers, \@invalid_columns ); >+ my $title_from_line1 = Koha::BackgroundJob::ImportKBARTFile::create_title_hash_from_line_data( >+ @{$lines}[0], $column_headers, >+ \@invalid_columns >+ ); >+ my $title_from_line2 = Koha::BackgroundJob::ImportKBARTFile::create_title_hash_from_line_data( >+ @{$lines}[1], $column_headers, >+ \@invalid_columns >+ ); > > my $line1_match = { > 'coverage_depth' => 'fulltext', >@@ -210,10 +214,14 @@ Nature Astronomy,,2397-3366,2017-01,1,1,,,,https://www.nature.com/natastron,,4bb > my ( $column_headers, $lines ) = Koha::BackgroundJob::ImportKBARTFile::read_file($file); > my @invalid_columns = ('invalid_column'); > >- my $title_from_line1 = >- Koha::BackgroundJob::ImportKBARTFile::create_title_hash_from_line_data( @{$lines}[0], $column_headers, \@invalid_columns ); >- my $title_from_line2 = >- Koha::BackgroundJob::ImportKBARTFile::create_title_hash_from_line_data( @{$lines}[1], $column_headers, \@invalid_columns ); >+ my $title_from_line1 = Koha::BackgroundJob::ImportKBARTFile::create_title_hash_from_line_data( >+ @{$lines}[0], $column_headers, >+ \@invalid_columns >+ ); >+ my $title_from_line2 = Koha::BackgroundJob::ImportKBARTFile::create_title_hash_from_line_data( >+ @{$lines}[1], $column_headers, >+ \@invalid_columns >+ ); > > my $line1_match = { > 'coverage_depth' => 'fulltext', >@@ -288,11 +296,11 @@ subtest 'process' => sub { > ); > > my $file = { >- filename => 'Test_file.tsv', >+ filename => 'Test_file.csv', > file_content => encode_base64( >- 'publication_title print_identifier online_identifier date_first_issue_online num_first_vol_online num_first_issue_online date_last_issue_online num_last_vol_online num_last_issue_online title_url first_author title_id embargo_info coverage_depth coverage_notes publisher_name publication_type date_monograph_published_print date_monograph_published_online monograph_volume monograph_edition first_editor parent_publication_title_id preceding_publication_title_id access_type >-Nature Plants 2055-0278 2015-01 1 1 https://www.nature.com/nplants 4aaa7 fulltext Hybrid (Open Choice) Nature Publishing Group UK serial P >-Nature Astronomy 2397-3366 2017-01 1 1 https://www.nature.com/natastron 4bbb0 fulltext Hybrid (Open Choice) Nature Publishing Group UK serial P' >+ 'publication_title,print_identifier,online_identifier,date_first_issue_online,num_first_vol_online,num_first_issue_online,date_last_issue_online,num_last_vol_online,num_last_issue_online,title_url,first_author,title_id,embargo_info,coverage_depth,coverage_notes,publisher_name,publication_type,date_monograph_published_print,date_monograph_published_online,monograph_volume,monograph_edition,first_editor,parent_publication_title_id,preceding_publication_title_id,access_type >+Nature Plants,,2055-0278,2015-01,1,1,,,,https://www.nature.com/nplants,,4aaa7,,fulltext,Hybrid (Open Choice),Nature Publishing Group UK,serial,,,,,,,,P >+Nature Astronomy,,2397-3366,2017-01,1,1,,,,https://www.nature.com/natastron,,4bbb0,,fulltext,Hybrid (Open Choice),Nature Publishing Group UK,serial,,,,,,,,P' > ) > }; > >@@ -453,4 +461,4 @@ Nature Astronomy 2397-3366 2017-01 1 1 https://www.nature.com/natastron 4bb > is( @{ $job4->messages }[1]->{code}, 'title_already_exists', 'Error message for a duplicate title' ); > > $schema->storage->txn_rollback; >-} >\ No newline at end of file >+ } >-- >2.39.2
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=166441">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 34788
:
161379
|
161380
|
161381
|
161382
|
161383
|
161384
|
161391
|
165189
|
165190
|
165191
|
165192
|
165193
|
165194
|
165195
|
165196
|
165197
|
165837
|
165887
|
165888
|
165889
|
165890
|
165891
|
165892
|
165893
|
165894
|
166420
|
166433
|
166434
|
166435
|
166436
|
166437
|
166438
|
166439
|
166440
| 166441 |
166571
|
166645