[Rt-commit] rt branch, 4.6/membersobj-check-for-group, created. rt-4.4.4-541-gdf8bea7866

[Craig Kaiser]

The branch, 4.6/membersobj-check-for-group has been created
        at  df8bea78666819b9665fdc389a47ff6e05203656 (commit)

- Log -----------------------------------------------------------------
commit 7a9ad58b3c29239d520467888aadd06db26f3965
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Tue Nov 26 16:56:38 2019 -0500

    Check that group object is defined before trying to limit
    
    If RT::Group object is unloaded then the limit call for MembersObj will
    result in a sql error.

diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index 5dfc3fe7af..49949a1475 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -755,12 +755,17 @@ sub MembersObj {
     my $self = shift;
     my $members_obj = RT::GroupMembers->new( $self->CurrentUser );
 
-    #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 );
-
-    return ( $members_obj );
+    if ( $self->PrincipalId ) {
+        #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 );
 
+        return ( $members_obj );
+    }
+    else {
+        RT::Logger->error( "Can't call MembersObj on unloaded group, returning undef." );
+        return undef;
+    }
 }
 
 

commit df8bea78666819b9665fdc389a47ff6e05203656
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Mon Dec 2 15:20:21 2019 -0500

    Add test for calling MembersObj on unloaded group object

diff --git a/t/api/group.t b/t/api/group.t
index f82dfc57e9..2b3f31c1ca 100644
--- a/t/api/group.t
+++ b/t/api/group.t
@@ -3,7 +3,7 @@ use strict;
 use warnings;
 use RT;
 use RT::Test nodata => 1, tests => undef;
-
+use Test::Warn;
 
 {
 
@@ -116,4 +116,32 @@ is($u->PrincipalObj->PrincipalType , 'Group' , "Principal 4 is a group");
     ok( !$id, "can't create duplicated group even case is different: $msg" );
 }
 
+diag 'create a custom role and try to load role group before one has been created.';
+{
+    my $engineer = RT::CustomRole->new(RT->SystemUser);
+    my ($ok, $msg) = $engineer->Create(
+        Name      => 'Engineer',
+        MaxValues => 1,
+    );
+    ok($ok, "created role: $msg");
+
+    my $general = RT::Test->load_or_create_queue( Name => 'General' );
+
+    ($ok, $msg) = $engineer->AddToObject($general->id);
+    ok($ok, "added engineer to General: $msg");
+
+    ok(my $t = RT::Ticket->new(RT->SystemUser));
+    ok((my $id, $msg) = $t->Create( Queue => $general->Id,
+        Subject => 'Testing'
+    ));
+
+    my $group = RT::Group->new(RT->SystemUser);
+    $group->LoadRoleGroup(Object => $t, Name => 'Engineer');
+    is($group->Id, undef);
+
+    warning_like {
+        is($group->MembersObj, undef);
+    } qr/Can't call MembersObj on unloaded group, returning undef./, "Generates a warning about unloaded group";
+}
+
 done_testing;

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