Bug 13417: Limit the permission to delete
authorJonathan Druart <jonathan.druart@biblibre.com>
Wed, 17 Dec 2014 15:27:11 +0000 (16:27 +0100)
committerTomas Cohen Arazi <tomascohen@gmail.com>
Fri, 13 Feb 2015 16:52:17 +0000 (13:52 -0300)
It's preferable to limit the permission to delete shelves.

Apply both patches before testing, then follow this test plan

Currently a public list can only be deleted by its owner.
This means lists can exist infinitely.
This will introduce a new permission for list. With this permission, a
staff member will be allow to delete any public lists.

Test plan:
1/ Add the manage_shelves permission to a patron.
2/ Login with this patron
3/ Go on the public list view
4/ You should be able to delete all public lists

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
C4/VirtualShelves.pm
C4/VirtualShelves/Page.pm
installer/data/mysql/en/mandatory/userpermissions.sql
installer/data/mysql/updatedatabase.pl
koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt

index f461d3c..8fd003a 100644 (file)
@@ -463,10 +463,10 @@ sub ShelfPossibleAction {
 
     return 0 unless defined($shelfnumber);
 
-    if ( $user > 0 ) {
+    if ( $user > 0 and $action eq 'delete_shelf' ) {
         my $borrower = C4::Members::GetMember( borrowernumber => $user );
         return 1
-            if C4::Auth::haspermission( $borrower->{userid}, { shelves => 'manage_shelves' } );
+            if C4::Auth::haspermission( $borrower->{userid}, { shelves => 'delete_shelves' } );
     }
 
     my $dbh = C4::Context->dbh;
@@ -520,7 +520,7 @@ sub ShelfPossibleAction {
             return (0, 5); # code 5: should be private list
         }
     }
-    elsif($action eq 'manage') {
+    elsif($action eq 'manage' or $action eq 'delete_shelf') {
         return 1 if $user && $shelf->{owner}==$user;
     }
     return 0;
index a5b3164..6117ea9 100644 (file)
@@ -310,6 +310,7 @@ sub shelfpage {
                 $showadd = 1;
                 my $i = 0;
                 my $manageshelf = ShelfPossibleAction( $loggedinuser, $shelfnumber, 'manage' );
+                my $can_delete_shelf = ShelfPossibleAction( $loggedinuser, $shelfnumber, 'delete_shelf' );
                 $template->param(
                     shelfname           => $shelfname,
                     shelfnumber         => $shelfnumber,
@@ -318,6 +319,7 @@ sub shelfpage {
                     manageshelf         => $manageshelf,
                     allowremovingitems  => ShelfPossibleAction( $loggedinuser, $shelfnumber, 'delete'),
                     allowaddingitem     => ShelfPossibleAction( $loggedinuser, $shelfnumber, 'add'),
+                    allowdeletingshelf  => $can_delete_shelf,
                     "category$category" => 1,
                     category            => $category,
                     itemsloop           => $items,
@@ -374,8 +376,10 @@ sub shelfpage {
                     $stay=0;
                     next;
                 }
-                #
-                unless ( ShelfPossibleAction( $loggedinuser, $number, 'manage' ) ) {
+
+                my $can_manage = ShelfPossibleAction( $loggedinuser, $number, 'manage' );
+                my $can_delete = ShelfPossibleAction( $loggedinuser, $number, 'delete_shelf' );
+                unless ( $can_manage or $can_delete ) {
                     push( @paramsloop, { nopermission => $shelfnumber } );
                     last;
                 }
@@ -428,8 +432,10 @@ sub shelfpage {
         my $category  = $shelflist->{$element}->{'category'};
         my $owner     = $shelflist->{$element}->{'owner'}||0;
         my $canmanage = ShelfPossibleAction( $loggedinuser, $element, 'manage' );
+        my $candelete = ShelfPossibleAction( $loggedinuser, $element, 'delete_shelf' );
         $shelflist->{$element}->{"viewcategory$category"} = 1;
         $shelflist->{$element}->{manageshelf} = $canmanage;
+        $shelflist->{$element}->{allowdeletingshelf} = $candelete;
         if($canmanage || ($loggedinuser && $owner==$loggedinuser)) {
             $shelflist->{$element}->{'mine'} = 1;
         }
index 12768b7..a5f574f 100644 (file)
@@ -72,5 +72,5 @@ INSERT INTO permissions (module_bit, code, description) VALUES
    (19, 'tool', 'Use tool plugins'),
    (19, 'report', 'Use report plugins'),
    (19, 'configure', 'Configure plugins'),
-   (20, 'manage_shelves', 'Manage shelves')
+   (20, 'delete_shelves', 'Delete shelves')
 ;
index b4d1953..bdb6efa 100755 (executable)
@@ -9767,9 +9767,9 @@ if ( CheckVersion($DBversion) ) {
     |);
     $dbh->do(q|
         INSERT INTO permissions (module_bit, code, description) VALUES
-        (20, 'manage_shelves', 'Manage shelves')
+        (20, 'delete_shelves', 'Delete shelves')
     |);
-    print "Upgrade to $DBversion done (Bug 13417: Add permission to manage shelves)\n";
+    print "Upgrade to $DBversion done (Bug 13417: Add permission to delete shelves)\n";
     SetVersion ($DBversion);
 }
 
index 97e47a7..bba00b3 100644 (file)
@@ -600,12 +600,14 @@ function placeHold () {
                        [% IF ( shelvesloo.viewcategory2 ) %]Public[% END %]
                </td>
         <td>
-            [% IF ( shelvesloo.manageshelf ) %]
+            [% IF shelvesloo.manageshelf %]
                                <form action="shelves.pl" method="get">
                                        <input type="hidden" name="shelfnumber" value="[% shelvesloo.shelf %]" />
                                        <input type="hidden" name="op" value="modif" />
                                        <input type="submit" class="editshelf" value="Edit" />
                                </form>
+            [% END %]
+            [% IF shelvesloo.manageshelf OR shelvesloo.allowdeletingshelf %]
                                <form action="shelves.pl" method="post">
                                        <input type="hidden" name="shelfoff" value="[% shelfoff %]" />
                                        <input type="hidden" name="shelves" value="1" />