From f75cca228a9149b6b1b5cb04f7037879cb465cf1 Mon Sep 17 00:00:00 2001 From: Colin Campbell Date: Thu, 8 Apr 2010 13:16:31 +0100 Subject: [PATCH] Basic refactoring of serials-edit.pm Removed some useless counts from serials-edit / Serials interfaces Removed old commented out code. Unrequired variables Reformatted some code so that improving logic can be done more easily --- C4/Serials.pm | 15 +- serials/serials-edit.pl | 444 +++++++++++++++++++++++----------------- 2 files changed, 256 insertions(+), 203 deletions(-) diff --git a/C4/Serials.pm b/C4/Serials.pm index c9b8246308..32c6cbe2f0 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -742,8 +742,8 @@ sub GetSerials { =over 4 -($totalissues,@serials) = GetSerials2($subscriptionid,$status); -this function get every serial waited for a given subscription +@serials = GetSerials2($subscriptionid,$status); +this function gets every serial waited for a given subscription as well as the number of issues registered in the database (all types) this number is used to see if a subscription can be deleted (=it must have only 1 issue) @@ -771,8 +771,7 @@ sub GetSerials2 { $line->{"publisheddate"} = format_date( $line->{"publisheddate"} ); push @serials, $line; } - my ($totalissues) = scalar(@serials); - return ( $totalissues, @serials ); + return @serials; } =head2 GetLatestSerials @@ -808,14 +807,6 @@ sub GetLatestSerials { push @serials, $line; } - # my $query = qq| - # SELECT count(*) - # FROM serial - # WHERE subscriptionid=? - # |; - # $sth=$dbh->prepare($query); - # $sth->execute($subscriptionid); - # my ($totalissues) = $sth->fetchrow; return \@serials; } diff --git a/serials/serials-edit.pl b/serials/serials-edit.pl index 528ecb39cc..1ee831da23 100755 --- a/serials/serials-edit.pl +++ b/serials/serials-edit.pl @@ -17,7 +17,6 @@ # with Koha; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - =head1 NAME serials-recieve.pl @@ -61,7 +60,6 @@ op can be : =cut - use strict; use warnings; use CGI; @@ -74,232 +72,296 @@ use C4::Output; use C4::Context; use C4::Serials; -#use Smart::Comments; - -my $query = new CGI; -my $dbh = C4::Context->dbh; -my @serialids = $query->param('serialid'); -my @serialseqs = $query->param('serialseq'); -my @planneddates = $query->param('planneddate'); -my @publisheddates = $query->param('publisheddate'); -my @status = $query->param('status'); -my @notes = $query->param('notes'); +my $query = CGI->new(); +my $dbh = C4::Context->dbh; +my @serialids = $query->param('serialid'); +my @serialseqs = $query->param('serialseq'); +my @planneddates = $query->param('planneddate'); +my @publisheddates = $query->param('publisheddate'); +my @status = $query->param('status'); +my @notes = $query->param('notes'); my @subscriptionids = $query->param('subscriptionid'); -my $op = $query->param('op'); -if (scalar(@subscriptionids)==1 && index($subscriptionids[0],",")>0){ - @subscriptionids =split (/,/,$subscriptionids[0]); +my $op = $query->param('op'); +if ( scalar(@subscriptionids) == 1 && index( $subscriptionids[0], q|,| ) > 0 ) { + @subscriptionids = split( /,/, $subscriptionids[0] ); } my @errors; my @errseq; + # If user comes from subscription details -unless (@serialids){ - foreach my $subscriptionid (@subscriptionids){ - my $serstatus=$query->param('serstatus'); - if ($serstatus){ - my ($count,@tmpser)=GetSerials2($subscriptionid,$serstatus); - foreach (@tmpser) { - push @serialids, $_->{'serialid'}; - } +unless (@serialids) { + foreach my $subscriptionid (@subscriptionids) { + my $serstatus = $query->param('serstatus'); + if ($serstatus) { + my @tmpser = GetSerials2( $subscriptionid, $serstatus ); + foreach (@tmpser) { + push @serialids, $_->{'serialid'}; + } + } } - } } -unless (scalar(@serialids)){ - my $string="serials-collection.pl?subscriptionid=".join(",",@subscriptionids); - $string=~s/,$//; -# warn $string; - print $query->redirect($string); +unless ( scalar(@serialids) ) { + my $string = + "serials-collection.pl?subscriptionid=" . join( ",", @subscriptionids ); + $string =~ s/,$//; + + print $query->redirect($string); } -my ($template, $loggedinuser, $cookie) -= get_template_and_user({template_name => "serials/serials-edit.tmpl", - query => $query, - type => "intranet", - authnotrequired => 0, - flagsrequired => {serials => 1}, - debug => 1, - }); +my ( $template, $loggedinuser, $cookie ) = get_template_and_user( + { + template_name => "serials/serials-edit.tmpl", + query => $query, + type => "intranet", + authnotrequired => 0, + flagsrequired => { serials => 1 }, + debug => 1, + } +); my @serialdatalist; my %processedserialid; -foreach my $tmpserialid (@serialids){ +foreach my $tmpserialid (@serialids) { + #filtering serialid for duplication #NEW serial should appear only once and are created afterwards - next unless (defined($tmpserialid) && $tmpserialid =~/^[0-9]+$/ && !$processedserialid{$tmpserialid}); - my $data=GetSerialInformation($tmpserialid); - $data->{publisheddate}=format_date($data->{publisheddate}); - $data->{planneddate}=format_date($data->{planneddate}); - $data->{'editdisable'}=((HasSubscriptionExpired($data->{subscriptionid})&& $data->{'status1'})||$data->{'cannotedit'}); - push @serialdatalist,$data; - $processedserialid{$tmpserialid}=1; + if ( defined($tmpserialid) + && $tmpserialid =~ /^[0-9]+$/ + && !$processedserialid{$tmpserialid} ) + { + my $data = GetSerialInformation($tmpserialid); + $data->{publisheddate} = format_date( $data->{publisheddate} ); + $data->{planneddate} = format_date( $data->{planneddate} ); + $data->{'editdisable'} = ( + ( + HasSubscriptionExpired( $data->{subscriptionid} ) + && $data->{'status1'} + ) + || $data->{'cannotedit'} + ); + push @serialdatalist, $data; + $processedserialid{$tmpserialid} = 1; + } } -my $bibdata=GetBiblioData($serialdatalist[0]->{'biblionumber'}); +my $bibdata = GetBiblioData( $serialdatalist[0]->{'biblionumber'} ); my @newserialloop; my @subscriptionloop; + # check, for each subscription edited, that we have an empty item line if applicable for the subscription my %processedsubscriptionid; -foreach my $subscriptionid (@subscriptionids){ - #Donot process subscriptionid twice if it was already processed. - my $subscriptiondetail = GetSubscription($subscriptionid); - next unless (defined($subscriptionid) && !$processedsubscriptionid{$subscriptionid}); - my $cell; - if ($serialdatalist[0]->{'serialsadditems'}){ - #Create New empty item - $cell = - PrepareItemrecordDisplay( $serialdatalist[0]->{'biblionumber'},'', $subscriptiondetail); - $cell->{serialsadditems} = 1; +foreach my $subscriptionid (@subscriptionids) { + + #Do not process subscriptionid twice if it was already processed. + if ( defined($subscriptionid) + && !$processedsubscriptionid{$subscriptionid} ) + { + my $cell; + if ( $serialdatalist[0]->{'serialsadditems'} ) { + + #Create New empty item + $cell = + PrepareItemrecordDisplay( $serialdatalist[0]->{'biblionumber'}, + '', GetSubscription($subscriptionid) ); + $cell->{serialsadditems} = 1; + } + $cell->{'subscriptionid'} = $subscriptionid; + $cell->{'itemid'} = "NNEW"; + $cell->{'serialid'} = "NEW"; + $cell->{'issuesatonce'} = 1; + push @newserialloop, $cell; + push @subscriptionloop, + { + 'subscriptionid' => $subscriptionid, + 'abouttoexpire' => abouttoexpire($subscriptionid), + 'subscriptionexpired' => HasSubscriptionExpired($subscriptionid), + }; + $processedsubscriptionid{$subscriptionid} = 1; } - $cell->{'subscriptionid'}=$subscriptionid; - $cell->{'itemid'} = "NNEW"; - $cell->{'serialid'} = "NEW"; - $cell->{'issuesatonce'} = 1; - push @newserialloop,$cell; - push @subscriptionloop, {'subscriptionid'=>$subscriptionid, - 'abouttoexpire'=>abouttoexpire($subscriptionid), - 'subscriptionexpired'=>HasSubscriptionExpired($subscriptionid), - }; - $processedsubscriptionid{$subscriptionid}=1; - $template->param(bibliotitle => $subscriptiondetail->{'bibliotitle'}, - callnumber => $subscriptiondetail->{'callnumber'}, - ); - } -$template->param(newserialloop=>\@newserialloop); -$template->param(subscriptions=>\@subscriptionloop); +$template->param( newserialloop => \@newserialloop ); +$template->param( subscriptions => \@subscriptionloop ); + +if ( $op and $op eq 'serialchangestatus' ) { -if ($op and $op eq 'serialchangestatus') { -# my $sth = $dbh->prepare("select status from serial where serialid=?"); my $newserial; - for (my $i=0;$i<=$#serialids;$i++) { -# $sth->execute($serialids[$i]); -# my ($oldstatus) = $sth->fetchrow; - if ($serialids[$i] && $serialids[$i] eq "NEW") { - if ($serialseqs[$i]){ + for ( my $i = 0 ; $i <= $#serialids ; $i++ ) { + + if ( $serialids[$i] && $serialids[$i] eq "NEW" ) { + if ( $serialseqs[$i] ) { + #IF newserial was provided a name Then we have to create a newSerial - ### FIXME if NewIssue is modified to use subscription biblionumber, then biblionumber would not be useful. - $newserial = NewIssue( $serialseqs[$i],$subscriptionids[$i],$serialdatalist[0]->{'biblionumber'}, - $status[$i], - format_date_in_iso($planneddates[$i]), - format_date_in_iso($publisheddates[$i]), - $notes[$i]); - } - }elsif ($serialids[$i]){ - ModSerialStatus($serialids[$i], - $serialseqs[$i], - format_date_in_iso($planneddates[$i]), - format_date_in_iso($publisheddates[$i]), - $status[$i], - $notes[$i]); + ### FIXME if NewIssue is modified to use subscription biblionumber, then biblionumber would not be useful. + $newserial = NewIssue( + $serialseqs[$i], + $subscriptionids[$i], + $serialdatalist[0]->{'biblionumber'}, + $status[$i], + format_date_in_iso( $planneddates[$i] ), + format_date_in_iso( $publisheddates[$i] ), + $notes[$i] + ); + } + } + elsif ( $serialids[$i] ) { + ModSerialStatus( + $serialids[$i], + $serialseqs[$i], + format_date_in_iso( $planneddates[$i] ), + format_date_in_iso( $publisheddates[$i] ), + $status[$i], + $notes[$i] + ); } } my @moditems = $query->param('moditem'); - if (scalar(@moditems)){ - my @tags = $query->param('tag'); - my @subfields = $query->param('subfield'); - my @field_values = $query->param('field_value'); - my @serials = $query->param('serial'); - my @bibnums = $query->param('bibnum'); - my @itemid = $query->param('itemid'); - my @ind_tag = $query->param('ind_tag'); - my @indicator = $query->param('indicator'); - #Rebuilding ALL the data for items into a hash - # parting them on $itemid. - my %itemhash; - my $countdistinct; - my $range=scalar(@itemid); - for (my $i=0; $i<$range; $i++){ - unless ($itemhash{$itemid[$i]}){ - if ($serials[$countdistinct] && $serials[$countdistinct] ne "NEW"){ - $itemhash{$itemid[$i]}->{'serial'}=$serials[$countdistinct]; - } else { - $itemhash{$itemid[$i]}->{'serial'}=$newserial; - } - $itemhash{$itemid[$i]}->{'bibnum'}=$bibnums[$countdistinct]; - $countdistinct++; - } - push @{$itemhash{$itemid[$i]}->{'tags'}},$tags[$i]; - push @{$itemhash{$itemid[$i]}->{'subfields'}},$subfields[$i]; - push @{$itemhash{$itemid[$i]}->{'field_values'}},$field_values[$i]; - push @{$itemhash{$itemid[$i]}->{'ind_tag'}},$ind_tag[$i]; - push @{$itemhash{$itemid[$i]}->{'indicator'}},$indicator[$i]; - } - foreach my $item (keys %itemhash){ - # Verify Itemization is "Valid", i.e. serial status is Arrived or Missing - my $index=-1; - for (my $i=0; $i{'serial'} eq $serialids[$i] || ($itemhash{$item}->{'serial'} == $newserial && $serialids[$i] eq "NEW")); - } - if ($index>=0 && $status[$index]==2){ - my $xml = TransformHtmlToXml( $itemhash{$item}->{'tags'}, - $itemhash{$item}->{'subfields'}, - $itemhash{$item}->{'field_values'}, - $itemhash{$item}->{'indicator'}, - $itemhash{$item}->{'ind_tag'}); - # warn $xml; - my $record=MARC::Record::new_from_xml($xml, 'UTF-8'); - if ($item=~/^N/){ - #New Item - - # if autoBarcode is set to 'incremental', calculate barcode... - my ($barcodetagfield,$barcodetagsubfield) = &GetMarcFromKohaField("items.barcode", GetFrameworkCode($serialdatalist[0]->{'biblionumber'})); - if (C4::Context->preference("autoBarcode") eq 'incremental' ) { - if (!$record->field($barcodetagfield)->subfield($barcodetagsubfield)) { - my $sth_barcode = $dbh->prepare("select max(abs(barcode)) from items"); - $sth_barcode->execute; - my ($newbarcode) = $sth_barcode->fetchrow; - # OK, we have the new barcode, add the entry in MARC record # FIXME -> should be using barcode plugin here. - $record->field($barcodetagfield)->update( $barcodetagsubfield => ++$newbarcode ); - } + if ( scalar(@moditems) ) { + my @tags = $query->param('tag'); + my @subfields = $query->param('subfield'); + my @field_values = $query->param('field_value'); + my @serials = $query->param('serial'); + my @bibnums = $query->param('bibnum'); + my @itemid = $query->param('itemid'); + my @ind_tag = $query->param('ind_tag'); + my @indicator = $query->param('indicator'); + + #Rebuilding ALL the data for items into a hash + # parting them on $itemid. + my %itemhash; + my $countdistinct; + my $range = scalar(@itemid); + for ( my $i = 0 ; $i < $range ; $i++ ) { + unless ( $itemhash{ $itemid[$i] } ) { + if ( $serials[$countdistinct] + && $serials[$countdistinct] ne "NEW" ) + { + $itemhash{ $itemid[$i] }->{'serial'} = + $serials[$countdistinct]; + } + else { + $itemhash{ $itemid[$i] }->{'serial'} = $newserial; + } + $itemhash{ $itemid[$i] }->{'bibnum'} = $bibnums[$countdistinct]; + $countdistinct++; } - # check for item barcode # being unique - my $exists; - if ($record->subfield($barcodetagfield,$barcodetagsubfield)) { - $exists = GetItemnumberFromBarcode($record->subfield($barcodetagfield,$barcodetagsubfield)); + push @{ $itemhash{ $itemid[$i] }->{'tags'} }, $tags[$i]; + push @{ $itemhash{ $itemid[$i] }->{'subfields'} }, $subfields[$i]; + push @{ $itemhash{ $itemid[$i] }->{'field_values'} }, + $field_values[$i]; + push @{ $itemhash{ $itemid[$i] }->{'ind_tag'} }, $ind_tag[$i]; + push @{ $itemhash{ $itemid[$i] }->{'indicator'} }, $indicator[$i]; + } + foreach my $item ( keys %itemhash ) { + + # Verify Itemization is "Valid", i.e. serial status is Arrived or Missing + my $index = -1; + for ( my $i = 0 ; $i < scalar(@serialids) ; $i++ ) { + if ( + $itemhash{$item}->{serial} eq $serialids[$i] + || ( $itemhash{$item}->{serial} == $newserial + && $serialids[$i] eq 'NEW' ) + ) { + $index = $i + } } - # push @errors,"barcode_not_unique" if($exists); - # if barcode exists, don't create, but report The problem. - if ($exists){ - push @errors,"barcode_not_unique" if($exists); - push @errseq,{"serialseq"=>$serialseqs[$index]}; - } else { - my ($biblionumber,$bibitemnum,$itemnumber) = AddItemFromMarc($record,$itemhash{$item}->{'bibnum'}); - AddItem2Serial($itemhash{$item}->{'serial'},$itemnumber); + if ( $index >= 0 && $status[$index] == 2 ) { + my $xml = TransformHtmlToXml( + $itemhash{$item}->{'tags'}, + $itemhash{$item}->{'subfields'}, + $itemhash{$item}->{'field_values'}, + $itemhash{$item}->{'indicator'}, + $itemhash{$item}->{'ind_tag'} + ); + + # warn $xml; + my $bib_record = MARC::Record::new_from_xml( $xml, 'UTF-8' ); + if ( $item =~ /^N/ ) { + + #New Item + + # if autoBarcode is set to 'incremental', calculate barcode... + my ( $barcodetagfield, $barcodetagsubfield ) = + GetMarcFromKohaField( + 'items.barcode', + GetFrameworkCode( + $serialdatalist[0]->{'biblionumber'} + ) + ); + if ( C4::Context->preference("autoBarcode") eq + 'incremental' ) + { + if ( !$bib_record->field($barcodetagfield) + ->subfield($barcodetagsubfield) ) + { + my $sth_barcode = $dbh->prepare( + "select max(abs(barcode)) from items"); + $sth_barcode->execute; + my ($newbarcode) = $sth_barcode->fetchrow; + +# OK, we have the new barcode, add the entry in MARC record # FIXME -> should be using barcode plugin here. + $bib_record->field($barcodetagfield) + ->update( $barcodetagsubfield => ++$newbarcode ); + } + } + + # check for item barcode # being unique + my $exists; + if ( + $bib_record->subfield( + $barcodetagfield, $barcodetagsubfield + ) + ) + { + $exists = GetItemnumberFromBarcode( + $bib_record->subfield( + $barcodetagfield, $barcodetagsubfield + ) + ); + } + + # push @errors,"barcode_not_unique" if($exists); + # if barcode exists, don't create, but report The problem. + if ($exists) { + push @errors, 'barcode_not_unique'; + push @errseq, { serialseq => $serialseqs[$index] }; + } + else { + my ( $biblionumber, $bibitemnum, $itemnumber ) = + AddItemFromMarc( $bib_record, + $itemhash{$item}->{bibnum} ); + AddItem2Serial( $itemhash{$item}->{serial}, + $itemnumber ); + } + } + else { + + #modify item + my ( $oldbiblionumber, $oldbibnum, $itemnumber ) = + ModItemFromMarc( $bib_record, + $itemhash{$item}->{'bibnum'}, $item ); + } } - } else { - #modify item - my ($oldbiblionumber,$oldbibnum,$itemnumber) = ModItemFromMarc($record,$itemhash{$item}->{'bibnum'},$item); - } } - } } -# ### FIXME this part of code is not very pretty. Nor is it very efficient... There MUST be a more perlish way to write it. But it works. -# my $redirect ="serials-home.pl?"; -# $redirect.=join("&",map{"serialseq=".$_} @serialseqs); -# $redirect.="&".join("&",map{"planneddate=".$_} @planneddates); -# $redirect.="&".join("&",map{"publisheddate=".$_} @publisheddates); -# $redirect.="&".join("&",map{"status=".$_} @status); -# $redirect.="&".join("&",map{"notes=".$_} @notes); - - if (scalar(@errors)>0){ - $template->param("Errors" => 1); - if (scalar(@errseq)>0){ - $template->param("barcode_not_unique" => 1); - $template->param('errseq'=>\@errseq); + + + if ( @errors ) { + $template->param( Errors => 1 ); + if ( @errseq ) { + $template->param( barcode_not_unique => 1, errseq => \@errseq ); } - } else { - my $redirect ="serials-collection.pl?"; - my %hashsubscription; - foreach (@subscriptionids) { - $hashsubscription{$_}=1; - } - $redirect.=join("&",map{"subscriptionid=".$_} sort keys %hashsubscription); - print $query->redirect("$redirect"); - } + } + else { + my $redirect = "serials-collection.pl?"; + $redirect .= join( '&', map { "subscriptionid=" . $_ } sort @subscriptionids );# ID The sort necessary + print $query->redirect($redirect); + } } $template->param( serialsadditems => $serialdatalist[0]->{'serialsadditems'}, - biblionumber => $serialdatalist[0]->{'biblionumber'}, - serialslist => \@serialdatalist, + bibliotitle => $bibdata->{'title'}, + biblionumber => $serialdatalist[0]->{'biblionumber'}, + serialslist => \@serialdatalist, ); output_html_with_http_headers $query, $cookie, $template->output; -- 2.39.5