From e3937b3f68699f0706236a2a2db5fec6fe5e61b1 Mon Sep 17 00:00:00 2001
From: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Date: Sat, 19 Jul 2014 20:58:17 -0300
Subject: [PATCH] Bug 12614: Remove CGI::scrolling_list from
 auth_subfields_structure.pl

This patch removes all instances on this file. Even removes some commented out instances.

Also re-enable editor feature to show/change kohafield value, can't find when or why it was removed.
Up to QA or RM for consideration, simply to remove.
But is strange to have a non editable field on auth frameworks.

To test:
1. Apply the patch
2. Go to Administration > Authority types
3. Clic MARC structure of any auth fw
4. Clic subfield on any tag, e.g. 031
5. Clic 'Edit subfields' button

Now the tests proper

6. 'Help input' box (editor bottom):
Eight(8) scrolling_list removed, they use to
feed the four pulldowns, so check each one
'Koha field' is new (for me at last)
'Authorised value',
'Thesaurus' (is this valid/useful here?)
'Plugin'
There is a difference between existing subtags and a new subtag,
the 'id' of the pulldowns (so the 8 scrolling list removed)
Check assign/remove/save/load values, search for regressions
If you edit subtag 942, it has a value on kohafield (default fw)

I think that this box could be prettified with fixed width for
pulldowns.

7. No more functional tests, there are 4 scrolling_list removed
that are commented (removed also commented code on tt file),
and two instances that are similar to previously removed
instances, evidently with no effect.
---
 admin/auth_subfields_structure.pl                  | 186 ++++++---------------
 .../en/modules/admin/auth_subfields_structure.tt   |  53 +++++-
 2 files changed, 97 insertions(+), 142 deletions(-)

diff --git a/admin/auth_subfields_structure.pl b/admin/auth_subfields_structure.pl
index c25090bf..67c7fac 100755
--- a/admin/auth_subfields_structure.pl
+++ b/admin/auth_subfields_structure.pl
@@ -148,62 +148,31 @@ if ($op eq 'add_form') {
                     id      => "ohidden$i",
                     default => $data->{'hidden'}
                     };
-		#$row_data{ihidden} = CGI::scrolling_list(-name=>'ihidden',
-		#			-id=>"ihidden$i",
-		#			-values=>['0','1','2'],
-		#			-labels => {'0'=>'Show','1'=>'Show Collapsed',
-		#							'2' =>'Hide',
-		#							},
-		#			-default=>substr($data->{'hidden'},1,1),
-		#			-size=>1,
-		#			-multiple=>0,
-		#			);
-		#$row_data{ehidden} = CGI::scrolling_list(-name=>'ehidden',
-		#			-id=>"ehidden$i",
-		#			-values=>['0','1','2'],
-		#			-labels => {'0'=>'Show','1'=>'Show Collapsed',
-		#							'2' =>'Hide',
-		#							},
-		#			-default=>substr($data->{'hidden'}."  ",2,1),
-		#			-size=>1,
-		#			-multiple=>0,
-		#			);
 		$row_data{tagsubfieldinput} = "<input type=\"hidden\" name=\"tagsubfield\" value=\"".$data->{'tagsubfield'}."\" id=\"tagsubfield\" />";
 		$row_data{tagsubfield} = $data->{'tagsubfield'};
 		$row_data{liblibrarian} = CGI::escapeHTML($data->{'liblibrarian'});
 		$row_data{libopac} = CGI::escapeHTML($data->{'libopac'});
 		$row_data{seealso} = CGI::escapeHTML($data->{'seealso'});
-		$row_data{kohafield}= CGI::scrolling_list( -name=>"kohafield",
-					-id=>"kohafield$i",
-					-values=> \@kohafields,
-					-default=> "$data->{'kohafield'}",
-					-size=>1,
-					-multiple=>0,
-					);
-		$row_data{authorised_value}  = CGI::scrolling_list(-name=>'authorised_value',
-					-id=>"authorised_value$i",
-					-values=> \@authorised_values,
-					-default=>$data->{'authorised_value'},
-					-size=>1,
-		 			-tabindex=>'',
-					-multiple=>0,
-					);
-		$row_data{frameworkcode}  = CGI::scrolling_list(-name=>'frameworkcode',
-					-id=>"frameworkcode$i",
-					-values=> \@authtypes,
-					-default=>$data->{'frameworkcode'},
-					-size=>1,
-		 			-tabindex=>'',
-					-multiple=>0,
-					);
-		$row_data{value_builder}  = CGI::scrolling_list(-name=>'value_builder',
-					-id=>"value_builder$i",
-					-values=> \@value_builder,
-					-default=>$data->{'value_builder'},
-					-size=>1,
-		 			-tabindex=>'',
-					-multiple=>0,
-					);
+        $row_data{kohafield} = {
+                    id      => "kohafield$i",
+                    values  => \@kohafields,
+                    default => "$data->{'kohafield'}",
+                    };
+        $row_data{authorised_value} = {
+                    id      => "authorised_value$i",
+                    values  => \@authorised_values,
+                    default => $data->{'authorised_value'},
+        };
+        $row_data{frameworkcode} = {
+                    id      => "frameworkcode$i",
+                    values  => \@authtypes,
+                    default => $data->{'frameworkcode'},
+        };
+        $row_data{value_builder} = {
+                    id      => "value_builder$i",
+                    values  => \@value_builder,
+                    default => $data->{'value_builder'},
+        };
 		
 		$row_data{repeatable} = CGI::checkbox(-name=>"repeatable$i",
 	-checked => $data->{'repeatable'}?'checked':'',
@@ -229,63 +198,15 @@ if ($op eq 'add_form') {
 	for (my $i=1;$i<=$more_subfields;$i++) {
 		my %row_data;  # get a fresh hash for the row data
         $row_data{'new_subfield'} = 1;
-		$row_data{tab} = CGI::scrolling_list(-name=>'tab',
-					-id => "tab$i",
-                                        -values =>
-                                        [ '-1', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '10' ],
-                                        -labels => {
-                                            '-1' => 'ignore',
-                                            '0'  => '0',
-                                            '1'  => '1',
-                                            '2'  => '2',
-                                            '3'  => '3',
-                                            '4'  => '4',
-                                            '5'  => '5',
-                                            '6'  => '6',
-                                            '7'  => '7',
-                                            '8'  => '8',
-                                            '9'  => '9',
-                                        },
-					-default=>"",
-					-size=>1,
-		 			-tabindex=>'',
-					-multiple=>0,
-					);
-		$row_data{ohidden} = CGI::scrolling_list(-name=>'ohidden',
-					-id=>"ohidden$i",
-					#-values=>['0','1','2'],
-					#-labels => {'0'=>'Show','1'=>'Show Collapsed',
-					#				'2' =>'Hide',
-					#				},
-					-values=>['0','-5'],
-					-labels => {'0'=>'Show All','-5'=>'Hide All',},
-					#-default=>"0",
-					-default=>$data->{'hidden'},
-					#-default=>"-5",
-					-size=>1,
-					-multiple=>0,
-					);
+        $row_data{tab} = {
+                    id      => "tab$i",
+                    default => $data->{'tab'},
+                    };
+        $row_data{ohidden} = {
+                    id      => "ohidden$i",
+                    default => $data->{'hidden'}
+                    };
 
-		#$row_data{ihidden} = CGI::scrolling_list(-name=>'ihidden',
-		#			-id=>"ihidden$i",
-		#			-values=>['0','1','2'],
-		#			-labels => {'0'=>'Show','1'=>'Show Collapsed',
-		#							'2' =>'Hide',
-		#							},
-		#			-default=>"0",
-		#			-size=>1,
-		#			-multiple=>0,
-		#			);
-		#$row_data{ehidden} = CGI::scrolling_list(-name=>'ehidden',
-		#			-id=>"ehidden$i",
-		#			-values=>['0','1','2'],
-		#			-labels => {'0'=>'Show','1'=>'Show Collapsed',
-		#							'2' =>'Hide',
-		#							},
-		#			-default=>"0",
-		#			-size=>1,
-		#			-multiple=>0,
-		#			);
 		$row_data{tagsubfieldinput} = "<input type=\"text\" name=\"tagsubfield\" value=\"".$data->{'tagsubfield'}."\" size=\"1\" id=\"tagsubfield\" maxlength=\"1\" />";
                 $row_data{tagsubfieldinput} = 
                         "<label><input type=\"text\" name=\"tagsubfield\" value=\""
@@ -311,36 +232,27 @@ if ($op eq 'add_form') {
 			-checked => '',
 			-value => 1,
 			-label => '');
-		$row_data{kohafield}= CGI::scrolling_list( -name=>'kohafield',
-					-id => "kohafield$i",
-					-values=> \@kohafields,
-					-default=> "",
-					-size=>1,
-					-multiple=>0,
-					);
-		$row_data{frameworkcode}  = CGI::scrolling_list(-name=>'frameworkcode',
-					-id=>'frameworkcode',
-					-values=> \@authtypes,
-					-default=>$data->{'frameworkcode'},
-					-size=>1,
-		 			-tabindex=>'',
-					-multiple=>0,
-					);
-		$row_data{authorised_value}  = CGI::scrolling_list(-name=>'authorised_value',
-					-id => 'authorised_value',
-					-values=> \@authorised_values,
-					-size=>1,
-		 			-tabindex=>'',
-					-multiple=>0,
-					);
-		$row_data{value_builder}  = CGI::scrolling_list(-name=>'value_builder',
-					-id=>'value_builder',
-					-values=> \@value_builder,
-					-default=>$data->{'value_builder'},
-					-size=>1,
-		 			-tabindex=>'',
-					-multiple=>0,
-					);
+        $row_data{kohafield} = {
+                    id      => "kohafield$i",
+                    values  => \@kohafields,
+                    default => "",
+                    };
+        $row_data{authorised_value} = {
+                    id      => "authorised_value",
+                    values  => \@authorised_values,
+                    default => "",
+        };
+        $row_data{frameworkcode} = {
+                    id      => "frameworkcode",
+                    values  => \@authtypes,
+                    default => $data->{'frameworkcode'},
+        };
+        $row_data{value_builder} = {
+                    id      => "value_builder",
+                    values  => \@value_builder,
+                    default => $data->{'value_builder'},
+        };
+
 		$row_data{row} = $i;
 		push(@loop_data, \%row_data);
 	}
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/auth_subfields_structure.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/auth_subfields_structure.tt
index 94d53bf..6de697d 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/auth_subfields_structure.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/auth_subfields_structure.tt
@@ -123,8 +123,6 @@ function displayMoreConstraint(numlayer){
                                     [%- END -%]
                                 </select>
                             </li>
-                            <!-- <li><label for="ihidden[% loo.row %]">Intranet:</label>[% loo.ihidden %] </li> -->
-                            <!-- <li><label for="ehidden[% loo.row %]">Editor:</label>[% loo.ehidden %] (see online help)</li> -->
                         </ol>
                     </fieldset>
                 </li>
@@ -143,9 +141,54 @@ function displayMoreConstraint(numlayer){
                 <li>
                     <fieldset><legend>Help input</legend>    
                         <ol>
-                            <li><label for="authorised_value[% loo.row %]">Authorized value:</label> [% loo.authorised_value %]</li>
-                            <li><label for="frameworkcode[% loo.row %]">Thesaurus:</label> [% loo.frameworkcode %]</li>
-                            <li><label for="value_builder[% loo.row %]">Plugin:</label> [% loo.value_builder %]</li>
+                            <li>
+                                <label for="kohafield[% loo.row %]">Koha field:</label>
+                                <select name="kohafield" id="[% loo.kohafield.id %]" size="1">
+                                [% FOREACH value IN loo.kohafield.values %]
+                                    [% IF ( value == loo.kohafield.default ) %]
+                                    <option value="[% value %]" selected>[% value %]</option>
+                                    [% ELSE %]
+                                    <option value="[% value %]">[% value %]</option>
+                                    [% END %]
+                                [% END %]
+                                </select>
+                            </li>
+                            <li>
+                                <label for="authorised_value[% loo.row %]">Authorized value:</label>
+                                <select name="authorised_value" id="[% loo.authorised_value.id %]" size="1">
+                                [% FOREACH value IN loo.authorised_value.values %]
+                                    [% IF ( value == loo.authorised_value.default ) %]
+                                    <option value="[% value %]" selected>[% value %]</option>
+                                    [% ELSE %]
+                                    <option value="[% value %]">[% value %]</option>
+                                    [% END %]
+                                [% END %]
+                                </select>
+                            </li>
+                            <li>
+                                <label for="frameworkcode[% loo.row %]">Thesaurus:</label>
+                                <select name="frameworkcode" id="[% loo.frameworkcode.id %]" size="1">
+                                [% FOREACH value IN loo.frameworkcode.values %]
+                                    [% IF ( value == loo.frameworkcode.default ) %]
+                                    <option value="[% value %]" selected>[% value %]</option>
+                                    [% ELSE %]
+                                    <option value="[% value %]">[% value %]</option>
+                                    [% END %]
+                                [% END %]
+                                </select>
+                            </li>
+                            <li>
+                                <label for="value_builder[% loo.row %]">Plugin:</label>
+                                <select name="value_builder" id="[% loo.value_builder.id %]" size="1">
+                                [% FOREACH value IN loo.value_builder.values %]
+                                    [% IF ( value == loo.value_builder.default ) %]
+                                    <option value="[% value %]" selected>[% value %]</option>
+                                    [% ELSE %]
+                                    <option value="[% value %]">[% value %]</option>
+                                    [% END %]
+                                [% END %]
+                                </select>
+                            </li>
                         </ol>
                     </fieldset>
                 </li>
-- 
1.9.1