Bugzilla – Attachment 51484 Details for
Bug 16519
Do not use global variables in [opac-]addbybiblionumbers.pl
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 16519: Replace 'our' with 'my' in [opac-]addbybiblionumbers.pl
Bug-16519-Replace-our-with-my-in-opac-addbybiblion.patch (text/plain), 19.29 KB, created by
Jonathan Druart
on 2016-05-13 20:02:23 UTC
(
hide
)
Description:
Bug 16519: Replace 'our' with 'my' in [opac-]addbybiblionumbers.pl
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2016-05-13 20:02:23 UTC
Size:
19.29 KB
patch
obsolete
>From 8943b0cc9f55e5e3cecde96a4e27741e9a2ca562 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Fri, 13 May 2016 20:57:33 +0100 >Subject: [PATCH] Bug 16519: Replace 'our' with 'my' in > [opac-]addbybiblionumbers.pl > >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 >--- > 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..8903151 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 <http://www.gnu.org/licenses>. > >-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 "<html><meta http-equiv=\"refresh\" content=\"0\" /><body onload=\"window.opener.location.reload(true);self.close();\"></body></html>"; >+ exit; > } >- >- #Reload the page where you came from >- print $query->header; >- print "<html><meta http-equiv=\"refresh\" content=\"0\" /><body onload=\"window.opener.location.reload(true);self.close();\"></body></html>"; > } >-} >- >-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 "<html><meta http-equiv=\"refresh\" content=\"0\" /><body onload=\"self.close();\"></body></html>"; >+ 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($bib); >+ 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 "<html><meta http-equiv=\"refresh\" content=\"0\" /><body onload=\"window.opener.location.reload(true);self.close();\"></body></html>"; >+ exit; > } >- #Reload the page where you came from >- print $query->header; >- print "<html><meta http-equiv=\"refresh\" content=\"0\" /><body onload=\"window.opener.location.reload(true);self.close();\"></body></html>"; >-} > >-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 "<html><meta http-equiv=\"refresh\" content=\"0\" /><body onload=\"self.close();\"></body></html>"; >+ 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; >-- >2.7.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 16519
:
51484
|
51796
|
51903
|
55662