From fbff1809a4034baeafaef3ddf394325201aa19e9 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 13 May 2016 20:57:33 +0100 Subject: [PATCH] Bug 16519: Replace 'our' with 'my' in [opac-]addbybiblionumbers.pl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To avoid bug like bug 16518 and to ease the readability/maintainability of these scripts, this patch replaces the use of 'our' with 'my' to avoid the use of global variables. Basically the code has been moved from subroutines to the appropriate places. Test plan: At the intranet and OPAC sides 1/ Add items to a list 2/ Add items to a list using an existing name 3/ Add items to a list you don't have right on it (by modifying the biblionumber in the url) 4/ At the OPAC, use the opac-addbybiblionumber.pl without being logged in to add items to a list Signed-off-by: Marc VĂ©ron --- opac/opac-addbybiblionumber.pl | 239 ++++++++++++++--------------------- virtualshelves/addbybiblionumber.pl | 210 +++++++++++++----------------- 2 files changed, 185 insertions(+), 264 deletions(-) diff --git a/opac/opac-addbybiblionumber.pl b/opac/opac-addbybiblionumber.pl index a5decf7..9227fa0 100755 --- a/opac/opac-addbybiblionumber.pl +++ b/opac/opac-addbybiblionumber.pl @@ -1,8 +1,7 @@ #!/usr/bin/perl -#script to provide virtualshelf management -# # Copyright 2000-2002 Katipo Communications +# Copyright 2016 Koha Development Team # # This file is part of Koha. # @@ -19,8 +18,7 @@ # You should have received a copy of the GNU General Public License # along with Koha; if not, see . -use strict; -use warnings; +use Modern::Perl; use CGI qw ( -utf8 ); use C4::Biblio; @@ -29,180 +27,139 @@ use C4::Auth; use Koha::Virtualshelves; -our $query = new CGI; -our @biblionumber = $query->param('biblionumber'); -our $selectedshelf = $query->param('selectedshelf'); -our $newshelf = $query->param('newshelf'); -our $shelfnumber = $query->param('shelfnumber'); -our $newvirtualshelf = $query->param('newvirtualshelf'); -our $category = $query->param('category'); -our $authorized = 1; -our $errcode = 0; -our @biblios = (); +my $query = new CGI; +my @biblionumbers = $query->multi_param('biblionumber'); +my $selectedshelf = $query->param('selectedshelf'); +my $newshelf = $query->param('newshelf'); +my $shelfnumber = $query->param('shelfnumber'); +my $newvirtualshelf = $query->param('newvirtualshelf'); +my $category = $query->param('category'); +my ( $errcode, $authorized ) = ( 0, 1 ); +my @biblios; # if virtualshelves is disabled, leave immediately -if ( ! C4::Context->preference('virtualshelves') ) { +if ( !C4::Context->preference('virtualshelves') ) { print $query->redirect("/cgi-bin/koha/errors/404.pl"); exit; } -if (scalar(@biblionumber) == 1) { - @biblionumber = (split /\//,$biblionumber[0]); +if ( scalar(@biblionumbers) == 1 ) { + @biblionumbers = ( split /\//, $biblionumbers[0] ); } -our ( $template, $loggedinuser, $cookie ) = get_template_and_user( - { - template_name => "opac-addbybiblionumber.tt", +my ( $template, $loggedinuser, $cookie ) = get_template_and_user( + { template_name => "opac-addbybiblionumber.tt", query => $query, type => "opac", authnotrequired => 0, } ); -if( $newvirtualshelf) { - HandleNewVirtualShelf(); - exit if $authorized; - ShowTemplate(); #error message -} -elsif($shelfnumber) { - HandleShelfNumber(); - exit if $authorized; - ShowTemplate(); #error message -} -elsif($selectedshelf) { - HandleSelectedShelf(); - LoadBib() if $authorized; - ShowTemplate(); -} -else { - HandleSelect(); - LoadBib() if $authorized; - ShowTemplate(); -} -#end - -sub HandleNewVirtualShelf { - if ( $loggedinuser > 0 and - ( - $category == 1 - or $category == 2 and $loggedinuser>0 && C4::Context->preference('OpacAllowPublicListCreation') - ) - ) { - my $shelf = eval { - Koha::Virtualshelf->new( - { - shelfname => $newvirtualshelf, - category => $category, - owner => $loggedinuser, - } - )->store; - }; +if ($newvirtualshelf) { + if ($loggedinuser > 0 + and ( $category == 1 + or $category == 2 and $loggedinuser > 0 && C4::Context->preference('OpacAllowPublicListCreation') ) + ) { + my $shelf = eval { Koha::Virtualshelf->new( { shelfname => $newvirtualshelf, category => $category, owner => $loggedinuser, } )->store; }; if ( $@ or not $shelf ) { + $errcode = 1; $authorized = 0; - $errcode = 1; - return; - } + } else { + for my $biblionumber (@biblionumbers) { + $shelf->add_biblio( $biblionumber, $loggedinuser ); + } - for my $bib (@biblionumber) { - $shelf->add_biblio( $bib, $loggedinuser ); + #Reload the page where you came from + print $query->header; + print ""; + exit; } - - #Reload the page where you came from - print $query->header; - print ""; } -} - -sub HandleShelfNumber { +} elsif ($shelfnumber) { my $shelfnumber = $query->param('shelfnumber'); - my $shelf = Koha::Virtualshelves->find( $shelfnumber ); - if ( $shelf->can_biblios_be_added( $loggedinuser ) ) { - for my $bib (@biblionumber) { - $shelf->add_biblio( $bib, $loggedinuser ); + my $shelf = Koha::Virtualshelves->find($shelfnumber); + if ( $shelf->can_biblios_be_added($loggedinuser) ) { + for my $biblionumber (@biblionumbers) { + $shelf->add_biblio( $biblionumber, $loggedinuser ); } + #Close this page and return print $query->header; print ""; + exit; } else { - # TODO + $authorized = 0; } -} - -sub HandleSelectedShelf { +} elsif ($selectedshelf) { my $shelfnumber = $query->param('selectedshelf'); - my $shelf = Koha::Virtualshelves->find( $shelfnumber ); - if ( $shelf->can_biblios_be_added( $loggedinuser ) ) { + my $shelf = Koha::Virtualshelves->find($shelfnumber); + if ( $shelf->can_biblios_be_added($loggedinuser) ) { + $template->param( + singleshelf => 1, + shelfnumber => $shelf->shelfnumber, + shelfname => $shelf->shelfname, + ); + } else { + $authorized = 0; + } +} else { + if ( $loggedinuser > 0 ) { + my $private_shelves = Koha::Virtualshelves->search( + { category => 1, + owner => $loggedinuser, + }, + { order_by => 'shelfname' } + ); + my $shelves_shared_with_me = Koha::Virtualshelves->search( + { category => 1, + 'virtualshelfshares.borrowernumber' => $loggedinuser, + -or => { + allow_add => 1, + owner => $loggedinuser, + } + }, + { join => 'virtualshelfshares', } + ); + my $public_shelves = Koha::Virtualshelves->search( + { category => 2, + -or => { + allow_add => 1, + owner => $loggedinuser, + } + }, + { order_by => 'shelfname' } + ); $template->param( - singleshelf => 1, - shelfnumber => $shelf->shelfnumber, - shelfname => $shelf->shelfname, + private_shelves => $private_shelves, + private_shelves_shared_with_me => $shelves_shared_with_me, + public_shelves => $public_shelves, ); } else { - # TODO + $authorized = 0; } } -sub HandleSelect { - return unless $authorized= $loggedinuser>0; - my $private_shelves = Koha::Virtualshelves->search( - { - category => 1, - owner => $loggedinuser, - }, - { order_by => 'shelfname' } - ); - my $shelves_shared_with_me = Koha::Virtualshelves->search( - { - category => 1, - 'virtualshelfshares.borrowernumber' => $loggedinuser, - -or => { - allow_add => 1, - owner => $loggedinuser, - } - }, - { - join => 'virtualshelfshares', - } - ); - my $public_shelves= Koha::Virtualshelves->search( - { - category => 2, - -or => { - allow_add => 1, - owner => $loggedinuser, +if ($authorized) { + for my $biblionumber (@biblionumbers) { + my $data = GetBiblioData($biblionumber); + push( + @biblios, + { biblionumber => $biblionumber, + title => $data->{'title'}, + author => $data->{'author'}, } - }, - { order_by => 'shelfname' } - ); - $template->param ( - private_shelves => $private_shelves, - private_shelves_shared_with_me => $shelves_shared_with_me, - public_shelves => $public_shelves, - ); -} - -sub LoadBib { - for my $bib (@biblionumber) { - my $data = GetBiblioData( $bib ); - push(@biblios, - { biblionumber => $bib, - title => $data->{'title'}, - author => $data->{'author'}, - } ); + ); } $template->param( - multiple => (scalar(@biblios) > 1), - total => scalar @biblios, - biblios => \@biblios, + multiple => ( scalar(@biblios) > 1 ), + total => scalar @biblios, + biblios => \@biblios, ); -} -sub ShowTemplate { - $template->param ( - newshelf => $newshelf||0, - authorized => $authorized, - errcode => $errcode, - OpacAllowPublicListCreation => C4::Context->preference('OpacAllowPublicListCreation'), + $template->param( + newshelf => $newshelf || 0, + OpacAllowPublicListCreation => C4::Context->preference('OpacAllowPublicListCreation'), ); - output_html_with_http_headers $query, $cookie, $template->output; } +$template->param( authorized => $authorized, errcode => $errcode, ); +output_html_with_http_headers $query, $cookie, $template->output; diff --git a/virtualshelves/addbybiblionumber.pl b/virtualshelves/addbybiblionumber.pl index 71270b7..6e058e9 100755 --- a/virtualshelves/addbybiblionumber.pl +++ b/virtualshelves/addbybiblionumber.pl @@ -1,9 +1,7 @@ #!/usr/bin/perl -#script to provide virtual shelf management -# -# # Copyright 2000-2002 Katipo Communications +# Copyright 2016 Koha Development Team # # This file is part of Koha. # @@ -58,8 +56,7 @@ addbybiblionumber.pl =cut -use strict; -use warnings; +use Modern::Perl; use CGI qw ( -utf8 ); use C4::Biblio; @@ -68,20 +65,25 @@ use C4::Auth; use Koha::Virtualshelves; -our $query = new CGI; -our @biblionumber = HandleBiblioPars(); -our $shelfnumber = $query->param('shelfnumber'); -our $newvirtualshelf = $query->param('newvirtualshelf'); -our $newshelf = $query->param('newshelf'); -our $category = $query->param('category'); -our $sortfield = $query->param('sortfield'); +my $query = new CGI; +my $shelfnumber = $query->param('shelfnumber'); +my $newvirtualshelf = $query->param('newvirtualshelf'); +my $newshelf = $query->param('newshelf'); +my $category = $query->param('category'); +my $sortfield = $query->param('sortfield'); my $confirmed = $query->param('confirmed') || 0; -our $authorized = 1; -our $errcode = 0; +my ( $errcode, $authorized ) = ( 0, 1 ); +my @biblionumbers = $query->multi_param('biblionumber'); + +if ( @biblionumbers == 0 && $query->param('biblionumbers') ) { + my $str = $query->param('biblionumbers'); + @biblionumbers = split '/', $str; +} elsif ( @biblionumbers == 1 && $biblionumbers[0] =~ /\// ) { + @biblionumbers = split '/', $biblionumbers[0]; +} -our ( $template, $loggedinuser, $cookie ) = get_template_and_user( - { - template_name => "virtualshelves/addbybiblionumber.tt", +my ( $template, $loggedinuser, $cookie ) = get_template_and_user( + { template_name => "virtualshelves/addbybiblionumber.tt", query => $query, type => "intranet", authnotrequired => 0, @@ -89,155 +91,117 @@ our ( $template, $loggedinuser, $cookie ) = get_template_and_user( } ); -if( $newvirtualshelf) { - HandleNewVirtualShelf(); - exit if $authorized; - ShowTemplate(); #error message -} -elsif($shelfnumber && $confirmed) { - HandleShelfNumber(); - exit if $authorized; - ShowTemplate(); #error message -} -elsif($shelfnumber) { #still needs confirmation - HandleSelectedShelf(); - LoadBib() if $authorized; - ShowTemplate(); -} -else { - HandleSelect(); - LoadBib(); - ShowTemplate(); -} -#end - -sub HandleBiblioPars { - my @bib= $query->multi_param('biblionumber'); - if(@bib==0 && $query->param('biblionumbers')) { - my $str= $query->param('biblionumbers'); - @bib= split '/', $str; - } - elsif(@bib==1 && $bib[0]=~/\//) { - @bib= split '/', $bib[0]; - } - return @bib; -} - -sub HandleNewVirtualShelf { +if ($newvirtualshelf) { my $shelf = eval { Koha::Virtualshelf->new( { shelfname => $newvirtualshelf, - category => $category, + category => $category, sortfield => $sortfield, - owner => $loggedinuser, + owner => $loggedinuser, } )->store; }; if ( $@ or not $shelf ) { - $authorized = 0; $errcode = 1; - return; - } + $authorized = 0; + } else { + + for my $biblionumber (@biblionumbers) { + $shelf->add_biblio( $biblionumber, $loggedinuser ); + } - for my $bib (@biblionumber){ - $shelf->add_biblio( $bib, $loggedinuser ); + #Reload the page where you came from + print $query->header; + print ""; + exit; } - #Reload the page where you came from - print $query->header; - print ""; -} -sub HandleShelfNumber { - my $shelf = Koha::Virtualshelves->find( $shelfnumber ); - if($authorized = $shelf->can_biblios_be_added( $loggedinuser ) ) { - for my $bib (@biblionumber){ - $shelf->add_biblio( $bib, $loggedinuser ); +} elsif ( $shelfnumber && $confirmed ) { + my $shelf = Koha::Virtualshelves->find($shelfnumber); + if ( $shelf->can_biblios_be_added($loggedinuser) ) { + for my $biblionumber (@biblionumbers) { + $shelf->add_biblio( $biblionumber, $loggedinuser ); } + #Close this page and return print $query->header; print ""; + exit; + } else { + $errcode = 2; #no perm + $authorized = 0; } - else { - $errcode=2; #no perm - } -} -sub HandleSelectedShelf { - my $shelf = Koha::Virtualshelves->find( $shelfnumber ); - if($authorized = $shelf->can_biblios_be_added( $loggedinuser ) ) { +} elsif ($shelfnumber) { #still needs confirmation + my $shelf = Koha::Virtualshelves->find($shelfnumber); + if ( $shelf->can_biblios_be_added($loggedinuser) ) { + #confirm adding to specific shelf $template->param( - singleshelf => 1, - shelfnumber => $shelf->shelfnumber, - shelfname => $shelf->shelfname, + singleshelf => 1, + shelfnumber => $shelf->shelfnumber, + shelfname => $shelf->shelfname, ); + } else { + $authorized = 0; + $errcode = 2; #no perm } - else { - $errcode=2; #no perm - } -} -sub HandleSelect { +} else { my $private_shelves = Koha::Virtualshelves->search( - { - category => 1, - owner => $loggedinuser, + { category => 1, + owner => $loggedinuser, }, { order_by => 'shelfname' } ); my $shelves_shared_with_me = Koha::Virtualshelves->search( - { - category => 1, + { category => 1, 'virtualshelfshares.borrowernumber' => $loggedinuser, - -or => { + -or => { allow_add => 1, - owner => $loggedinuser, + owner => $loggedinuser, } }, - { - join => 'virtualshelfshares', - } + { join => 'virtualshelfshares', } ); - my $public_shelves= Koha::Virtualshelves->search( - { - category => 2, - -or => { + my $public_shelves = Koha::Virtualshelves->search( + { category => 2, + -or => { allow_add => 1, - owner => $loggedinuser, + owner => $loggedinuser, } }, { order_by => 'shelfname' } ); - $template->param ( - private_shelves => $private_shelves, + $template->param( + private_shelves => $private_shelves, private_shelves_shared_with_me => $shelves_shared_with_me, - public_shelves => $public_shelves, + public_shelves => $public_shelves, ); -} -sub LoadBib { - my @biblios; - for my $bib (@biblionumber) { - my $data = GetBiblioData($bib); - push(@biblios, - { biblionumber => $bib, - title => $data->{'title'}, - author => $data->{'author'}, - } ); - } - $template->param( - multiple => (scalar(@biblios) > 1), - total => scalar @biblios, - biblios => \@biblios, - ); } -sub ShowTemplate { - $template->param ( - newshelf => $newshelf||0, - authorized => $authorized, - errcode => $errcode, +my @biblios; +for my $biblionumber (@biblionumbers) { + my $data = GetBiblioData($biblionumber); + push( + @biblios, + { biblionumber => $biblionumber, + title => $data->{'title'}, + author => $data->{'author'}, + } ); - output_html_with_http_headers $query, $cookie, $template->output; } +$template->param( + multiple => ( scalar(@biblios) > 1 ), + total => scalar @biblios, + biblios => \@biblios, +); + +$template->param( + newshelf => $newshelf || 0, + authorized => $authorized, + errcode => $errcode, +); +output_html_with_http_headers $query, $cookie, $template->output; -- 1.7.10.4