[Rt-commit] rt branch, 4.4/add-watcher-custom-role, repushed

[Jim Brandt]

The branch 4.4/add-watcher-custom-role was deleted and repushed:
       was ec537f98e6dd5ffb00960bd120808977986383ad
       now b9828c28eae8110d191eeea6f8b5f61085a2cd7c

1: 95e7b1bb0 ! 1: 81e18339b Add test showing custom roles can be set without Modify rights
    @@ -1,6 +1,8 @@
     Author: Jim Brandt <jbrandt at bestpractical.com>
     
    -    Add test showing custom roles can be set without ModifyTicket
    +    Add test showing custom roles can be set without Modify rights
    +    
    +    Tests custom role watchers on tickets and queues.
     
     diff --git a/t/ticket/add-watchers.t b/t/ticket/add-watchers.t
     --- a/t/ticket/add-watchers.t
    @@ -51,8 +53,31 @@
      ok(  $user->HasRight( Right => 'Watch',        Object => $queue ), "user can watch queue tickets" );
      ($rv, $msg) = $ticket2->AddWatcher( Type => 'Cc', PrincipalId => $user->PrincipalId );
     @@
    + ok( !$rv, "user can't add self as Cc" );
    + ($rv, $msg) = $queue2->AddWatcher( Type => 'Requestor', PrincipalId => $user->PrincipalId );
    + ok( !$rv, "user can't add self as Requestor" );
    ++($rv, $msg) = $queue2->AddWatcher( Type => $custom_role_type, PrincipalId => $user->PrincipalId );
    ++ok( !$rv, "user can't add self to Custom Role $msg" );
    ++
    + $principal->GrantRight( Right => 'Watch'  , Object => $queue );
    + ok(  $user->HasRight( Right => 'Watch',        Object => $queue ), "user can watch queue queues" );
    + ($rv, $msg) = $queue2->AddWatcher( Type => 'Cc', PrincipalId => $user->PrincipalId );
    +@@
    + ($rv, $msg) = $queue2->AddWatcher( Type => 'Requestor', PrincipalId => $user->PrincipalId );
    + ok(  $rv, "user can add self as Requestor by PrincipalId" );
    + 
    ++$principal->GrantRight( Right => 'ModifyQueueWatchers' , Object => $queue );
    ++ok( $user->HasRight( Right => 'ModifyQueueWatchers', Object => $queue ), "user can modify all queue watchers" );
    ++($rv, $msg) = $queue2->AddWatcher( Type => $custom_role_type, PrincipalId => $user->PrincipalId );
    ++ok( $rv, "user can add self to Custom Role $msg" );
    ++
    + # remove user and try adding with Email address
    + ($rv, $msg) = $queue->DeleteWatcher( Type => 'Cc',        PrincipalId => $user->PrincipalId );
    + ok( $rv, "watcher removed by PrincipalId" );
    +@@
      ($rv, $msg) = $queue2->AddWatcher( Type => 'Requestor', Email => $user->EmailAddress );
      ok(  $rv, "user can add self as Requestor by Email" );
      
    ++
     +done_testing();
     
2: ec537f98e ! 2: b9828c28e Fall through to false on watcher rights check
    @@ -8,11 +8,37 @@
         value was true, which meant that any watcher change that didn't
         match was allowed. All new custom roles won't match the existing
         watcher type checks, so users could by default edit custom roles
    -    even without ModifyTicket.
    +    even without ModifyTicket or ModifyQueueWatchers.
         
         Reverse the logic to return true for know rights and types and
         default to 0 for any unknown cases. At this point, ModifyTicket
    -    is required to modify custom roles.
    +    is required to modify custom roles on tickets and
    +    ModifyQueueWatchers on queues.
    +
    +diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
    +--- a/lib/RT/Queue.pm
    ++++ b/lib/RT/Queue.pm
    +@@
    +     return 1 if $self->CurrentUserHasRight('ModifyQueueWatchers');
    +     # If the watcher isn't the current user then the current user has no right
    +     return 0 unless $self->CurrentUser->PrincipalId == $principal->id;
    +-    # If it's an AdminCc and they don't have 'WatchAsAdminCc', bail
    +-    return 0 if $type eq 'AdminCc' and not $self->CurrentUserHasRight('WatchAsAdminCc');
    +-    # If it's a Requestor or Cc and they don't have 'Watch', bail
    +-    return 0 if ($type eq "Cc" or $type eq 'Requestor')
    +-        and not $self->CurrentUserHasRight('Watch');
    +-    return 1;
    ++    # If it's an AdminCc and they have 'WatchAsAdminCc', they can modify
    ++    return 1 if $type eq 'AdminCc' and $self->CurrentUserHasRight('WatchAsAdminCc');
    ++    # If it's a Requestor or Cc and they have 'Watch', they can modify
    ++    return 1 if ($type eq "Cc" or $type eq 'Requestor')
    ++        and $self->CurrentUserHasRight('Watch');
    ++
    ++    # Unknown type, so default to denied.
    ++    return 0;
    + }
    + 
    + 
     
     diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
     --- a/lib/RT/Ticket.pm