From a8f1a576e2a622d5df318debb5f0606621000cc0 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 17 Aug 2015 15:57:27 +0100 Subject: [PATCH] Bug 14544: Get rid of ShelfPossibleAction Bug 14544: (follow-up) Get rid of ShelfPossibleAction Signed-off-by: Alex Arnaud Signed-off-by: Marcel de Rooy Signed-off-by: Tomas Cohen Arazi --- C4/VirtualShelves.pm | 102 --------------------------------- opac/opac-addbybiblionumber.pl | 66 ++++++++++++--------- opac/opac-downloadshelf.pl | 2 - opac/opac-sendshelf.pl | 3 +- opac/opac-shareshelf.pl | 13 +++-- 5 files changed, 47 insertions(+), 139 deletions(-) diff --git a/C4/VirtualShelves.pm b/C4/VirtualShelves.pm index 73786d1459..3e8a6862ef 100644 --- a/C4/VirtualShelves.pm +++ b/C4/VirtualShelves.pm @@ -41,7 +41,6 @@ BEGIN { @ISA = qw(Exporter); @EXPORT = qw( &GetShelfContents - &ShelfPossibleAction ); @EXPORT_OK = qw( &ShelvesMax @@ -163,107 +162,6 @@ sub GetShelfContents { # or newer, for your version of DBI. } -=head2 ShelfPossibleAction - -ShelfPossibleAction($loggedinuser, $shelfnumber, $action); - -C<$loggedinuser,$shelfnumber,$action> - -$action can be "view", "add", "delete", "manage", "new_public", "new_private". -New additional actions are: invite, acceptshare. -Note that add/delete here refers to adding/deleting entries from the list. Deleting the list itself falls under manage. -new_public and new_private refers to creating a new public or private list. -The distinction between deleting your own entries from the list or entries from -others is made when deleting a content from the shelf. - -Returns 1 if the user can do the $action in the $shelfnumber shelf. -Returns 0 otherwise. -For the actions invite and acceptshare a second errorcode is returned if the -result is false. See opac-shareshelf.pl - -=cut - -sub ShelfPossibleAction { - my ( $user, $shelfnumber, $action ) = @_; - $action= 'view' unless $action; - $user=0 unless $user; - - if($action =~ /^new/) { #no shelfnumber needed - if($action eq 'new_private') { - return $user>0; - } - elsif($action eq 'new_public') { - return $user>0 && C4::Context->preference('OpacAllowPublicListCreation'); - } - return 0; - } - - return 0 unless defined($shelfnumber); - - if ( $user > 0 and $action eq 'delete_shelf' ) { - my $borrower = C4::Members::GetMember( borrowernumber => $user ); - require C4::Auth; - return 1 - if C4::Auth::haspermission( $borrower->{userid}, { lists => 'delete_public_lists' } ); - } - - my $dbh = C4::Context->dbh; - my $query = q{ - SELECT COALESCE(owner,0) AS owner, category, allow_add, allow_delete_own, allow_delete_other, COALESCE(sh.borrowernumber,0) AS borrowernumber - FROM virtualshelves vs - LEFT JOIN virtualshelfshares sh ON sh.shelfnumber=vs.shelfnumber - AND sh.borrowernumber=? - WHERE vs.shelfnumber=? - }; - my $sth = $dbh->prepare($query); - $sth->execute($user, $shelfnumber); - my $shelf= $sth->fetchrow_hashref; - - return 0 unless $shelf && ($shelf->{category}==2 || $shelf->{owner}==$user || ($user && $shelf->{borrowernumber}==$user)); - if($action eq 'view') { - #already handled in the above condition - return 1; - } - elsif($action eq 'add') { - return 0 if $user<=0; #should be logged in - return 1 if $shelf->{allow_add}==1 || $shelf->{owner}==$user; - #owner may always add - } - elsif($action eq 'delete') { - #this answer is just diplomatic: it says that you may be able to delete - #some items from that shelf - #it does not answer the question about a specific biblio - #Koha::Virtualshelf->remove_biblios checks the situation per biblio - return 1 if $user>0 && ($shelf->{allow_delete_own}==1 || $shelf->{allow_delete_other}==1); - } - elsif($action eq 'invite') { - #for sharing you must be the owner and the list must be private - if( $shelf->{category}==1 ) { - return 1 if $shelf->{owner}==$user; - return (0, 4); # code 4: should be owner - } - else { - return (0, 5); # code 5: should be private list - } - } - elsif($action eq 'acceptshare') { - #the key for accepting is checked later in Koha::Virtualshelf->share - #you must not be the owner, list must be private - if( $shelf->{category}==1 ) { - return (0, 8) if $shelf->{owner}==$user; - #code 8: should not be owner - return 1; - } - else { - return (0, 5); # code 5: should be private list - } - } - elsif($action eq 'manage' or $action eq 'delete_shelf') { - return 1 if $user && $shelf->{owner}==$user; - } - return 0; -} - =head2 ShelvesMax $howmany= ShelvesMax($context); diff --git a/opac/opac-addbybiblionumber.pl b/opac/opac-addbybiblionumber.pl index 43d448919a..b20702d156 100755 --- a/opac/opac-addbybiblionumber.pl +++ b/opac/opac-addbybiblionumber.pl @@ -87,47 +87,57 @@ sub AddBibliosToShelf { } sub HandleNewVirtualShelf { - if($authorized= ShelfPossibleAction($loggedinuser, undef, $category==1? 'new_private': 'new_public')) { - my $shelf = eval { - Koha::Virtualshelf->new( - { - shelfname => $newvirtualshelf, - category => $category, - owner => $loggedinuser, - } - ); - }; - if ( $@ or not $shelf ) { - $authorized=0; - $errcode=1; - return; - } - AddBibliosToShelf($shelfnumber, @biblionumber); - #Reload the page where you came from - print $query->header; - print ""; + 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, + } + ); + }; + if ( $@ or not $shelf ) { + $authorized = 0; + $errcode = 1; + return; + } + AddBibliosToShelf($shelfnumber, @biblionumber); + #Reload the page where you came from + print $query->header; + print ""; } } sub HandleShelfNumber { - if($authorized= ShelfPossibleAction($loggedinuser, $shelfnumber, 'add')) { - AddBibliosToShelf($shelfnumber,@biblionumber); - #Close this page and return - print $query->header; - print ""; + my $shelfnumber = $query->param('shelfnumber'); + my $shelf = Koha::Virtualshelves->find( $shelfnumber ); + if ( $shelf->can_biblios_be_added( $loggedinuser ) ) { + AddBibliosToShelf($shelfnumber,@biblionumber); + #Close this page and return + print $query->header; + print ""; + } else { + # TODO } } sub HandleSelectedShelf { - if($authorized= ShelfPossibleAction( $loggedinuser, $selectedshelf, 'add')){ - #adding to specific shelf - my $shelfnumber = $query->param('selectedshelf'); - my $shelf = Koha::Virtualshelves->find( $shelfnumber ); + my $shelfnumber = $query->param('selectedshelf'); + my $shelf = Koha::Virtualshelves->find( $shelfnumber ); + if ( $shelf->can_biblios_be_added( $loggedinuser ) ) { $template->param( singleshelf => 1, shelfnumber => $shelf->shelfnumber, shelfname => $shelf->shelfname, ); + } else { + # TODO } } diff --git a/opac/opac-downloadshelf.pl b/opac/opac-downloadshelf.pl index 4b932fca9b..f7e55716f5 100755 --- a/opac/opac-downloadshelf.pl +++ b/opac/opac-downloadshelf.pl @@ -107,8 +107,6 @@ if ( $shelf and $shelf->can_be_viewed( $borrowernumber ) ) { } else { - my $shelf = Koha::Virtualshelves->find( $shelfid ); - # if modal context is passed set a variable so that page markup can be different if($context eq "modal"){ $template->param(modal => 1); diff --git a/opac/opac-sendshelf.pl b/opac/opac-sendshelf.pl index 07353ac45d..b0215cc22d 100755 --- a/opac/opac-sendshelf.pl +++ b/opac/opac-sendshelf.pl @@ -52,7 +52,8 @@ my $email = $query->param('email'); my $dbh = C4::Context->dbh; -if ( ShelfPossibleAction( (defined($borrowernumber) ? $borrowernumber : -1), $shelfid, 'view' ) ) { +my $shelf = Koha::Virtualshelves->find( $shelfid ); +if ( $shelf->can_be_viewed( $borrowernumber ) ) { if ( $email ) { my $message = Koha::Email->new(); diff --git a/opac/opac-shareshelf.pl b/opac/opac-shareshelf.pl index a75875c8a7..ceafb334c8 100755 --- a/opac/opac-shareshelf.pl +++ b/opac/opac-shareshelf.pl @@ -115,13 +115,14 @@ sub confirm_invite { sub show_accept { my ($param) = @_; - my @rv = ShelfPossibleAction( $param->{loggedinuser}, - $param->{shelfnumber}, 'acceptshare' ); - $param->{errcode} = $rv[1] if !$rv[0]; - return if $param->{errcode}; + my $shelfnumber = $param->{shelfnumber}; + my $shelf = Koha::Virtualshelves->find( $shelfnumber ); - #errorcode 5: should be private list - #errorcode 8: should not be owner + # The key for accepting is checked later in Koha::Virtualshelf->share + # You must not be the owner and the list must be private + if ( $shelf->category == 2 or $shelf->owner == $param->{loggedinuser} ) { + return; + } # We could have used ->find with the share id, but we don't want to change # the url sent to the patron -- 2.39.5