[Rt-commit] rt branch, rightsmatrix, updated. rt-3.8.8-603-gb9634f3

[Thomas Sibley]

The branch, rightsmatrix has been updated
       via  b9634f3074184c00c22301b9ba720877cc5b02e7 (commit)
      from  c25dd8c7ee5b5c142c16998b3c535ce0aac4dada (commit)

Summary of changes:
 lib/RT/Interface/Web.pm              |   59 ++++++++++++++++++++++++++--------
 share/html/Admin/Elements/EditRights |    8 +++--
 2 files changed, 50 insertions(+), 17 deletions(-)

- Log -----------------------------------------------------------------
commit b9634f3074184c00c22301b9ba720877cc5b02e7
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Mon Aug 30 11:59:33 2010 -0400

    ProcessACLChanges now expects values from a series of checkboxes
    
    This breaks the old SelectRights element, but makes it work for the new
    rights editor.
    
    It now expects form inputs with names like
    SetRights-PrincipalId-ObjType-ObjId instead of Grant/RevokeRight.  Each
    input should be an array listing the rights the principal should have,
    and ProcessACLChanges will modify the current rights to match.
    Additionally, the previously unused CheckACL input listing
    PrincipalId-ObjType-ObjId is now used to catch cases when all the rights
    are removed from a principal and no SetRights input is submitted.

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 90e3f3f..5515211 100755
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -1367,25 +1367,30 @@ sub ParseDateToISO {
 
 sub ProcessACLChanges {
     my $ARGSref = shift;
+    my (%state, @results);
 
     #XXX: why don't we get ARGSref like in other Process* subs?
 
-    my @results;
+    my $CheckACL = $ARGSref->{'CheckACL'};
+    my @check = grep { defined } (ref $CheckACL eq 'ARRAY' ? @$CheckACL : $CheckACL);
 
+    # Build our rights state for each Principal-Object tuple
     foreach my $arg ( keys %$ARGSref ) {
-        next unless ( $arg =~ /^(GrantRight|RevokeRight)-(\d+)-(.+?)-(\d+)$/ );
-
-        my ( $method, $principal_id, $object_type, $object_id ) = ( $1, $2, $3, $4 );
+        next unless $arg =~ /^SetRights-(\d+-.+?-\d+)$/;
 
-        my @rights;
-        if ( UNIVERSAL::isa( $ARGSref->{$arg}, 'ARRAY' ) ) {
-            @rights = @{ $ARGSref->{$arg} };
-        } else {
-            @rights = $ARGSref->{$arg};
-        }
-        @rights = grep $_, @rights;
+        my $tuple  = $1;
+        my $value  = $ARGSref->{$arg};
+        my @rights = grep { $_ } (ref $value eq 'ARRAY' ? @$value : $value);
         next unless @rights;
 
+        $state{$tuple} = { map { $_ => 1 } @rights };
+    }
+
+    foreach my $tuple (@check) {
+        next unless $tuple =~ /^(\d+)-(.+?)-(\d+)$/;
+
+        my ( $principal_id, $object_type, $object_id ) = ( $1, $2, $3 );
+
         my $principal = RT::Principal->new( $session{'CurrentUser'} );
         $principal->Load($principal_id);
 
@@ -1405,9 +1410,35 @@ sub ProcessACLChanges {
             next;
         }
 
-        foreach my $right (@rights) {
-            my ( $val, $msg ) = $principal->$method( Object => $obj, Right => $right );
-            push( @results, $msg );
+        my $acls = RT::ACL->new($session{'CurrentUser'});
+        $acls->LimitToObject( $obj );
+        $acls->LimitToPrincipal( Id => $principal_id );
+
+        while ( my $ace = $acls->Next ) {
+            my $right = $ace->RightName;
+
+            # Has right and should have right
+            next if delete $state{$tuple}->{$right};
+
+            # Has right and shouldn't have right
+            my ($val, $msg) = $principal->RevokeRight( Object => $obj, Right => $right );
+            push @results, $msg;
+        }
+
+        # For everything left, they don't have the right but they should
+        for my $right (keys %{ $state{$tuple} || {} }) {
+            delete $state{$tuple}->{$right};
+            my ($val, $msg) = $principal->GrantRight( Object => $obj, Right => $right );
+            push @results, $msg;
+        }
+
+        # Check our state for leftovers
+        if ( keys %{ $state{$tuple} || {} } ) {
+            my $missed = join '|', %{$state{$tuple} || {}};
+            $RT::Logger->warn(
+               "Uh-oh, it looks like we somehow missed a right in "
+              ."ProcessACLChanges.  Here's what was leftover: $missed"
+            );
         }
     }
 
diff --git a/share/html/Admin/Elements/EditRights b/share/html/Admin/Elements/EditRights
index 25d5586..21d0434 100644
--- a/share/html/Admin/Elements/EditRights
+++ b/share/html/Admin/Elements/EditRights
@@ -71,22 +71,24 @@ for my $category (@$Principals) {
         my $id = "acl-$name-" . $obj->PrincipalId;
         $id =~ s/[^a-zA-Z0-9\-]/_/g;
 </%perl>
+
   <div id="<% $id %>">
     <h3>Rights for <% $loc ? loc($display) : $display %></h3>
     <ul class="rights-list">
 % for my $right (keys %available_rights) {
       <li>
         <input type="checkbox" class="checkbox"
-               name="GrantRight-<% $acldesc %>"
-               id="GrantRight-<% $acldesc %>-<% $right %>"
+               name="SetRights-<% $acldesc %>"
+               id="SetRights-<% $acldesc %>-<% $right %>"
                value="<% $right %>"
                <% $current_rights{$obj->PrincipalId}->{$right} ? 'checked' : '' %> />
-        <label for="GrantRight-<% $acldesc %>-<% $right %>">
+        <label for="SetRights-<% $acldesc %>-<% $right %>">
           <% loc($available_rights{$right}) %>
         </label>
       </li>
 % }
     </ul>
+    <input type="hidden" name="CheckACL" value="<% $acldesc %>" />
   </div>
 <%perl>
     }

-----------------------------------------------------------------------