Mailing List Archive

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@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@example.com",
"type" : "user",
"id" : "foo@example.com"
}
- ]
+ }
}
{ … },
…

commit 92361fac89328c1e01cd706ec11caa4f1f3e844b
Author: sunnavy <sunnavy@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@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@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@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@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@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@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@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@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@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@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@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@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@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@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@example.com', 'Single Member id in search result' );
- is( $ticket->{CustomRoles}{ $multi->GroupType }[0]{id}, 'multi@example.com', 'Multi Member id in search result' );
- is( $ticket->{CustomRoles}{ $multi->GroupType }[1]{id}, 'multi2@example.com', 'Multi Member id in search result' );
+ is( $ticket->{CustomRoles}{ $single->Name }{id}, 'single2@example.com', 'Single Member id in search result' );
+ is( $ticket->{CustomRoles}{ $multi->Name }[0]{id}, 'multi@example.com', 'Multi Member id in search result' );
+ is( $ticket->{CustomRoles}{ $multi->Name }[1]{id}, 'multi2@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@example.com', 'multi2@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@example.com',
+ _url => re(qr{$rest_base_path/user/multi\@example\.com$}),
+ },
+ {
+ type => 'user',
+ id => 'multi2@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@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
_______________________________________________
rt-commit mailing list
rt-commit@lists.bestpractical.com
https://lists.bestpractical.com/mailman/listinfo/rt-commit