[Rt-commit] rt branch, 4.2/role-roles, created. rt-4.1.5-241-g17af14d

Thomas Sibley trs at bestpractical.com
Thu Jan 10 18:49:06 EST 2013


The branch, 4.2/role-roles has been created
        at  17af14d2809bfae73cff4e92384f5a0fdd73b07d (commit)

- Log -----------------------------------------------------------------
commit 6dff44dcec9470fe1d9fb209755ff58d9f625e86
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jan 7 20:51:37 2013 -0500

    Move role-related methods into a role

diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index 99829c9..20bf27d 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -70,7 +70,7 @@ use warnings;
 use base 'RT::Record';
 
 use Role::Basic 'with';
-with "RT::Role::Record::Lifecycle";
+with "RT::Role::Record::Lifecycle", "RT::Role::Record::Roles";
 
 sub Table {'Queues'}
 
diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index 4132ae6..9188f4d 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -2166,442 +2166,6 @@ sub WikiBase {
     return RT->Config->Get('WebPath'). "/index.html?q=";
 }
 
-=head2 RegisterRole
-
-Registers an RT role which applies to this class for role-based access control.
-Arguments:
-
-=over 4
-
-=item Name
-
-Required.  The role name (i.e. Requestor, Owner, AdminCc, etc).
-
-=item EquivClasses
-
-Optional.  Array ref of classes through which this role percolates up to
-L<RT::System>.  You can think of this list as:
-
-    map { ref } $record_object->ACLEquivalenceObjects;
-
-You should not include L<RT::System> itself in this list.
-
-Simply calls RegisterRole on each equivalent class.
-
-=back
-
-=cut
-
-sub RegisterRole {
-    my $self  = shift;
-    my $class = ref($self) || $self;
-    my %role  = (
-        Name            => undef,
-        EquivClasses    => [],
-        @_
-    );
-    return unless $role{Name};
-
-    # Keep track of the class this role came from originally
-    $role{ Class } ||= $class;
-
-    # Some groups are limited to a single user
-    $role{ Single } = 1 if $role{Column};
-
-    # Stash the role on ourself
-    $class->_ROLES->{ $role{Name} } = \%role;
-
-    # Register it with any equivalent classes...
-    my $equiv = delete $role{EquivClasses} || [];
-
-    # ... and globally unless we ARE global
-    unless ($class eq "RT::System") {
-        require RT::System;
-        push @$equiv, "RT::System";
-    }
-
-    $_->RegisterRole(%role) for @$equiv;
-
-    # XXX TODO: Register which classes have roles on them somewhere?
-
-    return 1;
-}
-
-=head2 Roles
-
-Returns a list of role names registered for this class.
-
-=cut
-
-sub Roles { sort { $a cmp $b } keys %{ shift->_ROLES } }
-
-{
-    my %ROLES;
-    sub _ROLES {
-        my $class = ref($_[0]) || $_[0];
-        return $ROLES{$class} ||= {};
-    }
-}
-
-=head2 HasRole
-
-Returns true if the name provided is a registered role for this class.
-Otherwise returns false.
-
-=cut
-
-sub HasRole {
-    my $self = shift;
-    my $type = shift;
-    return scalar grep { $type eq $_ } $self->Roles;
-}
-
-=head2 RoleGroup
-
-Expects a role name as the first parameter which is used to load the
-L<RT::Group> for the specified role on this record.  Returns an unloaded
-L<RT::Group> object on failure.
-
-=cut
-
-sub RoleGroup {
-    my $self  = shift;
-    my $type  = shift;
-    my $group = RT::Group->new( $self->CurrentUser );
-
-    if ($self->HasRole($type)) {
-        $group->LoadRoleGroup(
-            Object  => $self,
-            Type    => $type,
-        );
-    }
-    return $group;
-}
-
-=head2 AddRoleMember
-
-Adds the described L<RT::Principal> to the specified role group for this record.
-
-Takes a set of key-value pairs:
-
-=over 4
-
-=item PrincipalId
-
-Optional.  The ID of the L<RT::Principal> object to add.
-
-=item User
-
-Optional.  The Name or EmailAddress of an L<RT::User> to use as the
-principal.  If an email address is given, but a user matching it cannot
-be found, a new user will be created.
-
-=item Group
-
-Optional.  The Name of an L<RT::Group> to use as the principal.
-
-=item Type
-
-Required.  One of the valid roles for this record, as returned by L</Roles>.
-
-=item ACL
-
-Optional.  A subroutine reference which will be passed the role type and
-principal being added.  If it returns false, the method will fail with a
-status of "Permission denied".
-
-=back
-
-One, and only one, of I<PrincipalId>, I<User>, or I<Group> is required.
-
-Returns a tuple of (principal object which was added, message).
-
-=cut
-
-sub AddRoleMember {
-    my $self = shift;
-    my %args = (@_);
-
-    return (0, $self->loc("One, and only one, of PrincipalId/User/Group is required"))
-        if 1 != grep { $_ } @args{qw/PrincipalId User Group/};
-
-    my $type = delete $args{Type};
-    return (0, $self->loc("No valid Type specified"))
-        unless $type and $self->HasRole($type);
-
-    if ($args{PrincipalId}) {
-        # Check the PrincipalId for loops
-        my $principal = RT::Principal->new( $self->CurrentUser );
-        $principal->Load($args{'PrincipalId'});
-        if ( $principal->id and $principal->IsUser and my $email = $principal->Object->EmailAddress ) {
-            return (0, $self->loc("[_1] is an address RT receives mail at. Adding it as a '[_2]' would create a mail loop",
-                                  $email, $self->loc($type)))
-                if RT::EmailParser->IsRTAddress( $email );
-        }
-    } else {
-        if ($args{User}) {
-            my $name = delete $args{User};
-            # Sanity check the address
-            return (0, $self->loc("[_1] is an address RT receives mail at. Adding it as a '[_2]' would create a mail loop",
-                                  $name, $self->loc($type) ))
-                if RT::EmailParser->IsRTAddress( $name );
-
-            # Create as the SystemUser, not the current user
-            my $user = RT::User->new(RT->SystemUser);
-            my ($pid, $msg) = $user->LoadOrCreateByEmail( $name );
-            unless ($pid) {
-                # If we can't find this watcher, we need to bail.
-                $RT::Logger->error("Could not load or create a user '$name' to add as a watcher: $msg");
-                return (0, $self->loc("Could not find or create user '$name'"));
-            }
-            $args{PrincipalId} = $pid;
-        }
-        elsif ($args{Group}) {
-            my $name = delete $args{Group};
-            my $group = RT::Group->new( $self->CurrentUser );
-            $group->LoadUserDefinedGroup($name);
-            unless ($group->id) {
-                $RT::Logger->error("Could not load group '$name' to add as a watcher");
-                return (0, $self->loc("Could not find group '$name'"));
-            }
-            $args{PrincipalId} = $group->PrincipalObj->id;
-        }
-    }
-
-    my $principal = RT::Principal->new( $self->CurrentUser );
-    $principal->Load( $args{PrincipalId} );
-
-    my $acl = delete $args{ACL};
-    return (0, $self->loc("Permission denied"))
-        if $acl and not $acl->($type => $principal);
-
-    my $group = $self->RoleGroup( $type );
-    return (0, $self->loc("Role group '$type' not found"))
-        unless $group->id;
-
-    return (0, $self->loc('[_1] is already a [_2]',
-                          $principal->Object->Name, $self->loc($type)) )
-            if $group->HasMember( $principal );
-
-    my ( $ok, $msg ) = $group->_AddMember( %args );
-    unless ($ok) {
-        $RT::Logger->error("Failed to add $args{PrincipalId} as a member of group ".$group->Id.": ".$msg);
-
-        return ( 0, $self->loc('Could not make [_1] a [_2]',
-                    $principal->Object->Name, $self->loc($type)) );
-    }
-
-    unless ($args{Silent}) {
-        $self->_NewTransaction(
-            Type     => 'AddWatcher', # use "watcher" for history's sake
-            NewValue => $args{PrincipalId},
-            Field    => $type,
-        );
-    }
-
-    return ($principal, $msg);
-}
-
-=head2 DeleteRoleMember
-
-Removes the specified L<RT::Principal> from the specified role group for this
-record.
-
-Takes a set of key-value pairs:
-
-=over 4
-
-=item PrincipalId
-
-Optional.  The ID of the L<RT::Principal> object to remove.
-
-=item User
-
-Optional.  The Name or EmailAddress of an L<RT::User> to use as the
-principal
-
-=item Type
-
-Required.  One of the valid roles for this record, as returned by L</Roles>.
-
-=back
-
-One, and only one, of I<PrincipalId> or I<User> is required.
-
-Returns a tuple of (principal object that was removed, message).
-
-=cut
-
-sub DeleteRoleMember {
-    my $self = shift;
-    my %args = (@_);
-
-    return (0, $self->loc("No valid Type specified"))
-        unless $args{Type} and $self->HasRole($args{Type});
-
-    if ($args{User}) {
-        my $user = RT::User->new( $self->CurrentUser );
-        $user->LoadByEmail( $args{User} );
-        $user->Load( $args{User} ) unless $user->id;
-        return (0, $self->loc("Could not load user '$args{User}'") )
-            unless $user->id;
-        $args{PrincipalId} = $user->PrincipalId;
-    }
-
-    return (0, $self->loc("No valid PrincipalId"))
-        unless $args{PrincipalId};
-
-    my $principal = RT::Principal->new( $self->CurrentUser );
-    $principal->Load( $args{PrincipalId} );
-
-    my $acl = delete $args{ACL};
-    return (0, $self->loc("Permission denied"))
-        if $acl and not $acl->($principal);
-
-    my $group = $self->RoleGroup( $args{Type} );
-    return (0, $self->loc("Role group '$args{Type}' not found"))
-        unless $group->id;
-
-    return ( 0, $self->loc( '[_1] is not a [_2]',
-                            $principal->Object->Name, $self->loc($args{Type}) ) )
-        unless $group->HasMember($principal);
-
-    my ($ok, $msg) = $group->_DeleteMember($args{PrincipalId});
-    unless ($ok) {
-        $RT::Logger->error("Failed to remove $args{PrincipalId} as a member of group ".$group->Id.": ".$msg);
-
-        return ( 0, $self->loc('Could not remove [_1] as a [_2]',
-                    $principal->Object->Name, $self->loc($args{Type})) );
-    }
-
-    unless ($args{Silent}) {
-        $self->_NewTransaction(
-            Type     => 'DelWatcher', # use "watcher" for history's sake
-            OldValue => $args{PrincipalId},
-            Field    => $args{Type},
-        );
-    }
-    return ($principal, $msg);
-}
-
-sub _ResolveRoles {
-    my $self = shift;
-    my ($roles, %args) = (@_);
-
-    my @errors;
-    for my $role ($self->Roles) {
-        if ($self->_ROLES->{$role}{Single}) {
-            # Default to nobody if unspecified
-            my $value = $args{$role} || RT->Nobody;
-            if (Scalar::Util::blessed($value) and $value->isa("RT::User")) {
-                # Accept a user; it may not be loaded, which we catch below
-                $roles->{$role} = $value->PrincipalObj;
-            } else {
-                # Try loading by id, name, then email.  If all fail, catch that below
-                my $user = RT::User->new( $self->CurrentUser );
-                $user->Load( $value );
-                # XXX: LoadOrCreateByEmail ?
-                $user->LoadByEmail( $value ) unless $user->id;
-                $roles->{$role} = $user->PrincipalObj;
-            }
-            unless ($roles->{$role}->id) {
-                push @errors, $self->loc("Invalid value for [_1]",loc($role));
-                $roles->{$role} = RT->Nobody->PrincipalObj unless $roles->{$role}->id;
-            }
-            # For consistency, we always return an arrayref
-            $roles->{$role} = [ $roles->{$role} ];
-        } else {
-            $roles->{$role} = [];
-            my @values = ref $args{ $role } ? @{ $args{$role} } : ($args{$role});
-            for my $value (grep {defined} @values) {
-                if ( $value =~ /^\d+$/ ) {
-                    # This implicitly allows groups, if passed by id.
-                    my $principal = RT::Principal->new( $self->CurrentUser );
-                    my ($ok, $msg) = $principal->Load( $value );
-                    if ($ok) {
-                        push @{ $roles->{$role} }, $principal;
-                    } else {
-                        push @errors,
-                            $self->loc("Couldn't load principal: [_1]", $msg);
-                    }
-                } else {
-                    my @addresses = RT::EmailParser->ParseEmailAddress( $value );
-                    for my $address ( @addresses ) {
-                        my $user = RT::User->new( RT->SystemUser );
-                        my ($id, $msg) = $user->LoadOrCreateByEmail( $address );
-                        if ( $id ) {
-                            # Load it back as us, not as the system
-                            # user, to be completely safe.
-                            $user = RT::User->new( $self->CurrentUser );
-                            $user->Load( $id );
-                            push @{ $roles->{$role} }, $user->PrincipalObj;
-                        } else {
-                            push @errors,
-                                $self->loc("Couldn't load or create user: [_1]", $msg);
-                        }
-                    }
-                }
-            }
-        }
-    }
-    return (@errors);
-}
-
-sub _CreateRoleGroups {
-    my $self = shift;
-    my %args = (@_);
-    for my $type ($self->Roles) {
-        my $type_obj = RT::Group->new($self->CurrentUser);
-        my ($id, $msg) = $type_obj->CreateRoleGroup(
-            Type    => $type,
-            Object  => $self,
-            %args,
-        );
-        unless ($id) {
-            $RT::Logger->error("Couldn't create a role group of type '$type' for ".ref($self)." ".
-                                   $self->Id.": ".$msg);
-            return(undef);
-        }
-    }
-    return(1);
-}
-
-sub _AddRolesOnCreate {
-    my $self = shift;
-    my ($roles, %acls) = @_;
-
-    my @errors;
-    {
-        my $changed = 0;
-
-        for my $role (keys %{$roles}) {
-            my $group = $self->RoleGroup($role);
-            my @left;
-            for my $principal (@{$roles->{$role}}) {
-                if ($acls{$role}->($principal)) {
-                    next if $group->HasMember($principal);
-                    my ($ok, $msg) = $group->_AddMember(
-                        PrincipalId       => $principal->id,
-                        InsideTransaction => 1,
-                        RecordTransaction => 0,
-                        Object            => $self,
-                    );
-                    push @errors, $self->loc("Couldn't set [_1] watcher: [_2]", $role, $msg)
-                        unless $ok;
-                    $changed++;
-                } else {
-                    push @left, $principal;
-                }
-            }
-            $roles->{$role} = [ @left ];
-        }
-
-        redo if $changed;
-    }
-
-    return @errors;
-}
-
 RT::Base->_ImportOverlays();
 
 1;
diff --git a/lib/RT/Role/Record.pm b/lib/RT/Role/Record.pm
index 1826023..f9eb85b 100644
--- a/lib/RT/Role/Record.pm
+++ b/lib/RT/Role/Record.pm
@@ -72,6 +72,7 @@ requires $_ for qw(
 
     _Set
     _Accessible
+    _NewTransaction
 );
 
 1;
diff --git a/lib/RT/Role/Record/Roles.pm b/lib/RT/Role/Record/Roles.pm
new file mode 100644
index 0000000..903a226
--- /dev/null
+++ b/lib/RT/Role/Record/Roles.pm
@@ -0,0 +1,513 @@
+# BEGIN BPS TAGGED BLOCK {{{
+#
+# COPYRIGHT:
+#
+# This software is Copyright (c) 1996-2012 Best Practical Solutions, LLC
+#                                          <sales at bestpractical.com>
+#
+# (Except where explicitly superseded by other copyright notices)
+#
+#
+# LICENSE:
+#
+# This work is made available to you under the terms of Version 2 of
+# the GNU General Public License. A copy of that license should have
+# been provided with this software, but in any event can be snarfed
+# from www.gnu.org.
+#
+# This work is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 or visit their web page on the internet at
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+#
+#
+# CONTRIBUTION SUBMISSION POLICY:
+#
+# (The following paragraph is not intended to limit the rights granted
+# to you to modify and distribute this software under the terms of
+# the GNU General Public License and is only of importance to you if
+# you choose to contribute your changes and enhancements to the
+# community by submitting them to Best Practical Solutions, LLC.)
+#
+# By intentionally submitting any modifications, corrections or
+# derivatives to this work, or any other work intended for use with
+# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+# you are the copyright holder for those contributions and you grant
+# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+# royalty-free, perpetual, license to use, copy, create derivative
+# works based on those contributions, and sublicense and distribute
+# those contributions and any derivatives thereof.
+#
+# END BPS TAGGED BLOCK }}}
+
+use strict;
+use warnings;
+
+package RT::Role::Record::Roles;
+use Role::Basic;
+use Scalar::Util qw(blessed);
+
+=head1 NAME
+
+RT::Role::Record::Roles - Common methods for records which "watchers" or "roles"
+
+=head1 REQUIRES
+
+=head2 L<RT::Role::Record>
+
+=cut
+
+with 'RT::Role::Record';
+
+require RT::System;
+require RT::Principal;
+require RT::Group;
+require RT::User;
+
+require RT::EmailParser;
+
+=head1 PROVIDES
+
+=head2 RegisterRole
+
+Registers an RT role which applies to this class for role-based access control.
+Arguments:
+
+=over 4
+
+=item Name
+
+Required.  The role name (i.e. Requestor, Owner, AdminCc, etc).
+
+=item EquivClasses
+
+Optional.  Array ref of classes through which this role percolates up to
+L<RT::System>.  You can think of this list as:
+
+    map { ref } $record_object->ACLEquivalenceObjects;
+
+You should not include L<RT::System> itself in this list.
+
+Simply calls RegisterRole on each equivalent class.
+
+=back
+
+=cut
+
+sub RegisterRole {
+    my $self  = shift;
+    my $class = ref($self) || $self;
+    my %role  = (
+        Name            => undef,
+        EquivClasses    => [],
+        @_
+    );
+    return unless $role{Name};
+
+    # Keep track of the class this role came from originally
+    $role{ Class } ||= $class;
+
+    # Some groups are limited to a single user
+    $role{ Single } = 1 if $role{Column};
+
+    # Stash the role on ourself
+    $class->_ROLES->{ $role{Name} } = \%role;
+
+    # Register it with any equivalent classes...
+    my $equiv = delete $role{EquivClasses} || [];
+
+    # ... and globally unless we ARE global
+    unless ($class eq "RT::System") {
+        push @$equiv, "RT::System";
+    }
+
+    $_->RegisterRole(%role) for @$equiv;
+
+    # XXX TODO: Register which classes have roles on them somewhere?
+
+    return 1;
+}
+
+=head2 Roles
+
+Returns a list of role names registered for this class.
+
+=cut
+
+sub Roles { sort { $a cmp $b } keys %{ shift->_ROLES } }
+
+{
+    my %ROLES;
+    sub _ROLES {
+        my $class = ref($_[0]) || $_[0];
+        return $ROLES{$class} ||= {};
+    }
+}
+
+=head2 HasRole
+
+Returns true if the name provided is a registered role for this class.
+Otherwise returns false.
+
+=cut
+
+sub HasRole {
+    my $self = shift;
+    my $type = shift;
+    return scalar grep { $type eq $_ } $self->Roles;
+}
+
+=head2 RoleGroup
+
+Expects a role name as the first parameter which is used to load the
+L<RT::Group> for the specified role on this record.  Returns an unloaded
+L<RT::Group> object on failure.
+
+=cut
+
+sub RoleGroup {
+    my $self  = shift;
+    my $type  = shift;
+    my $group = RT::Group->new( $self->CurrentUser );
+
+    if ($self->HasRole($type)) {
+        $group->LoadRoleGroup(
+            Object  => $self,
+            Type    => $type,
+        );
+    }
+    return $group;
+}
+
+=head2 AddRoleMember
+
+Adds the described L<RT::Principal> to the specified role group for this record.
+
+Takes a set of key-value pairs:
+
+=over 4
+
+=item PrincipalId
+
+Optional.  The ID of the L<RT::Principal> object to add.
+
+=item User
+
+Optional.  The Name or EmailAddress of an L<RT::User> to use as the
+principal.  If an email address is given, but a user matching it cannot
+be found, a new user will be created.
+
+=item Group
+
+Optional.  The Name of an L<RT::Group> to use as the principal.
+
+=item Type
+
+Required.  One of the valid roles for this record, as returned by L</Roles>.
+
+=item ACL
+
+Optional.  A subroutine reference which will be passed the role type and
+principal being added.  If it returns false, the method will fail with a
+status of "Permission denied".
+
+=back
+
+One, and only one, of I<PrincipalId>, I<User>, or I<Group> is required.
+
+Returns a tuple of (principal object which was added, message).
+
+=cut
+
+sub AddRoleMember {
+    my $self = shift;
+    my %args = (@_);
+
+    return (0, $self->loc("One, and only one, of PrincipalId/User/Group is required"))
+        if 1 != grep { $_ } @args{qw/PrincipalId User Group/};
+
+    my $type = delete $args{Type};
+    return (0, $self->loc("No valid Type specified"))
+        unless $type and $self->HasRole($type);
+
+    if ($args{PrincipalId}) {
+        # Check the PrincipalId for loops
+        my $principal = RT::Principal->new( $self->CurrentUser );
+        $principal->Load($args{'PrincipalId'});
+        if ( $principal->id and $principal->IsUser and my $email = $principal->Object->EmailAddress ) {
+            return (0, $self->loc("[_1] is an address RT receives mail at. Adding it as a '[_2]' would create a mail loop",
+                                  $email, $self->loc($type)))
+                if RT::EmailParser->IsRTAddress( $email );
+        }
+    } else {
+        if ($args{User}) {
+            my $name = delete $args{User};
+            # Sanity check the address
+            return (0, $self->loc("[_1] is an address RT receives mail at. Adding it as a '[_2]' would create a mail loop",
+                                  $name, $self->loc($type) ))
+                if RT::EmailParser->IsRTAddress( $name );
+
+            # Create as the SystemUser, not the current user
+            my $user = RT::User->new(RT->SystemUser);
+            my ($pid, $msg) = $user->LoadOrCreateByEmail( $name );
+            unless ($pid) {
+                # If we can't find this watcher, we need to bail.
+                $RT::Logger->error("Could not load or create a user '$name' to add as a watcher: $msg");
+                return (0, $self->loc("Could not find or create user '$name'"));
+            }
+            $args{PrincipalId} = $pid;
+        }
+        elsif ($args{Group}) {
+            my $name = delete $args{Group};
+            my $group = RT::Group->new( $self->CurrentUser );
+            $group->LoadUserDefinedGroup($name);
+            unless ($group->id) {
+                $RT::Logger->error("Could not load group '$name' to add as a watcher");
+                return (0, $self->loc("Could not find group '$name'"));
+            }
+            $args{PrincipalId} = $group->PrincipalObj->id;
+        }
+    }
+
+    my $principal = RT::Principal->new( $self->CurrentUser );
+    $principal->Load( $args{PrincipalId} );
+
+    my $acl = delete $args{ACL};
+    return (0, $self->loc("Permission denied"))
+        if $acl and not $acl->($type => $principal);
+
+    my $group = $self->RoleGroup( $type );
+    return (0, $self->loc("Role group '$type' not found"))
+        unless $group->id;
+
+    return (0, $self->loc('[_1] is already a [_2]',
+                          $principal->Object->Name, $self->loc($type)) )
+            if $group->HasMember( $principal );
+
+    my ( $ok, $msg ) = $group->_AddMember( %args );
+    unless ($ok) {
+        $RT::Logger->error("Failed to add $args{PrincipalId} as a member of group ".$group->Id.": ".$msg);
+
+        return ( 0, $self->loc('Could not make [_1] a [_2]',
+                    $principal->Object->Name, $self->loc($type)) );
+    }
+
+    unless ($args{Silent}) {
+        $self->_NewTransaction(
+            Type     => 'AddWatcher', # use "watcher" for history's sake
+            NewValue => $args{PrincipalId},
+            Field    => $type,
+        );
+    }
+
+    return ($principal, $msg);
+}
+
+=head2 DeleteRoleMember
+
+Removes the specified L<RT::Principal> from the specified role group for this
+record.
+
+Takes a set of key-value pairs:
+
+=over 4
+
+=item PrincipalId
+
+Optional.  The ID of the L<RT::Principal> object to remove.
+
+=item User
+
+Optional.  The Name or EmailAddress of an L<RT::User> to use as the
+principal
+
+=item Type
+
+Required.  One of the valid roles for this record, as returned by L</Roles>.
+
+=back
+
+One, and only one, of I<PrincipalId> or I<User> is required.
+
+Returns a tuple of (principal object that was removed, message).
+
+=cut
+
+sub DeleteRoleMember {
+    my $self = shift;
+    my %args = (@_);
+
+    return (0, $self->loc("No valid Type specified"))
+        unless $args{Type} and $self->HasRole($args{Type});
+
+    if ($args{User}) {
+        my $user = RT::User->new( $self->CurrentUser );
+        $user->LoadByEmail( $args{User} );
+        $user->Load( $args{User} ) unless $user->id;
+        return (0, $self->loc("Could not load user '$args{User}'") )
+            unless $user->id;
+        $args{PrincipalId} = $user->PrincipalId;
+    }
+
+    return (0, $self->loc("No valid PrincipalId"))
+        unless $args{PrincipalId};
+
+    my $principal = RT::Principal->new( $self->CurrentUser );
+    $principal->Load( $args{PrincipalId} );
+
+    my $acl = delete $args{ACL};
+    return (0, $self->loc("Permission denied"))
+        if $acl and not $acl->($principal);
+
+    my $group = $self->RoleGroup( $args{Type} );
+    return (0, $self->loc("Role group '$args{Type}' not found"))
+        unless $group->id;
+
+    return ( 0, $self->loc( '[_1] is not a [_2]',
+                            $principal->Object->Name, $self->loc($args{Type}) ) )
+        unless $group->HasMember($principal);
+
+    my ($ok, $msg) = $group->_DeleteMember($args{PrincipalId});
+    unless ($ok) {
+        $RT::Logger->error("Failed to remove $args{PrincipalId} as a member of group ".$group->Id.": ".$msg);
+
+        return ( 0, $self->loc('Could not remove [_1] as a [_2]',
+                    $principal->Object->Name, $self->loc($args{Type})) );
+    }
+
+    unless ($args{Silent}) {
+        $self->_NewTransaction(
+            Type     => 'DelWatcher', # use "watcher" for history's sake
+            OldValue => $args{PrincipalId},
+            Field    => $args{Type},
+        );
+    }
+    return ($principal, $msg);
+}
+
+sub _ResolveRoles {
+    my $self = shift;
+    my ($roles, %args) = (@_);
+
+    my @errors;
+    for my $role ($self->Roles) {
+        if ($self->_ROLES->{$role}{Single}) {
+            # Default to nobody if unspecified
+            my $value = $args{$role} || RT->Nobody;
+            if (Scalar::Util::blessed($value) and $value->isa("RT::User")) {
+                # Accept a user; it may not be loaded, which we catch below
+                $roles->{$role} = $value->PrincipalObj;
+            } else {
+                # Try loading by id, name, then email.  If all fail, catch that below
+                my $user = RT::User->new( $self->CurrentUser );
+                $user->Load( $value );
+                # XXX: LoadOrCreateByEmail ?
+                $user->LoadByEmail( $value ) unless $user->id;
+                $roles->{$role} = $user->PrincipalObj;
+            }
+            unless ($roles->{$role}->id) {
+                push @errors, $self->loc("Invalid value for [_1]",loc($role));
+                $roles->{$role} = RT->Nobody->PrincipalObj unless $roles->{$role}->id;
+            }
+            # For consistency, we always return an arrayref
+            $roles->{$role} = [ $roles->{$role} ];
+        } else {
+            $roles->{$role} = [];
+            my @values = ref $args{ $role } ? @{ $args{$role} } : ($args{$role});
+            for my $value (grep {defined} @values) {
+                if ( $value =~ /^\d+$/ ) {
+                    # This implicitly allows groups, if passed by id.
+                    my $principal = RT::Principal->new( $self->CurrentUser );
+                    my ($ok, $msg) = $principal->Load( $value );
+                    if ($ok) {
+                        push @{ $roles->{$role} }, $principal;
+                    } else {
+                        push @errors,
+                            $self->loc("Couldn't load principal: [_1]", $msg);
+                    }
+                } else {
+                    my @addresses = RT::EmailParser->ParseEmailAddress( $value );
+                    for my $address ( @addresses ) {
+                        my $user = RT::User->new( RT->SystemUser );
+                        my ($id, $msg) = $user->LoadOrCreateByEmail( $address );
+                        if ( $id ) {
+                            # Load it back as us, not as the system
+                            # user, to be completely safe.
+                            $user = RT::User->new( $self->CurrentUser );
+                            $user->Load( $id );
+                            push @{ $roles->{$role} }, $user->PrincipalObj;
+                        } else {
+                            push @errors,
+                                $self->loc("Couldn't load or create user: [_1]", $msg);
+                        }
+                    }
+                }
+            }
+        }
+    }
+    return (@errors);
+}
+
+sub _CreateRoleGroups {
+    my $self = shift;
+    my %args = (@_);
+    for my $type ($self->Roles) {
+        my $type_obj = RT::Group->new($self->CurrentUser);
+        my ($id, $msg) = $type_obj->CreateRoleGroup(
+            Type    => $type,
+            Object  => $self,
+            %args,
+        );
+        unless ($id) {
+            $RT::Logger->error("Couldn't create a role group of type '$type' for ".ref($self)." ".
+                                   $self->id.": ".$msg);
+            return(undef);
+        }
+    }
+    return(1);
+}
+
+sub _AddRolesOnCreate {
+    my $self = shift;
+    my ($roles, %acls) = @_;
+
+    my @errors;
+    {
+        my $changed = 0;
+
+        for my $role (keys %{$roles}) {
+            my $group = $self->RoleGroup($role);
+            my @left;
+            for my $principal (@{$roles->{$role}}) {
+                if ($acls{$role}->($principal)) {
+                    next if $group->HasMember($principal);
+                    my ($ok, $msg) = $group->_AddMember(
+                        PrincipalId       => $principal->id,
+                        InsideTransaction => 1,
+                        RecordTransaction => 0,
+                        Object            => $self,
+                    );
+                    push @errors, $self->loc("Couldn't set [_1] watcher: [_2]", $role, $msg)
+                        unless $ok;
+                    $changed++;
+                } else {
+                    push @left, $principal;
+                }
+            }
+            $roles->{$role} = [ @left ];
+        }
+
+        redo if $changed;
+    }
+
+    return @errors;
+}
+
+
+1;
diff --git a/lib/RT/System.pm b/lib/RT/System.pm
index 28fea07..2f326ee 100644
--- a/lib/RT/System.pm
+++ b/lib/RT/System.pm
@@ -72,6 +72,9 @@ use warnings;
 
 use base qw/RT::Record/;
 
+use Role::Basic 'with';
+with "RT::Role::Record::Roles";
+
 use RT::ACL;
 use RT::ACE;
 
@@ -135,7 +138,7 @@ sub AvailableRights {
     # Only return rights on classes which support the role asked for
     if ($principal and $principal->IsRoleGroup) {
         my $role = $principal->Object->Type;
-        @types   = grep { $_->HasRole($role) } @types;
+        @types   = grep { $_->DOES('RT::Role::Record::Roles') and $_->HasRole($role) } @types;
         %rights  = ();
     }
 
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 0d8f46a..b86237f 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -70,7 +70,7 @@ use warnings;
 use base 'RT::Record';
 
 use Role::Basic 'with';
-with "RT::Role::Record::Status";
+with "RT::Role::Record::Status", "RT::Role::Record::Roles";
 
 use RT::Queue;
 use RT::User;

commit ae81f155757eaf4a281fc2b018bfe1e630006cd7
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Wed Jan 9 16:46:53 2013 -0800

    Ensure any class returned by RT::Group->RoleClass does the Roles role
    
    This is more nicer than duck-typing HasRole, and catches uses of _ROLES
    elsewhere in RT::Group as well.

diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index 334314b..91c92f3 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -712,6 +712,7 @@ sub RoleClass {
     my $self = shift;
     my $domain = shift || $self->Domain;
     return unless $domain =~ /^(.+)-Role$/;
+    return unless $1->DOES("RT::Role::Record::Roles");
     return $1;
 }
 
@@ -729,7 +730,7 @@ sub ValidateRoleGroup {
     return 0 unless $args{Domain} and $args{Type};
 
     my $class = $self->RoleClass($args{Domain});
-    return 0 unless $class and $class->can('HasRole');
+    return 0 unless $class;
 
     return $class->HasRole($args{Type});
 }

commit 17af14d2809bfae73cff4e92384f5a0fdd73b07d
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Wed Jan 9 17:24:32 2013 -0800

    Warn during tests if role consumers override a role method silently
    
    This requires that classes explicitly -rename or -exclude any role
    methods they override.  Silent overrides make debugging more difficult
    and may lead to incorrect developer assumptions (for example, that you
    can use SUPER:: in an overriden role method to call the role's version).
    
    Tests will now flail very loudly if you don't declare your intent.

diff --git a/lib/RT/Test.pm b/lib/RT/Test.pm
index acba2ce..ad39285 100644
--- a/lib/RT/Test.pm
+++ b/lib/RT/Test.pm
@@ -54,6 +54,11 @@ use warnings;
 
 use base 'Test::More';
 
+BEGIN {
+    # Warn about role consumers overriding role methods so we catch it in tests.
+    $ENV{PERL_ROLE_OVERRIDE_WARN} = 1;
+}
+
 # We use the Test::NoWarnings catching and reporting functionality, but need to
 # wrap it in our own special handler because of the warn handler installed via
 # RT->InitLogging().
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index b86237f..4c3f44c 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -70,7 +70,12 @@ use warnings;
 use base 'RT::Record';
 
 use Role::Basic 'with';
-with "RT::Role::Record::Status", "RT::Role::Record::Roles";
+
+# SetStatus and _SetStatus are reimplemented below (using other pieces of the
+# role) to deal with ACLs, moving tickets between queues, and automatically
+# setting dates.
+with "RT::Role::Record::Status" => { -excludes => [qw(SetStatus _SetStatus)] },
+     "RT::Role::Record::Roles";
 
 use RT::Queue;
 use RT::User;

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


More information about the Rt-commit mailing list