[Rt-commit] rt branch, 4.4/reduce-cached-ticket-role-group-members, updated. rt-4.4.4-15-gb6b5e65bd

[? sunnavy]

The branch, 4.4/reduce-cached-ticket-role-group-members has been updated
       via  b6b5e65bd680742fa1b0b99f8f8ec92645c85d9e (commit)
       via  92d8f94ba7298f8bd73a1d0b27bc7d18e79a5d92 (commit)
      from  323c064975c440763b680725e4d8304416294c38 (commit)

Summary of changes:
 lib/RT/Group.pm             | 14 +++++++++++++-
 t/shredder/02group_member.t | 22 +++++++++++++++++++++-
 2 files changed, 34 insertions(+), 2 deletions(-)

- Log -----------------------------------------------------------------
commit 92d8f94ba7298f8bd73a1d0b27bc7d18e79a5d92
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Sat May 25 01:03:14 2019 +0800

    Don't shred subgroups' member relationships when shredding ticket role groups
    
    In 89c2d94f, we updated DeepMembersObj to return all members recursively
    for ticket role groups.  As ticket role groups are not directly
    connected to subgroups' members any more(see 9444ce14), we had to expand
    the search to cover subgroups' member relationships there.
    
    But to shred ticket role groups, we only want to shred their own direct
    relationships, instead of subgroups' ones. e.g. ticket 1's AdminCc role
    group has 2 direct members: user "alice" and group "managers", while
    group "managers" has user members "bob" and "richard". When we shred
    ticket 1, the AdminCc role group will be shredded, the following
    relationships need to be shredded too:
    
    * AdminCc role group <-> user "alice"
    * AdminCc role group <-> group "managers"
    
    But definitely not:
    
    * group "managers" <-> user "bob"
    * group "managers" <-> user "richard"

diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index bb0049708..0e4cb973d 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -1589,7 +1589,19 @@ sub __DependsOn {
     push( @$list, $objs );
 
 # Cached group members records
-    push( @$list, $self->DeepMembersObj );
+
+    if ( ( $self->Domain // '' ) eq 'RT::Ticket-Role' ) {
+
+        # For ticket role groups, do not delete subgroups' member
+        # relationships, as they are irrelevant here.
+
+        my $members_obj = RT::CachedGroupMembers->new( $self->CurrentUser );
+        $members_obj->LimitToMembersOfGroup( $self->PrincipalId );
+        push( @$list, $members_obj );
+    }
+    else {
+        push( @$list, $self->DeepMembersObj );
+    }
 
 # Cached group member records group belongs to
     $objs = RT::GroupMembers->new( $self->CurrentUser );

commit b6b5e65bd680742fa1b0b99f8f8ec92645c85d9e
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Sat May 25 01:26:12 2019 +0800

    Add tests to shred a ticket with groups in AdminCc

diff --git a/t/shredder/02group_member.t b/t/shredder/02group_member.t
index 692d361d3..9b9381de9 100644
--- a/t/shredder/02group_member.t
+++ b/t/shredder/02group_member.t
@@ -3,7 +3,7 @@ use strict;
 use warnings;
 
 use Test::Deep;
-use RT::Test::Shredder tests => 35;
+use RT::Test::Shredder tests => undef;
 my $test = "RT::Test::Shredder";
 
 ### nested membership check
@@ -146,3 +146,23 @@ my $test = "RT::Test::Shredder";
         is( $ticket->Owner, RT->Nobody->id, "owner switched back to nobody" );
         is( $ticket->OwnerGroup->MembersObj->First->MemberId, RT->Nobody->id, "and owner role group member is nobody");
 }
+
+diag "Shred a ticket with groups in AdminCc role group";
+{
+        $test->restore_savepoint('clean');
+
+        my $user = RT::Test->load_or_create_user( Name => 'alice' );
+        my $group = RT::Test->load_or_create_group( 'managers' );
+        my ( $status, $msg ) = $group->AddMember( $user->id );
+        ok( $status, "added user to group" ) or diag "error: $msg";
+
+        my $ticket = RT::Test->create_ticket( Queue => 'General', Subject => 'group requestor', AdminCc => $group->id );
+        $ticket->ApplyTransactionBatch;
+
+        my $shredder = $test->shredder_new();
+        $shredder->PutObjects( Objects => $ticket );
+        $shredder->WipeoutAll();
+        $test->db_is_valid;
+}
+
+done_testing;

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