From ff5996d65e4788e20d1c113ec70e80d0fd0710af Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Mon, 20 Oct 2014 12:36:36 +0200 Subject: [PATCH] Bug 10480: Use the framework plugin object in cataloguing This patch implements the use of Koha::FrameworkPlugin in Cataloguing, Authorities, Acquisition, Serials and Tools. The main change is architectural: see the commit message of the previous patch. No changes in behavior are expected, but the support of new events may provide additional functionality in the future. Some small bugs are resolved along the way. The change primarily focuses on the MARC and items editor in Cataloguing. But the MARC editor for Authorities and the item editor in Acquisition, Serials and Tools are touched too. This commit message gives some comments per module. NOTE FOR CATALOGUING: A new plugin without popup (or other click event code) now shows the title No popup when hovering over the tag editor image. The image alerts the user on a plugin, the title tells about its status. The noclick property allows for further style modifications in the template. Note that a follow-up patch will clean up the old style plugins too with the same effect. Some additional code in cataloging.js makes it possible to clone subfields with plugins (although only theoretically useful). The clones use the same javascript functions but event.data contains an updated id. This effectively resolves bug 13306. Note that if old plugins do not use the javascript parameter for the id but the perl variable, cloning does still operate on the wrong field (with and without this patch set). In the absence of report 12176 in master, it is not yet necessary to modify additem.tt. When it gets pushed, it should be an easy rebase. New style item plugins will no longer need an extra parameter. (The code in the FrameworkPlugin object actually takes care of that.) NOTE FOR AUTHORITIES: This patch also adds class name tag_editor to the buttonDot anchors. This effectively makes the same tag editor image appear as in Cataloguing. Futhermore it removes the button from the tab sequence if there is no click event (really effective after conversion to the new style, since the old style plugins contain empty onclicks and launchers). Both small adjustments increase consistency between auth and bib edits. NOTE FOR ACQUISITION: In Acquisition two scripts use an item editor, but in a different way. The scripts addorderiso2709 and neworderempty both rely on the routine PrepareItemrecordDisplay in C4::Items, but neworderempty creates item blocks dynamically via an ajax call to services/itemrecorddisplay.pl. In order to make the dynamic item blocks work with plugins, some code changes were needed in additem.js. (Normally the event binding is done at document ready time; now it must be done later.) At this moment the routine in Items.pm contains the html tags, and this makes changes to the following templates not necessary for now: * acqui/addorderiso2709.tt * services/itemrecorddisplay.tt Report 13397 has been opened to address moving the html to the templates. NOTE FOR SERIALS: Script serial-edit relies also on C4::Items (just as in Acquisition). This makes changes to serials/serials-edit.tt not necessary for now. NOTE FOR TOOLS: The current code in tools/batchMod.pl allows the use of plugins for batch modification of items. This patch just converts that code to use the new object. Most item plugins however may not be very useful for operating on multiple items at once. PERFORMANCE: I have benchmarked build_tabs in addbiblio to see how especially the additional processing of the javascript in the FrameworkPlugin object would impact performance. Testing default MARC21 framework with 8 plugins gave the following figures: - Old situation: 851 ms - New situation: 942 ms (+10,7%) - New situation after plugin cleanup: 881 ms (+3,4%) Note also that adding lines for event binding is compensated by removing lines for unused events. Page load should essentially be the same. TEST PLAN: Suggestion: If you also apply the next patch with the EXAMPLE plugin, you can test with a rather harmless plugin (with popup) on various places :) But your test should also include old style plugins, with[out] popups. If you want to test a new plugin without popup, rename/remove Click$id in the javascript code of the $builder definition (temporarily). [1] Test Cataloguing: - Add/Edit biblio. Try plugins with and without popup. - Add/Edit items. (EXAMPLE can be used as an item plugin with popup.) - Clone a subfield with plugin (use EXAMPLE): Verify that the plugin works on both original and clone with the respective field values. Is the value put back in the right field too? [2] Test Authorities: Edit an authority record. Try plugins with an without popup. [3] Test Acquisition: Set system preference AcqCreateItem to "placing an order". Check the item editor in the following two places: a- addorderiso2709: Open a basket, add an order from a staged file. Select a file, click Add orders, and go to tab Item information. b- neworderempty: Open a basket, add an order from a new empty record. [4] Test Serials: Check the item editor on serials-edit. Go to subscription detail. Click Receive. Choose "Click to add item". (Note that this subscription should create an item record when receiving this serial.) [5] Test Tools: Check the item editor for batch item modification. Enter a few valid barcodes and press Continue to reach the item editor. Signed-off-by: Brendan Gallagher Signed-off-by: Jonathan Druart Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi --- C4/Items.pm | 33 ++++++----- authorities/authorities.pl | 56 +++++++++---------- cataloguing/addbiblio.pl | 39 +++++-------- cataloguing/additem.pl | 38 ++++++------- cataloguing/plugin_launcher.pl | 23 +++----- koha-tmpl/intranet-tmpl/prog/en/js/additem.js | 14 +++++ .../intranet-tmpl/prog/en/js/cataloging.js | 49 ++++++++++------ .../en/modules/authorities/authorities.tt | 8 ++- .../prog/en/modules/cataloguing/addbiblio.tt | 10 +++- .../prog/en/modules/tools/batchMod-edit.tt | 8 ++- tools/batchMod.pl | 30 +++++----- 11 files changed, 169 insertions(+), 139 deletions(-) diff --git a/C4/Items.pm b/C4/Items.pm index 22a8bc2d08..f443d38f6a 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -3022,21 +3022,24 @@ sub PrepareItemrecordDisplay { labels => \%authorised_lib, }; } elsif ( $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, undef, $tagslib, $subfield_data{id}, undef ); - my ( $function_name, $javascript ) = plugin_javascript( $dbh, undef, $tagslib, $subfield_data{id}, undef ); - $subfield_data{random} = int(rand(1000000)); # why do we need 2 different randoms? - $subfield_data{marc_value} = qq[ - ... - $javascript]; - } else { - warn "Plugin Failed: $plugin"; - $subfield_data{marc_value} = qq(); # supply default input form - } + # it is a plugin + require Koha::FrameworkPlugin; + my $plugin = Koha::FrameworkPlugin->new({ + name => $tagslib->{$tag}->{$subfield}->{value_builder}, + item_style => 1, + }); + my $pars = { dbh => $dbh, record => undef, tagslib =>$tagslib, id => $subfield_data{id}, tabloop => undef }; + $plugin->build( $pars ); + if( !$plugin->errstr ) { + #TODO Move html to template; see report 12176/13397 + my $tab= $plugin->noclick? '-1': ''; + my $class= $plugin->noclick? ' disabled': ''; + my $title= $plugin->noclick? 'No popup': 'Tag editor'; + $subfield_data{marc_value} = qq[...\n].$plugin->javascript; + } else { + warn $plugin->errstr; + $subfield_data{marc_value} = qq(); # supply default input form + } } elsif ( $tag eq '' ) { # it's an hidden field $subfield_data{marc_value} = qq(); diff --git a/authorities/authorities.pl b/authorities/authorities.pl index f9a79ad0e8..88eb1560d8 100755 --- a/authorities/authorities.pl +++ b/authorities/authorities.pl @@ -212,37 +212,37 @@ sub create_input { value => $value, authtypecode => $tagslib->{$tag}->{$subfield}->{authtypecode}, }; - # it's a plugin field } - elsif ( $tagslib->{$tag}->{$subfield}->{'value_builder'} ) { - - # opening plugin. Just check whether we are on a developer computer on a production one - # (the cgidir differs) - my $cgidir = C4::Context->intranetdir . "/cgi-bin/cataloguing/value_builder"; - unless (-r $cgidir and -d $cgidir) { - $cgidir = C4::Context->intranetdir . "/cataloguing/value_builder"; + elsif ( $tagslib->{$tag}->{$subfield}->{'value_builder'} ) { # plugin + require Koha::FrameworkPlugin; + my $plugin = Koha::FrameworkPlugin->new({ + name => $tagslib->{$tag}->{$subfield}->{'value_builder'}, + }); + my $pars= { dbh => $dbh, record => $rec, tagslib =>$tagslib, + id => $subfield_data{id}, tabloop => $tabloop }; + $plugin->build( $pars ); + if( !$plugin->errstr ) { + $subfield_data{marc_value} = { + type => 'text2', + id => $subfield_data{id}, + name => $subfield_data{id}, + value => $value, + maxlength => $max_length, + javascript => $plugin->javascript, + noclick => $plugin->noclick, + }; + } else { # warn and supply default field + warn $plugin->errstr; + $subfield_data{marc_value} = { + type => 'text', + id => $subfield_data{id}, + name => $subfield_data{id}, + value => $value, + maxlength => $max_length, + }; } - my $plugin = $cgidir . "/" . $tagslib->{$tag}->{$subfield}->{'value_builder'}; - do $plugin || die "Plugin Failed: ".$plugin; - my $extended_param; - eval{ - $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,$extended_param ); - - $subfield_data{marc_value} = { - type => 'text2', - id => $subfield_data{id}, - name => $subfield_data{id}, - value => $value, - maxlength => $max_length, - function => $function_name, - index_tag => $index_tag, - javascript => $javascript, - }; - # it's an hidden field } + # it's an hidden field elsif ( $tag eq '' ) { $subfield_data{marc_value} = { type => 'hidden', diff --git a/cataloguing/addbiblio.pl b/cataloguing/addbiblio.pl index 9013a4e4be..ecc87f2818 100755 --- a/cataloguing/addbiblio.pl +++ b/cataloguing/addbiblio.pl @@ -391,21 +391,15 @@ sub create_input { }; # it's a plugin field - } - elsif ( $tagslib->{$tag}->{$subfield}->{'value_builder'} ) { - - # opening plugin. Just check whether we are on a developer 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'}; - 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 ); - + } elsif ( $tagslib->{$tag}->{$subfield}->{'value_builder'} ) { + require Koha::FrameworkPlugin; + my $plugin = Koha::FrameworkPlugin->new( { + name => $tagslib->{$tag}->{$subfield}->{'value_builder'}, + }); + my $pars= { dbh => $dbh, record => $rec, tagslib => $tagslib, + id => $subfield_data{id}, tabloop => $tabloop }; + $plugin->build( $pars ); + if( !$plugin->errstr ) { $subfield_data{marc_value} = { type => 'text_complex', id => $subfield_data{id}, @@ -413,13 +407,11 @@ sub create_input { value => $value, size => 67, maxlength => $subfield_data{maxlength}, - function_name => $function_name, - index_tag => $index_tag, - javascript => $javascript, + javascript => $plugin->javascript, + noclick => $plugin->noclick, }; - } else { - warn "Plugin Failed: $plugin"; + warn $plugin->errstr; # supply default input form $subfield_data{marc_value} = { type => 'text', @@ -430,11 +422,10 @@ sub create_input { maxlength => $subfield_data{maxlength}, readonly => 0, }; - } - # it's an hidden field - } - elsif ( $tag eq '' ) { + + # it's an hidden field + } elsif ( $tag eq '' ) { $subfield_data{marc_value} = { type => 'hidden', id => $subfield_data{id}, diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl index 6ff3bbd4c6..0e8f2ef4ee 100755 --- a/cataloguing/additem.pl +++ b/cataloguing/additem.pl @@ -120,7 +120,6 @@ sub generate_subfield_form { $subfield_data{tag} = $tag; $subfield_data{subfield} = $subfieldtag; - $subfield_data{random} = int(rand(1000000)); # why do we need 2 different randoms? $subfield_data{marc_lib} ="{lib}."\">".$subfieldlib->{lib}.""; $subfield_data{mandatory} = $subfieldlib->{mandatory}; $subfield_data{repeatable} = $subfieldlib->{repeatable}; @@ -267,25 +266,24 @@ sub generate_subfield_form { "; } # it's a plugin field - elsif ( $subfieldlib->{value_builder} ) { - # opening plugin - my $plugin = C4::Context->intranetdir . "/cataloguing/value_builder/" . $subfieldlib->{'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 ); - my $change = index($javascript, 'function Change') > -1 ? - "return Change$function_name($subfield_data{random}, '$subfield_data{id}');" : - 'return 1;'; - $subfield_data{marc_value} = qq[ - ... - $javascript]; - } else { - warn "Plugin Failed: $plugin"; - $subfield_data{marc_value} = ""; # supply default input form - } + elsif ( $subfieldlib->{value_builder} ) { # plugin + require Koha::FrameworkPlugin; + my $plugin = Koha::FrameworkPlugin->new({ + name => $subfieldlib->{'value_builder'}, + item_style => 1, + }); + my $pars= { dbh => $dbh, record => $temp, tagslib =>$tagslib, + id => $subfield_data{id}, tabloop => $loop_data }; + $plugin->build( $pars ); + if( !$plugin->errstr ) { + #TODO Report 12176 will make this even better ! + my $class= 'buttonDot'. ( $plugin->noclick? ' disabled': '' ); + my $title= $plugin->noclick? 'No popup': 'Tag editor'; + $subfield_data{marc_value} = qq[...\n].$plugin->javascript; + } else { + warn $plugin->errstr; + $subfield_data{marc_value} = ""; # supply default input form + } } elsif ( $tag eq '' ) { # it's an hidden field $subfield_data{marc_value} = qq(); diff --git a/cataloguing/plugin_launcher.pl b/cataloguing/plugin_launcher.pl index 50aeb3279d..ff4d60eaab 100755 --- a/cataloguing/plugin_launcher.pl +++ b/cataloguing/plugin_launcher.pl @@ -1,6 +1,5 @@ #!/usr/bin/perl - # Copyright 2000-2002 Katipo Communications # # This file is part of Koha. @@ -18,21 +17,13 @@ # with Koha; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -use strict; -#use warnings; FIXME - Bug 2505 +use Modern::Perl; use CGI qw ( -utf8 ); -use C4::Context; -use C4::Output; -my $input = new CGI; -my $plugin_name="cataloguing/value_builder/".$input->param("plugin_name"); +use Koha::FrameworkPlugin; -# opening plugin. Just check whether we are on a developer computer on a production one -# (the cgidir differs) -my $cgidir = C4::Context->intranetdir ."/cgi-bin"; -my $vbdir = "$cgidir/cataloguing/value_builder"; -unless (-r $vbdir and -d $vbdir) { - $cgidir = C4::Context->intranetdir; -} -do $cgidir."/".$plugin_name; -&plugin($input); +my $input = new CGI; +my $plugin= Koha::FrameworkPlugin->new( { + name => $input->param("plugin_name"), +}); +$plugin->launch({ cgi => $input }); diff --git a/koha-tmpl/intranet-tmpl/prog/en/js/additem.js b/koha-tmpl/intranet-tmpl/prog/en/js/additem.js index aa19155925..de3ee71ee8 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/js/additem.js +++ b/koha-tmpl/intranet-tmpl/prog/en/js/additem.js @@ -161,10 +161,24 @@ function cloneItemBlock(index, unique_item_fields) { }); $("#outeritemblock").append(clone); + BindPluginEvents(data); } }); } +function BindPluginEvents(data) { +// the script tag in data for plugins contains a document ready that binds +// the events for the plugin +// when we append, this code does not get executed anymore; so we do it here + var events= data.match(/BindEventstag_\d+_subfield_._\d+/g); + var i; + for(i=0; i ... [% ELSIF ( mv.type == 'text2' ) %] - - ... + + [% IF mv.noclick %] + ... + [% ELSE %] + ... + [% END %] [% mv.javascript %] [% ELSIF ( mv.type == 'text' ) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt index c530ce0120..7c6b48d80d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt @@ -626,7 +626,15 @@ function Changefwk(FwkList) { Tag editor [% END %] [% ELSIF ( mv.type == 'text_complex' ) %] - Tag editor[% mv.javascript %] + + + [% IF mv.noclick %] + + [% ELSE %] + Tag editor + [% END %] + + [% mv.javascript %] [% ELSIF ( mv.type == 'hidden' ) %] [% ELSIF ( mv.type == 'textarea' ) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-edit.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-edit.tt index ab9c2deca2..38a76cecbf 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-edit.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-edit.tt @@ -193,8 +193,12 @@ $(document).ready(function(){ ... [% ELSIF ( mv.type == 'text2' ) %] - - ... + + [% IF mv.noclick %] + ... + [% ELSE %] + ... + [% END %] [% mv.javascript %] [% ELSIF ( mv.type == 'text' ) %] diff --git a/tools/batchMod.pl b/tools/batchMod.pl index 4ff95e836b..afb0abc16b 100755 --- a/tools/batchMod.pl +++ b/tools/batchMod.pl @@ -326,8 +326,6 @@ foreach my $tag (sort keys %{$tagslib}) { } $subfield_data{tag} = $tag; $subfield_data{subfield} = $subfield; - $subfield_data{random} = int(rand(1000000)); # why do we need 2 different randoms? - # $subfield_data{marc_lib} = $tagslib->{$tag}->{$subfield}->{lib}; $subfield_data{marc_lib} ="{$tag}->{$subfield}->{lib}."\">".$tagslib->{$tag}->{$subfield}->{lib}.""; $subfield_data{mandatory} = $tagslib->{$tag}->{$subfield}->{mandatory}; $subfield_data{repeatable} = $tagslib->{$tag}->{$subfield}->{repeatable}; @@ -411,26 +409,28 @@ foreach my $tag (sort keys %{$tagslib}) { value => $value, authtypecode => $tagslib->{$tag}->{$subfield}->{authtypecode}, } - # it's a plugin field } - elsif ( $tagslib->{$tag}->{$subfield}->{value_builder} ) { - # opening plugin - my $plugin = C4::Context->intranetdir . "/cataloguing/value_builder/" . $tagslib->{$tag}->{$subfield}->{'value_builder'}; - if (do $plugin) { - my $temp; - 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 ); + elsif ( $tagslib->{$tag}->{$subfield}->{value_builder} ) { # plugin + require Koha::FrameworkPlugin; + my $plugin = Koha::FrameworkPlugin->new( { + name => $tagslib->{$tag}->{$subfield}->{'value_builder'}, + item_style => 1, + }); + my $temp; + my $pars= { dbh => $dbh, record => $temp, tagslib => $tagslib, + id => $subfield_data{id}, tabloop => \@loop_data }; + $plugin->build( $pars ); + if( !$plugin->errstr ) { $subfield_data{marc_value} = { type => 'text2', id => $subfield_data{id}, value => $value, - function => $function_name, - random => $subfield_data{random}, - javascript => $javascript, + javascript => $plugin->javascript, + noclick => $plugin->noclick, }; } else { - warn "Plugin Failed: $plugin"; - $subfield_data{marc_value} = { # supply default input form + warn $plugin->errstr; + $subfield_data{marc_value} = { # supply default input form type => 'text', id => $subfield_data{id}, value => $value, -- 2.39.5