[Bps-public-commit] rt-extension-rest2 branch, master, updated. 9d62b8514679adcbcfb457f232fffa8d5034c7bb
Shawn Moore
shawn at bestpractical.com
Mon Jun 26 19:36:00 EDT 2017
The branch, master has been updated
via 9d62b8514679adcbcfb457f232fffa8d5034c7bb (commit)
via 45f5638560a65d46c12f5ff81f8dd2ba4fc56186 (commit)
via c843f05da07c0f7626fdce8a13e9ed69a6c43873 (commit)
via 987534a4ff3a2db276f2c559f77d750efd52a247 (commit)
via 34f22055f4b9f2c1731e581fd34a02d018461204 (commit)
via de556e657f9f607cc39acfbca6ec73eb0a7ddb78 (commit)
via 70404dc6ab13e70357ce704c2378ba8a03f8795b (commit)
from b5222f00615ff4618f4e141bc92b779cef7f5a18 (commit)
Summary of changes:
lib/RT/Extension/REST2/Dispatcher.pm | 6 +-
lib/RT/Extension/REST2/Resource/Attachment.pm | 2 +-
lib/RT/Extension/REST2/Resource/Queue.pm | 2 +-
lib/RT/Extension/REST2/Resource/Record/Writable.pm | 82 ++++++-
lib/RT/Extension/REST2/Resource/Ticket.pm | 7 +-
lib/RT/Extension/REST2/Resource/Transaction.pm | 2 +-
lib/RT/Extension/REST2/Resource/User.pm | 20 ++
lib/RT/Extension/REST2/Resource/Users.pm | 7 +
lib/RT/Extension/REST2/Util.pm | 11 +-
t/ticket-customfields.t | 118 ++++++++--
t/user-customfields.t | 249 +++++++++++++++++++++
11 files changed, 473 insertions(+), 33 deletions(-)
create mode 100644 t/user-customfields.t
- Log -----------------------------------------------------------------
commit 70404dc6ab13e70357ce704c2378ba8a03f8795b
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Mon Jun 26 22:29:56 2017 +0000
Use a hash of CustomFields id -> value
This way, apps can support multiple CFs with the same name, using the ID
internally. Later we'll provide CF info as hypermedia
diff --git a/lib/RT/Extension/REST2/Resource/Record/Writable.pm b/lib/RT/Extension/REST2/Resource/Record/Writable.pm
index 1edb800..8244155 100644
--- a/lib/RT/Extension/REST2/Resource/Record/Writable.pm
+++ b/lib/RT/Extension/REST2/Resource/Record/Writable.pm
@@ -60,19 +60,20 @@ sub _update_custom_fields {
my $record = $self->record;
my @results;
- foreach my $arg ( keys %$data ) {
- next unless $arg =~ /^CustomField-(\d+)$/i;
- my $cfid = $1;
+ return unless $data->{CustomFields};
+
+ foreach my $cfid (keys %{ $data->{CustomFields} }) {
+ my $val = $data->{CustomFields}{$cfid};
+
my $cf = $record->LoadCustomFieldByIdentifier($cfid);
next unless $cf->ObjectTypeFromLookupType($cf->__Value('LookupType'))->isa(ref $record);
if ($cf->SingleValue) {
- my $val = $data->{$arg};
if (ref($val) eq 'ARRAY') {
$val = $val->[0];
}
elsif (ref($val)) {
- die "Invalid value type for $arg";
+ die "Invalid value type for CustomField $cfid";
}
my ($ok, $msg) = $record->AddCustomFieldValue(
@@ -104,7 +105,23 @@ sub update_resource {
sub create_record {
my $self = shift;
my $data = shift;
- return $self->record->Create( %$data );
+
+ my $record = $self->record;
+ my %args = %$data;
+
+ # 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
+ if ($record->isa('RT::Ticket')) {
+ if (my $cfs = delete $args{CustomFields}) {
+ while (my ($id, $value) = each(%$cfs)) {
+ $args{"CustomField-$id"} = $value;
+ }
+ }
+ }
+
+ my ($ok, @rest) = $record->Create(%args);
+ return ($ok, @rest);
}
sub create_resource {
diff --git a/lib/RT/Extension/REST2/Resource/Ticket.pm b/lib/RT/Extension/REST2/Resource/Ticket.pm
index 0545666..8c80de8 100644
--- a/lib/RT/Extension/REST2/Resource/Ticket.pm
+++ b/lib/RT/Extension/REST2/Resource/Ticket.pm
@@ -11,7 +11,8 @@ with (
'RT::Extension::REST2::Resource::Record::Hypermedia'
=> { -alias => { hypermedia_links => '_default_hypermedia_links' } },
'RT::Extension::REST2::Resource::Record::Deletable',
- 'RT::Extension::REST2::Resource::Record::Writable',
+ 'RT::Extension::REST2::Resource::Record::Writable'
+ => { -alias => { create_record => '_create_record' } },
);
sub dispatch_rules {
@@ -28,7 +29,7 @@ sub dispatch_rules {
sub create_record {
my $self = shift;
my $data = shift;
- my ($ok, $txn, $msg) = $self->record->Create(%$data);
+ my ($ok, $txn, $msg) = $self->_create_record($data);
return ($ok, $msg);
}
diff --git a/lib/RT/Extension/REST2/Util.pm b/lib/RT/Extension/REST2/Util.pm
index d2d1c76..a15c52c 100644
--- a/lib/RT/Extension/REST2/Util.pm
+++ b/lib/RT/Extension/REST2/Util.pm
@@ -95,11 +95,12 @@ sub serialize_record {
# Custom fields; no role yet, but we have registered lookup types
my %registered_type = map {; $_ => 1 } RT::CustomField->LookupTypes;
if ($registered_type{$record->CustomFieldLookupType}) {
+ my %values;
+
my $cfs = $record->CustomFields;
while (my $cf = $cfs->Next) {
- # Multiple CFs with the same name will use the same key
- my $key = "CF." . $cf->Name;
- my $values = $data{$key} ||= [];
+ my $key = $cf->Id;
+ my $values = $values{$key} ||= [];
my $ocfvs = $cf->ValuesForObject( $record );
my $type = $cf->Type;
while (my $ocfv = $ocfvs->Next) {
@@ -117,6 +118,8 @@ sub serialize_record {
push @$values, $content;
}
}
+
+ $data{CustomFields} = \%values;
}
return \%data;
}
@@ -129,6 +132,8 @@ sub deserialize_record {
# Sanitize input for the Perl API
for my $field (sort keys %$data) {
+ next if $field eq 'CustomFields';
+
my $value = $data->{$field};
next unless ref $value;
if (looks_like_uid($value)) {
diff --git a/t/ticket-customfields.t b/t/ticket-customfields.t
index 89a432f..cfd18bd 100644
--- a/t/ticket-customfields.t
+++ b/t/ticket-customfields.t
@@ -28,7 +28,9 @@ my ($ticket_url, $ticket_id);
To => 'rt at localhost',
Queue => 'General',
Content => 'Testing ticket creation using REST API.',
- 'CustomField-' . $single_cf->Id => 'Hello world!',
+ CustomFields => {
+ $single_cf->Id => 'Hello world!',
+ },
};
# Rights Test - No CreateTicket
@@ -85,7 +87,7 @@ my ($ticket_url, $ticket_id);
is($content->{Type}, 'ticket');
is($content->{Status}, 'new');
is($content->{Subject}, 'Ticket creation using REST');
- is_deeply($content->{'CF.Freeform'}, undef, 'Ticket custom field not present');
+ is_deeply($content->{'CustomFields'}, {}, 'Ticket custom field not present');
}
# Rights Test - With ShowTicket and SeeCustomField
@@ -102,7 +104,7 @@ my ($ticket_url, $ticket_id);
is($content->{Type}, 'ticket');
is($content->{Status}, 'new');
is($content->{Subject}, 'Ticket creation using REST');
- is_deeply($content->{'CF.Freeform'}, [], 'Ticket custom field');
+ is_deeply($content->{CustomFields}, { $single_cf->Id => [], $multi_cf->Id => [] }, 'No ticket custom field values');
}
# Ticket Update without ModifyCustomField
@@ -110,7 +112,9 @@ my ($ticket_url, $ticket_id);
my $payload = {
Subject => 'Ticket update using REST',
Priority => 42,
- 'CustomField-'.$single_cf->Id => 'Modified CF',
+ CustomFields => {
+ $single_cf->Id => 'Modified CF',
+ },
};
# Rights Test - No ModifyTicket
@@ -141,7 +145,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->{'CF.Freeform'}, [], 'No update to CF');
+ is_deeply($content->{CustomFields}, { $single_cf->Id => [], $multi_cf->Id => [] }, 'No update to CF');
}
# Ticket Update with ModifyCustomField
@@ -150,7 +154,9 @@ my ($ticket_url, $ticket_id);
my $payload = {
Subject => 'More updates using REST',
Priority => 43,
- 'CustomField-'.$single_cf->Id => 'Modified CF',
+ CustomFields => {
+ $single_cf->Id => 'Modified CF',
+ },
};
my $res = $mech->put_json($ticket_url,
$payload,
@@ -167,10 +173,10 @@ my ($ticket_url, $ticket_id);
my $content = $mech->json_response;
is($content->{Subject}, 'More updates using REST');
is($content->{Priority}, 43);
- is_deeply($content->{'CF.Freeform'}, ['Modified CF'], 'New CF value');
+ is_deeply($content->{CustomFields}, { $single_cf->Id => ['Modified CF'], $multi_cf->Id => [] }, 'New CF value');
# make sure changing the CF doesn't add a second OCFV
- $payload->{'CustomField-'.$single_cf->Id} = 'Modified Again';
+ $payload->{CustomFields}{$single_cf->Id} = 'Modified Again';
$res = $mech->put_json($ticket_url,
$payload,
'Authorization' => $auth,
@@ -184,10 +190,10 @@ my ($ticket_url, $ticket_id);
is($res->code, 200);
$content = $mech->json_response;
- is_deeply($content->{'CF.Freeform'}, ['Modified Again'], 'New CF value');
+ is_deeply($content->{CustomFields}, { $single_cf->Id => ['Modified Again'], $multi_cf->Id => [] }, 'New CF value');
# stop changing the CF, change something else, make sure CF sticks around
- delete $payload->{'CustomField-'.$single_cf->Id};
+ delete $payload->{CustomFields}{$single_cf->Id};
$payload->{Subject} = 'No CF change';
$res = $mech->put_json($ticket_url,
$payload,
@@ -202,7 +208,7 @@ my ($ticket_url, $ticket_id);
is($res->code, 200);
$content = $mech->json_response;
- is_deeply($content->{'CF.Freeform'}, ['Modified Again'], 'Same CF value');
+ is_deeply($content->{CustomFields}, { $single_cf->Id => ['Modified Again'], $multi_cf->Id => [] }, 'Same CF value');
}
# Ticket Creation with ModifyCustomField
@@ -213,7 +219,9 @@ my ($ticket_url, $ticket_id);
To => 'rt at localhost',
Queue => 'General',
Content => 'Testing ticket creation using REST API.',
- 'CustomField-' . $single_cf->Id => 'Hello world!',
+ CustomFields => {
+ $single_cf->Id => 'Hello world!',
+ },
};
my $res = $mech->post_json("$rest_base_path/ticket",
@@ -237,7 +245,7 @@ my ($ticket_url, $ticket_id);
is($content->{Type}, 'ticket');
is($content->{Status}, 'new');
is($content->{Subject}, 'Ticket creation using REST');
- is_deeply($content->{'CF.Freeform'}, ['Hello world!'], 'Ticket custom field');
+ is_deeply($content->{'CustomFields'}{$single_cf->Id}, ['Hello world!'], 'Ticket custom field');
}
done_testing;
commit de556e657f9f607cc39acfbca6ec73eb0a7ddb78
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Mon Jun 26 22:50:21 2017 +0000
Support and tests for multivalue CFs
diff --git a/lib/RT/Extension/REST2/Resource/Record/Writable.pm b/lib/RT/Extension/REST2/Resource/Record/Writable.pm
index 8244155..f34a8ae 100644
--- a/lib/RT/Extension/REST2/Resource/Record/Writable.pm
+++ b/lib/RT/Extension/REST2/Resource/Record/Writable.pm
@@ -6,6 +6,7 @@ use Moose::Role;
use namespace::autoclean;
use JSON ();
use RT::Extension::REST2::Util qw( deserialize_record error_as_json );
+use List::MoreUtils 'uniq';
with 'RT::Extension::REST2::Resource::Role::RequestBodyIsJSON'
=> { type => 'HASH' };
@@ -83,6 +84,50 @@ sub _update_custom_fields {
push @results, $msg;
}
else {
+ my %count;
+ my @vals = ref($val) eq 'ARRAY' ? @$val : $val;
+ for (@vals) {
+ $count{$_}++;
+ }
+
+ my $ocfvs = $cf->ValuesForObject( $record );
+ my %ocfv_id;
+ while (my $ocfv = $ocfvs->Next) {
+ my $content = $ocfv->Content;
+ $count{$content}--;
+ push @{ $ocfv_id{$content} }, $ocfv->Id;
+ }
+
+ # we want to provide a stable order, so first go by the order
+ # provided in the argument list, and then for any custom fields
+ # that are being removed, remove in sorted order
+ for my $key (uniq(@vals, sort keys %count)) {
+ my $count = $count{$key};
+ if ($count == 0) {
+ # new == old, no change needed
+ }
+ elsif ($count > 0) {
+ # new > old, need to add new
+ while ($count-- > 0) {
+ my ($ok, $msg) = $record->AddCustomFieldValue(
+ Field => $cf,
+ Value => $key,
+ );
+ push @results, $msg;
+ }
+ }
+ elsif ($count < 0) {
+ # old > new, need to remove old
+ while ($count++ < 0) {
+ my $id = shift @{ $ocfv_id{$key} };
+ my ($ok, $msg) = $record->DeleteCustomFieldValue(
+ Field => $cf,
+ ValueId => $id,
+ );
+ push @results, $msg;
+ }
+ }
+ }
}
}
diff --git a/t/ticket-customfields.t b/t/ticket-customfields.t
index cfd18bd..40fd217 100644
--- a/t/ticket-customfields.t
+++ b/t/ticket-customfields.t
@@ -92,7 +92,7 @@ my ($ticket_url, $ticket_id);
# Rights Test - With ShowTicket and SeeCustomField
{
- $user->PrincipalObj->GrantRight( Right => 'SeeCustomField', Object => $single_cf);
+ $user->PrincipalObj->GrantRight( Right => 'SeeCustomField');
my $res = $mech->get($ticket_url,
'Authorization' => $auth,
@@ -150,7 +150,7 @@ my ($ticket_url, $ticket_id);
# Ticket Update with ModifyCustomField
{
- $user->PrincipalObj->GrantRight( Right => 'ModifyCustomField', Object => $single_cf);
+ $user->PrincipalObj->GrantRight( Right => 'ModifyCustomField' );
my $payload = {
Subject => 'More updates using REST',
Priority => 43,
@@ -248,5 +248,85 @@ my ($ticket_url, $ticket_id);
is_deeply($content->{'CustomFields'}{$single_cf->Id}, ['Hello world!'], 'Ticket custom field');
}
+# Ticket Creation for multi-value CF
+for my $value (
+ 'scalar',
+ ['array reference'],
+ ['multiple', 'values'],
+) {
+ my $payload = {
+ Subject => 'Multi-value CF',
+ Queue => 'General',
+ CustomFields => {
+ $multi_cf->Id => $value,
+ },
+ };
+
+ my $res = $mech->post_json("$rest_base_path/ticket",
+ $payload,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 201);
+ ok($ticket_url = $res->header('location'));
+ ok(($ticket_id) = $ticket_url =~ qr[/ticket/(\d+)]);
+
+ $res = $mech->get($ticket_url,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+
+ my $content = $mech->json_response;
+ is($content->{id}, $ticket_id);
+ is($content->{Type}, 'ticket');
+ is($content->{Status}, 'new');
+ 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');
+}
+
+{
+ sub modify_multi_ok {
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+ my $input = shift;
+ my $messages = shift;
+ my $output = shift;
+ my $name = shift;
+
+ my $payload = {
+ CustomFields => {
+ $multi_cf->Id => $input,
+ },
+ };
+ my $res = $mech->put_json($ticket_url,
+ $payload,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+ is_deeply($mech->json_response, $messages);
+
+ $res = $mech->get($ticket_url,
+ 'Authorization' => $auth,
+ );
+ 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');
+ }
+
+ # starting point: ['multiple', 'values'],
+ modify_multi_ok(['multiple', 'values'], [], ['multiple', 'values'], 'no change');
+ modify_multi_ok(['multiple', 'values', 'new'], ['new added as a value for Multi'], ['multiple', 'new', 'values'], 'added "new"');
+ modify_multi_ok(['multiple', 'new'], ['values is no longer a value for custom field Multi'], ['multiple', 'new'], 'removed "values"');
+ modify_multi_ok('replace all', ['replace all added as a value for Multi', 'multiple is no longer a value for custom field Multi', 'new is no longer a value for custom field Multi'], ['replace all'], 'replaced all values');
+ modify_multi_ok([], ['replace all is no longer a value for custom field Multi'], [], 'removed all values');
+
+ modify_multi_ok(['foo', 'foo', 'bar'], ['foo added as a value for Multi', undef, 'bar added as a value for Multi'], ['bar', 'foo'], 'multiple values with the same name');
+ modify_multi_ok(['foo', 'bar'], [], ['bar', 'foo'], 'multiple values with the same name');
+ modify_multi_ok(['bar'], ['foo is no longer a value for custom field Multi'], ['bar'], 'multiple values with the same name');
+ modify_multi_ok(['bar', 'bar', 'bar'], [undef, undef], ['bar'], 'multiple values with the same name');
+}
+
done_testing;
commit 34f22055f4b9f2c1731e581fd34a02d018461204
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Mon Jun 26 22:57:26 2017 +0000
Allow optional / after record id in paths like /ticket/1/
diff --git a/lib/RT/Extension/REST2/Resource/Attachment.pm b/lib/RT/Extension/REST2/Resource/Attachment.pm
index 165ac85..93c71b7 100644
--- a/lib/RT/Extension/REST2/Resource/Attachment.pm
+++ b/lib/RT/Extension/REST2/Resource/Attachment.pm
@@ -15,7 +15,7 @@ sub dispatch_rules {
block => sub { { record_class => 'RT::Attachment' } },
),
Path::Dispatcher::Rule::Regex->new(
- regex => qr{^/attachment/(\d+)$},
+ regex => qr{^/attachment/(\d+)/?$},
block => sub { { record_class => 'RT::Attachment', record_id => shift->pos(1) } },
)
}
diff --git a/lib/RT/Extension/REST2/Resource/Queue.pm b/lib/RT/Extension/REST2/Resource/Queue.pm
index 0019277..0d68da5 100644
--- a/lib/RT/Extension/REST2/Resource/Queue.pm
+++ b/lib/RT/Extension/REST2/Resource/Queue.pm
@@ -19,7 +19,7 @@ sub dispatch_rules {
block => sub { { record_class => 'RT::Queue' } },
),
Path::Dispatcher::Rule::Regex->new(
- regex => qr{^/queue/(\d+)$},
+ regex => qr{^/queue/(\d+)/?$},
block => sub { { record_class => 'RT::Queue', record_id => shift->pos(1) } },
)
}
diff --git a/lib/RT/Extension/REST2/Resource/Ticket.pm b/lib/RT/Extension/REST2/Resource/Ticket.pm
index 8c80de8..0b81df5 100644
--- a/lib/RT/Extension/REST2/Resource/Ticket.pm
+++ b/lib/RT/Extension/REST2/Resource/Ticket.pm
@@ -21,7 +21,7 @@ sub dispatch_rules {
block => sub { { record_class => 'RT::Ticket' } },
),
Path::Dispatcher::Rule::Regex->new(
- regex => qr{^/ticket/(\d+)$},
+ regex => qr{^/ticket/(\d+)/?$},
block => sub { { record_class => 'RT::Ticket', record_id => shift->pos(1) } },
)
}
diff --git a/lib/RT/Extension/REST2/Resource/Transaction.pm b/lib/RT/Extension/REST2/Resource/Transaction.pm
index 805bf5a..03c26e0 100644
--- a/lib/RT/Extension/REST2/Resource/Transaction.pm
+++ b/lib/RT/Extension/REST2/Resource/Transaction.pm
@@ -16,7 +16,7 @@ sub dispatch_rules {
block => sub { { record_class => 'RT::Transaction' } },
),
Path::Dispatcher::Rule::Regex->new(
- regex => qr{^/transaction/(\d+)$},
+ regex => qr{^/transaction/(\d+)/?$},
block => sub { { record_class => 'RT::Transaction', record_id => shift->pos(1) } },
)
}
commit 987534a4ff3a2db276f2c559f77d750efd52a247
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Mon Jun 26 23:21:36 2017 +0000
Dispatch rules for /user, /user/1, and /users
diff --git a/lib/RT/Extension/REST2/Resource/User.pm b/lib/RT/Extension/REST2/Resource/User.pm
index 44b2367..91dc224 100644
--- a/lib/RT/Extension/REST2/Resource/User.pm
+++ b/lib/RT/Extension/REST2/Resource/User.pm
@@ -12,6 +12,17 @@ with (
'RT::Extension::REST2::Resource::Record::Writable',
);
+sub dispatch_rules {
+ Path::Dispatcher::Rule::Regex->new(
+ regex => qr{^/user/?$},
+ block => sub { { record_class => 'RT::User' } },
+ ),
+ Path::Dispatcher::Rule::Regex->new(
+ regex => qr{^/user/(\d+)/?$},
+ block => sub { { record_class => 'RT::User', record_id => shift->pos(1) } },
+ ),
+}
+
around 'serialize' => sub {
my $orig = shift;
my $self = shift;
diff --git a/lib/RT/Extension/REST2/Resource/Users.pm b/lib/RT/Extension/REST2/Resource/Users.pm
index 13f4752..aa6d6f9 100644
--- a/lib/RT/Extension/REST2/Resource/Users.pm
+++ b/lib/RT/Extension/REST2/Resource/Users.pm
@@ -8,6 +8,13 @@ use namespace::autoclean;
extends 'RT::Extension::REST2::Resource::Collection';
with 'RT::Extension::REST2::Resource::Collection::QueryByJSON';
+sub dispatch_rules {
+ Path::Dispatcher::Rule::Regex->new(
+ regex => qr{^/users/?$},
+ block => sub { { collection_class => 'RT::Users' } },
+ ),
+}
+
sub searchable_fields {
my $class = $_[0]->collection->RecordClass;
grep {
commit c843f05da07c0f7626fdce8a13e9ed69a6c43873
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Mon Jun 26 23:22:41 2017 +0000
Fixup multivalue
diff --git a/lib/RT/Extension/REST2/Resource/Record/Writable.pm b/lib/RT/Extension/REST2/Resource/Record/Writable.pm
index f34a8ae..588f840 100644
--- a/lib/RT/Extension/REST2/Resource/Record/Writable.pm
+++ b/lib/RT/Extension/REST2/Resource/Record/Writable.pm
@@ -46,7 +46,7 @@ sub update_record {
AttributesRef => [ $self->record->WritableAttributes ],
);
- push @results, $self->_update_custom_fields($data);
+ push @results, $self->_update_custom_fields($data->{CustomFields});
# XXX TODO: Figure out how to return success/failure? Core RT::Record's
# ->Update will need to be replaced or improved.
@@ -61,10 +61,8 @@ sub _update_custom_fields {
my $record = $self->record;
my @results;
- return unless $data->{CustomFields};
-
- foreach my $cfid (keys %{ $data->{CustomFields} }) {
- my $val = $data->{CustomFields}{$cfid};
+ foreach my $cfid (keys %{ $data }) {
+ my $val = $data->{$cfid};
my $cf = $record->LoadCustomFieldByIdentifier($cfid);
next unless $cf->ObjectTypeFromLookupType($cf->__Value('LookupType'))->isa(ref $record);
@@ -154,18 +152,26 @@ sub create_record {
my $record = $self->record;
my %args = %$data;
+ my $cfs = delete $args{CustomFields};
+
# 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
if ($record->isa('RT::Ticket')) {
- if (my $cfs = delete $args{CustomFields}) {
+ if ($cfs) {
while (my ($id, $value) = each(%$cfs)) {
+ delete $cfs->{id};
$args{"CustomField-$id"} = $value;
}
}
}
my ($ok, @rest) = $record->Create(%args);
+
+ if ($ok && $cfs) {
+ $self->_update_custom_fields($cfs);
+ }
+
return ($ok, @rest);
}
commit 45f5638560a65d46c12f5ff81f8dd2ba4fc56186
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Mon Jun 26 23:23:43 2017 +0000
User CF tests
diff --git a/t/user-customfields.t b/t/user-customfields.t
new file mode 100644
index 0000000..fb30bb1
--- /dev/null
+++ b/t/user-customfields.t
@@ -0,0 +1,241 @@
+use strict;
+use warnings;
+use lib 't/lib';
+use RT::Extension::REST2::Test tests => undef;
+
+my $mech = RT::Extension::REST2::Test->mech;
+
+my $auth = RT::Extension::REST2::Test->authorization_header;
+my $rest_base_path = '/REST/2.0';
+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 $ocf = RT::ObjectCustomField->new( RT->SystemUser );
+( $ok, $msg ) = $ocf->Add( CustomField => $single_cf->id, ObjectId => 0 );
+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);
+$ocf = RT::ObjectCustomField->new( RT->SystemUser );
+( $ok, $msg ) = $ocf->Add( CustomField => $multi_cf->id, ObjectId => 0 );
+ok($ok, "Applied globally" );
+
+# User Creation with no AdminUsers
+my ($user_url, $user_id);
+{
+ my $payload = {
+ Name => 'user1',
+ EmailAddress => 'user1 at example.com',
+ CustomFields => {
+ $single_cf->Id => 'Hello world!',
+ },
+ };
+
+ # Rights Test - No AdminUsers
+ my $res = $mech->post_json("$rest_base_path/user",
+ $payload,
+ 'Authorization' => $auth,
+ );
+ TODO: {
+ local $TODO = "this should return 403";
+ is($res->code, 403);
+ }
+
+ # Rights Test - With AdminUsers
+ $user->PrincipalObj->GrantRight( Right => 'AdminUsers' );
+ $res = $mech->post_json("$rest_base_path/user",
+ $payload,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 201);
+ ok($user_url = $res->header('location'));
+ ok(($user_id) = $user_url =~ qr[/user/(\d+)]);
+}
+
+# Rights Test - no SeeCustomField
+{
+ my $res = $mech->get($user_url,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+
+ my $content = $mech->json_response;
+ 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');
+}
+
+# Rights Test - With SeeCustomField
+{
+ $user->PrincipalObj->GrantRight( Right => 'SeeCustomField');
+
+ my $res = $mech->get($user_url,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+
+ my $content = $mech->json_response;
+ 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');
+}
+
+# User Update without ModifyCustomField
+{
+ my $payload = {
+ Name => 'User1',
+ CustomFields => {
+ $single_cf->Id => 'Modified CF',
+ },
+ };
+
+ my $res = $mech->put_json($user_url,
+ $payload,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+ is_deeply($mech->json_response, ["User User1: Name changed from 'user1' to 'User1'", 'Could not add new custom field value: Permission Denied']);
+
+ $res = $mech->get($user_url,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+
+ my $content = $mech->json_response;
+ 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');
+}
+
+# User Update with ModifyCustomField
+{
+ $user->PrincipalObj->GrantRight( Right => 'ModifyCustomField' );
+ my $payload = {
+ EmailAddress => 'user1+rt at example.com',
+ CustomFields => {
+ $single_cf->Id => 'Modified CF',
+ },
+ };
+ my $res = $mech->put_json($user_url,
+ $payload,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+ is_deeply($mech->json_response, ["User User1: EmailAddress changed from 'user1\@example.com' to 'user1+rt\@example.com'", 'Freeform Modified CF added']);
+
+ $res = $mech->get($user_url,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+
+ 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');
+
+ # make sure changing the CF doesn't add a second OCFV
+ $payload->{CustomFields}{$single_cf->Id} = 'Modified Again';
+ $res = $mech->put_json($user_url,
+ $payload,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+ is_deeply($mech->json_response, ['Freeform Modified CF changed to Modified Again']);
+
+ $res = $mech->get($user_url,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+
+ $content = $mech->json_response;
+ is_deeply($content->{CustomFields}, { $single_cf->Id => ['Modified Again'], $multi_cf->Id => [] }, 'New CF value');
+
+ # stop changing the CF, change something else, make sure CF sticks around
+ delete $payload->{CustomFields}{$single_cf->Id};
+ $payload->{EmailAddress} = 'user1+rt.test at example.com';
+ $res = $mech->put_json($user_url,
+ $payload,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+ is_deeply($mech->json_response, ["User User1: EmailAddress changed from 'user1+rt\@example.com' to 'user1+rt.test\@example.com'"]);
+
+ $res = $mech->get($user_url,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+
+ $content = $mech->json_response;
+ is_deeply($content->{CustomFields}, { $single_cf->Id => ['Modified Again'], $multi_cf->Id => [] }, 'Same CF value');
+}
+
+# User Creation with ModifyCustomField
+{
+ my $payload = {
+ Name => 'user2',
+ CustomFields => {
+ $single_cf->Id => 'Hello world!',
+ },
+ };
+
+ my $res = $mech->post_json("$rest_base_path/user",
+ $payload,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 201);
+ ok($user_url = $res->header('location'));
+ ok(($user_id) = $user_url =~ qr[/user/(\d+)]);
+}
+
+# Rights Test - With SeeCustomField
+{
+ my $res = $mech->get($user_url,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+
+ 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');
+}
+
+# User Creation for multi-value CF
+for my $value (
+ 'scalar',
+ ['array reference'],
+ ['multiple', 'values'],
+) {
+ my $payload = {
+ Name => "user-$value",
+ CustomFields => {
+ $multi_cf->Id => $value,
+ },
+ };
+
+ my $res = $mech->post_json("$rest_base_path/user",
+ $payload,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 201);
+ ok($user_url = $res->header('location'));
+ ok(($user_id) = $user_url =~ qr[/user/(\d+)]);
+
+ $res = $mech->get($user_url,
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+
+ 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');
+}
+
+done_testing;
+
commit 9d62b8514679adcbcfb457f232fffa8d5034c7bb
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Mon Jun 26 23:27:19 2017 +0000
Support /user/name
The dispatcher needed to get a little bit smarter because now one
resource can match twice (/user/1 and /user/name both match the
/user/.* rule). Since that is intentional -- the purpose of the
dispatcher checking for only one match is to catch accidental
collisions -- we allow it by checking that only one resource matched,
not that only one rule matched.
We still use the first matching rule, so resource need to provide
rules in most specific to least specific. (which makes intuitive sense
anyway)
diff --git a/lib/RT/Extension/REST2/Dispatcher.pm b/lib/RT/Extension/REST2/Dispatcher.pm
index 449771f..1c7f780 100644
--- a/lib/RT/Extension/REST2/Dispatcher.pm
+++ b/lib/RT/Extension/REST2/Dispatcher.pm
@@ -5,6 +5,7 @@ use Moose;
use Web::Machine;
use Path::Dispatcher;
use Plack::Request;
+use List::MoreUtils 'uniq';
use Module::Pluggable (
search_path => ['RT::Extension::REST2::Resource'],
@@ -48,8 +49,9 @@ sub to_psgi_app {
if !$dispatch->has_matches;
my @matches = $dispatch->matches;
- if (@matches > 1) {
- RT->Logger->error("Path $env->{PATH_INFO} erroneously matched " . scalar(@matches) . " resources: " . (join ', ', map { $_->rule->{_rest2_resource} } @matches) . ". Refusing to dispatch.");
+ my @matched_resources = uniq map { $_->rule->{_rest2_resource} } @matches;
+ if (@matched_resources > 1) {
+ RT->Logger->error("Path $env->{PATH_INFO} erroneously matched " . scalar(@matched_resources) . " resources: " . (join ', ', @matched_resources) . ". Refusing to dispatch.");
return [500, ['Content-Type' => 'text/plain'], 'Internal Server Error']
}
diff --git a/lib/RT/Extension/REST2/Resource/User.pm b/lib/RT/Extension/REST2/Resource/User.pm
index 91dc224..640e05d 100644
--- a/lib/RT/Extension/REST2/Resource/User.pm
+++ b/lib/RT/Extension/REST2/Resource/User.pm
@@ -21,6 +21,15 @@ sub dispatch_rules {
regex => qr{^/user/(\d+)/?$},
block => sub { { record_class => 'RT::User', record_id => shift->pos(1) } },
),
+ Path::Dispatcher::Rule::Regex->new(
+ regex => qr{^/user/(.*?)/?$},
+ block => sub {
+ my ($match, $req) = @_;
+ my $user = RT::User->new($req->env->{"rt.current_user"});
+ $user->Load($match->pos(1));
+ return { record => $user };
+ },
+ ),
}
around 'serialize' => sub {
diff --git a/t/user-customfields.t b/t/user-customfields.t
index fb30bb1..5118435 100644
--- a/t/user-customfields.t
+++ b/t/user-customfields.t
@@ -67,6 +67,14 @@ my ($user_url, $user_id);
is($content->{EmailAddress}, 'user1 at example.com');
is($content->{Name}, 'user1');
is_deeply($content->{'CustomFields'}, {}, 'User custom field not present');
+
+ # can fetch user by name too
+ $res = $mech->get("$rest_base_path/user/user1",
+ 'Authorization' => $auth,
+ );
+ is($res->code, 200);
+
+ is_deeply($mech->json_response, $content, 'requesting user by name is same as user by id');
}
# Rights Test - With SeeCustomField
-----------------------------------------------------------------------
More information about the Bps-public-commit
mailing list