[Rt-commit] rt branch, 4.4/membersobj-check-for-group, created. rt-4.4.4-83-g829e7b89ed

[Craig Kaiser]

The branch, 4.4/membersobj-check-for-group has been created
        at  829e7b89ed470782e5ca819e2819e69c0823180d (commit)

- Log -----------------------------------------------------------------
commit e5ba359d13c5ae77f6d102577f9bd5856b4bdbb6
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Wed Jan 29 12:15:39 2020 -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 19a41ef7a9..1d3d833522 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 829e7b89ed470782e5ca819e2819e69c0823180d
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Wed Jan 29 12:16:14 2020 -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;

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