From fe558c6486ec0f4d7fbfdbc0c4649085b4ceb1a4 Mon Sep 17 00:00:00 2001 From: Jesse Weaver Date: Mon, 30 Nov 2015 20:05:57 -0700 Subject: [PATCH] Bug 19266: Allow 001 checking on save to batch and 005 update on import of records To test: 1 - Export a set of records with control numbers from your system 2 - Stage those records 3 - Note during staging a new option to update the 005 4 - Check records after staging 5 - Note 005 is updated or not as the option is selected 6 - Open the advanced cataloging editor 7 - Under settings choose Import Batches 8 - Set the batch you just imported as a save target 9 - Select that batch only as save target 10 - Create a new record, give it the control number of a record in the batch 11 - Attempt to save to the batch 12 - You get an error 13 - Save overriding warnings 14 - The record is then added to the batch --- C4/Biblio.pm | 12 +- C4/ImportBatch.pm | 155 +++++++++++++++------ .../prog/en/modules/tools/stage-marc-import.tt | 30 ++++ misc/stage_file.pl | 34 +++-- svc/cataloguing/import_batches | 30 +--- tools/stage-marc-import.pl | 59 ++++---- 6 files changed, 206 insertions(+), 114 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 98f138c..bb91183 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -3240,9 +3240,9 @@ sub _koha_delete_biblio_metadata { =head1 UNEXPORTED FUNCTIONS -=head2 Update005Time +=head2 UpdateMarcTimestamp - &Update005Time( $record ); + &UpdateMarcTimestamp( $record ); Updates the 005 timestamp of the given record to the current time. @@ -3257,7 +3257,13 @@ sub UpdateMarcTimestamp { my @a = (localtime) [5,4,3,2,1,0]; $a[0]+=1900; $a[1]++; # YY MM DD HH MM SS (update year and month) my $f005 = $record->field('005'); - $f005->update( sprintf( "%4d%02d%02d%02d%02d%04.1f",@a ) ) if $f005; + + unless ($f005) { + $f005 = MARC::Field->new('005'); + $record->insert_fields_ordered( $f005 ); + } + + $f005->update( sprintf( "%4d%02d%02d%02d%02d%04.1f",@a ) ); } } diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm index 721f893..a9c6ce1 100644 --- a/C4/ImportBatch.pm +++ b/C4/ImportBatch.pm @@ -29,6 +29,7 @@ use C4::AuthoritiesMarc; use C4::MarcModificationTemplates; use DateTime; use DateTime::Format::Strptime; +use Koha::Database; use Koha::Plugins::Handler; use Koha::Logger; @@ -49,6 +50,7 @@ BEGIN { AddItemsToImportBiblio ModAuthorityInBatch ModBiblioInBatch + CheckRecordControlNumberInBatch BatchStageMarcRecords BatchFindDuplicates @@ -312,6 +314,41 @@ sub ModBiblioInBatch { } +=head2 CheckRecordControlNumberInBatch + + CheckRecordControlNumberInBatch( $marc_record, $batch_id[, $import_record_id] ); + +Check to see if the given record's control number is already in use in the given batch. If provided, +will check only for conflicts in records besides the given C<$import_batch_id>. + +If a record with the given control number in the given batch exists, will return its C. + +=cut + +sub CheckRecordControlNumberInBatch { + my ( $marc_record, $batch_id, $import_record_id ) = @_; + + return unless ( $marc_record->field('001') ); + + my %extra_conditions; + + $extra_conditions{'import_record.import_record_id'} = { '!=', $import_record_id } if ( $import_record_id ); + + my $control_number = $marc_record->field('001')->data; + + my $schema = Koha::Database->new()->schema(); + return $schema->resultset('ImportBiblio')->search( + { + control_number => $control_number, + 'import_record.import_batch_id' => $batch_id, + %extra_conditions + }, + { + join => 'import_record', + } + )->get_column('import_record.import_batch_id')->first(); +} + =head2 AddAuthToBatch my $import_record_id = AddAuthToBatch($batch_id, $record_sequence, @@ -352,77 +389,82 @@ sub ModAuthInBatch { =head2 BatchStageMarcRecords -( $batch_id, $num_records, $num_items, @invalid_records ) = - BatchStageMarcRecords( - $record_type, $encoding, - $marc_records, $file_name, - $marc_modification_template, $comments, - $branch_code, $parse_items, - $leave_as_staging, $progress_interval, - $progress_callback - ); +{ + batch_id => $batch_id, + num_records => $num_records, + num_items => $num_items, + invalid_records => \@invalid_records +} = BatchStageMarcRecords( { + record_type => $record_type, + encoding => $encoding, + marc_records => $marc_records, + file_name => $file_name, + comments => $comments, + [ control_number_handling => 'preserve' | 'overwrite' ], + [ existing_batch_id => $existing_batch_id, ] + [ leave_as_staging => 1 ] + [ marc_modification_template => $marc_modification_template, ] + [ parse_items => 1 ] + [ progress_interval => $progress_interval, + progress_callback => \&progress_callback, ] + [ timestamp_update => 'now' ], + [ to_marc_plugin => $to_marc_plugin, ] +} ); + +Any optional parameters are assumed to be no / off if omitted. If C is specified, C must be as well. =cut sub BatchStageMarcRecords { - my $record_type = shift; - my $encoding = shift; - my $marc_records = shift; - my $file_name = shift; - my $marc_modification_template = shift; - my $comments = shift; - my $branch_code = shift; - my $parse_items = shift; - my $leave_as_staging = shift; - + my $params = shift; # optional callback to monitor status # of job my $progress_interval = 0; my $progress_callback = undef; - if ($#_ >= 1) { - $progress_interval = shift; - $progress_callback = shift; + if ( $params->{progress_interval} ) { + $progress_interval = $params->{progress_interval}; + $progress_callback = $params->{progress_callback}; $progress_interval = 0 unless $progress_interval =~ /^\d+$/ and $progress_interval > 0; $progress_interval = 0 unless 'CODE' eq ref $progress_callback; } - my $existing_batch_id = shift; my $batch_id; - if ( $existing_batch_id ) { - $batch_id = $existing_batch_id; + if ( $params->{existing_batch_id} ) { + $batch_id = $params->{existing_batch_id}; } else { $batch_id = AddImportBatch( { overlay_action => 'create_new', import_status => 'staging', batch_type => 'batch', - file_name => $file_name, - comments => $comments, - record_type => $record_type, + file_name => $params->{file_name}, + comments => $params->{comments}, + record_type => $params->{record_type}, } ); } - if ($parse_items) { + if ($params->{parse_items}) { SetImportBatchItemAction($batch_id, 'always_add'); } else { SetImportBatchItemAction($batch_id, 'ignore'); } - my $marc_type = C4::Context->preference('marcflavour'); - $marc_type .= 'AUTH' if ($marc_type eq 'UNIMARC' && $record_type eq 'auth'); + $marc_type .= 'AUTH' if ($marc_type eq 'UNIMARC' && $params->{record_type} eq 'auth'); my @invalid_records = (); my $num_valid = 0; my $num_items = 0; + my $num_matched_control_number = 0; # FIXME - for now, we're dealing only with bibs my $rec_num = 0; - foreach my $marc_record (@$marc_records) { + foreach my $marc_record (@{$params->{marc_records}}) { + next unless $marc_record; $rec_num++; if ($progress_interval and (0 == ($rec_num % $progress_interval))) { &$progress_callback($rec_num); } - ModifyRecordWithTemplate( $marc_modification_template, $marc_record ) if ( $marc_modification_template ); + ModifyRecordWithTemplate( $params->{marc_modification_template}, $marc_record ) if ( $params->{marc_modification_template} ); my $import_record_id; if (scalar($marc_record->fields()) == 0) { @@ -431,25 +473,55 @@ sub BatchStageMarcRecords { # Normalize the record so it doesn't have separated diacritics SetUTF8Flag($marc_record); - $num_valid++; - if ($record_type eq 'biblio') { - $import_record_id = AddBiblioToBatch($batch_id, $rec_num, $marc_record, $encoding, int(rand(99999)), 0); - if ($parse_items) { + + C4::Biblio::UpdateMarcTimestamp( $marc_record ) if ( $params->{timestamp_update} eq 'now' ); + my $existing_import_record_id; + + if ( $params->{control_number_handling} ) { + $existing_import_record_id = CheckRecordControlNumberInBatch( $marc_record, $batch_id ); + + if ( $existing_import_record_id ) { + $num_matched_control_number++; + + next if ( $params->{control_number_handling} eq 'preserve' ); + } + } + + if ($params->{record_type} eq 'biblio') { + if ( $existing_import_record_id ) { + $import_record_id = $existing_import_record_id; + ModBiblioInBatch( $import_record_id, $marc_record ); + } else { + $import_record_id = AddBiblioToBatch($batch_id, $rec_num, $marc_record, $params->{encoding}, int(rand(99999)), 0); + } + + if ($params->{parse_items}) { my @import_items_ids = AddItemsToImportBiblio($batch_id, $import_record_id, $marc_record, 0); $num_items += scalar(@import_items_ids); } - } elsif ($record_type eq 'auth') { - $import_record_id = AddAuthToBatch($batch_id, $rec_num, $marc_record, $encoding, int(rand(99999)), 0, $marc_type); + } elsif ($params->{record_type} eq 'auth') { + if ( $existing_import_record_id ) { + $import_record_id = $existing_import_record_id; + ModAuthInBatch( $import_record_id, $marc_record ); + } else { + $import_record_id = AddAuthToBatch($batch_id, $rec_num, $marc_record, $params->{encoding}, int(rand(99999)), 0, $marc_type); + } } } } - unless ($leave_as_staging) { + unless ($params->{leave_as_staging}) { SetImportBatchStatus($batch_id, 'staged'); } # FIXME branch_code, number of bibs, number of items _update_batch_record_counts($batch_id); - return ($batch_id, $num_valid, $num_items, @invalid_records); + return { + batch_id => $batch_id, + num_items => $num_items, + num_valid => $num_valid, + num_matched_control_number => $num_matched_control_number, + invalid_records => \@invalid_records + }; } =head2 AddItemsToImportBiblio @@ -468,6 +540,7 @@ sub AddItemsToImportBiblio { my @import_items_ids = (); my $dbh = C4::Context->dbh; + $dbh->do( "DELETE FROM import_items WHERE import_record_id = ?", undef, $import_record_id ); my ($item_tag,$item_subfield) = &GetMarcFromKohaField("items.itemnumber",''); foreach my $item_field ($marc_record->field($item_tag)) { my $item_marc = MARC::Record->new(); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt index 67a48c7..ce6520a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt @@ -40,6 +40,16 @@
  • [% total %] records in file
  • [% import_errors %] records not staged because of MARC error
  • [% staged %] records staged
  • + [% IF control_number_handling %] +
  • + [% num_matched_control_number %] records with matching control number + [% IF control_number_handling == 'preserve' %] + (preserved) + [% ELSIF control_number_handling == 'overwrite' %] + (overwritten) + [% END %] +
  • + [% END %] [% IF ( checked_matches ) %]
  • [% matched %] records with at least one match in catalog per matching rule "[% matcher_code %]"
  • @@ -166,6 +176,26 @@ +
    + Automatically update records? +
      + [% IF existing_batch_id %] +
    1. + +
    2. + [% END %] +
    3. + +
    4. +
    +
    Check for embedded item record data?
      diff --git a/misc/stage_file.pl b/misc/stage_file.pl index 1f11b6a..5433863 100755 --- a/misc/stage_file.pl +++ b/misc/stage_file.pl @@ -115,16 +115,24 @@ sub process_batch { print "... staging MARC records -- please wait\n"; #FIXME: We should really allow the use of marc modification frameworks and to_marc plugins here if possible - my ($batch_id, $num_valid_records, $num_items, @import_errors) = - BatchStageMarcRecords($record_type, $params->{encoding}, $marc_records, $params->{input_file}, undef, $params->{batch_comment}, '', $params->{add_items}, 0, - 100, \&print_progress_and_commit); + my (@import_errors) = + my $stage_results = BatchStageMarcRecords( { + record_type => $record_type, + encoding => $encoding, + marc_records => $marc_records, + file_name => $input_file, + comments => $batch_comment, + parse_items => $add_items, + progress_interval => 100, + progress_callback => \&print_progress_and_commit + } ); print "... finished staging MARC records\n"; my $num_with_matches = 0; if ( $params->{match} ) { my $matcher = C4::Matcher->fetch( $params->{match} ); if (defined $matcher) { - SetImportBatchMatcher( $batch_id, $params->{match} ); + SetImportBatchMatcher($stage_results->{batch_id}, $params->{match}); } elsif ($record_type eq 'biblio') { $matcher = C4::Matcher->new($record_type); $matcher->add_simple_matchpoint('isbn', 1000, '020', 'a', -1, 0, ''); @@ -132,15 +140,15 @@ sub process_batch { '245', 'a', -1, 0, ''); } # set default record overlay behavior - SetImportBatchOverlayAction( $batch_id, $params->{no_replace} ? 'ignore' : 'replace' ); - SetImportBatchNoMatchAction( $batch_id, $params->{no_create} ? 'ignore' : 'create_new' ); - SetImportBatchItemAction( $batch_id, $params->{item_action} ); + SetImportBatchOverlayAction($stage_results->{batch_id}, $params->{no_replace} ? 'ignore' : 'replace'); + SetImportBatchNoMatchAction($stage_results->{batch_id}, $params->{no_create} ? 'ignore' : 'create_new'); + SetImportBatchItemAction($stage_results->{batch_id}, $params->{item_action} ); print "... looking for matches with records already in database\n"; - $num_with_matches = BatchFindDuplicates($batch_id, $matcher, 10, 100, \&print_progress_and_commit); + $num_with_matches = BatchFindDuplicates($stage_results->{batch_id}, $matcher, 10, 100, \&print_progress_and_commit); print "... finished looking for matches\n"; } - my $num_invalid_records = scalar(@import_errors); + my $num_invalid_records = scalar( @{ $stage_results->{invalid_records} } ); print <<_SUMMARY_; MARC record staging report @@ -148,7 +156,7 @@ MARC record staging report Input file: $params->{input_file} Record type: $record_type Number of input records: $num_input_records -Number of valid records: $num_valid_records +Number of valid records: $stage_results->{num_valid} Number of invalid records: $num_invalid_records _SUMMARY_ if( $params->{match} ) { @@ -157,15 +165,15 @@ _SUMMARY_ print "Incoming records not matched against existing records (--match option not supplied)\n"; } if ($record_type eq 'biblio') { - if ( $params->{add_items} ) { - print "Number of items parsed: $num_items\n"; + if ($params->{add_items} ) { + print "Number of items parsed: $stage_results->{num_items}\n"; } else { print "No items parsed (--add-items option not supplied)\n"; } } print "\n"; - print "Batch number assigned: $batch_id\n"; + print "Batch number assigned: $stage_results->{batch_id}\n"; print "\n"; } diff --git a/svc/cataloguing/import_batches b/svc/cataloguing/import_batches index b8b2d09..e868db4 100755 --- a/svc/cataloguing/import_batches +++ b/svc/cataloguing/import_batches @@ -21,7 +21,7 @@ use Modern::Perl; use C4::Biblio qw(); use C4::Context; -use C4::ImportBatch qw( AddBiblioToBatch AddImportBatch GetImportRecordMarcXML ModBiblioInBatch ); +use C4::ImportBatch qw( AddBiblioToBatch AddImportBatch GetImportRecordMarcXML ModBiblioInBatch CheckRecordControlNumberInBatch ); use C4::Service; use DateTime; use DateTime::Format::Strptime; @@ -132,30 +132,6 @@ sub get_record { C4::Service->return_success( $response ); } -sub _control_number_already_used { - my ( $marc_record, $batch_id, $import_record_id ) = @_; - - return unless ( $marc_record->field('001') ); - - my %extra_conditions; - - $extra_conditions{'import_record.import_record_id'} = { '!=', $import_record_id } if ( $import_record_id ); - - my $control_number = $marc_record->field('001')->data; - - my $schema = Koha::Database->new()->schema(); - return $schema->resultset('ImportBiblio')->count( - { - control_number => $control_number, - 'import_record.import_batch_id' => $batch_id, - %extra_conditions - }, - { - join => 'import_record', - } - ); -} - sub create_record { my ( $batch_id ) = @_; @@ -165,7 +141,7 @@ sub create_record { C4::Service->return_error( 'failed', $@ ); } - if ( !$query->param('allow_control_number_conflict') && _control_number_already_used( $marc_record, $batch_id ) ) { + if ( !$query->param('allow_control_number_conflict') && CheckRecordControlNumberInBatch( $marc_record, $batch_id ) ) { C4::Service->return_error( 'control_number_match', '' ); } @@ -184,7 +160,7 @@ sub update_record { C4::Service->return_error( 'failed', $@ ); } - if ( !$query->param('allow_control_number_conflict') && _control_number_already_used( $marc_record, $batch_id, $import_record_id ) ) { + if ( !$query->param('allow_control_number_conflict') && CheckRecordControlNumberInBatch( $marc_record, $batch_id, $import_record_id ) ) { C4::Service->return_error( 'control_number_match', '' ); } diff --git a/tools/stage-marc-import.pl b/tools/stage-marc-import.pl index aca8c4e..c5c3e76 100755 --- a/tools/stage-marc-import.pl +++ b/tools/stage-marc-import.pl @@ -59,6 +59,8 @@ my $encoding = $input->param('encoding') || 'UTF-8'; my $format = $input->param('format') || 'ISO2709'; my $marc_modification_template = $input->param('marc_modification_template_id'); my $existing_batch_id = $input->param('existing_batch_id'); +my $control_number_handling = $input->param('control_number_handling'); +my $timestamp_update = $input->param('timestamp_update'); my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { @@ -139,16 +141,20 @@ if ($completedJobID) { $dbh = C4::Context->dbh({new => 1}); $dbh->{AutoCommit} = 0; # FIXME branch code - my ( $batch_id, $num_valid, $num_items, @import_errors ) = - BatchStageMarcRecords( - $record_type, $encoding, - $marcrecords, $filename, - $marc_modification_template, - $comments, '', - $parse_items, 0, - 50, staging_progress_callback( $job, $dbh ), - $existing_batch_id - ); + my $stage_results = BatchStageMarcRecords( { + record_type => $record_type, + encoding => $encoding, + marc_records => $marcrecords, + file_name => $filename, + marc_modification_template => $marc_modification_template, + comments => $comments, + parse_items => $parse_items, + progress_interval => 50, + progress_callback => staging_progress_callback( $job, $dbh ), + existing_batch_id => $existing_batch_id, + control_number_handling => $control_number_handling, + timestamp_update => $timestamp_update, + } ); my $num_with_matches = 0; my $checked_matches = 0; @@ -160,12 +166,12 @@ if ($completedJobID) { $checked_matches = 1; $matcher_code = $matcher->code(); $num_with_matches = - BatchFindDuplicates( $batch_id, $matcher, 10, 50, + BatchFindDuplicates( $stage_results->{batch_id}, $matcher, 10, 50, matching_progress_callback( $job, $dbh ) ); - SetImportBatchMatcher($batch_id, $matcher_id); - SetImportBatchOverlayAction($batch_id, $overlay_action); - SetImportBatchNoMatchAction($batch_id, $nomatch_action); - SetImportBatchItemAction($batch_id, $item_action); + SetImportBatchMatcher( $stage_results->{batch_id}, $matcher_id ); + SetImportBatchOverlayAction( $stage_results->{batch_id}, $overlay_action ); + SetImportBatchNoMatchAction( $stage_results->{batch_id}, $nomatch_action ); + SetImportBatchItemAction( $stage_results->{batch_id}, $item_action ); $dbh->commit(); } else { $matcher_failed = 1; @@ -175,30 +181,23 @@ if ($completedJobID) { } my $results = { - staged => $num_valid, + staged => $stage_results->{num_valid}, matched => $num_with_matches, - num_items => $num_items, - import_errors => scalar(@import_errors), - total => $num_valid + scalar(@import_errors), + num_items => $stage_results->{num_items}, + import_errors => scalar( @{ $stage_results->{invalid_records} } ), + total => $stage_results->{num_valid} + scalar( @{ $stage_results->{invalid_records} } ), checked_matches => $checked_matches, matcher_failed => $matcher_failed, matcher_code => $matcher_code, - import_batch_id => $batch_id + import_batch_id => $stage_results->{batch_id}, + control_number_handling => $control_number_handling, + num_matched_control_number => $stage_results->{num_matched_control_number}, }; if ($runinbackground) { $job->finish($results); exit 0; } else { - $template->param(staged => $num_valid, - matched => $num_with_matches, - num_items => $num_items, - import_errors => scalar(@import_errors), - total => $num_valid + scalar(@import_errors), - checked_matches => $checked_matches, - matcher_failed => $matcher_failed, - matcher_code => $matcher_code, - import_batch_id => $batch_id - ); + $template->param( %$results ); } } else { -- 2.1.4