From a1e9453f1dcd5cc477ccd7fbecadb2a36176a4be Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 29 Sep 2020 13:29:33 +0200 Subject: [PATCH] Bug 26518: Use Koha::Biblio[item] in AddBiblio Bug 26518: Does not return from the catch block Signed-off-by: Victor Grousset/tuxayo Signed-off-by: Nick Clemens Signed-off-by: Jonathan Druart --- C4/Biblio.pm | 212 +++++++++++++++------------------------- t/db_dependent/Biblio.t | 29 +++++- 2 files changed, 102 insertions(+), 139 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 90893399af..44428156d5 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -216,26 +216,89 @@ sub AddBiblio { # transform the data into koha-table style data SetUTF8Flag($record); my $olddata = TransformMarcToKoha( $record, $frameworkcode ); - ( $biblionumber, $error ) = _koha_add_biblio( $dbh, $olddata, $frameworkcode ); - $olddata->{'biblionumber'} = $biblionumber; - ( $biblioitemnumber, $error ) = _koha_add_biblioitem( $dbh, $olddata ); + my $schema = Koha::Database->schema; + try { + $schema->txn_do(sub { + + my $biblio = Koha::Biblio->new( + { + frameworkcode => $frameworkcode, + author => $olddata->{author}, + title => $olddata->{title}, + subtitle => $olddata->{subtitle}, + medium => $olddata->{medium}, + part_number => $olddata->{part_number}, + part_name => $olddata->{part_name}, + unititle => $olddata->{unititle}, + notes => $olddata->{notes}, + serial => + ( $olddata->{serial} || $olddata->{seriestitle} ? 1 : 0 ), + seriestitle => $olddata->{seriestitle}, + copyrightdate => $olddata->{copyrightdate}, + datecreated => \'NOW()', + abstract => $olddata->{abstract}, + } + )->store; + $biblionumber = $biblio->biblionumber; + + my ($cn_sort) = GetClassSort( $olddata->{'biblioitems.cn_source'}, $olddata->{'cn_class'}, $olddata->{'cn_item'} ); + my $biblioitem = Koha::Biblioitem->new( + { + biblionumber => $biblionumber, + volume => $olddata->{volume}, + number => $olddata->{number}, + itemtype => $olddata->{itemtype}, + isbn => $olddata->{isbn}, + issn => $olddata->{issn}, + publicationyear => $olddata->{publicationyear}, + publishercode => $olddata->{publishercode}, + volumedate => $olddata->{volumedate}, + volumedesc => $olddata->{volumedesc}, + collectiontitle => $olddata->{collectiontitle}, + collectionissn => $olddata->{collectionissn}, + collectionvolume => $olddata->{collectionvolume}, + editionstatement => $olddata->{editionstatement}, + editionresponsibility => $olddata->{editionresponsibility}, + illus => $olddata->{illus}, + pages => $olddata->{pages}, + notes => $olddata->{bnotes}, + size => $olddata->{size}, + place => $olddata->{place}, + lccn => $olddata->{lccn}, + url => $olddata->{url}, + cn_source => $olddata->{'biblioitems.cn_source'}, + cn_class => $olddata->{cn_class}, + cn_item => $olddata->{cn_item}, + cn_suffix => $olddata->{cn_suff}, + cn_sort => $cn_sort, + totalissues => $olddata->{totalissues}, + ean => $olddata->{ean}, + agerestriction => $olddata->{agerestriction}, + } + )->store; + $biblioitemnumber = $biblioitem->biblioitemnumber; - _koha_marc_update_bib_ids( $record, $frameworkcode, $biblionumber, $biblioitemnumber ); + _koha_marc_update_bib_ids( $record, $frameworkcode, $biblionumber, $biblioitemnumber ); - # update MARC subfield that stores biblioitems.cn_sort - _koha_marc_update_biblioitem_cn_sort( $record, $olddata, $frameworkcode ); + # update MARC subfield that stores biblioitems.cn_sort + _koha_marc_update_biblioitem_cn_sort( $record, $olddata, $frameworkcode ); - # now add the record - ModBiblioMarc( $record, $biblionumber, $frameworkcode ) unless $defer_marc_save; + # now add the record + ModBiblioMarc( $record, $biblionumber, $frameworkcode ) unless $defer_marc_save; - # update OAI-PMH sets - if(C4::Context->preference("OAI-PMH:AutoUpdateSets")) { - C4::OAI::Sets::UpdateOAISetsBiblio($biblionumber, $record); - } + # update OAI-PMH sets + if(C4::Context->preference("OAI-PMH:AutoUpdateSets")) { + C4::OAI::Sets::UpdateOAISetsBiblio($biblionumber, $record); + } - _after_biblio_action_hooks({ action => 'create', biblio_id => $biblionumber }); + _after_biblio_action_hooks({ action => 'create', biblio_id => $biblionumber }); - logaction( "CATALOGUING", "ADD", $biblionumber, "biblio" ) if C4::Context->preference("CataloguingLog"); + logaction( "CATALOGUING", "ADD", $biblionumber, "biblio" ) if C4::Context->preference("CataloguingLog"); + }); + } catch { + warn $_; + ( $biblionumber, $biblioitemnumber ) = ( undef, undef ); + }; return ( $biblionumber, $biblioitemnumber ); } @@ -2676,61 +2739,6 @@ sub _koha_marc_update_biblioitem_cn_sort { } } -=head2 _koha_add_biblio - - my ($biblionumber,$error) = _koha_add_biblio($dbh,$biblioitem); - -Internal function to add a biblio ($biblio is a hash with the values) - -=cut - -sub _koha_add_biblio { - my ( $dbh, $biblio, $frameworkcode ) = @_; - - my $error; - - # set the series flag - unless (defined $biblio->{'serial'}){ - $biblio->{'serial'} = 0; - if ( $biblio->{'seriestitle'} ) { $biblio->{'serial'} = 1 } - } - - my $query = "INSERT INTO biblio - SET frameworkcode = ?, - author = ?, - title = ?, - subtitle = ?, - medium = ?, - part_number = ?, - part_name = ?, - unititle =?, - notes = ?, - serial = ?, - seriestitle = ?, - copyrightdate = ?, - datecreated=NOW(), - abstract = ? - "; - my $sth = $dbh->prepare($query); - $sth->execute( - $frameworkcode, $biblio->{'author'}, $biblio->{'title'}, $biblio->{'subtitle'}, - $biblio->{'medium'}, $biblio->{'part_number'}, $biblio->{'part_name'}, $biblio->{'unititle'}, - $biblio->{'notes'}, $biblio->{'serial'}, $biblio->{'seriestitle'}, $biblio->{'copyrightdate'}, - $biblio->{'abstract'} - ); - - my $biblionumber = $dbh->{'mysql_insertid'}; - if ( $dbh->errstr ) { - $error .= "ERROR in _koha_add_biblio $query" . $dbh->errstr; - warn $error; - } - - $sth->finish(); - - #warn "LEAVING _koha_add_biblio: ".$biblionumber."\n"; - return ( $biblionumber, $error ); -} - =head2 _koha_modify_biblio my ($biblionumber,$error) == _koha_modify_biblio($dbh,$biblio,$frameworkcode); @@ -2841,72 +2849,6 @@ sub _koha_modify_biblioitem_nonmarc { return ( $biblioitem->{'biblioitemnumber'}, $error ); } -=head2 _koha_add_biblioitem - - my ($biblioitemnumber,$error) = _koha_add_biblioitem( $dbh, $biblioitem ); - -Internal function to add a biblioitem - -=cut - -sub _koha_add_biblioitem { - my ( $dbh, $biblioitem ) = @_; - my $error; - - my ($cn_sort) = GetClassSort( $biblioitem->{'biblioitems.cn_source'}, $biblioitem->{'cn_class'}, $biblioitem->{'cn_item'} ); - my $query = "INSERT INTO biblioitems SET - biblionumber = ?, - volume = ?, - number = ?, - itemtype = ?, - isbn = ?, - issn = ?, - publicationyear = ?, - publishercode = ?, - volumedate = ?, - volumedesc = ?, - collectiontitle = ?, - collectionissn = ?, - collectionvolume= ?, - editionstatement= ?, - editionresponsibility = ?, - illus = ?, - pages = ?, - notes = ?, - size = ?, - place = ?, - lccn = ?, - url = ?, - cn_source = ?, - cn_class = ?, - cn_item = ?, - cn_suffix = ?, - cn_sort = ?, - totalissues = ?, - ean = ?, - agerestriction = ? - "; - my $sth = $dbh->prepare($query); - $sth->execute( - $biblioitem->{'biblionumber'}, $biblioitem->{'volume'}, $biblioitem->{'number'}, $biblioitem->{'itemtype'}, - $biblioitem->{'isbn'}, $biblioitem->{'issn'}, $biblioitem->{'publicationyear'}, $biblioitem->{'publishercode'}, - $biblioitem->{'volumedate'}, $biblioitem->{'volumedesc'}, $biblioitem->{'collectiontitle'}, $biblioitem->{'collectionissn'}, - $biblioitem->{'collectionvolume'}, $biblioitem->{'editionstatement'}, $biblioitem->{'editionresponsibility'}, $biblioitem->{'illus'}, - $biblioitem->{'pages'}, $biblioitem->{'bnotes'}, $biblioitem->{'size'}, $biblioitem->{'place'}, - $biblioitem->{'lccn'}, $biblioitem->{'url'}, $biblioitem->{'biblioitems.cn_source'}, - $biblioitem->{'cn_class'}, $biblioitem->{'cn_item'}, $biblioitem->{'cn_suffix'}, $cn_sort, - $biblioitem->{'totalissues'}, $biblioitem->{'ean'}, $biblioitem->{'agerestriction'} - ); - my $bibitemnum = $dbh->{'mysql_insertid'}; - - if ( $dbh->errstr ) { - $error .= "ERROR in _koha_add_biblioitem $query" . $dbh->errstr; - warn $error; - } - $sth->finish(); - return ( $bibitemnum, $error ); -} - =head2 _koha_delete_biblio $error = _koha_delete_biblio($dbh,$biblionumber); diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t index 21d2c07105..6876d8f7f5 100755 --- a/t/db_dependent/Biblio.t +++ b/t/db_dependent/Biblio.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 14; +use Test::More tests => 15; use Test::MockModule; use List::MoreUtils qw( uniq ); use MARC::Record; @@ -42,6 +42,30 @@ Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-" ); my $builder = t::lib::TestBuilder->new; +subtest 'AddBiblio' => sub { + plan tests => 3; + + my $marcflavour = 'MARC21'; + t::lib::Mocks::mock_preference( 'marcflavour', $marcflavour ); + my $record = MARC::Record->new(); + + my ( $f, $sf ) = GetMarcFromKohaField('biblioitems.lccn'); + my $lccn_field = MARC::Field->new( $f, ' ', ' ', + $sf => 'ThisisgoingtobetoomanycharactersfortheLCCNfield' ); + $record->append_fields($lccn_field); + + my $nb_biblios = Koha::Biblios->count; + my ( $biblionumber, $biblioitemnumber ) = + C4::Biblio::AddBiblio( $record, '' ); + is( $biblionumber, undef, + 'AddBiblio returns undef for biblionumber if something went wrong' ); + is( $biblioitemnumber, undef, + 'AddBiblio returns undef for biblioitemnumber if something went wrong' + ); + is( Koha::Biblios->count, $nb_biblios, + 'No biblio should have been added if something went wrong' ); +}; + subtest 'GetMarcSubfieldStructureFromKohaField' => sub { plan tests => 25; @@ -758,9 +782,6 @@ subtest "LinkBibHeadingsToAuthorities record generation tests" => sub { "Tolkien, J. R. R. (John Ronald Reuel), 1892-1973. Lord of the rings", "The generated record contains the correct subfields" ); - - - }; # Cleanup -- 2.39.5