[Rt-commit] rt branch, 4.2/role-roles, created. rt-4.1.5-274-g4b15643
Alex Vandiver
alexmv at bestpractical.com
Fri Jan 11 19:00:52 EST 2013
The branch, 4.2/role-roles has been created
at 4b15643d3a8b94592d9d2c28eb4f6d00c3da0e1e (commit)
- Log -----------------------------------------------------------------
commit 3133ff4e8fbd09bf58668818729fbed17ba5ffa2
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 3bbf6c2..fca9726 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 af3b870..5b70dd3 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;
commit 4ad5d6bd3acad1b0f1600edfba28e61d6dcd2e60
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Jan 10 12:27:34 2013 -0800
Document the Single and Column attributes of roles
diff --git a/lib/RT/Role/Record/Roles.pm b/lib/RT/Role/Record/Roles.pm
index 903a226..1a1b01a 100644
--- a/lib/RT/Role/Record/Roles.pm
+++ b/lib/RT/Role/Record/Roles.pm
@@ -96,6 +96,20 @@ You should not include L<RT::System> itself in this list.
Simply calls RegisterRole on each equivalent class.
+=item Single
+
+Optional. A true value indicates that this role may only contain a single user
+as a member at any given time. When adding a new member to a Single role, any
+existing member will be removed. If all members are removed, L<RT/Nobody> is
+added automatically.
+
+=item Column
+
+Optional, implies Single. Specifies a column on the announcing class into
+which the single role member's user ID is denormalized. The column will be
+kept updated automatically as the role member changes. This is used, for
+example, for ticket owners and makes searching simpler (among other benefits).
+
=back
=cut
commit a208829efbabc7fceefb3d08b60bf7e4da6b49f9
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 a2a87c1..4338062 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 6dfcc3e..608d86d 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -2175,442 +2175,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 b2a3127..af3b870 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 9790159e021cfa8e40815d50963f008a5b6aaf4c
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Jan 10 13:37:30 2013 -0800
Store the original role attributes as a shallow copy
This was the original intention since it leaves EquivClasses for the
announcing class but strips it from the role attributes in the
equivalent classes themselves.
The array ref value of EquivClasses is still modified by RegisterRole to
add RT::System, but that's OK since it is simply an implicit equivalent
class. It should be introspectable later rather than hidden.
diff --git a/lib/RT/Role/Record/Roles.pm b/lib/RT/Role/Record/Roles.pm
index 1a1b01a..8768e5c 100644
--- a/lib/RT/Role/Record/Roles.pm
+++ b/lib/RT/Role/Record/Roles.pm
@@ -131,7 +131,7 @@ sub RegisterRole {
$role{ Single } = 1 if $role{Column};
# Stash the role on ourself
- $class->_ROLES->{ $role{Name} } = \%role;
+ $class->_ROLES->{ $role{Name} } = { %role };
# Register it with any equivalent classes...
my $equiv = delete $role{EquivClasses} || [];
commit 43ab6f55a8d3cb070bce4a5c07d954f1070a581e
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 3e7ace9..5d5ec07 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -709,6 +709,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;
}
@@ -726,7 +727,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 d9c0fc520f1a74a01eb210eb82b5d9403684ffbb
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Jan 10 12:44:57 2013 -0800
Provide a method to access role attributes rather than using _ROLES directly
This is a bit cleaner, documented, and doesn't let the caller change any
attributes using the returned reference. The next step would be to
start using objects to represent role metadata, but that feels much
heavier an abstraction than is needed. A hashref is fine for a
read-only handful of flags.
diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index 5d5ec07..898babd 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -740,15 +740,17 @@ sub SingleMemberRoleGroup {
my $self = shift;
my $class = $self->RoleClass;
return unless $class;
- return $class->_ROLES->{$self->Type}{Single};
+ return $class->Role($self->Type)->{Single};
}
sub SingleMemberRoleGroupColumn {
my $self = shift;
my ($class) = $self->Domain =~ /^(.+)-Role$/;
return unless $class;
- return unless $class->_ROLES->{$self->Type}{Class} eq $class;
- return $class->_ROLES->{$self->Type}{Column};
+
+ my $role = $class->Role($self->Type);
+ return unless $role->{Class} eq $class;
+ return $role->{Column};
}
sub RoleGroupObject {
diff --git a/lib/RT/Role/Record/Roles.pm b/lib/RT/Role/Record/Roles.pm
index 8768e5c..e25dd1f 100644
--- a/lib/RT/Role/Record/Roles.pm
+++ b/lib/RT/Role/Record/Roles.pm
@@ -148,6 +148,29 @@ sub RegisterRole {
return 1;
}
+=head2 Role
+
+Takes a role name; returns a hashref describing the role. This hashref
+contains the same attributes used to register the role (see L</RegisterRole>),
+as well as some extras, including:
+
+=over
+
+=item Class
+
+The original class which announced the role. This is set automatically by
+L</RegisterRole> and is the same across all EquivClasses.
+
+=back
+
+Returns an empty hashref if the role doesn't exist.
+
+=cut
+
+sub Role {
+ return \%{ $_[0]->_ROLES->{$_[1]} || {} };
+}
+
=head2 Roles
Returns a list of role names registered for this class.
commit 0f06dcd7da29825212100c6304326cfbb954120d
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Jan 10 15:17:32 2013 -0800
Optionally filter returned role names by the presence or lack of role flags
These flags are accessible via the ->Role() method as well, but this
filtering mechanism avoids an additional map and grep on the caller
side.
diff --git a/lib/RT/Role/Record/Roles.pm b/lib/RT/Role/Record/Roles.pm
index e25dd1f..f7506a0 100644
--- a/lib/RT/Role/Record/Roles.pm
+++ b/lib/RT/Role/Record/Roles.pm
@@ -175,9 +175,32 @@ sub Role {
Returns a list of role names registered for this class.
+Optionally takes a hash specifying attributes the returned roles must possess
+or lack. Testing is done on a simple truthy basis and the actual values of
+the role attributes and arguments you pass are not compared string-wise or
+numerically; they must simply evaluate to the same truthiness.
+
+For example:
+
+ # Return role names which are denormalized into a column; note that the
+ # role's Column attribute contains a string.
+ $object->Roles( Column => 1 );
+
=cut
-sub Roles { sort { $a cmp $b } keys %{ shift->_ROLES } }
+sub Roles {
+ my $self = shift;
+ my %attr = @_;
+
+ return grep {
+ my $ok = 1;
+ my $role = $self->Role($_);
+ for my $k (keys %attr) {
+ $ok = 0, last if $attr{$k} xor $role->{$k};
+ }
+ $ok;
+ } sort { $a cmp $b } keys %{ $self->_ROLES };
+}
{
my %ROLES;
commit 47a506ee5bebfc92f22c2116703c0d362217e51d
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Jan 10 15:21:35 2013 -0800
Introduce the ACLOnly and ACLOnlyInEquiv flags to role metadata
These are advisory flags indicating when a role is only used for
granting rights and generally shouldn't contain any members. For
example, the Owner and Requestor ticket roles should not contain members
at the queue or system level. These flags are useful in the UI when
generating a list of selectable roles to display for objects.
diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index 4338062..160abff 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -649,9 +649,7 @@ them, see L</Roles>.
=cut
sub ManageableRoleGroupTypes {
- # This grep is a little hacky, but I don't want to introduce the concept of
- # manageable vs. unmanageable roles globally (yet).
- return grep { not /^(Requestor|Owner)$/ } shift->Roles;
+ shift->Roles( ACLOnly => 0 )
}
=head2 IsManageableRoleGroupType
@@ -663,12 +661,7 @@ Returns whether the passed-in type is a manageable role group type.
sub IsManageableRoleGroupType {
my $self = shift;
my $type = shift;
-
- for my $valid_type ($self->ManageableRoleGroupTypes) {
- return 1 if $type eq $valid_type;
- }
-
- return 0;
+ return not $self->Role($type)->{ACLOnly};
}
diff --git a/lib/RT/Role/Record/Roles.pm b/lib/RT/Role/Record/Roles.pm
index f7506a0..21efbca 100644
--- a/lib/RT/Role/Record/Roles.pm
+++ b/lib/RT/Role/Record/Roles.pm
@@ -110,6 +110,19 @@ which the single role member's user ID is denormalized. The column will be
kept updated automatically as the role member changes. This is used, for
example, for ticket owners and makes searching simpler (among other benefits).
+=item ACLOnly
+
+Optional. A true value indicates this role is only used for ACLs and should
+not be populated with members.
+
+This flag is advisory only, and the Perl API still allows members to be added
+to ACLOnly roles.
+
+=item ACLOnlyInEquiv
+
+Optional. Automatically sets the ACLOnly flag for all EquivClasses, but not
+the announcing class.
+
=back
=cut
@@ -141,6 +154,9 @@ sub RegisterRole {
push @$equiv, "RT::System";
}
+ # ... marked as "for ACLs only" if flagged as such by the announcing class
+ $role{ACLOnly} = 1 if delete $role{ACLOnlyInEquiv};
+
$_->RegisterRole(%role) for @$equiv;
# XXX TODO: Register which classes have roles on them somewhere?
@@ -182,6 +198,9 @@ numerically; they must simply evaluate to the same truthiness.
For example:
+ # Return role names which are not only for ACL purposes
+ $object->Roles( ACLOnly => 0 );
+
# Return role names which are denormalized into a column; note that the
# role's Column attribute contains a string.
$object->Roles( Column => 1 );
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 5b70dd3..f18b7d2 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -106,7 +106,8 @@ for my $role (sort keys %ROLES) {
RT::Ticket->RegisterRole(
Name => $role,
EquivClasses => ['RT::Queue'],
- ( $role eq "Owner" ? ( Column => "Owner") : () ),
+ ( $role eq "Owner" ? ( Column => "Owner") : () ),
+ ( $role !~ /Cc/ ? ( ACLOnlyInEquiv => 1) : () ),
);
}
commit 1d2ac18bfa3d6189869d1d4d61dbfa310614ab7e
Merge: 43ab6f5 47a506e
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Fri Jan 11 18:42:28 2013 -0500
Merge branch '4.2/role-inspection' into 4.2/role-roles
commit 093efccd6d00ceb928bc5bd7d42d7d17ed07d1fa
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Fri Jan 11 04:42:13 2013 -0500
Strip out "bundling" support effectively removed in 3.7.1
27ed2c9 disabled the %can_bundle hash, which controlled all "bundling"
of similar queries. This functionality has since been re-implemented
more correctly by caching join clauses. Remove the old unreachable code
which the %can_bundle hash mediated access to.
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index fc3a7f1..0308c5d 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -176,7 +176,6 @@ our %dispatch = (
CUSTOMFIELD => \&_CustomFieldLimit,
HASATTRIBUTE => \&_HasAttributeLimit,
);
-our %can_bundle = ();# WATCHERFIELD => "yes", );
# Default EntryAggregator per type
# if you specify OP, you must specify all valid OPs
@@ -224,7 +223,6 @@ my %DefaultEA = (
# into Tickets_SQL.
sub FIELDS { return \%FIELD_METADATA }
sub dispatch { return \%dispatch }
-sub can_bundle { return \%can_bundle }
# Bring in the clowns.
require RT::Tickets_SQL;
diff --git a/lib/RT/Tickets_SQL.pm b/lib/RT/Tickets_SQL.pm
index ec1bb49..63e3be3 100644
--- a/lib/RT/Tickets_SQL.pm
+++ b/lib/RT/Tickets_SQL.pm
@@ -57,7 +57,7 @@ use RT::SQL;
# Import configuration data from the lexcial scope of __PACKAGE__ (or
# at least where those two Subroutines are defined.)
-our (%FIELD_METADATA, %dispatch, %can_bundle);
+our (%FIELD_METADATA, %dispatch);
# Lower Case version of FIELDS, for case insensitivity
my %lcfields = map { ( lc($_) => $_ ) } (keys %FIELD_METADATA);
@@ -138,49 +138,15 @@ just handed off the SearchBuilder)
=cut
-sub _close_bundle {
- my ($self, @bundle) = @_;
- return unless @bundle;
-
- if ( @bundle == 1 ) {
- $bundle[0]->{'dispatch'}->(
- $self,
- $bundle[0]->{'key'},
- $bundle[0]->{'op'},
- $bundle[0]->{'val'},
- SUBCLAUSE => '',
- ENTRYAGGREGATOR => $bundle[0]->{ea},
- SUBKEY => $bundle[0]->{subkey},
- );
- }
- else {
- my @args;
- foreach my $chunk (@bundle) {
- push @args, [
- $chunk->{key},
- $chunk->{op},
- $chunk->{val},
- SUBCLAUSE => '',
- ENTRYAGGREGATOR => $chunk->{ea},
- SUBKEY => $chunk->{subkey},
- ];
- }
- $bundle[0]->{dispatch}->( $self, \@args );
- }
-}
-
sub _parser {
my ($self,$string) = @_;
- my @bundle;
my $ea = '';
my %callback;
$callback{'OpenParen'} = sub {
- $self->_close_bundle(@bundle); @bundle = ();
$self->_OpenParen
};
$callback{'CloseParen'} = sub {
- $self->_close_bundle(@bundle); @bundle = ();
$self->_CloseParen;
};
$callback{'EntryAggregator'} = sub { $ea = $_[0] || '' };
@@ -208,37 +174,15 @@ sub _parser {
}
my $sub = $dispatch{ $class };
- if ( $can_bundle{ $class }
- && ( !@bundle
- || ( $bundle[-1]->{dispatch} == $sub
- && $bundle[-1]->{key} eq $key
- && $bundle[-1]->{subkey} eq $subkey
- )
- )
- )
- {
- push @bundle, {
- dispatch => $sub,
- key => $key,
- op => $op,
- val => $value,
- ea => $ea,
- subkey => $subkey,
- };
- }
- else {
- $self->_close_bundle(@bundle); @bundle = ();
- $sub->( $self, $key, $op, $value,
- SUBCLAUSE => '', # don't need anymore
- ENTRYAGGREGATOR => $ea,
- SUBKEY => $subkey,
- );
- }
+ $sub->( $self, $key, $op, $value,
+ SUBCLAUSE => '', # don't need anymore
+ ENTRYAGGREGATOR => $ea,
+ SUBKEY => $subkey,
+ );
$self->{_sql_looking_at}{lc $key} = 1;
$ea = '';
};
RT::SQL::Parse($string, \%callback);
- $self->_close_bundle(@bundle); @bundle = ();
}
=head2 ClausesToSQL
commit 3333e58ed3521b8129aa4e5d655ae49df0e10b98
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Fri Jan 11 04:49:09 2013 -0500
Remove unnecessary explicit empty SUBCLAUSE
This line, along with its comment, was added in ca592e4 in RT 2.1.59
when Tickets_Overlay_SQL.pm was first added. It merely provides an
unnecessary default -- and the comment is just as true now as when it
was added. Remove it.
diff --git a/lib/RT/Tickets_SQL.pm b/lib/RT/Tickets_SQL.pm
index 63e3be3..b0b3f01 100644
--- a/lib/RT/Tickets_SQL.pm
+++ b/lib/RT/Tickets_SQL.pm
@@ -175,7 +175,6 @@ sub _parser {
my $sub = $dispatch{ $class };
$sub->( $self, $key, $op, $value,
- SUBCLAUSE => '', # don't need anymore
ENTRYAGGREGATOR => $ea,
SUBKEY => $subkey,
);
commit d404d30910bccb113bf9aeb6dfc0f941d6c92d53
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Fri Jan 11 05:00:12 2013 -0500
Allow providing a more explit controlled message to RT->Deprecated
diff --git a/lib/RT.pm b/lib/RT.pm
index 467adf9..f7b0b52 100644
--- a/lib/RT.pm
+++ b/lib/RT.pm
@@ -828,6 +828,11 @@ A suggestion of what to use in place of the deprecated API
Used if not the entire method is being removed, merely a manner of
calling it; names the arguments which are deprecated.
+=item Message
+
+Overrides the auto-built phrasing of C<Calling function ____ is
+deprecated> with a custom message.
+
=back
=cut
@@ -838,6 +843,7 @@ sub Deprecated {
Arguments => undef,
Remove => undef,
Instead => undef,
+ Message => undef,
@_,
);
@@ -856,7 +862,9 @@ sub Deprecated {
$stack =~ s/^.*?\n//; # Strip off call to ->Deprecated
my $msg;
- if ($args{Arguments}) {
+ if ($args{Message}) {
+ $msg = $args{Message};
+ } elsif ($args{Arguments}) {
$msg = "Calling $function with $args{Arguments} is deprecated";
} else {
$msg = "The $function is deprecated";
commit 9e7da014a8150e1e781a3b18dd69d779e27580aa
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Tue Jan 8 17:57:05 2013 -0500
Rename Limit in RT::Tickets to LimitField, so ->Limit acts as elsewhere
The old-style LimitFoo methods now call LimitField, which goes through
the Restrctions and Clauses machinery. For compatibility with the rest
of SearchBuilder, however, it is exceedingly useful to have ->Limit act
immediately.
diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index 608d86d..66c5e6e 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -1058,12 +1058,9 @@ sub HasUnresolvedDependencies {
my $deps = $self->UnresolvedDependencies;
if ($args{Type}) {
- $deps->Limit( FIELD => 'Type',
- OPERATOR => '=',
- VALUE => $args{Type});
- }
- else {
- $deps->IgnoreType;
+ $deps->LimitType( VALUE => $args{Type} );
+ } else {
+ $deps->IgnoreType;
}
if ($deps->Count > 0) {
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index f18b7d2..cef3bde 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -2129,11 +2129,11 @@ sub Merged {
if $MERGE_CACHE{'merged'}{ $id };
my $mergees = RT::Tickets->new( $self->CurrentUser );
- $mergees->Limit(
+ $mergees->LimitField(
FIELD => 'EffectiveId',
VALUE => $id,
);
- $mergees->Limit(
+ $mergees->LimitField(
FIELD => 'id',
OPERATOR => '!=',
VALUE => $id,
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index 0308c5d..ef09167 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -2006,16 +2006,35 @@ sub OrderByCols {
}
+sub Limit {
+ my $self = shift;
+ my %args = @_;
+ $self->{'must_redo_search'} = 1;
+ delete $self->{'raw_rows'};
+ delete $self->{'count_all'};
+
+ $args{ALIAS} ||= 'main';
+ $self->{'looking_at_effective_id'} = 1
+ if $args{'ALIAS'} eq 'main' and $args{'FIELD'} eq 'EffectiveId';
+ $self->{'looking_at_type'} = 1
+ if $args{'ALIAS'} eq 'main' and $args{'FIELD'} eq 'Type';
+
+ if ($self->{'using_restrictions'}) {
+ RT->Deprecated( Message => "Mixing old-style LimitFoo methods with Limit is deprecated" );
+ $self->LimitField(@_);
+ }
+ $self->SUPER::Limit(@_);
+}
-=head2 Limit
+=head2 LimitField
Takes a paramhash with the fields FIELD, OPERATOR, VALUE and DESCRIPTION
Generally best called from LimitFoo methods
=cut
-sub Limit {
+sub LimitField {
my $self = shift;
my %args = (
FIELD => undef,
@@ -2030,6 +2049,12 @@ sub Limit {
)
if ( !defined $args{'DESCRIPTION'} );
+
+ if ($self->_isLimited > 1) {
+ RT->Deprecated( Message => "Mixing old-style LimitFoo methods with Limit is deprecated" );
+ }
+ $self->{using_restrictions} = 1;
+
my $index = $self->_NextIndex;
# make the TicketRestrictions hash the equivalent of whatever we just passed in;
@@ -2038,20 +2063,6 @@ sub Limit {
$self->{'RecalcTicketLimits'} = 1;
-# If we're looking at the effective id, we don't want to append the other clause
-# which limits us to tickets where id = effective id
- if ( $args{'FIELD'} eq 'EffectiveId'
- && ( !$args{'ALIAS'} || $args{'ALIAS'} eq 'main' ) )
- {
- $self->{'looking_at_effective_id'} = 1;
- }
-
- if ( $args{'FIELD'} eq 'Type'
- && ( !$args{'ALIAS'} || $args{'ALIAS'} eq 'main' ) )
- {
- $self->{'looking_at_type'} = 1;
- }
-
return ($index);
}
@@ -2087,7 +2098,7 @@ sub LimitQueue {
#TODO check for a valid queue here
- $self->Limit(
+ $self->LimitField(
FIELD => 'Queue',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2119,7 +2130,7 @@ sub LimitStatus {
OPERATOR => '=',
@_
);
- $self->Limit(
+ $self->LimitField(
FIELD => 'Status',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2205,12 +2216,12 @@ sub LimitType {
VALUE => undef,
@_
);
- $self->Limit(
+ $self->LimitField(
FIELD => 'Type',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
DESCRIPTION => join( ' ',
- $self->loc('Type'), $args{'OPERATOR'}, $args{'Limit'}, ),
+ $self->loc('Type'), $args{'OPERATOR'}, $args{'VALUE'}, ),
);
}
@@ -2229,7 +2240,7 @@ VALUE is a string to search for in the subject of the ticket.
sub LimitSubject {
my $self = shift;
my %args = (@_);
- $self->Limit(
+ $self->LimitField(
FIELD => 'Subject',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2258,7 +2269,7 @@ sub LimitId {
@_
);
- $self->Limit(
+ $self->LimitField(
FIELD => 'id',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2280,7 +2291,7 @@ VALUE is a value to match the ticket's priority against
sub LimitPriority {
my $self = shift;
my %args = (@_);
- $self->Limit(
+ $self->LimitField(
FIELD => 'Priority',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2304,7 +2315,7 @@ VALUE is a value to match the ticket's initial priority against
sub LimitInitialPriority {
my $self = shift;
my %args = (@_);
- $self->Limit(
+ $self->LimitField(
FIELD => 'InitialPriority',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2327,7 +2338,7 @@ VALUE is a value to match the ticket's final priority against
sub LimitFinalPriority {
my $self = shift;
my %args = (@_);
- $self->Limit(
+ $self->LimitField(
FIELD => 'FinalPriority',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2350,7 +2361,7 @@ VALUE is a value to match the ticket's TimeWorked attribute
sub LimitTimeWorked {
my $self = shift;
my %args = (@_);
- $self->Limit(
+ $self->LimitField(
FIELD => 'TimeWorked',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2373,7 +2384,7 @@ VALUE is a value to match the ticket's TimeLeft attribute
sub LimitTimeLeft {
my $self = shift;
my %args = (@_);
- $self->Limit(
+ $self->LimitField(
FIELD => 'TimeLeft',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2398,7 +2409,7 @@ VALUE is a string to search for in the body of the ticket
sub LimitContent {
my $self = shift;
my %args = (@_);
- $self->Limit(
+ $self->LimitField(
FIELD => 'Content',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2421,7 +2432,7 @@ VALUE is a string to search for in the body of the ticket
sub LimitFilename {
my $self = shift;
my %args = (@_);
- $self->Limit(
+ $self->LimitField(
FIELD => 'Filename',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2443,7 +2454,7 @@ VALUE is a content type to search ticket attachments for
sub LimitContentType {
my $self = shift;
my %args = (@_);
- $self->Limit(
+ $self->LimitField(
FIELD => 'ContentType',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2476,7 +2487,7 @@ sub LimitOwner {
$owner->Load( $args{'VALUE'} );
# FIXME: check for a valid $owner
- $self->Limit(
+ $self->LimitField(
FIELD => 'Owner',
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2517,7 +2528,7 @@ sub LimitWatcher {
$watcher_type = "Watcher";
}
- $self->Limit(
+ $self->LimitField(
FIELD => $watcher_type,
VALUE => $args{'VALUE'},
OPERATOR => $args{'OPERATOR'},
@@ -2553,7 +2564,7 @@ sub LimitLinkedTo {
@_
);
- $self->Limit(
+ $self->LimitField(
FIELD => 'LinkedTo',
BASE => undef,
TARGET => $args{'TARGET'},
@@ -2596,7 +2607,7 @@ sub LimitLinkedFrom {
my $type = $args{'TYPE'};
$type = $fromToMap{$type} if exists( $fromToMap{$type} );
- $self->Limit(
+ $self->LimitField(
FIELD => 'LinkedTo',
TARGET => undef,
BASE => $args{'BASE'},
@@ -2718,7 +2729,7 @@ sub LimitDate {
. $args{'VALUE'} . " GMT";
}
- $self->Limit(%args);
+ $self->LimitField(%args);
}
@@ -2792,7 +2803,7 @@ sub LimitTransactionDate {
. $args{'VALUE'} . " GMT";
}
- $self->Limit(%args);
+ $self->LimitField(%args);
}
@@ -2866,7 +2877,7 @@ sub LimitCustomField {
@rest = ( ENTRYAGGREGATOR => 'AND' )
if ( $CF->Type eq 'SelectMultiple' );
- $self->Limit(
+ $self->LimitField(
VALUE => $args{VALUE},
FIELD => "CF"
.(defined $args{'QUEUE'}? ".{$args{'QUEUE'}}" : '' )
@@ -3111,6 +3122,8 @@ sub CurrentUserCanSee {
Right => 'SuperUser', Object => $RT::System
);
+ local $self->{using_restrictions};
+
my $id = $self->CurrentUser->id;
# directly can see in all queues then we have nothing to do
@@ -3467,6 +3480,7 @@ sub _ProcessRestrictions {
my $sql = $self->Query; # Violating the _SQL namespace
if ( !$sql || $self->{'RecalcTicketLimits'} ) {
+ local $self->{using_restrictions};
# "Restrictions to Clauses Branch\n";
my $clauseRef = eval { $self->_RestrictionsToClauses; };
if ($@) {
diff --git a/lib/RT/Tickets_SQL.pm b/lib/RT/Tickets_SQL.pm
index b0b3f01..08c1ed8 100644
--- a/lib/RT/Tickets_SQL.pm
+++ b/lib/RT/Tickets_SQL.pm
@@ -78,19 +78,10 @@ sub _InitSQL {
sub _SQLLimit {
my $self = shift;
my %args = (@_);
- if ($args{'FIELD'} eq 'EffectiveId' &&
- (!$args{'ALIAS'} || $args{'ALIAS'} eq 'main' ) ) {
- $self->{'looking_at_effective_id'} = 1;
- }
-
- if ($args{'FIELD'} eq 'Type' &&
- (!$args{'ALIAS'} || $args{'ALIAS'} eq 'main' ) ) {
- $self->{'looking_at_type'} = 1;
- }
# All SQL stuff goes into one SB subclause so we can deal with all
# the aggregation
- $self->SUPER::Limit(%args,
+ $self->Limit(%args,
SUBCLAUSE => 'ticketsql');
}
diff --git a/share/html/Approvals/Elements/PendingMyApproval b/share/html/Approvals/Elements/PendingMyApproval
index c4e19a8..36c1fa6 100644
--- a/share/html/Approvals/Elements/PendingMyApproval
+++ b/share/html/Approvals/Elements/PendingMyApproval
@@ -87,7 +87,7 @@ my $created_before = RT::Date->new( $session{'CurrentUser'} );
my $created_after = RT::Date->new( $session{'CurrentUser'} );
foreach ($tickets, $group_tickets) {
- $_->Limit( FIELD => 'Type', VALUE => 'approval' );
+ $_->LimitType( VALUE => 'approval' );
if ( $ARGS{'ShowResolved'} ) {
$_->LimitStatus( VALUE => 'resolved' );
diff --git a/share/html/Ticket/Elements/ShowDependencyStatus b/share/html/Ticket/Elements/ShowDependencyStatus
index 4de49ee..2d7a620 100644
--- a/share/html/Ticket/Elements/ShowDependencyStatus
+++ b/share/html/Ticket/Elements/ShowDependencyStatus
@@ -66,12 +66,12 @@ $Ticket
<%init>
# normal tickets
my $deps = $Ticket->UnresolvedDependencies;
-$deps->Limit( FIELD => 'Type', VALUE => 'ticket' );
+$deps->LimitType( VALUE => 'ticket' );
my $depends = $deps->Count || 0;
# approvals
$deps = $Ticket->UnresolvedDependencies;
-$deps->Limit( FIELD => 'Type', VALUE => 'approval' );
+$deps->LimitType( VALUE => 'approval' );
my $approvals = $deps->Count || 0;
return unless $depends or $approvals;
commit a800789fb8f1dadf8858d92b55e979ff2b55d003
Merge: 1d2ac18 9e7da01
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Fri Jan 11 18:44:24 2013 -0500
Merge branch '4.2/ticket-limit' into 4.2/role-roles
commit 4b15643d3a8b94592d9d2c28eb4f6d00c3da0e1e
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Fri Jan 11 04:56:22 2013 -0500
Factor out role/watcher methods on tickets into a role
diff --git a/lib/RT/Role/SearchBuilder.pm b/lib/RT/Role/SearchBuilder.pm
new file mode 100644
index 0000000..766cd13
--- /dev/null
+++ b/lib/RT/Role/SearchBuilder.pm
@@ -0,0 +1,77 @@
+# 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::SearchBuilder;
+use Role::Basic;
+
+=head1 NAME
+
+RT::Role::SearchBuilder - Common requirements for roles which are consumed by collections
+
+=head1 DESCRIPTION
+
+Various L<RT::SearchBuilder> (and by inheritance L<DBIx::SearchBuilder>)
+methods are required by this role. It provides no methods on its own but is
+simply a contract for other roles to require (usually under the
+I<RT::Role::SearchBuilder::> namespace).
+
+=cut
+
+requires $_ for qw(
+ Join
+ Limit
+ NewItem
+ CurrentUser
+ _OpenParen
+ _CloseParen
+);
+
+1;
diff --git a/lib/RT/Role/SearchBuilder/Roles.pm b/lib/RT/Role/SearchBuilder/Roles.pm
new file mode 100644
index 0000000..7eb61a9
--- /dev/null
+++ b/lib/RT/Role/SearchBuilder/Roles.pm
@@ -0,0 +1,343 @@
+# 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::SearchBuilder::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::SearchBuilder>
+
+=cut
+
+with 'RT::Role::SearchBuilder';
+
+require RT::System;
+require RT::Principal;
+require RT::Group;
+require RT::User;
+
+require RT::EmailParser;
+
+=head1 PROVIDES
+
+=cut
+
+sub _RoleGroupsJoin {
+ my $self = shift;
+ my %args = (New => 0, Class => 'Ticket', Type => '', @_);
+ $args{Class} =~ s/^RT:://;
+ return $self->{'_sql_role_group_aliases'}{ $args{'Class'} .'-'. $args{'Type'} }
+ if $self->{'_sql_role_group_aliases'}{ $args{'Class'} .'-'. $args{'Type'} }
+ && !$args{'New'};
+
+ # we always have watcher groups for ticket, so we use INNER join
+ my $groups = $self->Join(
+ ALIAS1 => 'main',
+ FIELD1 => $args{'Class'} eq 'Queue'? 'Queue': 'id',
+ TABLE2 => 'Groups',
+ FIELD2 => 'Instance',
+ ENTRYAGGREGATOR => 'AND',
+ );
+ $self->Limit(
+ LEFTJOIN => $groups,
+ ALIAS => $groups,
+ FIELD => 'Domain',
+ VALUE => 'RT::'. $args{'Class'} .'-Role',
+ );
+ $self->Limit(
+ LEFTJOIN => $groups,
+ ALIAS => $groups,
+ FIELD => 'Type',
+ VALUE => $args{'Type'},
+ ) if $args{'Type'};
+
+ $self->{'_sql_role_group_aliases'}{ $args{'Class'} .'-'. $args{'Type'} } = $groups
+ unless $args{'New'};
+
+ return $groups;
+}
+
+sub _GroupMembersJoin {
+ my $self = shift;
+ my %args = (New => 1, GroupsAlias => undef, Left => 1, @_);
+
+ return $self->{'_sql_group_members_aliases'}{ $args{'GroupsAlias'} }
+ if $self->{'_sql_group_members_aliases'}{ $args{'GroupsAlias'} }
+ && !$args{'New'};
+
+ my $alias = $self->Join(
+ $args{'Left'} ? (TYPE => 'LEFT') : (),
+ ALIAS1 => $args{'GroupsAlias'},
+ FIELD1 => 'id',
+ TABLE2 => 'CachedGroupMembers',
+ FIELD2 => 'GroupId',
+ ENTRYAGGREGATOR => 'AND',
+ );
+ $self->Limit(
+ $args{'Left'} ? (LEFTJOIN => $alias) : (),
+ ALIAS => $alias,
+ FIELD => 'Disabled',
+ VALUE => 0,
+ );
+
+ $self->{'_sql_group_members_aliases'}{ $args{'GroupsAlias'} } = $alias
+ unless $args{'New'};
+
+ return $alias;
+}
+
+=head2 _WatcherJoin
+
+Helper function which provides joins to a watchers table both for limits
+and for ordering.
+
+=cut
+
+sub _WatcherJoin {
+ my $self = shift;
+ my $type = shift || '';
+
+
+ my $groups = $self->_RoleGroupsJoin( Type => $type );
+ my $group_members = $self->_GroupMembersJoin( GroupsAlias => $groups );
+ # XXX: work around, we must hide groups that
+ # are members of the role group we search in,
+ # otherwise them result in wrong NULLs in Users
+ # table and break ordering. Now, we know that
+ # RT doesn't allow to add groups as members of the
+ # ticket roles, so we just hide entries in CGM table
+ # with MemberId == GroupId from results
+ $self->Limit(
+ LEFTJOIN => $group_members,
+ FIELD => 'GroupId',
+ OPERATOR => '!=',
+ VALUE => "$group_members.MemberId",
+ QUOTEVALUE => 0,
+ );
+ my $users = $self->Join(
+ TYPE => 'LEFT',
+ ALIAS1 => $group_members,
+ FIELD1 => 'MemberId',
+ TABLE2 => 'Users',
+ FIELD2 => 'id',
+ );
+ return ($groups, $group_members, $users);
+}
+
+
+sub RoleLimit {
+ my $self = shift;
+ my %args = (
+ TYPE => '',
+ FIELD => undef,
+ OPERATOR => '=',
+ VALUE => undef,
+ @_
+ );
+
+ my $class = blessed($self->NewItem);
+
+ my $type = delete $args{TYPE};
+ if ($type) {
+ unless ($class->HasRole($type)) {
+ RT->Logger->warn("RoleLimit called with invalid role $type for $class");
+ return;
+ }
+ my $column = $class->Role($type)->{Column};
+ if ( $column ) {
+ if ( $args{OPERATOR} =~ /^!?=$/
+ && (!$args{FIELD} || $args{FIELD} eq 'Name' || $args{FIELD} eq 'EmailAddress') ) {
+ my $o = RT::User->new( $self->CurrentUser );
+ my $method = ($args{FIELD}||'') eq 'EmailAddress' ? 'LoadByEmail': 'Load';
+ $o->$method( $args{VALUE} );
+ $self->Limit(
+ %args,
+ FIELD => $column,
+ VALUE => $o->id,
+ );
+ return;
+ }
+ if ( $args{FIELD} and $args{FIELD} eq 'id' ) {
+ $self->Limit(
+ %args,
+ FIELD => $column,
+ );
+ return;
+ }
+ }
+ }
+
+ $args{FIELD} ||= 'EmailAddress';
+
+ my $groups = $self->_RoleGroupsJoin( Type => $type, Class => $class, New => !$type );
+
+ $self->_OpenParen( $args{SUBCLAUSE} ) if $args{SUBCLAUSE};
+ if ( $args{OPERATOR} =~ /^IS(?: NOT)?$/i ) {
+ # is [not] empty case
+
+ my $group_members = $self->_GroupMembersJoin( GroupsAlias => $groups );
+ # to avoid joining the table Users into the query, we just join GM
+ # and make sure we don't match records where group is member of itself
+ $self->Limit(
+ LEFTJOIN => $group_members,
+ FIELD => 'GroupId',
+ OPERATOR => '!=',
+ VALUE => "$group_members.MemberId",
+ QUOTEVALUE => 0,
+ );
+ $self->Limit(
+ %args,
+ ALIAS => $group_members,
+ FIELD => 'GroupId',
+ OPERATOR => $args{OPERATOR},
+ VALUE => $args{VALUE},
+ );
+ }
+ elsif ( $args{OPERATOR} =~ /^!=$|^NOT\s+/i ) {
+ # negative condition case
+
+ # reverse op
+ $args{OPERATOR} =~ s/!|NOT\s+//i;
+
+ # XXX: we have no way to build correct "Watcher.X != 'Y'" when condition
+ # "X = 'Y'" matches more then one user so we try to fetch two records and
+ # do the right thing when there is only one exist and semi-working solution
+ # otherwise.
+ my $users_obj = RT::Users->new( $self->CurrentUser );
+ $users_obj->Limit(
+ FIELD => $args{FIELD},
+ OPERATOR => $args{OPERATOR},
+ VALUE => $args{VALUE},
+ );
+ $users_obj->OrderBy;
+ $users_obj->RowsPerPage(2);
+ my @users = @{ $users_obj->ItemsArrayRef };
+
+ my $group_members = $self->_GroupMembersJoin( GroupsAlias => $groups );
+ if ( @users <= 1 ) {
+ my $uid = 0;
+ $uid = $users[0]->id if @users;
+ $self->Limit(
+ LEFTJOIN => $group_members,
+ ALIAS => $group_members,
+ FIELD => 'MemberId',
+ VALUE => $uid,
+ );
+ $self->Limit(
+ %args,
+ ALIAS => $group_members,
+ FIELD => 'id',
+ OPERATOR => 'IS',
+ VALUE => 'NULL',
+ );
+ } else {
+ $self->Limit(
+ LEFTJOIN => $group_members,
+ FIELD => 'GroupId',
+ OPERATOR => '!=',
+ VALUE => "$group_members.MemberId",
+ QUOTEVALUE => 0,
+ );
+ my $users = $self->Join(
+ TYPE => 'LEFT',
+ ALIAS1 => $group_members,
+ FIELD1 => 'MemberId',
+ TABLE2 => 'Users',
+ FIELD2 => 'id',
+ );
+ $self->Limit(
+ LEFTJOIN => $users,
+ ALIAS => $users,
+ FIELD => $args{FIELD},
+ OPERATOR => $args{OPERATOR},
+ VALUE => $args{VALUE},
+ CASESENSITIVE => 0,
+ );
+ $self->Limit(
+ %args,
+ ALIAS => $users,
+ FIELD => 'id',
+ OPERATOR => 'IS',
+ VALUE => 'NULL',
+ );
+ }
+ } else {
+ # positive condition case
+
+ my $group_members = $self->_GroupMembersJoin(
+ GroupsAlias => $groups, New => 1, Left => 0
+ );
+ my $users = $self->Join(
+ TYPE => 'LEFT',
+ ALIAS1 => $group_members,
+ FIELD1 => 'MemberId',
+ TABLE2 => 'Users',
+ FIELD2 => 'id',
+ );
+ $self->Limit(
+ %args,
+ ALIAS => $users,
+ FIELD => $args{FIELD},
+ OPERATOR => $args{OPERATOR},
+ VALUE => $args{VALUE},
+ CASESENSITIVE => 0,
+ );
+ }
+ $self->_CloseParen( $args{SUBCLAUSE} ) if $args{SUBCLAUSE};
+}
+
+1;
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index ef09167..abc27ca 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -83,6 +83,9 @@ use warnings;
use base 'RT::SearchBuilder';
+use Role::Basic 'with';
+with 'RT::Role::SearchBuilder::Roles';
+
use RT::Ticket;
sub Table { 'Tickets'}
@@ -899,250 +902,14 @@ sub _WatcherLimit {
die "Invalid watcher subfield: '$rest{SUBKEY}'";
}
- # Owner was ENUM field, so "Owner = 'xxx'" allowed user to
- # search by id and Name at the same time, this is workaround
- # to preserve backward compatibility
- if ( $field eq 'Owner' ) {
- if ( $op =~ /^!?=$/ && (!$rest{'SUBKEY'} || $rest{'SUBKEY'} eq 'Name' || $rest{'SUBKEY'} eq 'EmailAddress') ) {
- my $o = RT::User->new( $self->CurrentUser );
- my $method = ($rest{'SUBKEY'}||'') eq 'EmailAddress' ? 'LoadByEmail': 'Load';
- $o->$method( $value );
- $self->_SQLLimit(
- FIELD => 'Owner',
- OPERATOR => $op,
- VALUE => $o->id,
- %rest,
- );
- return;
- }
- if ( ($rest{'SUBKEY'}||'') eq 'id' ) {
- $self->_SQLLimit(
- FIELD => 'Owner',
- OPERATOR => $op,
- VALUE => $value,
- %rest,
- );
- return;
- }
- }
- $rest{SUBKEY} ||= 'EmailAddress';
-
- my $groups = $self->_RoleGroupsJoin( Type => $type, Class => $class, New => !$type );
-
- $self->_OpenParen;
- if ( $op =~ /^IS(?: NOT)?$/i ) {
- # is [not] empty case
-
- my $group_members = $self->_GroupMembersJoin( GroupsAlias => $groups );
- # to avoid joining the table Users into the query, we just join GM
- # and make sure we don't match records where group is member of itself
- $self->SUPER::Limit(
- LEFTJOIN => $group_members,
- FIELD => 'GroupId',
- OPERATOR => '!=',
- VALUE => "$group_members.MemberId",
- QUOTEVALUE => 0,
- );
- $self->_SQLLimit(
- ALIAS => $group_members,
- FIELD => 'GroupId',
- OPERATOR => $op,
- VALUE => $value,
- %rest,
- );
- }
- elsif ( $op =~ /^!=$|^NOT\s+/i ) {
- # negative condition case
-
- # reverse op
- $op =~ s/!|NOT\s+//i;
-
- # XXX: we have no way to build correct "Watcher.X != 'Y'" when condition
- # "X = 'Y'" matches more then one user so we try to fetch two records and
- # do the right thing when there is only one exist and semi-working solution
- # otherwise.
- my $users_obj = RT::Users->new( $self->CurrentUser );
- $users_obj->Limit(
- FIELD => $rest{SUBKEY},
- OPERATOR => $op,
- VALUE => $value,
- );
- $users_obj->OrderBy;
- $users_obj->RowsPerPage(2);
- my @users = @{ $users_obj->ItemsArrayRef };
-
- my $group_members = $self->_GroupMembersJoin( GroupsAlias => $groups );
- if ( @users <= 1 ) {
- my $uid = 0;
- $uid = $users[0]->id if @users;
- $self->SUPER::Limit(
- LEFTJOIN => $group_members,
- ALIAS => $group_members,
- FIELD => 'MemberId',
- VALUE => $uid,
- );
- $self->_SQLLimit(
- %rest,
- ALIAS => $group_members,
- FIELD => 'id',
- OPERATOR => 'IS',
- VALUE => 'NULL',
- );
- } else {
- $self->SUPER::Limit(
- LEFTJOIN => $group_members,
- FIELD => 'GroupId',
- OPERATOR => '!=',
- VALUE => "$group_members.MemberId",
- QUOTEVALUE => 0,
- );
- my $users = $self->Join(
- TYPE => 'LEFT',
- ALIAS1 => $group_members,
- FIELD1 => 'MemberId',
- TABLE2 => 'Users',
- FIELD2 => 'id',
- );
- $self->SUPER::Limit(
- LEFTJOIN => $users,
- ALIAS => $users,
- FIELD => $rest{SUBKEY},
- OPERATOR => $op,
- VALUE => $value,
- CASESENSITIVE => 0,
- );
- $self->_SQLLimit(
- %rest,
- ALIAS => $users,
- FIELD => 'id',
- OPERATOR => 'IS',
- VALUE => 'NULL',
- );
- }
- } else {
- # positive condition case
-
- my $group_members = $self->_GroupMembersJoin(
- GroupsAlias => $groups, New => 1, Left => 0
- );
- my $users = $self->Join(
- TYPE => 'LEFT',
- ALIAS1 => $group_members,
- FIELD1 => 'MemberId',
- TABLE2 => 'Users',
- FIELD2 => 'id',
- );
- $self->_SQLLimit(
- %rest,
- ALIAS => $users,
- FIELD => $rest{'SUBKEY'},
- VALUE => $value,
- OPERATOR => $op,
- CASESENSITIVE => 0,
- );
- }
- $self->_CloseParen;
-}
-
-sub _RoleGroupsJoin {
- my $self = shift;
- my %args = (New => 0, Class => 'Ticket', Type => '', @_);
- return $self->{'_sql_role_group_aliases'}{ $args{'Class'} .'-'. $args{'Type'} }
- if $self->{'_sql_role_group_aliases'}{ $args{'Class'} .'-'. $args{'Type'} }
- && !$args{'New'};
-
- # we always have watcher groups for ticket, so we use INNER join
- my $groups = $self->Join(
- ALIAS1 => 'main',
- FIELD1 => $args{'Class'} eq 'Queue'? 'Queue': 'id',
- TABLE2 => 'Groups',
- FIELD2 => 'Instance',
- ENTRYAGGREGATOR => 'AND',
- );
- $self->SUPER::Limit(
- LEFTJOIN => $groups,
- ALIAS => $groups,
- FIELD => 'Domain',
- VALUE => 'RT::'. $args{'Class'} .'-Role',
- );
- $self->SUPER::Limit(
- LEFTJOIN => $groups,
- ALIAS => $groups,
- FIELD => 'Type',
- VALUE => $args{'Type'},
- ) if $args{'Type'};
-
- $self->{'_sql_role_group_aliases'}{ $args{'Class'} .'-'. $args{'Type'} } = $groups
- unless $args{'New'};
-
- return $groups;
-}
-
-sub _GroupMembersJoin {
- my $self = shift;
- my %args = (New => 1, GroupsAlias => undef, Left => 1, @_);
-
- return $self->{'_sql_group_members_aliases'}{ $args{'GroupsAlias'} }
- if $self->{'_sql_group_members_aliases'}{ $args{'GroupsAlias'} }
- && !$args{'New'};
-
- my $alias = $self->Join(
- $args{'Left'} ? (TYPE => 'LEFT') : (),
- ALIAS1 => $args{'GroupsAlias'},
- FIELD1 => 'id',
- TABLE2 => 'CachedGroupMembers',
- FIELD2 => 'GroupId',
- ENTRYAGGREGATOR => 'AND',
- );
- $self->SUPER::Limit(
- $args{'Left'} ? (LEFTJOIN => $alias) : (),
- ALIAS => $alias,
- FIELD => 'Disabled',
- VALUE => 0,
- );
-
- $self->{'_sql_group_members_aliases'}{ $args{'GroupsAlias'} } = $alias
- unless $args{'New'};
-
- return $alias;
-}
-
-=head2 _WatcherJoin
-
-Helper function which provides joins to a watchers table both for limits
-and for ordering.
-
-=cut
-
-sub _WatcherJoin {
- my $self = shift;
- my $type = shift || '';
-
-
- my $groups = $self->_RoleGroupsJoin( Type => $type );
- my $group_members = $self->_GroupMembersJoin( GroupsAlias => $groups );
- # XXX: work around, we must hide groups that
- # are members of the role group we search in,
- # otherwise them result in wrong NULLs in Users
- # table and break ordering. Now, we know that
- # RT doesn't allow to add groups as members of the
- # ticket roles, so we just hide entries in CGM table
- # with MemberId == GroupId from results
- $self->SUPER::Limit(
- LEFTJOIN => $group_members,
- FIELD => 'GroupId',
- OPERATOR => '!=',
- VALUE => "$group_members.MemberId",
- QUOTEVALUE => 0,
- );
- my $users = $self->Join(
- TYPE => 'LEFT',
- ALIAS1 => $group_members,
- FIELD1 => 'MemberId',
- TABLE2 => 'Users',
- FIELD2 => 'id',
+ $self->RoleLimit(
+ TYPE => $type,
+ FIELD => $rest{SUBKEY},
+ OPERATOR => $op,
+ VALUE => $value,
+ SUBCLAUSE => "ticketsql",
+ %rest,
);
- return ($groups, $group_members, $users);
}
=head2 _WatcherMembershipLimit
@@ -1195,30 +962,13 @@ sub _WatcherMembershipLimit {
my $users = $self->NewAlias('Users');
my $memberships = $self->NewAlias('CachedGroupMembers');
- if ( ref $field ) { # gross hack
- my @bundle = @$field;
- $self->_OpenParen;
- for my $chunk (@bundle) {
- ( $field, $op, $value, @rest ) = @$chunk;
- $self->_SQLLimit(
- ALIAS => $memberships,
- FIELD => 'GroupId',
- VALUE => $value,
- OPERATOR => $op,
- @rest,
- );
- }
- $self->_CloseParen;
- }
- else {
- $self->_SQLLimit(
- ALIAS => $memberships,
- FIELD => 'GroupId',
- VALUE => $value,
- OPERATOR => $op,
- @rest,
- );
- }
+ $self->_SQLLimit(
+ ALIAS => $memberships,
+ FIELD => 'GroupId',
+ VALUE => $value,
+ OPERATOR => $op,
+ @rest,
+ );
# Tie to groups for tickets we care about
$self->_SQLLimit(
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list