[Koha-patches] [PATCH 3/3] Basic refactoring of serials-edit.pl
Colin Campbell
colin.campbell at ptfs-europe.com
Fri Nov 20 20:41:09 CET 2009
Removed some useless counts from serials-edit / Serials interfaces
Removed old commented out code. Unrequired variables
Reformatted code so that improving logic can be done
more easily
---
C4/Serials.pm | 15 +--
serials/serials-edit.pl | 442 +++++++++++++++++++++++++++--------------------
2 files changed, 257 insertions(+), 200 deletions(-)
diff --git a/C4/Serials.pm b/C4/Serials.pm
index 8cf80e0..1635f36 100644
--- a/C4/Serials.pm
+++ b/C4/Serials.pm
@@ -772,8 +772,8 @@ sub GetSerials {
=over 4
-($totalissues, at serials) = GetSerials2($subscriptionid,$status);
-this function get every serial waited for a given subscription
+ at 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)
@@ -799,8 +799,7 @@ sub GetSerials2 {
$line->{"publisheddate"} = format_date($line->{"publisheddate"});
push @serials,$line;
}
- my ($totalissues) = scalar(@serials);
- return ($totalissues, at serials);
+ return @serials;
}
=head2 GetLatestSerials
@@ -837,14 +836,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 08eae33..0ebb8f6 100755
--- a/serials/serials-edit.pl
+++ b/serials/serials-edit.pl
@@ -17,7 +17,6 @@
# Koha; if not, write to the Free Software Foundation, Inc., 59 Temple Place,
# Suite 330, Boston, MA 02111-1307 USA
-
=head1 NAME
serials-recieve.pl
@@ -61,7 +60,6 @@ op can be :
=cut
-
use strict;
use warnings;
use CGI;
@@ -74,228 +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, at 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(",", at 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.
- next unless (defined($subscriptionid) && !$processedsubscriptionid{$subscriptionid});
- my $cell;
- if ($serialdatalist[0]->{'serialsadditems'}){
- #Create New empty item
- $cell =
- PrepareItemrecordDisplay( $serialdatalist[0]->{'biblionumber'},'', GetSubscription($subscriptionid));
- $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(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<scalar(@serialids);$i++){
- $index = $i if ($itemhash{$item}->{'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}->{'ind_tag'},
- $itemhash{$item}->{'indicator'});
- # 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'},
- bibliotitle => $bibdata->{'title'},
- biblionumber => $serialdatalist[0]->{'biblionumber'},
- serialslist => \@serialdatalist,
+ serialsadditems => $serialdatalist[0]->{'serialsadditems'},
+ bibliotitle => $bibdata->{'title'},
+ biblionumber => $serialdatalist[0]->{'biblionumber'},
+ serialslist => \@serialdatalist,
);
output_html_with_http_headers $query, $cookie, $template->output;
--
1.6.5.2
More information about the Koha-patches
mailing list