[Rt-commit] rt branch, 4.4/reduce-cached-ticket-role-group-members, created. rt-4.4.4-13-g323c06497
? sunnavy
sunnavy at bestpractical.com
Fri Mar 15 16:44:59 EDT 2019
The branch, 4.4/reduce-cached-ticket-role-group-members has been created
at 323c064975c440763b680725e4d8304416294c38 (commit)
- Log -----------------------------------------------------------------
commit 9444ce1427bc9a110838a70d84682e93ef529d85
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat Mar 9 02:09:40 2019 +0800
Don't recursively add members to ticket role groups in CachedGroupMembers
CachedGroupMembers is used to quickly check if a user/group is a member
of another group, recursively. The table contains flat relationships
between groups and their members(including recurisve members), e.g.
- top
- sub
- user
In CachedGroupMembers, the rows are like(simplified and also excluded
self to self rows):
Group Member
top sub
sub user
top user
You can see there is a direct relationship between "top" to "user" in
CachedGroupMembers.
The performance becomes quite bad when processing updates of groups that
are in roles of a huge list of tickets. e.g.
- AdminCc(for ticket 1)
- group1
- AdminCc(for ticket 2)
- group1
- AdminCc(for ticket 3)
- group1
...
If we add a new member to "group1", we need to create "AdminCc <-> the
new member" rows in CachedGroupMembers for all the tickets above, which
could be quite time consuming: as the complexity is O(n), it will be
linearly slower as the ticket list grows.
To get around it, let's not create those rows in CachedGroupMembers and
tweak logic in RT instead.
diff --git a/lib/RT/CachedGroupMember.pm b/lib/RT/CachedGroupMember.pm
index c7c130937..f148e2bc1 100644
--- a/lib/RT/CachedGroupMember.pm
+++ b/lib/RT/CachedGroupMember.pm
@@ -156,7 +156,7 @@ sub Create {
return $id if $args{'Member'}->id == $args{'Group'}->id;
- if ( $args{'Member'}->IsGroup() ) {
+ if ( $args{'Group'}->Object->Domain ne 'RT::Ticket-Role' && $args{'Member'}->IsGroup() ) {
my $GroupMembers = $args{'Member'}->Object->MembersObj();
while ( my $member = $GroupMembers->Next() ) {
my $cached_member =
diff --git a/lib/RT/GroupMember.pm b/lib/RT/GroupMember.pm
index d7331aea5..b59acd625 100644
--- a/lib/RT/GroupMember.pm
+++ b/lib/RT/GroupMember.pm
@@ -123,6 +123,20 @@ sub _InsertCGM {
QUOTEVALUE => 0,
ENTRYAGGREGATOR => 'AND',
);
+ my $groups = $cgm->Join(
+ ALIAS1 => 'main',
+ FIELD1 => 'GroupId',
+ TABLE2 => 'Groups',
+ FIELD2 => 'id',
+ );
+ $cgm->Limit(
+ ALIAS => $groups,
+ FIELD => 'Domain',
+ OPERATOR => '!=',
+ VALUE => 'RT::Ticket-Role',
+ ENTRYAGGREGATOR => 'AND',
+ CASESENSITIVE => 0,
+ );
while ( my $parent_member = $cgm->Next ) {
my $parent_id = $parent_member->MemberId;
commit 89c2d94fee4b027174f6d975d96097915059544d
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat Mar 9 02:48:56 2019 +0800
Check direct group members in recursive member methods for ticket role groups
As we don't maintain a full flat relationship in CachedGroupMembers for
ticket role groups(in 9444ce14), we need to check all their direct group
members too, which have full flat relationships themselves.
Note Domain is set for all the rows, the expression of "$self->Domain //
''" is to eliminate uninitialized warnings for cases where $self hasn't
loaded a row yet(it happens in tests).
diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index 19a41ef7a..bb0049708 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -738,6 +738,12 @@ sub DeepMembersObj {
#If we don't have rights, don't include any results
# TODO XXX WHY IS THERE NO ACL CHECK HERE?
$members_obj->LimitToMembersOfGroup( $self->PrincipalId );
+ if ( ( $self->Domain // '' ) eq 'RT::Ticket-Role' ) {
+ my $groups = $self->GroupMembersObj( Recursively => 0 );
+ while ( my $group = $groups->Next ) {
+ $members_obj->LimitToMembersOfGroup( $group->PrincipalId );
+ }
+ }
return ( $members_obj );
@@ -790,9 +796,16 @@ sub GroupMembersObj {
ALIAS2 => $groups->PrincipalsAlias, FIELD2 => 'id',
);
$groups->Limit(
- ALIAS => $members_alias,
- FIELD => 'GroupId',
- VALUE => $self->PrincipalId,
+ ALIAS => $members_alias,
+ FIELD => 'GroupId',
+ $args{Recursively} && ( $self->Domain // '' ) eq 'RT::Ticket-Role'
+ ? ( OPERATOR => 'IN',
+ VALUE => [
+ $self->PrincipalId,
+ map { $_->PrincipalId } @{ $self->GroupMembersObj( Recursively => 0 )->ItemsArrayRef }
+ ]
+ )
+ : ( VALUE => $self->PrincipalId, )
);
$groups->Limit(
ALIAS => $members_alias,
@@ -832,7 +845,14 @@ sub UserMembersObj {
$users->Limit(
ALIAS => $members_alias,
FIELD => 'GroupId',
- VALUE => $self->PrincipalId,
+ $args{Recursively} && ( $self->Domain // '' ) eq 'RT::Ticket-Role'
+ ? ( OPERATOR => 'IN',
+ VALUE => [
+ $self->PrincipalId,
+ map { $_->PrincipalId } @{ $self->GroupMembersObj( Recursively => 0 )->ItemsArrayRef }
+ ]
+ )
+ : ( VALUE => $self->PrincipalId, )
);
$users->Limit(
ALIAS => $members_alias,
@@ -1109,9 +1129,14 @@ sub HasMemberRecursively {
if ( my $member_id = $member_obj->id ) {
return $member_id;
}
- else {
- return (undef);
+ elsif ( ( $self->Domain // '' ) eq 'RT::Ticket-Role' ) {
+ my $groups = $self->GroupMembersObj( Recursively => 0 );
+ while ( my $group = $groups->Next ) {
+ my $ret = $group->HasMemberRecursively($principal);
+ return $ret if $ret;
+ }
}
+ return (undef);
}
commit f26dafbd0cb5bec312b4d5e86badf6f51d1562c2
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat Mar 9 03:01:37 2019 +0800
Check direct group members in rights check for ticket role groups
It covers both ticket search and rights check on a single ticket.
See also 9444ce14 and 89c2d94f
diff --git a/lib/RT/Principal.pm b/lib/RT/Principal.pm
index aec96f7a2..f5c395588 100644
--- a/lib/RT/Principal.pm
+++ b/lib/RT/Principal.pm
@@ -574,6 +574,10 @@ sub _HasRoleRightQuery {
@_
);
+ my $groups = RT::Groups->new( RT->SystemUser );
+ $groups->LimitToUserDefinedGroups;
+ $groups->WithMember( PrincipalId => $self->id, Recursively => 1 );
+
my $query =
" FROM Groups, Principals, CachedGroupMembers WHERE "
@@ -589,7 +593,7 @@ sub _HasRoleRightQuery {
# also, check to see if the right is being granted _directly_ to this principal,
# as is the case when we want to look up group rights
. "AND Principals.id = CachedGroupMembers.GroupId "
- . "AND CachedGroupMembers.MemberId = " . $self->Id . " "
+ . "AND CachedGroupMembers.MemberId IN (" . ( join ',', $self->Id, map { $_->id } @{ $groups->ItemsArrayRef } ) . ") "
;
if ( $args{'Roles'} ) {
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index 5dcd50f07..3cccf2048 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -2693,11 +2693,16 @@ sub CurrentUserCanSee {
if ( $join_roles ) {
$role_group_alias = $self->_RoleGroupsJoin( New => 1 );
$cgm_alias = $self->_GroupMembersJoin( GroupsAlias => $role_group_alias );
+
+ my $groups = RT::Groups->new( RT->SystemUser );
+ $groups->LimitToUserDefinedGroups;
+ $groups->WithMember( PrincipalId => $id, Recursively => 1 );
+
$self->Limit(
LEFTJOIN => $cgm_alias,
FIELD => 'MemberId',
- OPERATOR => '=',
- VALUE => $id,
+ OPERATOR => 'IN',
+ VALUE => [ $id, map { $_->id } @{ $groups->ItemsArrayRef } ],
);
}
my $limit_queues = sub {
commit 987966888e536c38205a85b003a8ea5e9c969d75
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat Mar 9 03:11:50 2019 +0800
Exclude ticket role groups for recursive validity check in CachedGroupMembers
See also 9444ce14
diff --git a/sbin/rt-validator.in b/sbin/rt-validator.in
index 33767827f..51479bd21 100644
--- a/sbin/rt-validator.in
+++ b/sbin/rt-validator.in
@@ -573,9 +573,13 @@ FROM
AND cgm3.MemberId = gm2.MemberId
AND cgm3.Via = cgm1.id
AND cgm3.ImmediateParentId = cgm1.MemberId )
+ LEFT JOIN Groups g ON (
+ cgm1.GroupId = g.id
+ )
WHERE cgm1.GroupId != cgm1.MemberId
AND gm2.GroupId = cgm1.MemberId
AND cgm3.id IS NULL
+AND g.Domain != 'RT::Ticket-Role'
END
my $action = sub {
commit 2509f50a93cbbcc5ce060d9996e90459a51152e2
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat Mar 9 04:56:22 2019 +0800
Update shrink-cgm-table to remove indirect members of ticket role groups
diff --git a/etc/upgrade/shrink-cgm-table.in b/etc/upgrade/shrink-cgm-table.in
index 8cd8ce2ae..ffe4fba4c 100644
--- a/etc/upgrade/shrink-cgm-table.in
+++ b/etc/upgrade/shrink-cgm-table.in
@@ -90,20 +90,37 @@ $cgms->Limit(
ENTRYAGGREGATOR => 'AND',
);
-my $total = $cgms->Count;
+my $cgms2 = RT::CachedGroupMembers->new( RT->SystemUser );
+$cgms2->Limit( FIELD => 'ImmediateParentId', VALUE => 'main.GroupId', OPERATOR => '!=', QUOTEVALUE => 0 );
+my $groups = $cgms2->Join(
+ ALIAS1 => 'main',
+ FIELD1 => 'GroupId',
+ TABLE2 => 'Groups',
+ FIELD2 => 'id'
+);
+$cgms2->Limit(
+ ALIAS => $groups,
+ FIELD => 'Domain',
+ VALUE => 'RT::Ticket-Role',
+ CASESENSITIVE => 0,
+);
+
+my $total = $cgms->Count + $cgms2->Count;
my $i = 0;
-FetchNext( $cgms, 'init' );
-while ( my $rec = FetchNext( $cgms ) ) {
- $i++;
- printf("\r%0.2f %%", 100 * $i / $total);
- $RT::Handle->BeginTransaction;
- my ($status) = $rec->Delete;
- unless ($status) {
- $RT::Logger->error( "Couldn't delete CGM #". $rec->id );
- exit 1;
+for my $cgms ( $cgms, $cgms2 ) {
+ FetchNext( $cgms, 'init' );
+ while ( my $rec = FetchNext($cgms) ) {
+ $i++;
+ printf( "\r%0.2f %%", 100 * $i / $total );
+ $RT::Handle->BeginTransaction;
+ my ($status) = $rec->Delete;
+ unless ($status) {
+ $RT::Logger->error( "Couldn't delete CGM #" . $rec->id );
+ exit 1;
+ }
+ $RT::Handle->Commit;
}
- $RT::Handle->Commit;
}
use constant PAGE_SIZE => 10000;
commit 6974cf0c934a1526f966c7ba729a5db0804a1340
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat Mar 16 01:00:43 2019 +0800
Check role's direct group members for ticket watcher searches
For queries like "AdminCc.Name = 'foo'", the change is straightforward
as we can find all the user defined groups of user "foo" and search them
too.
For queries like "AdminCc.Name LIKE 'foo'" when there are multiple users
whose names contain "foo", it could logically work if we iterate all
those users and then search them using the above "=" strategy, which is
equivalent to "AdminCc.Name = 'foobar' OR AdminCc.Name = 'foobaz' OR
...", but I bet the performance will be bad as user list grows.
For criteria involving multiple users, previous code did a joined SQL of
CGMs and Users. As users might not be linked to role groups directly in
CGMs, "CGMs <-> Users" join is not enough, instead of checking if user
is a member of role group, we need another CGMs join to test if user is
a member of role group or its direct member groups.
Note that as role groups are treated as their own members in CGMs, we
can technially only test if user is a member of role group's direct
group members.
i.e. from
CGMs(MemberId) <-> Users(id)
to
CGMs(MemberId) <-> CGMs_2(GroupId)|CGMs_2(MemberId) <-> Users(id)
For queries like "AdminCc.Name NOT LIKE 'foo'" when there are multiple
users whose names contain "foo", previous behavior is to return tickets
which have at least a member in AdminCc that doesn't have name matching
"foo". i.e. if a ticket's AdminCc contains users "foo" and "bar", then
"AdminCc.Name NOT LIKE 'foo'" will still return that ticket. Because of
this behavior, there is no need to adjust code here and it will work as
before: if a ticket role contains user defined groups, then that ticket
will always return.
This was commented as "semi-working solution" and I'm afraid it'll be
quite hard if possible to properly correct it to make it totally
opposite of "LIKE". One workaround I can think of is to get user ids
first, then search them using "!=" instead, i.e. something like
"AdminCc.id != 2 AND AdminCc.id != 3 AND ...", which deserves another
branch, so it's not covered here.
See also 9444ce14
diff --git a/lib/RT/SearchBuilder/Role/Roles.pm b/lib/RT/SearchBuilder/Role/Roles.pm
index 5980882d3..d7a7d110b 100644
--- a/lib/RT/SearchBuilder/Role/Roles.pm
+++ b/lib/RT/SearchBuilder/Role/Roles.pm
@@ -312,11 +312,21 @@ sub RoleLimit {
if ( @users <= 1 ) {
my $uid = 0;
$uid = $users[0]->id if @users;
+
+ my @ids;
+ {
+ my $groups = RT::Groups->new( RT->SystemUser );
+ $groups->LimitToUserDefinedGroups;
+ $groups->WithMember( PrincipalId => $uid, Recursively => 1 );
+ @ids = ( $uid, map { $_->id } @{ $groups->ItemsArrayRef } );
+ }
+
$self->Limit(
LEFTJOIN => $group_members,
ALIAS => $group_members,
FIELD => 'MemberId',
- VALUE => $uid,
+ VALUE => \@ids,
+ OPERATOR => 'IN',
);
$self->Limit(
%args,
@@ -363,19 +373,43 @@ sub RoleLimit {
GroupsAlias => $groups, New => 1, Left => 0
);
if ($args{FIELD} eq "id") {
+ my @ids;
+ {
+ my $groups = RT::Groups->new( RT->SystemUser );
+ $groups->LimitToUserDefinedGroups;
+ $groups->WithMember( PrincipalId => $args{VALUE}, Recursively => 1 );
+ @ids = ( $args{VALUE}, map { $_->id } @{ $groups->ItemsArrayRef } );
+ }
+
# Save a left join to Users, if possible
$self->Limit(
%args,
ALIAS => $group_members,
FIELD => "MemberId",
- OPERATOR => $args{OPERATOR},
- VALUE => $args{VALUE},
+ OPERATOR => 'IN',
+ VALUE => \@ids,
CASESENSITIVE => 0,
);
} else {
+
+ my $cgm_2 = $self->NewAlias('CachedGroupMembers');
+ my $group_members_2 = $self->Join(
+ ALIAS1 => $group_members,
+ FIELD1 => 'MemberId',
+ ALIAS2 => $cgm_2,
+ FIELD2 => 'GroupId',
+ );
+ $self->Limit(
+ LEFTJOIN => $group_members_2,
+ ALIAS => $cgm_2,
+ FIELD => 'Disabled',
+ VALUE => 0,
+ ENTRYAGGREGATOR => 'AND',
+ );
+
$users ||= $self->Join(
TYPE => 'LEFT',
- ALIAS1 => $group_members,
+ ALIAS1 => $group_members_2,
FIELD1 => 'MemberId',
TABLE2 => 'Users',
FIELD2 => 'id',
commit 8b7c241b618b1e4a6b317b14bf2243513e769609
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat Mar 16 01:05:19 2019 +0800
Check role's direct group members for ticket watcher group searches
For queries like "AdminCcGroup = 'foo'", we need to check if there is a
member in a ticket's AdminCc role that is in group "foo" or not.
Previously we did the joined search like:
CGMs(MemberId) <-> Users(id) <-> CGMs_2(MemberId)
And check GroupId of CGMs_2 agaist the id of group "foo".
As users might not be linked to role groups directly in CGMs, the first
"CGMs <-> Users" join is not enough, we need another CGMs join to test
if user is a member of role group's direct member groups, i.e.
CGMs(MemberId) <-> CGMs_2(GroupId)|CGMs_2(MemberId) <-> Users(id) <-> CGMs_3(MemberId)
And then check GroupId of CGMs_3 agaist the id of group "foo".
See also 9444ce14 and 6974cf0c
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index 3cccf2048..da1f3a294 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -1161,34 +1161,81 @@ sub _WatcherMembershipLimit {
my $meta = $FIELD_METADATA{$field};
my $type = $meta->[1] || '';
- my ($members_alias, $members_column);
if ( $type eq 'Owner' ) {
- ($members_alias, $members_column) = ('main', 'Owner');
- } else {
- (undef, undef, $members_alias) = $self->_WatcherJoin( New => 1, Name => $type );
- $members_column = 'id';
+ my $cgm_alias = $self->Join(
+ ALIAS1 => 'main',
+ FIELD1 => 'Owner',
+ TABLE2 => 'CachedGroupMembers',
+ FIELD2 => 'MemberId',
+ );
+ $self->Limit(
+ ALIAS => $cgm_alias,
+ FIELD => 'GroupId',
+ VALUE => $value,
+ OPERATOR => $op,
+ %rest,
+ );
}
+ else {
+ my $groups = $self->_RoleGroupsJoin( Name => $type, Class => $self->_RoleGroupClass, New => 1 );
+ my $group_members = $self->_GroupMembersJoin( GroupsAlias => $groups );
- my $cgm_alias = $self->Join(
- ALIAS1 => $members_alias,
- FIELD1 => $members_column,
- TABLE2 => 'CachedGroupMembers',
- FIELD2 => 'MemberId',
- );
- $self->Limit(
- LEFTJOIN => $cgm_alias,
- ALIAS => $cgm_alias,
- FIELD => 'Disabled',
- VALUE => 0,
- );
+ my $cgm = $self->NewAlias('CachedGroupMembers');
+ my $group_members_2 = $self->Join(
+ TYPE => 'LEFT',
+ ALIAS1 => $group_members,
+ FIELD1 => 'MemberId',
+ ALIAS2 => $cgm,
+ FIELD2 => 'GroupId',
+ );
+ $self->Limit(
+ LEFTJOIN => $group_members_2,
+ FIELD => 'GroupId',
+ OPERATOR => '!=',
+ VALUE => "$group_members_2.MemberId",
+ QUOTEVALUE => 0,
+ );
- $self->Limit(
- ALIAS => $cgm_alias,
- FIELD => 'GroupId',
- VALUE => $value,
- OPERATOR => $op,
- %rest,
- );
+ $self->Limit(
+ LEFTJOIN => $group_members_2,
+ ALIAS => $cgm,
+ FIELD => 'Disabled',
+ VALUE => 0,
+ ENTRYAGGREGATOR => 'AND',
+ );
+
+ my $users = $self->Join(
+ TYPE => 'LEFT',
+ ALIAS1 => $group_members_2,
+ FIELD1 => 'MemberId',
+ TABLE2 => 'Users',
+ FIELD2 => 'id',
+ );
+
+ my $cgm_2 = $self->NewAlias('CachedGroupMembers');
+ my $group_members_3 = $self->Join(
+ TYPE => 'LEFT',
+ ALIAS1 => $users,
+ FIELD1 => 'id',
+ ALIAS2 => $cgm_2,
+ FIELD2 => 'MemberId',
+ );
+
+ $self->Limit(
+ LEFTJOIN => $cgm_2,
+ ALIAS => $cgm_2,
+ FIELD => 'Disabled',
+ VALUE => 0,
+ );
+
+ $self->Limit(
+ ALIAS => $group_members_3,
+ FIELD => 'GroupId',
+ VALUE => $value,
+ OPERATOR => $op,
+ %rest,
+ );
+ }
}
=head2 _CustomFieldDecipher
commit 323c064975c440763b680725e4d8304416294c38
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat Mar 9 05:44:28 2019 +0800
Add tests for CGM changes of ticket role groups
As we no longer create indirect member rows in CachedGroupMembers for
ticket role groups, here are the tests to confirm RT still behaves
correctly.
diff --git a/t/api/group.t b/t/api/group.t
index f82dfc57e..7ec3a341e 100644
--- a/t/api/group.t
+++ b/t/api/group.t
@@ -116,4 +116,69 @@ is($u->PrincipalObj->PrincipalType , 'Group' , "Principal 4 is a group");
ok( !$id, "can't create duplicated group even case is different: $msg" );
}
+diag "Ticket role group members";
+{
+ RT::Test->load_or_create_queue( Name => 'General' );
+ my $ticket = RT::Test->create_ticket( Queue => 'General', Subject => 'test ticket role group' );
+ my $admincc = $ticket->RoleGroup('AdminCc');
+ my $delegates = RT::Test->load_or_create_group('delegates');
+ my $core = RT::Test->load_or_create_group('core team');
+ my $alice = RT::Test->load_or_create_user( Name => 'alice' );
+ my $bob = RT::Test->load_or_create_user( Name => 'bob' );
+
+ ok( $admincc->AddMember( $delegates->PrincipalId ), 'Add delegates to AdminCc' );
+ ok( $delegates->AddMember( $core->PrincipalId ), 'Add core team to delegates' );
+ ok( $delegates->AddMember( $bob->PrincipalId ), 'Add bob to delegates' );
+ ok( $core->AddMember( $alice->PrincipalId ), 'Add alice to core team' );
+
+ ok( $admincc->HasMember( $delegates->PrincipalId ), 'AdminCc has direct member of delegates' );
+ ok( !$admincc->HasMember( $core->PrincipalId ), "AdminCc doesn't have member of core" );
+ ok( !$admincc->HasMember( $bob->PrincipalId ), "AdminCc doesn't have member of bob" );
+ ok( !$admincc->HasMember( $alice->PrincipalId ), "AdminCc doesn't have member of bob" );
+ ok( $admincc->HasMemberRecursively( $core->PrincipalId ), "AdminCc recursively has member of core" );
+ ok( $admincc->HasMemberRecursively( $bob->PrincipalId ), "AdminCc recursively has member of bob" );
+ ok( $admincc->HasMemberRecursively( $alice->PrincipalId ), "AdminCc recursively has member of alice" );
+
+ my $CGM = RT::CachedGroupMember->new( RT->SystemUser );
+ $CGM->LoadByCols( GroupId => $admincc->PrincipalId, MemberId => $delegates->PrincipalId );
+ ok( $CGM->id, 'CGM record for admincc <-> delegates' );
+
+ $CGM->LoadByCols( GroupId => $delegates->PrincipalId, MemberId => $core->PrincipalId );
+ ok( $CGM->id, 'CGM record for delegates <-> core' );
+
+ $CGM->LoadByCols( GroupId => $core->PrincipalId, MemberId => $alice->PrincipalId );
+ ok( $CGM->id, 'CGM record for core <-> alice' );
+
+ $CGM->LoadByCols( GroupId => $delegates->PrincipalId, MemberId => $alice->PrincipalId );
+ ok( $CGM->id, 'CGM record for delegates <-> alice' );
+
+ $CGM->LoadByCols( GroupId => $admincc->PrincipalId, MemberId => $core->PrincipalId );
+ ok( !$CGM->id, 'No CGM record for admincc <-> core' );
+
+ $CGM->LoadByCols( GroupId => $admincc->PrincipalId, MemberId => $alice->PrincipalId );
+ ok( !$CGM->id, 'No CGM record for admincc <-> alice' );
+
+ $CGM->LoadByCols( GroupId => $admincc->PrincipalId, MemberId => $bob->PrincipalId );
+ ok( !$CGM->id, 'No CGM record for admincc <-> bob' );
+
+ ok( $admincc->DeleteMember( $delegates->PrincipalId ), 'Delete delegates from AdminCc' );
+
+ $CGM->LoadByCols( GroupId => $admincc->PrincipalId, MemberId => $delegates->PrincipalId );
+ ok( !$CGM->id, 'No CGM record for admincc <-> delegates' );
+
+ ok( $admincc->AddMember( $delegates->PrincipalId ), 'Add delegates to AdminCc again' );
+
+ $CGM->LoadByCols( GroupId => $admincc->PrincipalId, MemberId => $delegates->PrincipalId );
+ ok( $CGM->id, 'CGM record for admincc <-> delegates again' );
+
+ $CGM->LoadByCols( GroupId => $admincc->PrincipalId, MemberId => $core->PrincipalId );
+ ok( !$CGM->id, 'No CGM record for admincc <-> corei still' );
+
+ $CGM->LoadByCols( GroupId => $admincc->PrincipalId, MemberId => $alice->PrincipalId );
+ ok( !$CGM->id, 'No CGM record for admincc <-> alice still' );
+
+ $CGM->LoadByCols( GroupId => $admincc->PrincipalId, MemberId => $bob->PrincipalId );
+ ok( !$CGM->id, 'No CGM record for admincc <-> bob still' );
+}
+
done_testing;
diff --git a/t/api/rights.t b/t/api/rights.t
index a6346a737..def221dc0 100644
--- a/t/api/rights.t
+++ b/t/api/rights.t
@@ -1,4 +1,4 @@
-use RT::Test nodata => 1, tests => 38;
+use RT::Test nodata => 1, tests => undef;
use strict;
use warnings;
@@ -174,3 +174,81 @@ note "Right name canonicalization";
ok $ok, "Granted ShowTicket: $msg";
ok $user->HasRight( Right => "showticket", Object => RT->System ), "HasRight showticket";
}
+
+diag "Ticket role rights for users in groups that are added in ticket roles";
+{
+ RT::Test->load_or_create_queue( Name => 'General' );
+ my $ticket = RT::Test->create_ticket( Queue => 'General', Subject => 'test ticket role group' );
+ my $admincc = $ticket->RoleGroup('AdminCc');
+ ok( $admincc->PrincipalObj->GrantRight( Right => 'ShowTicket', Object => $ticket->QueueObj ),
+ 'Grant AdminCc ShowTicket right' );
+
+ my $delegates = RT::Test->load_or_create_group('delegates');
+ my $core = RT::Test->load_or_create_group('core team');
+ my $alice = RT::Test->load_or_create_user( Name => 'alice' );
+ my $bob = RT::Test->load_or_create_user( Name => 'bob' );
+ ok( $delegates->AddMember( $core->PrincipalId ), 'Add core team to delegates' );
+ ok( $delegates->AddMember( $bob->PrincipalId ), 'Add bob to delegates' );
+ ok( $core->AddMember( $alice->PrincipalId ), 'Add alice to core team' );
+
+ my $current_alice = RT::CurrentUser->new( RT->SystemUser );
+ $current_alice->Load( $alice->id );
+ my $current_bob = RT::CurrentUser->new( RT->SystemUser );
+ $current_bob->Load( $bob->id );
+
+ for my $current_user ( $current_alice, $current_bob ) {
+ ok( !$current_user->HasRight( Object => $ticket, Right => 'ShowTicket' ),
+ 'No ShowTicket right for ' . $current_user->Name );
+ my $tickets = RT::Tickets->new($current_user);
+ $tickets->FromSQL("Subject = 'test ticket role group'");
+ ok( !$tickets->Count, 'No tickets found for user ' . $current_user->Name );
+ }
+
+ ok( $admincc->AddMember( $delegates->PrincipalId ), 'Add delegates to AdminCc' );
+
+ for my $current_user ( $current_alice, $current_bob ) {
+ ok( $current_user->HasRight( Object => $ticket, Right => 'ShowTicket' ),
+ 'Has ShowTicket right for ' . $current_user->Name );
+ my $tickets = RT::Tickets->new($current_user);
+ $tickets->FromSQL("Subject = 'test ticket role group'");
+ is( $tickets->Count, 1, 'Found 1 ticket for ' . $current_user->Name );
+ is( $tickets->First->id, $ticket->id, 'Found the ticket for ' . $current_user->Name );
+ }
+
+ ok( $admincc->DeleteMember( $delegates->PrincipalId ), 'Delete delegates from AdminCc' );
+ for my $current_user ( $current_alice, $current_bob ) {
+ ok( !$current_user->HasRight( Object => $ticket, Right => 'ShowTicket' ),
+ 'No ShowTicket right any more for ' . $current_user->Name
+ );
+ my $tickets = RT::Tickets->new($current_user);
+ $tickets->FromSQL("Subject = 'test ticket role group'");
+ ok( !$tickets->Count, 'No tickets found any more for ' . $current_user->Name );
+ }
+
+
+ ok( $admincc->AddMember( $delegates->PrincipalId ), 'Add delegates to AdminCc again' );
+
+ for my $current_user ( $current_alice, $current_bob ) {
+ ok( $current_user->HasRight( Object => $ticket, Right => 'ShowTicket' ),
+ 'Has ShowTicket right again for ' . $current_user->Name
+ );
+ my $tickets = RT::Tickets->new($current_user);
+ $tickets->FromSQL("Subject = 'test ticket role group'");
+ is( $tickets->Count, 1, 'Found 1 ticket again for ' . $current_user->Name );
+ is( $tickets->First->id, $ticket->id, 'Found the ticket again for ' . $current_user->Name );
+ }
+
+ ok( $admincc->PrincipalObj->RevokeRight( Right => 'ShowTicket', Object => $ticket->QueueObj ),
+ 'Revoke AdminCc ShowTicket right' );
+
+ for my $current_user ( $current_alice, $current_bob ) {
+ ok( !$current_user->HasRight( Object => $ticket, Right => 'ShowTicket' ),
+ 'No ShowTicket right any more for ' . $current_user->Name
+ );
+ my $tickets = RT::Tickets->new($current_user);
+ $tickets->FromSQL("Subject = 'test ticket role group'");
+ ok( !$tickets->Count, 'No tickets found any more for ' . $current_user->Name );
+ }
+}
+
+done_testing;
diff --git a/t/api/tickets.t b/t/api/tickets.t
index 407981baa..facaefa7d 100644
--- a/t/api/tickets.t
+++ b/t/api/tickets.t
@@ -161,4 +161,83 @@ ok( $unlimittickets->Count > 0, "UnLimited tickets object should return tickets"
);
}
+diag "Ticket role group members";
+{
+ my $ticket = RT::Test->create_ticket( Queue => 'General', Subject => 'test ticket role group' );
+ my $admincc = $ticket->RoleGroup('AdminCc');
+
+ my $delegates = RT::Test->load_or_create_group('delegates');
+ my $core = RT::Test->load_or_create_group('core team');
+ my $alice = RT::Test->load_or_create_user( Name => 'alice' );
+ my $bob = RT::Test->load_or_create_user( Name => 'bob' );
+ ok( $delegates->AddMember( $core->PrincipalId ), 'Add core team to delegates' );
+ ok( $delegates->AddMember( $bob->PrincipalId ), 'Add bob to delegates' );
+ ok( $core->AddMember( $alice->PrincipalId ), 'Add alice to core team' );
+
+ for my $name ( 'alice', 'bob' ) {
+ my $tickets = RT::Tickets->new( RT->SystemUser );
+ $tickets->FromSQL("Subject = 'test ticket role group' AND AdminCc.Name = '$name'");
+ ok( !$tickets->Count, 'No tickets found' );
+
+ $tickets->FromSQL("Subject = 'test ticket role group' AND AdminCc.Name != '$name'");
+ is( $tickets->Count, 1, 'Found 1 ticket' );
+ is( $tickets->First->id, $ticket->id, 'Found the ticket' );
+
+ $tickets->FromSQL("Subject = 'test ticket role group' AND AdminCc.Name LIKE '$name'");
+ ok( !$tickets->Count, 'No tickets found' );
+
+ $tickets->FromSQL("Subject = 'test ticket role group' AND AdminCc.Name NOT LIKE '$name'");
+ is( $tickets->Count, 1, 'Found 1 ticket' );
+ is( $tickets->First->id, $ticket->id, 'Found the ticket' );
+ }
+
+ ok( $admincc->AddMember( $delegates->PrincipalId ), 'Add delegates to AdminCc' );
+
+ for my $name ( 'alice', 'bob' ) {
+ my $tickets = RT::Tickets->new( RT->SystemUser );
+ $tickets->FromSQL("Subject = 'test ticket role group' AND AdminCc.Name = '$name'");
+ is( $tickets->Count, 1, 'Found 1 ticket' );
+ is( $tickets->First->id, $ticket->id, 'Found the ticket' );
+
+ $tickets->FromSQL("Subject = 'test ticket role group' AND AdminCc.Name != '$name'");
+ ok( !$tickets->Count, 'No tickets found' );
+
+ $tickets->FromSQL("Subject = 'test ticket role group' AND AdminCc.Name LIKE '$name'");
+ is( $tickets->Count, 1, 'Found 1 ticket' );
+ is( $tickets->First->id, $ticket->id, 'Found the ticket' );
+
+ $tickets->FromSQL("Subject = 'test ticket role group' AND AdminCc.Name NOT LIKE '$name'");
+ ok( !$tickets->Count, 'No tickets found' );
+ }
+
+ my $abc = RT::Test->load_or_create_user( Name => 'abc' ); # so there are multiple users to search
+ my $abc_ticket = RT::Test->create_ticket( Queue => 'General', Subject => 'test ticket role group' );
+ ok( $abc_ticket->RoleGroup('AdminCc')->AddMember( $abc->PrincipalId ), 'Add abc to AdminCc' );
+
+ my $tickets = RT::Tickets->new( RT->SystemUser );
+ $tickets->FromSQL("Subject = 'test ticket role group' AND AdminCc.Name LIKE 'a'");
+ is( $tickets->Count, 2, 'Found 2 ticket' );
+
+ $tickets->FromSQL("Subject = 'test ticket role group' AND AdminCc.Name NOT LIKE 'a'");
+ TODO: {
+ local $TODO = <<EOF;
+Searching NOT LIKE with multiple users is not the opposite of "LIKE", e.g.
+
+ "alice", "bob" are AdminCcs of ticket 1, abc is AdminCc of ticket 2:
+ "AdminCc.Name LIKE 'a'" returns tickets 1 and 2.
+ "AdminCc.Name NOT LIKE 'a'" returns ticket 1 because it has AdminCc "bob" which doesn't match "a".
+
+EOF
+ ok( !$tickets->Count, 'No tickets found' );
+ }
+ if ( $tickets->Count ) {
+ is( $tickets->Count, 1, 'Found 1 ticket' );
+ is( $tickets->First->id, $ticket->id, 'Found the ticket' );
+ }
+
+ $tickets->FromSQL("Subject = 'test ticket role group' AND AdminCcGroup = 'delegates'");
+ is( $tickets->Count, 1, 'Found 1 ticket' );
+ is( $tickets->First->id, $ticket->id, 'Found the ticket' );
+}
+
done_testing;
diff --git a/t/validator/group_members.t b/t/validator/group_members.t
index 0fd1a749a..ada0815a1 100644
--- a/t/validator/group_members.t
+++ b/t/validator/group_members.t
@@ -125,4 +125,21 @@ RT::Test->db_is_valid;
RT::Test->db_is_valid;
}
+diag "CGM recurisve check for ticket role groups";
+{
+ my $ticket = RT::Test->create_ticket( Queue => 'General', Subject => 'test ticket role group' );
+ my $admincc = $ticket->RoleGroup('AdminCc');
+ my $delegates = RT::Test->load_or_create_group('delegates');
+ my $core = RT::Test->load_or_create_group('core team');
+ my $alice = RT::Test->load_or_create_user( Name => 'alice' );
+ my $bob = RT::Test->load_or_create_user( Name => 'bob' );
+
+ ok( $admincc->AddMember( $delegates->PrincipalId ), 'Add delegates to AdminCc' );
+ ok( $delegates->AddMember( $core->PrincipalId ), 'Add core team to delegates' );
+ ok( $delegates->AddMember( $bob->PrincipalId ), 'Add bob to delegates' );
+ ok( $core->AddMember( $alice->PrincipalId ), 'Add alice to core team' );
+
+ RT::Test->db_is_valid;
+}
+
done_testing;
-----------------------------------------------------------------------
More information about the rt-commit
mailing list