Bug 14544: Get rid of ShelfPossibleAction
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Mon, 17 Aug 2015 14:57:27 +0000 (15:57 +0100)
committerTomas Cohen Arazi <tomascohen@theke.io>
Thu, 5 Nov 2015 12:58:01 +0000 (09:58 -0300)
Bug 14544: (follow-up) Get rid of ShelfPossibleAction

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
C4/VirtualShelves.pm
opac/opac-addbybiblionumber.pl
opac/opac-downloadshelf.pl
opac/opac-sendshelf.pl
opac/opac-shareshelf.pl

index 73786d1..3e8a686 100644 (file)
@@ -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);
index 43d4489..b20702d 100755 (executable)
@@ -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 "<html><meta http-equiv=\"refresh\" content=\"0\" /><body onload=\"window.opener.location.reload(true);self.close();\"></body></html>";
+    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 "<html><meta http-equiv=\"refresh\" content=\"0\" /><body onload=\"window.opener.location.reload(true);self.close();\"></body></html>";
     }
 }
 
 sub HandleShelfNumber {
-    if($authorized= ShelfPossibleAction($loggedinuser, $shelfnumber, 'add')) {
-    AddBibliosToShelf($shelfnumber,@biblionumber);
-    #Close this page and return
-    print $query->header;
-    print "<html><meta http-equiv=\"refresh\" content=\"0\" /><body onload=\"self.close();\"></body></html>";
+    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 "<html><meta http-equiv=\"refresh\" content=\"0\" /><body onload=\"self.close();\"></body></html>";
+    } 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
     }
 }
 
index 4b932fc..f7e5571 100755 (executable)
@@ -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);
index 07353ac..b0215cc 100755 (executable)
@@ -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();
index a75875c..ceafb33 100755 (executable)
@@ -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