[Bps-public-commit] rt-extension-rest2 branch, custom-field-handling-improvements, created. 1.07-4-g8f50238

Aaron Trevena ast at bestpractical.com
Fri Feb 28 09:35:53 EST 2020


The branch, custom-field-handling-improvements has been created
        at  8f50238a9a2d8c48ec748613bd291cf7cc5654b4 (commit)

- Log -----------------------------------------------------------------
commit 586143263e1eddff81096998b56bb58c814f03da
Author: Andrew Ruthven <puck at catalystcloud.nz>
Date:   Fri Feb 28 10:05:04 2020 +0000

    Treat CustomFields in a manner that is more consistent to other object types

diff --git a/Changes b/Changes
index c756d8c..e6c887b 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,8 @@
 Revision history for RT-Extension-REST2
 
+Unreleased
+ - Return more information about CustomFields when returning ticket
+
 1.07 2019-05-24
  - Accept 'Content' as a parameter on create. The documentation previously showed
    this in examples, but it wasn't yet supported. Now it works as documented.
diff --git a/lib/RT/Extension/REST2/Util.pm b/lib/RT/Extension/REST2/Util.pm
index e814c1c..ec3defd 100644
--- a/lib/RT/Extension/REST2/Util.pm
+++ b/lib/RT/Extension/REST2/Util.pm
@@ -115,8 +115,13 @@ sub serialize_record {
     if (my $cfs = custom_fields_for($record)) {
         my %values;
         while (my $cf = $cfs->Next) {
-            my $key    = $cf->Id;
-            my $values = $values{$key} ||= [];
+            if (! defined $values{$cf->Id}) {
+                $values{$cf->Id} = {
+                    %{ expand_uid($cf->UID) },
+                    name   => $cf->Name,
+                    values => [],
+                };
+            }
             my $ocfvs  = $cf->ValuesForObject( $record );
             my $type   = $cf->Type;
             while (my $ocfv = $ocfvs->Next) {
@@ -131,11 +136,12 @@ sub serialize_record {
                         _url         => RT::Extension::REST2->base_uri . "/download/cf/" . $ocfv->id,
                     };
                 }
-                push @$values, $content;
+                push @{ $values{$cf->Id}{values} }, $content;
             }
         }
 
-        $data{CustomFields} = \%values;
+        push @{ $data{CustomFields} }, values %values;
+        $data{CustomFieldsValues} = { map { $_ => $values{$_}{values} } keys %values };
     }
     return \%data;
 }

commit bbf999c19a312abdff608ef9d26e4259e605c352
Author: Andrew Ruthven <puck at catalystcloud.nz>
Date:   Fri Feb 28 10:05:39 2020 +0000

    New and updated Tests for CustomFields handling

diff --git a/xt/asset-customfields.t b/xt/asset-customfields.t
index 499c400..3003665 100644
--- a/xt/asset-customfields.t
+++ b/xt/asset-customfields.t
@@ -96,10 +96,15 @@ my ($asset_url, $asset_id);
     is($content->{id}, $asset_id);
     is($content->{Status}, 'new');
     is($content->{Name}, 'Asset creation using REST');
-    is_deeply($content->{'CustomFields'}, {}, 'Asset custom field not present');
+    is_deeply($content->{'CustomFields'}, [], 'Asset custom field not present');
     is_deeply([grep { $_->{ref} eq 'customfield' } @{ $content->{'_hyperlinks'} }], [], 'No CF hypermedia');
 }
 
+my $no_asset_cf_values = bag(
+  { name => 'Single', id => $single_cf_id, type => 'customfield', _url => ignore(), values => [] },
+  { name => 'Multi',  id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => [] },
+);
+
 # Rights Test - With ShowAsset and SeeCustomField
 {
     $user->PrincipalObj->GrantRight( Right => 'SeeCustomField');
@@ -113,7 +118,9 @@ my ($asset_url, $asset_id);
     is($content->{id}, $asset_id);
     is($content->{Status}, 'new');
     is($content->{Name}, 'Asset creation using REST');
-    is_deeply($content->{CustomFields}, { $single_cf_id => [], $multi_cf_id => [] }, 'No asset custom field values');
+    cmp_deeply($content->{CustomFields}, $no_asset_cf_values, 'No asset custom field values');
+
+
     cmp_deeply(
         [grep { $_->{ref} eq 'customfield' } @{ $content->{'_hyperlinks'} }],
         [{
@@ -198,7 +205,7 @@ my ($asset_url, $asset_id);
     my $content = $mech->json_response;
     is($content->{Name}, 'Asset update using REST');
     is($content->{Status}, 'allocated');
-    is_deeply($content->{CustomFields}, { $single_cf_id => [], $multi_cf_id => [] }, 'No update to CF');
+    cmp_deeply($content->{CustomFields}, $no_asset_cf_values, 'No update to CF');
 }
 
 # Asset Update with ModifyCustomField
@@ -223,10 +230,15 @@ my ($asset_url, $asset_id);
     );
     is($res->code, 200);
 
+    my $modified_asset_cf_values = bag(
+        { name => 'Single', id => $single_cf_id, type => 'customfield', _url => ignore(), values => ['Modified CF'] },
+        { name => 'Multi',  id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => [] },
+    );
+
     my $content = $mech->json_response;
     is($content->{Name}, 'More updates using REST');
     is($content->{Status}, 'in-use');
-    is_deeply($content->{CustomFields}, { $single_cf_id => ['Modified CF'], $multi_cf_id => [] }, 'New CF value');
+    cmp_deeply($content->{CustomFields}, $modified_asset_cf_values, 'New CF value');
 
     # make sure changing the CF doesn't add a second OCFV
     $payload->{CustomFields}{$single_cf_id} = 'Modified Again';
@@ -242,8 +254,13 @@ my ($asset_url, $asset_id);
     );
     is($res->code, 200);
 
+    $modified_asset_cf_values = bag(
+        { name => 'Single', id => $single_cf_id, type => 'customfield', _url => ignore(), values => ['Modified Again'] },
+        { name => 'Multi',  id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => [] },
+    );
+
     $content = $mech->json_response;
-    is_deeply($content->{CustomFields}, { $single_cf_id => ['Modified Again'], $multi_cf_id => [] }, 'New CF value');
+    cmp_deeply($content->{CustomFields}, $modified_asset_cf_values, 'New CF value');
 
     # stop changing the CF, change something else, make sure CF sticks around
     delete $payload->{CustomFields}{$single_cf_id};
@@ -261,7 +278,7 @@ my ($asset_url, $asset_id);
     is($res->code, 200);
 
     $content = $mech->json_response;
-    is_deeply($content->{CustomFields}, { $single_cf_id => ['Modified Again'], $multi_cf_id => [] }, 'Same CF value');
+    cmp_deeply($content->{CustomFields}, $modified_asset_cf_values, 'Same CF value');
 }
 
 # Asset Creation with ModifyCustomField
@@ -290,11 +307,16 @@ my ($asset_url, $asset_id);
     );
     is($res->code, 200);
 
+    my $asset_cf_values = bag(
+        { name => 'Single', id => $single_cf_id, type => 'customfield', _url => ignore(), values => ['Hello world!'] },
+        { name => 'Multi',  id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => [] },
+    );
+
     my $content = $mech->json_response;
     is($content->{id}, $asset_id);
     is($content->{Status}, 'new');
     is($content->{Name}, 'Asset creation using REST');
-    is_deeply($content->{'CustomFields'}{$single_cf_id}, ['Hello world!'], 'Asset custom field');
+    cmp_deeply($content->{'CustomFields'}, $asset_cf_values, 'Asset custom field');
 }
 
 # Asset Creation for multi-value CF
@@ -330,7 +352,12 @@ for my $value (
     is($content->{Name}, 'Multi-value CF');
 
     my $output = ref($value) ? $value : [$value]; # scalar input comes out as array reference
-    is_deeply($content->{'CustomFields'}, { $multi_cf_id => $output, $single_cf_id => [] }, 'Asset custom field');
+    my $asset_cf_values = bag(
+        { name => 'Single', id => $single_cf_id, type => 'customfield', _url => ignore(), values => [] },
+        { name => 'Multi',  id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => $output },
+    );
+
+    cmp_deeply($content->{'CustomFields'}, $asset_cf_values, 'Asset custom field');
 }
 
 {
@@ -359,8 +386,13 @@ for my $value (
         is($res->code, 200);
 
         my $content = $mech->json_response;
-        my @values = sort @{ $content->{CustomFields}{$multi_cf_id} };
-        is_deeply(\@values, $output, $name || 'New CF value');
+        my $values;
+        for my $cf (@{ $content->{CustomFields} }) {
+            next unless $cf->{id} == $multi_cf_id;
+
+            $values = [ sort @{ $cf->{values} } ];
+        }
+        cmp_deeply($values, $output, $name || 'New CF value');
     }
 
     # starting point: ['multiple', 'values'],
diff --git a/xt/ticket-customfields.t b/xt/ticket-customfields.t
index 5e8174f..949253b 100644
--- a/xt/ticket-customfields.t
+++ b/xt/ticket-customfields.t
@@ -91,10 +91,15 @@ my ($ticket_url, $ticket_id);
     is($content->{Type}, 'ticket');
     is($content->{Status}, 'new');
     is($content->{Subject}, 'Ticket creation using REST');
-    is_deeply($content->{'CustomFields'}, {}, 'Ticket custom field not present');
+    is_deeply($content->{'CustomFields'}, [], 'Ticket custom field not present');
     is_deeply([grep { $_->{ref} eq 'customfield' } @{ $content->{'_hyperlinks'} }], [], 'No CF hypermedia');
 }
 
+my $no_ticket_cf_values = bag(
+  { name => 'Single', id => $single_cf_id, type => 'customfield', _url => ignore(), values => [] },
+  { name => 'Multi',  id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => [] },
+);
+
 # Rights Test - With ShowTicket and SeeCustomField
 {
     $user->PrincipalObj->GrantRight( Right => 'SeeCustomField');
@@ -109,7 +114,7 @@ my ($ticket_url, $ticket_id);
     is($content->{Type}, 'ticket');
     is($content->{Status}, 'new');
     is($content->{Subject}, 'Ticket creation using REST');
-    is_deeply($content->{CustomFields}, { $single_cf_id => [], $multi_cf_id => [] }, 'No ticket custom field values');
+    cmp_deeply($content->{CustomFields}, $no_ticket_cf_values, 'No ticket custom field values');
     cmp_deeply(
         [grep { $_->{ref} eq 'customfield' } @{ $content->{'_hyperlinks'} }],
         [{
@@ -194,7 +199,7 @@ my ($ticket_url, $ticket_id);
     my $content = $mech->json_response;
     is($content->{Subject}, 'Ticket update using REST');
     is($content->{Priority}, 42);
-    is_deeply($content->{CustomFields}, { $single_cf_id => [], $multi_cf_id => [] }, 'No update to CF');
+    cmp_deeply($content->{CustomFields}, $no_ticket_cf_values, 'No update to CF');
 }
 
 # Ticket Update with ModifyCustomField
@@ -219,10 +224,15 @@ my ($ticket_url, $ticket_id);
     );
     is($res->code, 200);
 
+    my $modified_single_cf_value = bag(
+        { name => 'Single', id => $single_cf_id, type => 'customfield', _url => ignore(), values => ['Modified CF'] },
+        { name => 'Multi',  id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => [] },
+    );
+
     my $content = $mech->json_response;
     is($content->{Subject}, 'More updates using REST');
     is($content->{Priority}, 43);
-    is_deeply($content->{CustomFields}, { $single_cf_id => ['Modified CF'], $multi_cf_id => [] }, 'New CF value');
+    cmp_deeply($content->{CustomFields}, $modified_single_cf_value, 'New CF value');
 
     # make sure changing the CF doesn't add a second OCFV
     $payload->{CustomFields}{$single_cf_id} = 'Modified Again';
@@ -238,8 +248,13 @@ my ($ticket_url, $ticket_id);
     );
     is($res->code, 200);
 
+    my $modified_again_single_cf_value = bag(
+        { name => 'Single', id => $single_cf_id, type => 'customfield', _url => ignore(), values => ['Modified Again'] },
+        { name => 'Multi',  id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => [] },
+    );
+
     $content = $mech->json_response;
-    is_deeply($content->{CustomFields}, { $single_cf_id => ['Modified Again'], $multi_cf_id => [] }, 'New CF value');
+    cmp_deeply($content->{CustomFields}, $modified_again_single_cf_value, 'New CF value');
 
     # stop changing the CF, change something else, make sure CF sticks around
     delete $payload->{CustomFields}{$single_cf_id};
@@ -257,7 +272,7 @@ my ($ticket_url, $ticket_id);
     is($res->code, 200);
 
     $content = $mech->json_response;
-    is_deeply($content->{CustomFields}, { $single_cf_id => ['Modified Again'], $multi_cf_id => [] }, 'Same CF value');
+    cmp_deeply($content->{CustomFields}, $modified_again_single_cf_value, 'Same CF value');
 }
 
 # Ticket Creation with ModifyCustomField
@@ -289,12 +304,17 @@ my ($ticket_url, $ticket_id);
     );
     is($res->code, 200);
 
+    my $ticket_cf_value = bag(
+        { name => 'Single', id => $single_cf_id, type => 'customfield', _url => ignore(), values => ['Hello world!'] },
+        { name => 'Multi',  id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => [] },
+    );
+
     my $content = $mech->json_response;
     is($content->{id}, $ticket_id);
     is($content->{Type}, 'ticket');
     is($content->{Status}, 'new');
     is($content->{Subject}, 'Ticket creation using REST');
-    is_deeply($content->{'CustomFields'}{$single_cf_id}, ['Hello world!'], 'Ticket custom field');
+    cmp_deeply($content->{CustomFields}, $ticket_cf_value, 'Ticket custom field');
 }
 
 # Ticket Creation for multi-value CF
@@ -331,7 +351,11 @@ for my $value (
     is($content->{Subject}, 'Multi-value CF');
 
     my $output = ref($value) ? $value : [$value]; # scalar input comes out as array reference
-    is_deeply($content->{'CustomFields'}, { $multi_cf_id => $output, $single_cf_id => [] }, 'Ticket custom field');
+    my $ticket_cf_value = bag(
+        { name => 'Single', id => $single_cf_id, type => 'customfield', _url => ignore(), values => [] },
+        { name => 'Multi',  id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => $output },
+    );
+    cmp_deeply($content->{'CustomFields'}, $ticket_cf_value, 'Ticket custom field');
 }
 
 {
@@ -359,9 +383,13 @@ for my $value (
         );
         is($res->code, 200);
 
+        my $ticket_cf_value = bag(
+            { name => 'Single', id => $single_cf_id, type => 'customfield', _url => ignore(), values => [] },
+            { name => 'Multi',  id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => bag(@$output) },
+        );
+
         my $content = $mech->json_response;
-        my @values = sort @{ $content->{CustomFields}{$multi_cf_id} };
-        is_deeply(\@values, $output, $name || 'New CF value');
+        cmp_deeply($content->{'CustomFields'}, $ticket_cf_value, $name || 'New CF value');
     }
 
     # starting point: ['multiple', 'values'],
diff --git a/xt/ticket-customroles.t b/xt/ticket-customroles.t
index 9ccfcdf..a2bc034 100644
--- a/xt/ticket-customroles.t
+++ b/xt/ticket-customroles.t
@@ -505,7 +505,7 @@ $user->PrincipalObj->GrantRight( Right => $_ )
         id           => $group->Id,
         Name         => 'Watcher Group',
         Domain       => 'UserDefined',
-        CustomFields => {},
+        CustomFields => [],
         Members      => [{
             type => 'user',
             id   => 'multi at example.com',
diff --git a/xt/ticket-watchers.t b/xt/ticket-watchers.t
index 1ceae9f..e6bf631 100644
--- a/xt/ticket-watchers.t
+++ b/xt/ticket-watchers.t
@@ -570,7 +570,7 @@ $user->PrincipalObj->GrantRight( Right => $_ )
         id           => $group->Id,
         Name         => 'Watcher Group',
         Domain       => 'UserDefined',
-        CustomFields => {},
+        CustomFields => [],
         Members      => [{
             type => 'user',
             id   => 'admincc at example.com',
diff --git a/xt/user-customfields.t b/xt/user-customfields.t
index 7d277c5..d46a1c0 100644
--- a/xt/user-customfields.t
+++ b/xt/user-customfields.t
@@ -1,6 +1,7 @@
 use strict;
 use warnings;
 use RT::Extension::REST2::Test tests => undef;
+use Test::Deep;
 
 my $mech = RT::Extension::REST2::Test->mech;
 
@@ -11,6 +12,7 @@ my $user = RT::Extension::REST2::Test->user;
 my $single_cf = RT::CustomField->new( RT->SystemUser );
 my ($ok, $msg) = $single_cf->Create( Name => 'Freeform', Type => 'FreeformSingle', LookupType => RT::User->CustomFieldLookupType );
 ok($ok, $msg);
+my $single_cf_id = $single_cf->Id();
 my $ocf = RT::ObjectCustomField->new( RT->SystemUser );
 ( $ok, $msg ) = $ocf->Add( CustomField => $single_cf->id, ObjectId => 0 );
 ok($ok, "Applied globally" );
@@ -18,6 +20,7 @@ ok($ok, "Applied globally" );
 my $multi_cf = RT::CustomField->new( RT->SystemUser );
 ($ok, $msg) = $multi_cf->Create( Name => 'Multi', Type => 'FreeformMultiple', LookupType => RT::User->CustomFieldLookupType );
 ok($ok, $msg);
+my $multi_cf_id = $multi_cf->Id();
 $ocf = RT::ObjectCustomField->new( RT->SystemUser );
 ( $ok, $msg ) = $ocf->Add( CustomField => $multi_cf->id, ObjectId => 0 );
 ok($ok, "Applied globally" );
@@ -65,7 +68,7 @@ my ($user_url, $user_id);
     is($content->{id}, $user_id);
     is($content->{EmailAddress}, 'user1 at example.com');
     is($content->{Name}, 'user1');
-    is_deeply($content->{'CustomFields'}, {}, 'User custom field not present');
+    is_deeply($content->{'CustomFields'}, [], 'User custom field not present');
 
     # can fetch user by name too
     $res = $mech->get("$rest_base_path/user/user1",
@@ -76,6 +79,11 @@ my ($user_url, $user_id);
     is_deeply($mech->json_response, $content, 'requesting user by name is same as user by id');
 }
 
+my $no_user_cf_values = bag(
+  { name => 'Freeform', id => $single_cf_id, type => 'customfield', _url => ignore(), values => [] },
+  { name => 'Multi',    id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => [] },
+);
+
 # Rights Test - With SeeCustomField
 {
     $user->PrincipalObj->GrantRight( Right => 'SeeCustomField');
@@ -89,7 +97,7 @@ my ($user_url, $user_id);
     is($content->{id}, $user_id);
     is($content->{EmailAddress}, 'user1 at example.com');
     is($content->{Name}, 'user1');
-    is_deeply($content->{'CustomFields'}, { $single_cf->Id => [], $multi_cf->Id => [] }, 'User custom field not present');
+    cmp_deeply($content->{'CustomFields'}, $no_user_cf_values, 'User custom field not present');
 }
 
 # User Update without ModifyCustomField
@@ -117,7 +125,7 @@ my ($user_url, $user_id);
     is($content->{id}, $user_id);
     is($content->{EmailAddress}, 'user1 at example.com');
     is($content->{Name}, 'User1');
-    is_deeply($content->{CustomFields}, { $single_cf->Id => [], $multi_cf->Id => [] }, 'No update to CF');
+    cmp_deeply($content->{'CustomFields'}, $no_user_cf_values, 'User custom field not present');
 }
 
 # User Update with ModifyCustomField
@@ -126,7 +134,7 @@ my ($user_url, $user_id);
     my $payload = {
         EmailAddress  => 'user1+rt at example.com',
         CustomFields => {
-            $single_cf->Id => 'Modified CF',
+            $single_cf_id => 'Modified CF',
         },
     };
     my $res = $mech->put_json($user_url,
@@ -143,7 +151,12 @@ my ($user_url, $user_id);
 
     my $content = $mech->json_response;
     is($content->{EmailAddress}, 'user1+rt at example.com');
-    is_deeply($content->{CustomFields}, { $single_cf->Id => ['Modified CF'], $multi_cf->Id => [] }, 'New CF value');
+
+    my $set_user_cf_values = bag(
+        { name => 'Freeform', id => $single_cf_id, type => 'customfield', _url => ignore(), values => ['Modified CF'] },
+        { name => 'Multi',    id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => [] },
+    );
+    cmp_deeply($content->{'CustomFields'}, $set_user_cf_values, 'New CF value');
 
     # make sure changing the CF doesn't add a second OCFV
     $payload->{CustomFields}{$single_cf->Id} = 'Modified Again';
@@ -160,7 +173,12 @@ my ($user_url, $user_id);
     is($res->code, 200);
 
     $content = $mech->json_response;
-    is_deeply($content->{CustomFields}, { $single_cf->Id => ['Modified Again'], $multi_cf->Id => [] }, 'New CF value');
+
+    $set_user_cf_values = bag(
+        { name => 'Freeform', id => $single_cf_id, type => 'customfield', _url => ignore(), values => ['Modified Again'] },
+        { name => 'Multi',    id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => [] },
+    );
+    cmp_deeply($content->{'CustomFields'}, $set_user_cf_values, 'New CF value');
 
     # stop changing the CF, change something else, make sure CF sticks around
     delete $payload->{CustomFields}{$single_cf->Id};
@@ -178,7 +196,7 @@ my ($user_url, $user_id);
     is($res->code, 200);
 
     $content = $mech->json_response;
-    is_deeply($content->{CustomFields}, { $single_cf->Id => ['Modified Again'], $multi_cf->Id => [] }, 'Same CF value');
+    cmp_deeply($content->{'CustomFields'}, $set_user_cf_values, 'Same CF value');
 }
 
 # User Creation with ModifyCustomField
@@ -209,7 +227,12 @@ my ($user_url, $user_id);
     my $content = $mech->json_response;
     is($content->{id}, $user_id);
     is($content->{Name}, 'user2');
-    is_deeply($content->{'CustomFields'}{$single_cf->Id}, ['Hello world!'], 'User custom field');
+
+    my $set_user_cf_values = bag(
+        { name => 'Freeform', id => $single_cf_id, type => 'customfield', _url => ignore(), values => ['Hello world!'] },
+        { name => 'Multi',    id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => [] },
+    );
+    cmp_deeply($content->{'CustomFields'}, $set_user_cf_values, 'User custom field');
 }
 
 # User Creation for multi-value CF
@@ -241,7 +264,11 @@ for my $value (
     my $content = $mech->json_response;
     is($content->{id}, $user_id);
     my $output = ref($value) ? $value : [$value]; # scalar input comes out as array reference
-    is_deeply($content->{'CustomFields'}, { $multi_cf->Id => $output, $single_cf->Id => [] }, 'User custom field');
+    my $set_user_cf_values = bag(
+        { name => 'Freeform', id => $single_cf_id, type => 'customfield', _url => ignore(), values => [] },
+        { name => 'Multi',    id => $multi_cf_id,  type => 'customfield', _url => ignore(), values => $output },
+    );
+    cmp_deeply($content->{'CustomFields'}, $set_user_cf_values, 'User custom field');
 }
 
 done_testing;

commit 1b7f65373d7ac96449da3fe6744a90053ed9d3fd
Author: Andrew Ruthven <puck at catalystcloud.nz>
Date:   Fri Feb 28 14:31:09 2020 +0000

    When creating an object, allow CustomFields to use names
    
    The docs all show examples of specifying CustomFields by name, yet that
    doesn't actually work.

diff --git a/lib/RT/Extension/REST2/Resource/Record/Writable.pm b/lib/RT/Extension/REST2/Resource/Record/Writable.pm
index 6d04d3b..3868426 100644
--- a/lib/RT/Extension/REST2/Resource/Record/Writable.pm
+++ b/lib/RT/Extension/REST2/Resource/Record/Writable.pm
@@ -290,6 +290,25 @@ sub create_record {
 
     my $cfs = delete $args{CustomFields};
 
+    # Lookup CustomFields by name.
+    if ($cfs) {
+        foreach my $id (keys(%$cfs)) {
+            if ($id !~ /^\d+$/) {
+                my $cf = $record->LoadCustomFieldByIdentifier($id);
+
+                if ($cf->Id) {
+                    $cfs->{$cf->Id} = $cfs->{$id};
+                    delete $cfs->{$id};
+                } else {
+                    # I would really like to return an error message, but, how?
+                    # RT appears to treat missing permission to a CF or
+                    # non-existance of a CF as a non-fatal error.
+                    RT->Logger->error( $record->loc( "Custom field [_1] not found", $id ) );
+                }
+            }
+        }
+    }
+
     # if a record class handles CFs in ->Create, use it (so it doesn't generate
     # spurious transactions and interfere with default values, etc). Otherwise,
     # add OCFVs after ->Create

commit 8f50238a9a2d8c48ec748613bd291cf7cc5654b4
Author: Andrew Ruthven <puck at catalystcloud.nz>
Date:   Fri Feb 28 14:32:37 2020 +0000

    Tests for creating an object with custom fields specified by name

diff --git a/xt/ticket-customfields.t b/xt/ticket-customfields.t
index 949253b..cfb3cd1 100644
--- a/xt/ticket-customfields.t
+++ b/xt/ticket-customfields.t
@@ -23,6 +23,7 @@ my $multi_cf_id = $multi_cf->Id;
 
 # Ticket Creation with no ModifyCustomField
 my ($ticket_url, $ticket_id);
+my ($ticket_url_cf_by_name, $ticket_id_cf_by_name);
 {
     my $payload = {
         Subject => 'Ticket creation using REST',
@@ -35,10 +36,35 @@ my ($ticket_url, $ticket_id);
         },
     };
 
+    my $payload_cf_by_name = {
+        Subject => 'Ticket creation using REST - CF By Name',
+        From    => 'test at bestpractical.com',
+        To      => 'rt at localhost',
+        Queue   => 'General',
+        Content => 'Testing ticket creation using REST API.',
+        CustomFields => {
+            'Single' => 'Hello world! Again.',
+        },
+    };
+
+    my $payload_cf_by_name_invalid = {
+        Subject => 'Ticket creation using REST - CF By Name (Invalid)',
+        From    => 'test at bestpractical.com',
+        To      => 'rt at localhost',
+        Queue   => 'General',
+        Content => 'Testing ticket creation using REST API.',
+        CustomFields => {
+            'Not Existant CF' => 'Hello world!',
+        },
+    };
+
+
     # 4.2.3 introduced a bug (e092e23) in CFs fixed in 4.2.9 (ab7ea15)
-    delete $payload->{CustomFields}
-        if RT::Handle::cmp_version($RT::VERSION, '4.2.3') >= 0
-        && RT::Handle::cmp_version($RT::VERSION, '4.2.8') <= 0;
+    if (   RT::Handle::cmp_version($RT::VERSION, '4.2.3') >= 0
+        && RT::Handle::cmp_version($RT::VERSION, '4.2.8') <= 0) {
+        delete $payload->{CustomFields};
+        delete $payload_cf_by_name->{CustomFields};
+    };
 
     # Rights Test - No CreateTicket
     my $res = $mech->post_json("$rest_base_path/ticket",
@@ -62,10 +88,39 @@ my ($ticket_url, $ticket_id);
     ok($ticket_url = $res->header('location'));
     ok(($ticket_id) = $ticket_url =~ qr[/ticket/(\d+)]);
 
+    # Create CF using name, mising right, how to fail it?
+    $res = $mech->post_json("$rest_base_path/ticket",
+        $payload_cf_by_name,
+        'Authorization' => $auth,
+    );
+    is($res->code, 201);
+
+    # To be able to lookup a CustomField by name, the user needs to have
+    # that right.
+    $user->PrincipalObj->GrantRight( Right => 'SeeCustomField');
+
+    # Create CF using name
+    $res = $mech->post_json("$rest_base_path/ticket",
+        $payload_cf_by_name,
+        'Authorization' => $auth,
+    );
+    is($res->code, 201);
+    ok($ticket_url_cf_by_name = $res->header('location'));
+    ok(($ticket_id_cf_by_name) = $ticket_url_cf_by_name =~ qr[/ticket/(\d+)]);
+
+    # Create CF using name (invalid)
+    $res = $mech->post_json("$rest_base_path/ticket",
+        $payload_cf_by_name_invalid,
+        'Authorization' => $auth,
+    );
+    is($res->code, 201);
+
    TODO: {
-       local $TODO = "this warns due to specifying a CF with no permission to see" if RT::Handle::cmp_version($RT::VERSION, '4.4.0') >= 0;
-       is(@warnings, 0, "no warnings");
+       local $TODO = "this warns due to specifying a CF with no permission to see" if RT::Handle::cmp_version($RT::VERSION, '4.4.0') || RT::Handle::cmp_version($RT::VERSION, '4.4.4') >= 0;
+       is(@warnings, 0, "no warnings") or diag(join("\n",'warnings : ', @warnings));
    }
+
+    $user->PrincipalObj->RevokeRight( Right => 'SeeCustomField');
 }
 
 # Ticket Display
@@ -104,6 +159,7 @@ my $no_ticket_cf_values = bag(
 {
     $user->PrincipalObj->GrantRight( Right => 'SeeCustomField');
 
+    # CustomField by Id
     my $res = $mech->get($ticket_url,
         'Authorization' => $auth,
     );
@@ -159,6 +215,34 @@ my $no_ticket_cf_values = bag(
 	Name       => 'Multi',
 	Type       => 'Freeform',
     }), 'multi cf');
+
+    # CustomField by Name
+    $res = $mech->get($ticket_url_cf_by_name,
+        'Authorization' => $auth,
+    );
+    is($res->code, 200);
+
+    $content = $mech->json_response;
+    is($content->{id}, $ticket_id_cf_by_name);
+    is($content->{Type}, 'ticket');
+    is($content->{Status}, 'new');
+    is($content->{Subject}, 'Ticket creation using REST - CF By Name');
+    is_deeply($content->{CustomFieldsValues}, { $single_cf_id => [], $multi_cf_id => [] }, 'No ticket custom field values');
+    cmp_deeply(
+        [grep { $_->{ref} eq 'customfield' } @{ $content->{'_hyperlinks'} }],
+        [{
+            ref => 'customfield',
+            id  => $single_cf_id,
+            type => 'customfield',
+            _url => re(qr[$rest_base_path/customfield/$single_cf_id$]),
+        }, {
+            ref => 'customfield',
+            id  => $multi_cf_id,
+            type => 'customfield',
+            _url => re(qr[$rest_base_path/customfield/$multi_cf_id$]),
+        }],
+        'Two CF hypermedia',
+    );
 }
 
 # Ticket Update without ModifyCustomField
@@ -288,6 +372,17 @@ my $no_ticket_cf_values = bag(
         },
     };
 
+    my $payload_cf_by_name = {
+        Subject => 'Ticket creation using REST - CF By Name',
+        From    => 'test at bestpractical.com',
+        To      => 'rt at localhost',
+        Queue   => 'General',
+        Content => 'Testing ticket creation using REST API.',
+        CustomFields => {
+            'Single' => 'Hello world! Again.',
+        },
+    };
+
     my $res = $mech->post_json("$rest_base_path/ticket",
         $payload,
         'Authorization' => $auth,
@@ -295,10 +390,19 @@ my $no_ticket_cf_values = bag(
     is($res->code, 201);
     ok($ticket_url = $res->header('location'));
     ok(($ticket_id) = $ticket_url =~ qr[/ticket/(\d+)]);
+
+    $res = $mech->post_json("$rest_base_path/ticket",
+        $payload_cf_by_name,
+        'Authorization' => $auth,
+    );
+    is($res->code, 201);
+    ok($ticket_url_cf_by_name = $res->header('location'));
+    ok(($ticket_id_cf_by_name) = $ticket_url_cf_by_name =~ qr[/ticket/(\d+)]);
 }
 
 # Rights Test - With ShowTicket and SeeCustomField
 {
+    # CustomField by Id
     my $res = $mech->get($ticket_url,
         'Authorization' => $auth,
     );
@@ -314,7 +418,22 @@ my $no_ticket_cf_values = bag(
     is($content->{Type}, 'ticket');
     is($content->{Status}, 'new');
     is($content->{Subject}, 'Ticket creation using REST');
-    cmp_deeply($content->{CustomFields}, $ticket_cf_value, 'Ticket custom field');
+
+    is_deeply($content->{'CustomFieldsValues'}{$single_cf_id}, ['Hello world!'], 'Ticket custom field');
+    cmp_deeply($content->{'CustomFields'}, $ticket_cf_value, 'Ticket custom field');
+
+    # CustomField by Name
+    $res = $mech->get($ticket_url_cf_by_name,
+        'Authorization' => $auth,
+    );
+    is($res->code, 200);
+
+    $content = $mech->json_response;
+    is($content->{id}, $ticket_id_cf_by_name);
+    is($content->{Type}, 'ticket');
+    is($content->{Status}, 'new');
+    is($content->{Subject}, 'Ticket creation using REST - CF By Name');
+    is_deeply($content->{'CustomFieldsValues'}{$single_cf_id}, ['Hello world! Again.'], 'Ticket custom field (by name)');
 }
 
 # Ticket Creation for multi-value CF

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


More information about the Bps-public-commit mailing list