[Koha-patches] [PATCH] Begin cleanup on additem.

[Joe Atzberger]

REFACTOR logic out of conditional branches when the assignment ($nextop) is the same.
Be sure to EXIT after printing a redirect, instead of wasting clock filling in the
template for a process the user will never see.  Remove  the now inapplicable logic for
differentiation between "/cgi-bin" and "non /cgi-bin" installations.  There is no
longer any /cgi-bin in the actual directory file path.

This does not fix the value_builder js errors like "Blurbarcode52251 is not defined",
but it does not cause them either (see Bug 2919).
---
 cataloguing/additem.pl |   72 ++++++++++++++++++++---------------------------
 1 files changed, 31 insertions(+), 41 deletions(-)

diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl
index f8780c2..b197589 100755
--- a/cataloguing/additem.pl
+++ b/cataloguing/additem.pl
@@ -61,10 +61,10 @@ sub get_item_from_barcode {
 
 my $input = new CGI;
 my $dbh = C4::Context->dbh;
-my $error = $input->param('error');
+my $error        = $input->param('error');
 my $biblionumber = $input->param('biblionumber');
-my $itemnumber = $input->param('itemnumber');
-my $op = $input->param('op');
+my $itemnumber   = $input->param('itemnumber');
+my $op           = $input->param('op');
 
 my ($template, $loggedinuser, $cookie)
     = get_template_and_user({template_name => "cataloguing/additem.tmpl",
@@ -119,11 +119,9 @@ if ($op eq "additem") {
     push @errors,"barcode_not_unique" if($exist_itemnumber);
     # if barcode exists, don't create, but report The problem.
     my ($oldbiblionumber,$oldbibnum,$oldbibitemnum) = AddItemFromMarc($record,$biblionumber) unless ($exist_itemnumber);
+    $nextop = "additem";
     if ($exist_itemnumber) {
-        $nextop = "additem";
         $itemrecord = $record;
-    } else {
-        $nextop = "additem";
     }
 #-------------------------------------------------------------------------------
 } elsif ($op eq "edititem") {
@@ -139,34 +137,32 @@ if ($op eq "additem") {
     $sth->execute($itemnumber);
     my $onloan=$sth->fetchrow;
 	$sth->finish();
-    push @errors,"book_on_loan" if ($onloan); ##error book_on_loan added to template as well
+    $nextop="additem";
     if ($onloan){
-		$nextop="additem";
+        push @errors,"book_on_loan";
     } else {
 		# check it doesnt have a waiting reserve
 		$sth=$dbh->prepare("SELECT * FROM reserves WHERE found = 'W' AND itemnumber = ?");
 		$sth->execute($itemnumber);
 		my $reserve=$sth->fetchrow;
-		if ($reserve){
-			push @errors,"book_reserved";
-			$nextop="additem";
-		}
-		else {
+		unless ($reserve){
 			&DelItem($dbh,$biblionumber,$itemnumber);
 			print $input->redirect("additem.pl?biblionumber=$biblionumber&frameworkcode=$frameworkcode");
+            exit;
 		}
+        push @errors,"book_reserved";
     }
 #-------------------------------------------------------------------------------
 } elsif ($op eq "saveitem") {
 #-------------------------------------------------------------------------------
     # rebuild
-    my @tags = $input->param('tag');
+    my @tags      = $input->param('tag');
     my @subfields = $input->param('subfield');
-    my @values = $input->param('field_value');
+    my @values    = $input->param('field_value');
     # build indicator hash.
-    my @ind_tag = $input->param('ind_tag');
+    my @ind_tag   = $input->param('ind_tag');
     my @indicator = $input->param('indicator');
-    #    my $itemnumber = $input->param('itemnumber');
+    # my $itemnumber = $input->param('itemnumber');
     my $xml = TransformHtmlToXml(\@tags,\@subfields,\@values,\@indicator,\@ind_tag,'ITEM');
     my $itemtosave=MARC::Record::new_from_xml($xml, 'UTF-8');
     # MARC::Record builded => now, record in DB
@@ -195,8 +191,8 @@ my @fields = $temp->fields();
 my %witness; #---- stores the list of subfields used at least once, with the "meaning" of the code
 my @big_array;
 #---- finds where items.itemnumber is stored
-my ($itemtagfield,$itemtagsubfield) = &GetMarcFromKohaField("items.itemnumber",$frameworkcode);
-my ($branchtagfield,$branchtagsubfield) = &GetMarcFromKohaField("items.homebranch",$frameworkcode);
+my (  $itemtagfield,   $itemtagsubfield) = &GetMarcFromKohaField("items.itemnumber", $frameworkcode);
+my ($branchtagfield, $branchtagsubfield) = &GetMarcFromKohaField("items.homebranch", $frameworkcode);
 
 foreach my $field (@fields) {
     next if ($field->tag()<10);
@@ -283,8 +279,8 @@ foreach my $tag (sort keys %{$tagslib}) {
     $subfield_data{subfield}=$subfield;
     $subfield_data{random}=int(rand(1000000)); 
 #        $subfield_data{marc_lib}=$tagslib->{$tag}->{$subfield}->{lib};
-    $subfield_data{marc_lib}="<span id=\"error$i\" title=\"".$tagslib->{$tag}->{$subfield}->{lib}."\">".$tagslib->{$tag}->{$subfield}->{lib}."</span>";
-    $subfield_data{mandatory}=$tagslib->{$tag}->{$subfield}->{mandatory};
+    $subfield_data{marc_lib}  ="<span id=\"error$i\" title=\"".$tagslib->{$tag}->{$subfield}->{lib}."\">".$tagslib->{$tag}->{$subfield}->{lib}."</span>";
+    $subfield_data{mandatory} =$tagslib->{$tag}->{$subfield}->{mandatory};
     $subfield_data{repeatable}=$tagslib->{$tag}->{$subfield}->{repeatable};
     my ($x,$value);
     ($x,$value) = find_value($tag,$subfield,$itemrecord) if ($itemrecord);
@@ -301,22 +297,22 @@ foreach my $tag (sort keys %{$tagslib}) {
         $value =~ s/DD/$day/g;
     }
     $subfield_data{visibility} = "display:none;" if (($tagslib->{$tag}->{$subfield}->{hidden} > 4) || ($tagslib->{$tag}->{$subfield}->{hidden} < -4));
-    #testing branch value if IndependantBranches.
-    my $test = (C4::Context->preference("IndependantBranches")) &&
-              ($tag eq $branchtagfield) && ($subfield eq $branchtagsubfield) &&
-              (C4::Context->userenv->{flags} != 1) && ($value) && ($value ne C4::Context->userenv->{branch}) ;
-#         print $input->redirect(".pl?biblionumber=$biblionumber") if ($test);
+    # testing branch value if IndependantBranches.
+    # my $test = (C4::Context->preference("IndependantBranches")) &&
+    #          ($tag eq $branchtagfield) && ($subfield eq $branchtagsubfield) &&
+    #          (C4::Context->userenv->{flags} != 1) && ($value) && ($value ne C4::Context->userenv->{branch}) ;
+    # $test and print $input->redirect(".pl?biblionumber=$biblionumber") and exit;
         # search for itemcallnumber if applicable
     if (!$value && $tagslib->{$tag}->{$subfield}->{kohafield} eq 'items.itemcallnumber' && C4::Context->preference('itemcallnumber')) {
-        my $CNtag = substr(C4::Context->preference('itemcallnumber'),0,3);
-        my $CNsubfield = substr(C4::Context->preference('itemcallnumber'),3,1);
+        my $CNtag       = substr(C4::Context->preference('itemcallnumber'),0,3);
+        my $CNsubfield  = substr(C4::Context->preference('itemcallnumber'),3,1);
         my $CNsubfield2 = substr(C4::Context->preference('itemcallnumber'),4,1);
         my $temp2 = $temp->field($CNtag);
         if ($temp2) {
-                $value = ($temp2->subfield($CNsubfield)).' '.($temp2->subfield($CNsubfield2));
-#remove any trailing space incase one subfield is used
-        $value=~s/^\s+|\s+$//g;
-      }
+            $value = ($temp2->subfield($CNsubfield)).' '.($temp2->subfield($CNsubfield2));
+            #remove any trailing space incase one subfield is used
+            $value=~s/^\s+|\s+$//g;
+        }
     }
     if ( $tagslib->{$tag}->{$subfield}->{authorised_value} ) {
       my @authorised_values;
@@ -390,7 +386,7 @@ foreach my $tag (sort keys %{$tagslib}) {
               $authorised_lib{$value} = $lib;
           }
       }
-      $subfield_data{marc_value} =CGI::scrolling_list(
+      $subfield_data{marc_value} =CGI::scrolling_list( # FIXME: factor out scrolling_list
           -name     => "field_value",
           -values   => \@authorised_values,
           -default  => $value,
@@ -422,14 +418,8 @@ foreach my $tag (sort keys %{$tagslib}) {
     }
     elsif ( $tagslib->{$tag}->{$subfield}->{'value_builder'} ) {
 
-        # opening plugin. Just check wether we are on a developper computer on a production one
-        # (the cgidir differs)
-        my $cgidir = C4::Context->intranetdir . "/cgi-bin/cataloguing/value_builder";
-        unless ( opendir( DIR, "$cgidir" ) ) {
-            $cgidir = C4::Context->intranetdir . "/cataloguing/value_builder";
-            closedir( DIR );
-        }
-        my $plugin = $cgidir . "/" . $tagslib->{$tag}->{$subfield}->{'value_builder'};
+        # opening plugin
+        my $plugin = C4::Context->intranetdir . "/cataloguing/value_builder" . $tagslib->{$tag}->{$subfield}->{'value_builder'};
         if (do $plugin) {
             my $extended_param = plugin_parameters( $dbh, $temp, $tagslib, $subfield_data{id}, \@loop_data );
             my ( $function_name, $javascript ) = plugin_javascript( $dbh, $temp, $tagslib, $subfield_data{id}, \@loop_data );
-- 
1.5.5.GIT