[Rt-commit] rt branch 5.0/rest2-custom-roles-keys created. rt-5.0.2-225-g9ec3f553ef

BPS Git Server git at git.bestpractical.com
Wed May 18 16:15:55 UTC 2022


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".

The branch, 5.0/rest2-custom-roles-keys has been created
        at  9ec3f553efbbea4b40cde56c78051f760a286586 (commit)

- Log -----------------------------------------------------------------
commit 9ec3f553efbbea4b40cde56c78051f760a286586
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Wed May 18 20:43:18 2022 +0800

    Update REST2 doc to prefer "CustomRoles" top key for ticket search
    
    Fields like "RT::CustomRole-1" is still supported for back
    compatibility, but using "CustomRoles" is more pragmatic, and also
    consistent with other examples that create/update tickets.

diff --git a/lib/RT/REST2.pm b/lib/RT/REST2.pm
index 1f4c8e4bba..b1ac6df014 100644
--- a/lib/RT/REST2.pm
+++ b/lib/RT/REST2.pm
@@ -1305,7 +1305,7 @@ You can use additional fields parameters to expand child blocks, for
 example (line wrapping inserted for readability):
 
     XX_RT_URL_XX/REST/2.0/tickets
-      ?fields=Owner,Status,Created,Subject,Queue,CustomFields,Requestor,Cc,AdminCc,RT::CustomRole-1
+      ?fields=Owner,Status,Created,Subject,Queue,CustomFields,Requestor,Cc,AdminCc,CustomRoles
       &fields[Queue]=Name,Description
 
 Says that in the result set for tickets, the extra fields for Owner, Status,
@@ -1360,13 +1360,13 @@ ticket is displayed in this example):
             }
          ],
          "AdminCc" : [],
-         "RT::CustomRole-1" : [
-            {
+         "CustomRoles" : {
+            "Manager": {
                "_url" : "XX_RT_URL_XX/REST/2.0/user/foo at example.com",
                "type" : "user",
                "id" : "foo at example.com"
             }
-         ]
+         }
       }
       { … },
       …

commit 92361fac89328c1e01cd706ec11caa4f1f3e844b
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Wed May 18 20:26:03 2022 +0800

    Update tests for the keys change of CustomRoles in REST2

diff --git a/t/rest2/ticket-correspond-customroles.t b/t/rest2/ticket-correspond-customroles.t
index f52cb5a947..c629c97a74 100644
--- a/t/rest2/ticket-correspond-customroles.t
+++ b/t/rest2/ticket-correspond-customroles.t
@@ -71,10 +71,12 @@ my ($ticket_url, $ticket_id);
     is($content->{Status}, 'new');
     is($content->{Subject}, 'Ticket creation using REST');
 
-    ok(exists $content->{$_}, "Content exists for $_") for qw(AdminCc TimeEstimated Started Cc
-                                     LastUpdated TimeWorked Resolved
-                                     RT::CustomRole-1 RT::CustomRole-2
-                                     Created Due Priority EffectiveId);
+    ok( exists $content->{$_}, "Content exists for $_" ) for qw(AdminCc TimeEstimated Started Cc
+        LastUpdated TimeWorked Resolved Created Due Priority EffectiveId CustomRoles);
+
+    # Remove this in RT 5.2
+    ok( exists $content->{$_}, "Content exists for $_" ) for 'Single Member', 'Multi Member';
+
 }
 
 diag "Correspond with custom roles";
diff --git a/t/rest2/ticket-customroles.t b/t/rest2/ticket-customroles.t
index 8831c6869f..9a70b9408f 100644
--- a/t/rest2/ticket-customroles.t
+++ b/t/rest2/ticket-customroles.t
@@ -58,8 +58,8 @@ $user->PrincipalObj->GrantRight( Right => $_ )
     is($res->code, 200);
 
     my $content = $mech->json_response;
-    cmp_deeply($content->{$multi->GroupType}, [], 'no Multi Member');
-    cmp_deeply($content->{$single->GroupType}, {
+    cmp_deeply($content->{$multi->Name}, [], 'no Multi Member');
+    cmp_deeply($content->{$single->Name}, {
         type => 'user',
         id   => 'Nobody',
         _url => re(qr{$rest_base_path/user/Nobody$}),
@@ -86,7 +86,7 @@ $user->PrincipalObj->GrantRight( Right => $_ )
     my ($single_url) = map { $_->{_url} } grep { $_->{ref} eq 'customrole' && $_->{id} == $single_id } @{ $content->{'_hyperlinks'} };
     my ($multi_url) = map { $_->{_url} } grep { $_->{ref} eq 'customrole' && $_->{id} == $multi_id } @{ $content->{'_hyperlinks'} };
 
-    $res = $mech->get($content->{$single->GroupType}{_url},
+    $res = $mech->get($content->{$single->Name}{_url},
         'Authorization' => $auth,
     );
     is($res->code, 200);
@@ -142,13 +142,13 @@ $user->PrincipalObj->GrantRight( Right => $_ )
     is($res->code, 200);
 
     my $content = $mech->json_response;
-    cmp_deeply($content->{$multi->GroupType}, [{
+    cmp_deeply($content->{$multi->Name}, [{
         type => 'user',
         id   => 'multi at example.com',
         _url => re(qr{$rest_base_path/user/multi\@example\.com$}),
     }], 'one Multi Member');
 
-    cmp_deeply($content->{$single->GroupType}, {
+    cmp_deeply($content->{$single->Name}, {
         type => 'user',
         id   => 'test at localhost',
         _url => re(qr{$rest_base_path/user/test\@localhost$}),
@@ -178,7 +178,7 @@ $user->PrincipalObj->GrantRight( Right => $_ )
     is($res->code, 200);
 
     my $content = $mech->json_response;
-    cmp_deeply($content->{$multi->GroupType}, [{
+    cmp_deeply($content->{$multi->Name}, [{
         type => 'user',
         id   => 'multi at example.com',
         _url => re(qr{$rest_base_path/user/multi\@example\.com$}),
@@ -188,7 +188,7 @@ $user->PrincipalObj->GrantRight( Right => $_ )
         _url => re(qr{$rest_base_path/user/multi2\@example\.com$}),
     }], 'two Multi Members');
 
-    cmp_deeply($content->{$single->GroupType}, {
+    cmp_deeply($content->{$single->Name}, {
         type => 'user',
         id   => 'test at localhost',
         _url => re(qr{$rest_base_path/user/test\@localhost}),
@@ -218,7 +218,7 @@ diag 'Create and view ticket with custom roles by name';
     is($res->code, 200);
 
     my $content = $mech->json_response;
-    cmp_deeply($content->{$multi->GroupType}, [{
+    cmp_deeply($content->{$multi->Name}, [{
         type => 'user',
         id   => 'multi at example.com',
         _url => re(qr{$rest_base_path/user/multi\@example\.com$}),
@@ -228,7 +228,7 @@ diag 'Create and view ticket with custom roles by name';
         _url => re(qr{$rest_base_path/user/multi2\@example\.com$}),
     }], 'two Multi Members');
 
-    cmp_deeply($content->{$single->GroupType}, {
+    cmp_deeply($content->{$single->Name}, {
         type => 'user',
         id   => 'test at localhost',
         _url => re(qr{$rest_base_path/user/test\@localhost}),
@@ -255,7 +255,7 @@ diag 'Create and view ticket with custom roles by name';
     );
     is($res->code, 200);
 
-    cmp_deeply($mech->json_response->{$single->GroupType}, {
+    cmp_deeply($mech->json_response->{$single->Name}, {
         type => 'user',
         id   => 'Nobody',
         _url => re(qr{$rest_base_path/user/Nobody$}),
@@ -277,7 +277,7 @@ diag 'Create and view ticket with custom roles by name';
         );
         is($res->code, 200);
 
-        cmp_deeply($mech->json_response->{$single->GroupType}, {
+        cmp_deeply($mech->json_response->{$single->Name}, {
             type => 'user',
             id   => 'test',
             _url => re(qr{$rest_base_path/user/test$}),
@@ -298,7 +298,7 @@ diag 'Create and view ticket with custom roles by name';
         );
         is($res->code, 200);
 
-        cmp_deeply($mech->json_response->{$single->GroupType}, {
+        cmp_deeply($mech->json_response->{$single->Name}, {
             type => 'user',
             id   => 'Nobody',
             _url => re(qr{$rest_base_path/user/Nobody$}),
@@ -321,7 +321,7 @@ diag 'Create and view ticket with custom roles by name';
         );
         is($res->code, 200);
 
-        cmp_deeply($mech->json_response->{$single->GroupType}, {
+        cmp_deeply($mech->json_response->{$single->Name}, {
             type => 'user',
             id   => 'test',
             _url => re(qr{$rest_base_path/user/test$}),
@@ -342,7 +342,7 @@ diag 'Create and view ticket with custom roles by name';
         );
         is($res->code, 200);
 
-        cmp_deeply($mech->json_response->{$single->GroupType}, {
+        cmp_deeply($mech->json_response->{$single->Name}, {
             type => 'user',
             id   => 'Nobody',
             _url => re(qr{$rest_base_path/user/Nobody$}),
@@ -371,7 +371,7 @@ diag 'Create and view ticket with custom roles by name';
     is($res->code, 200);
 
     my $content = $mech->json_response;
-    cmp_deeply($content->{$multi->GroupType}, [], 'no Multi Member');
+    cmp_deeply($content->{$multi->Name}, [], 'no Multi Member');
 
     $payload = {
         $multi->GroupType => 'multi at example.com',
@@ -388,7 +388,7 @@ diag 'Create and view ticket with custom roles by name';
     );
     is($res->code, 200);
     $content = $mech->json_response;
-    cmp_deeply($content->{$multi->GroupType}, [{
+    cmp_deeply($content->{$multi->Name}, [{
         type => 'user',
         id   => 'multi at example.com',
         _url => re(qr{$rest_base_path/user/multi\@example\.com$}),
@@ -409,7 +409,7 @@ diag 'Create and view ticket with custom roles by name';
     );
     is($res->code, 200);
     $content = $mech->json_response;
-    cmp_deeply($content->{$multi->GroupType}, [{
+    cmp_deeply($content->{$multi->Name}, [{
         type => 'user',
         id   => 'multi2 at example.com',
         _url => re(qr{$rest_base_path/user/multi2\@example\.com$}),
@@ -430,7 +430,7 @@ diag 'Create and view ticket with custom roles by name';
     );
     is($res->code, 200);
     $content = $mech->json_response;
-    cmp_deeply($content->{$multi->GroupType}, bag({
+    cmp_deeply($content->{$multi->Name}, bag({
         type => 'user',
         id   => 'multi2 at example.com',
         _url => re(qr{$rest_base_path/user/multi2\@example\.com$}),
@@ -478,7 +478,7 @@ diag 'Create and view ticket with custom roles by name';
         );
         is($res->code, 200);
         $content = $mech->json_response;
-        cmp_deeply($content->{$multi->GroupType}, bag({
+        cmp_deeply($content->{$multi->Name}, bag({
             type => 'user',
             id   => 'multi2 at example.com',
             _url => re(qr{$rest_base_path/user/multi2\@example\.com$}),
@@ -502,7 +502,7 @@ diag 'Create and view ticket with custom roles by name';
     is( $res->code, 200 );
     $content = $mech->json_response;
     cmp_deeply(
-        $content->{ $multi->GroupType },
+        $content->{ $multi->Name },
         bag({   type => 'user',
                 id   => 'test at localhost',
                 _url => re(qr{$rest_base_path/user/test\@localhost$}),
@@ -549,7 +549,7 @@ diag 'Create and view ticket with custom roles by name';
     is($res->code, 200);
 
     my $content = $mech->json_response;
-    cmp_deeply($content->{$multi->GroupType}, [{
+    cmp_deeply($content->{$multi->Name}, [{
         id   => $group_id,
         type => 'group',
         _url => re(qr{$rest_base_path/group/$group_id$}),
@@ -570,7 +570,7 @@ diag 'Create and view ticket with custom roles by name';
     );
     is($res->code, 200);
     $content = $mech->json_response;
-    cmp_deeply($content->{$multi->GroupType}, [{
+    cmp_deeply($content->{$multi->Name}, [{
         type => 'user',
         id   => 'multi at example.com',
         _url => re(qr{$rest_base_path/user/multi\@example\.com$}),
@@ -591,7 +591,7 @@ diag 'Create and view ticket with custom roles by name';
     );
     is($res->code, 200);
     $content = $mech->json_response;
-    cmp_deeply($content->{$multi->GroupType}, [{
+    cmp_deeply($content->{$multi->Name}, [{
         type => 'user',
         id   => 'multi at example.com',
         _url => re(qr{$rest_base_path/user/multi\@example\.com$}),
@@ -602,7 +602,7 @@ diag 'Create and view ticket with custom roles by name';
         _url => re(qr{$rest_base_path/group/$group_id$}),
     }], 'Multi Member user and group');
 
-    $res = $mech->get($content->{$multi->GroupType}[1]{_url},
+    $res = $mech->get($content->{$multi->Name}[1]{_url},
         'Authorization' => $auth,
     );
     is($res->code, 200);
@@ -660,8 +660,8 @@ diag 'Create and view ticket with custom roles by name';
     is($res->code, 200);
 
     my $content = $mech->json_response;
-    cmp_deeply($content->{$later_multi->GroupType}, [], 'no Later Multi Member');
-    cmp_deeply($content->{$later_single->GroupType}, {
+    cmp_deeply($content->{$later_multi->Name}, [], 'no Later Multi Member');
+    cmp_deeply($content->{$later_single->Name}, {
         type => 'user',
         id   => 'Nobody',
         _url => re(qr{$rest_base_path/user/Nobody$}),
@@ -704,9 +704,79 @@ diag 'Create and view ticket with custom roles by name';
     is( scalar @{ $content->{items} }, 1 );
 
     $ticket = $content->{items}->[0];
-    is( $ticket->{CustomRoles}{ $single->GroupType }{id}, 'single2 at example.com',  'Single Member id in search result' );
-    is( $ticket->{CustomRoles}{ $multi->GroupType }[0]{id}, 'multi at example.com',  'Multi Member id in search result' );
-    is( $ticket->{CustomRoles}{ $multi->GroupType }[1]{id}, 'multi2 at example.com', 'Multi Member id in search result' );
+    is( $ticket->{CustomRoles}{ $single->Name }{id}, 'single2 at example.com',  'Single Member id in search result' );
+    is( $ticket->{CustomRoles}{ $multi->Name }[0]{id}, 'multi at example.com',  'Multi Member id in search result' );
+    is( $ticket->{CustomRoles}{ $multi->Name }[1]{id}, 'multi2 at example.com', 'Multi Member id in search result' );
+}
+
+diag 'Test custom role name conflicts';
+{
+    my $creator = RT::CustomRole->new( RT->SystemUser );
+    ( $ok, $msg ) = $creator->Create( Name => 'Creator' );
+    ok( $ok, $msg );
+    my $creator_id = $creator->Id;
+
+    ( $ok, $msg ) = $creator->AddToObject( $queue->id );
+    ok( $ok, $msg );
+
+    my $payload = {
+        Subject     => 'Ticket with creator watchers',
+        Queue       => 'General',
+        CustomRoles => { Creator => [ 'multi at example.com', 'multi2 at example.com' ] },
+    };
+
+    my $res = $mech->post_json( "$rest_base_path/ticket", $payload, 'Authorization' => $auth, );
+    is( $res->code, 201 );
+    ok( my $ticket_url = $res->header('location') );
+    ok( ( my $ticket_id ) = $ticket_url =~ qr[/ticket/(\d+)] );
+
+    my @warnings;
+    local $SIG{__WARN__} = sub {
+        push @warnings, @_;
+    };
+
+    $res = $mech->get( $ticket_url, 'Authorization' => $auth, );
+    is( $res->code, 200 );
+    my $content = $mech->json_response;
+    cmp_deeply(
+        $content->{ 'CustomRole.{' . $creator->Name . '}' },
+        [
+            {
+                type => 'user',
+                id   => 'multi at example.com',
+                _url => re(qr{$rest_base_path/user/multi\@example\.com$}),
+            },
+            {
+                type => 'user',
+                id   => 'multi2 at example.com',
+                _url => re(qr{$rest_base_path/user/multi2\@example\.com$}),
+            }
+        ],
+        'two Creator Members'
+    );
+    cmp_deeply(
+        $content->{Creator},
+        {
+            type => 'user',
+            id   => 'test',
+            _url => re(qr{$rest_base_path/user/test$}),
+        },
+        'two Multi Members'
+    );
+    is( scalar @warnings, 1, 'Got one warning' );
+    like(
+        $warnings[0],
+        qr/\QCustomRole Creator conflicts with core field Creator, renaming its key to CustomRole.{Creator}\E/,
+        'Got the name conflict warning'
+    );
+
+    is_deeply(
+        $content->{_comments},
+        [
+            'Top level individual custom role keys are depcreated and will be removed in RT 5.2. Please use "CustomRoles" instead.'
+        ],
+        'Got the deprecated comment'
+    );
 }
 
 done_testing;

commit 6c66d8530e84c6d8508d066fab34a47b689a26d1
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Tue May 17 23:36:37 2022 +0800

    Use custom role names as keys for ticket endpoints in REST2
    
    Previously we used the GroupType(e.g. "RT::CustomRole-2"), which didn't
    contain useful info compared to Name. This commit changes it to Name to
    be more consistent with core roles. It also adds a separate
    "CustomRoles" top key, just like "CustomFields".
    
    As there is a "CustomRoles" top key, we can deprecate the usage of
    individual top level custom role keys, considering it could conflict
    with other fields(e.g. custom role "Creator" VS core field "Creator").
    If there is a collsion, the key for custom role is automatically
    renamed for now, e.g. from "Creator" to "CustomRole.{Creator}".

diff --git a/lib/RT/REST2/Resource.pm b/lib/RT/REST2/Resource.pm
index 777023404c..498ba0fa50 100644
--- a/lib/RT/REST2/Resource.pm
+++ b/lib/RT/REST2/Resource.pm
@@ -116,23 +116,29 @@ sub expand_field {
         if ( $item->DOES("RT::Record::Role::Roles") ) {
             my %data;
             for my $role ( $item->Roles( ACLOnly => 0 ) ) {
-                next unless $role =~ /^RT::CustomRole-/;
-                $data{$role} = [];
+                next unless $role =~ /^RT::CustomRole-(\d+)$/;
+                my $role_id = $1;
+                my $role_object = RT::CustomRole->new( $item->CurrentUser );
+                $role_object->Load($role_id);
+                next unless $role_object->Id;
+                my $role_name = $role_object->Name;
+
+                $data{$role_name} = [];
 
                 my $group = $item->RoleGroup($role);
                 if ( !$group->Id ) {
-                    $data{$role} = $self->_expand_object( RT->Nobody->UserObj, $field, $param_prefix )
+                    $data{$role_name} = $self->_expand_object( RT->Nobody->UserObj, $field, $param_prefix )
                         if $item->_ROLES->{$role}{Single};
                     next;
                 }
 
                 my $gms = $group->MembersObj;
                 while ( my $gm = $gms->Next ) {
-                    push @{ $data{$role} }, $self->_expand_object( $gm->MemberObj->Object, $field, $param_prefix );
+                    push @{ $data{$role_name} }, $self->_expand_object( $gm->MemberObj->Object, $field, $param_prefix );
                 }
 
                 # Avoid the extra array ref for single member roles
-                $data{$role} = shift @{$data{$role}} if $group->SingleMemberRoleGroup;
+                $data{$role_name} = shift @{$data{$role_name}} if $group->SingleMemberRoleGroup;
             }
             return \%data;
         }
diff --git a/lib/RT/REST2/Util.pm b/lib/RT/REST2/Util.pm
index f9bc8e5fa9..646654b296 100644
--- a/lib/RT/REST2/Util.pm
+++ b/lib/RT/REST2/Util.pm
@@ -164,11 +164,33 @@ sub serialize_record {
 
     # Include role members, if applicable
     if ($record->DOES("RT::Record::Role::Roles")) {
+        my %custom_role;
         for my $role ($record->Roles(ACLOnly => 0)) {
-            my $members = $data{$role} = [];
+            my $role_name;
+            if ( $role =~ /^RT::CustomRole-(\d+)$/ ) {
+                my $role_id = $1;
+                my $role_object = RT::CustomRole->new( $record->CurrentUser );
+                $role_object->Load($role_id);
+                next unless $role_object->Id;
+                $role_name = $role_object->Name;
+                $custom_role{$role_name} = 1;
+
+                if ( $record->_Accessible( $role_name => 'read' ) ) {
+                    RT->Logger->warning(
+                        "CustomRole $role_name conflicts with core field $role_name, renaming its key to CustomRole.{$role_name}"
+                    );
+                    $role_name = "CustomRole.{$role_name}";
+                }
+            }
+            else {
+                # Core role
+                $role_name = $role;
+            }
+
+            my $members = $data{$role_name} = [];
             my $group = $record->RoleGroup($role);
             if ( !$group->Id ) {
-                $data{$role} = expand_uid( RT->Nobody->UserObj->UID ) if $record->_ROLES->{$role}{Single};
+                $data{$role_name} = expand_uid( RT->Nobody->UserObj->UID ) if $record->_ROLES->{$role}{Single};
                 next;
             }
 
@@ -178,9 +200,17 @@ sub serialize_record {
             }
 
             # Avoid the extra array ref for single member roles
-            $data{$role} = shift @$members
+            $data{$role_name} = shift @$members
                 if $group->SingleMemberRoleGroup;
         }
+
+        if (%custom_role) {
+            $data{CustomRoles} = { map { $_ => $data{$_} || $data{"CustomRole.{$_}"} } keys %custom_role };
+            push @{ $data{_comments} },
+                $record->loc(
+                'Top level individual custom role keys are depcreated and will be removed in RT 5.2. Please use "CustomRoles" instead.'
+                );
+        }
     }
 
     if (my $cfs = custom_fields_for($record)) {

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


hooks/post-receive
-- 
rt


More information about the rt-commit mailing list