From 317315df49dd1114f159d5a41657c570e0ba39d2 Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Mon, 17 Jun 2013 13:02:55 +0200
Subject: [PATCH] Bug 10480: New cataloging plugin style without redefining
Content-Type: text/plain; charset=utf-8

This reports adds the way we handle cataloging plugins.
Currently, three functions are redefined: plugin_parameters, plugin_javascript
and plugin. In this new proposal, communication between addbiblio and the
plugin goes via a global variable. The plugin script detects that way if we are
building the plugin or executing it.

A example plugin is added just to illustrate the concept.
Plugin marc21_leader.pl is adjusted to the new style (plus some cosmetic
changes: removing tabs etc.)

Test plan:
Run the MARC editor and test your current plugins.
If you use marc21_leader, test it again.
Temporarily connect new_example_plugin to some field and test that.
Check the log for warnings about redefines.
---
 cataloguing/addbiblio.pl                        |   37 ++++++-
 cataloguing/value_builder/marc21_leader.pl      |  119 ++++++++++++-----------
 cataloguing/value_builder/new_example_plugin.pl |   68 +++++++++++++
 3 files changed, 163 insertions(+), 61 deletions(-)
 create mode 100755 cataloguing/value_builder/new_example_plugin.pl

diff --git a/cataloguing/addbiblio.pl b/cataloguing/addbiblio.pl
index 3a5b7c5..b686f95 100755
--- a/cataloguing/addbiblio.pl
+++ b/cataloguing/addbiblio.pl
@@ -46,6 +46,10 @@ if ( C4::Context->preference('marcflavour') eq 'UNIMARC' ) {
 }
 
 our($tagslib,$authorised_values_sth,$is_a_modif,$usedTagsLib,$mandatory_z3950);
+our $building_plugins=[];
+    #arrayref of hashes { record => .., fieldno => .., tabno => .., }
+    #used in building the new cataloging plugins (without redefinitions)
+    #plugin script should add function and javascript to hash
 
 =head1 FUNCTIONS
 
@@ -406,10 +410,8 @@ sub create_input {
             closedir( DIR );
         }
         my $plugin = $cgidir . "/" . $tagslib->{$tag}->{$subfield}->{'value_builder'};
-        if (do $plugin) {
-            my $extended_param = plugin_parameters( $dbh, $rec, $tagslib, $subfield_data{id}, $tabloop );
-            my ( $function_name, $javascript ) = plugin_javascript( $dbh, $rec, $tagslib, $subfield_data{id}, $tabloop );
-        
+        my ( $function_name, $javascript ) = _new_plugin_builder( $plugin, $rec, $subfield_data{id}, $tabloop );
+        if ($function_name) {
             $subfield_data{marc_value} =
                     "<input tabindex=\"1\"
                             type=\"text\"
@@ -491,6 +493,33 @@ sub create_input {
     return \%subfield_data;
 }
 
+sub _new_plugin_builder {
+    my ($plugin, $record, $fieldno, $tab) = @_;
+
+    my $n= @$building_plugins;
+    push @$building_plugins,
+        {
+            record  => $record,
+            fieldno => $fieldno,
+            tab     => $tab,
+        };
+    do $plugin || return;
+    if( $building_plugins->[$n]->{function} &&
+            exists $building_plugins->[$n]->{javascript} ) {
+            #this is the new way without redefines
+        return ($building_plugins->[$n]->{function},
+            $building_plugins->[$n]->{javascript} );
+    }
+
+    #arriving here, we try the old way with redefines: we are PHASING this OUT
+    #next call is of no use, commented
+    #my $extended_param = plugin_parameters( $dbh, $rec, $tagslib, $subfield_data{id}, $tabloop );
+    my ( $function_name, $javascript ) = plugin_javascript( undef, $record, $tagslib, $fieldno, $tab );
+        #first param was dbh, not needed: use C4::Context->dbh in plugin
+        #note also that tagslib is global (so actually superfluous)
+    return ( $function_name, $javascript ) if $function_name && $javascript;
+    return;
+}
 
 =head2 format_indicator
 
diff --git a/cataloguing/value_builder/marc21_leader.pl b/cataloguing/value_builder/marc21_leader.pl
index 69b79d0..3c8c69b 100755
--- a/cataloguing/value_builder/marc21_leader.pl
+++ b/cataloguing/value_builder/marc21_leader.pl
@@ -1,6 +1,5 @@
 #!/usr/bin/perl
 
-
 # Copyright 2000-2002 Katipo Communications
 #
 # This file is part of Koha.
@@ -19,29 +18,34 @@
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
 use strict;
-#use warnings; FIXME - Bug 2505
-use C4::Auth;
+use warnings;
+
 use CGI;
-use C4::Context;
 
+use C4::Auth;
+use C4::Context;
 use C4::Search;
 use C4::Output;
 
-=head1
+# This plugin has been moved to the new style using $building_plugins
 
-plugin_parameters : other parameters added when the plugin is called by the dopop function
+our $building_plugins;
+if(defined $building_plugins) {
+    pass_script_for_marc21_leader();
+}
+else {
+    run_marc21_leader_plugin();
+}
 
-=cut
+#-------------------------------------------------------------------------------
 
-sub plugin_parameters {
-my ($dbh,$record,$tagslib,$i,$tabloop) = @_;
-return "";
-}
+sub pass_script_for_marc21_leader {
+    my $n=@$building_plugins;
+    return if $n==0;
+    my $field_number= $building_plugins->[$n-1]->{fieldno};
 
-sub plugin_javascript {
-my ($dbh,$record,$tagslib,$field_number,$tabloop) = @_;
-my $function_name= $field_number;
-my $res="
+    my $function_name= $field_number;
+    my $res="
 <script type=\"text/javascript\">
 //<![CDATA[
 
@@ -53,56 +57,57 @@ function Focus$function_name(subfield_managed) {
 }
 
 function Blur$function_name(subfield_managed) {
-	return 1;
+    return 1;
 }
 
 function Clic$function_name(i) {
-	defaultvalue=document.getElementById(\"$field_number\").value;
-	newin=window.open(\"../cataloguing/plugin_launcher.pl?plugin_name=marc21_leader.pl&index=$field_number&result=\"+defaultvalue,\"tag_editor\",'width=1000,height=600,toolbar=false,scrollbars=yes');
-
+    defaultvalue=document.getElementById(\"$field_number\").value;
+    newin=window.open(\"../cataloguing/value_builder/marc21_leader.pl?index=$field_number&result=\"+defaultvalue,\"tag_editor\",'width=1000,height=600,toolbar=false,scrollbars=yes');
 }
 //]]>
 </script>
 ";
-
-return ($function_name,$res);
+    $building_plugins->[$n-1]->{function}= $function_name;
+    $building_plugins->[$n-1]->{javascript}= $res;
 }
-sub plugin {
-my ($input) = @_;
-	my $index= $input->param('index');
-	my $result= $input->param('result');
-
-
-	my $dbh = C4::Context->dbh;
-
-my ($template, $loggedinuser, $cookie)
-    = get_template_and_user({template_name => "cataloguing/value_builder/marc21_leader.tmpl",
-			     query => $input,
-			     type => "intranet",
-			     authnotrequired => 0,
-			     flagsrequired => {editcatalogue => '*'},
-			     debug => 1,
-			     });
-	$result = "     nam a22     7a 4500" unless $result;
-	my $f5 = substr($result,5,1);
-	my $f6 = substr($result,6,1);
-	my $f7 = substr($result,7,1);
-	my $f8 = substr($result,8,1);
-	my $f17 = substr($result,17,1);
-	my $f18 = substr($result,18,1);
-	my $f19 = substr($result,19,1);
-	my $f2023 = substr($result,20,4);
-	$template->param(index => $index,
-							"f5$f5" => 1,
-							"f6$f6" => 1,
-							"f7$f7" => 1,
-							"f8$f8" => 1,
-							"f17$f17" => 1,
-							"f18$f18" => 1,
-							"f19$f19" => 1,
-							"f2023" => $f2023,
-					);
-        output_html_with_http_headers $input, $cookie, $template->output;
+
+sub run_marc21_leader_plugin {
+    my $input = new CGI;
+    my $index= $input->param('index');
+    my $result= $input->param('result');
+
+    my $dbh = C4::Context->dbh;
+
+    my ($template, $loggedinuser, $cookie) = get_template_and_user(
+        {
+            template_name => "cataloguing/value_builder/marc21_leader.tmpl",
+            query => $input,
+            type => "intranet",
+            authnotrequired => 0,
+            flagsrequired => {editcatalogue => '*'},
+            debug => 1,
+        });
+    $result = "     nam a22     7a 4500" unless $result;
+    my $f5 = substr($result,5,1);
+    my $f6 = substr($result,6,1);
+    my $f7 = substr($result,7,1);
+    my $f8 = substr($result,8,1);
+    my $f17 = substr($result,17,1);
+    my $f18 = substr($result,18,1);
+    my $f19 = substr($result,19,1);
+    my $f2023 = substr($result,20,4);
+    $template->param(
+        index => $index,
+        "f5$f5" => 1,
+        "f6$f6" => 1,
+        "f7$f7" => 1,
+        "f8$f8" => 1,
+        "f17$f17" => 1,
+        "f18$f18" => 1,
+        "f19$f19" => 1,
+        "f2023" => $f2023,
+    );
+    output_html_with_http_headers $input, $cookie, $template->output;
 }
 
 1;
diff --git a/cataloguing/value_builder/new_example_plugin.pl b/cataloguing/value_builder/new_example_plugin.pl
new file mode 100755
index 0000000..b22856c
--- /dev/null
+++ b/cataloguing/value_builder/new_example_plugin.pl
@@ -0,0 +1,68 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use CGI;
+
+#-------------------------------------------------------------------------------
+
+# example of new cataloging plugin style without redefines
+# if addbiblio calls this script, the global $building_plugins exists
+# when copying this example, pick two new names to prevent redefining
+
+our $building_plugins;
+if(defined $building_plugins) {
+    pass_script_for_myexample_plugin(); #TODO: pick a new name !
+}
+else {
+    run_myexample_plugin(); #TODO: pick a new name !
+}
+
+#-------------------------------------------------------------------------------
+
+sub pass_script_for_myexample_plugin { #TODO: pick a new name
+    #always use the following lines
+    #note that you also have a record and tab parameter (
+    my $n=@$building_plugins;
+    return if $n==0;
+    my $field_number= $building_plugins->[$n-1]->{fieldno};
+
+    #now return a function name and javascript via global $building_plugins
+    #your javascript normally contains a Blur, Clic and Focus function
+    #call your plugin NO LONGER via plugin_launcher but directly
+    #see also marc21_leader.pl
+    my $function_name= $field_number;
+        #does not need to be so..
+    my $javascript=qq|
+<script type=\"text/javascript\">
+//<![CDATA[
+function Focus$function_name(subfield_managed) {
+    return 1;
+}
+function Blur$function_name(subfield_managed) {
+    document.getElementById(\"$field_number\").value= '12345';
+    return 1;
+}
+function Clic$function_name(i) {
+    return 1;
+}
+//]]>
+</script>
+|; #as you may be aware, this example just puts 12345 in your field and no more
+
+    $building_plugins->[$n-1]->{function}= $function_name;
+    $building_plugins->[$n-1]->{javascript}= $javascript;
+}
+
+
+#-------------------------------------------------------------------------------
+
+sub run_myexample_plugin { #TODO: pick a new name !
+    my $query=new CGI;
+
+    #does nothing now
+    #rest of your plugin script comes here
+}
+
+1;
-- 
1.7.7.6