Add submitting txn CFs with comments/correspondence to REST API

Andrew Ruthven <[email protected]> Tue, 26 Jan 2016 17:16:33 +1300
Newsgroups gmane.comp.bug-tracking.request-tracker.devel
Message-ID <[email protected]>
[Resent from an account which is subscribed to rt-devel@...]

Hi,

I asked on rt-users last week about being able to set transaction
custom fields when submitting comments and correspondence via the REST
API. The response was that I wasn't crazy and that this wasn't
currently possible.

The attached patches provide this functionality and (hopefully)
appropriate test cases.

These are against RT 4.2 - I haven't tried applying them to 4.4.

You can also view them here:
https://github.com/catalyst/rt/tree/4.2/add-txn-rest-customfield

I'm happy to make a pull request if you'd like, but I'm sending this
email as requested in docs/hacking.pod .

Cheers,
Andrew

-- 
Andrew Ruthven, Wellington, New Zealand
[email protected]             |     linux.conf.au 2016 
  New Zealand's only Cloud:   | LCA By the Bay, Geelong, AU
https://catalyst.net.nz/cloud | http://lca2016.linux.org.au
0001-Don-t-try-and-use-vsplit-if-the-values-are-undefined.patch (text/x-patch, 1.8 KB)
From 8d87c4914f892ce2dd558a520ad564d35ce6b5da Mon Sep 17 00:00:00 2001
From: Andrew Ruthven <[email protected]>
Date: Tue, 26 Jan 2016 15:50:24 +1300
Subject: [PATCH 1/3] Don't try and use vsplit if the values are undefined.

This causes a warning to be thrown if you send comments via REST
without these fields being set.
---
 share/html/REST/1.0/Forms/ticket/comment | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/share/html/REST/1.0/Forms/ticket/comment b/share/html/REST/1.0/Forms/ticket/comment
index 22b70ff..c6d6779 100644
--- a/share/html/REST/1.0/Forms/ticket/comment
+++ b/share/html/REST/1.0/Forms/ticket/comment
@@ -78,7 +78,8 @@ unless ($action =~ /^(?:Comment|Correspond)$/) {
 }
 
 my $text = $changes{Text};
-my @atts = @{ vsplit($changes{Attachment}) };
+my @atts = @{ vsplit($changes{Attachment}) }
+    if defined $changes{Attachment};
 
 if (!$changes{Text} && @atts == 0) {
     $e = 1;
@@ -117,12 +118,12 @@ unless ($ticket->CurrentUserHasRight('ModifyTicket') ||
     goto OUTPUT;
 }
 
-my $cc = join ", ", @{ vsplit($changes{Cc}) };
-my $bcc = join ", ", @{ vsplit($changes{Bcc}) };
-my ($n, $s) = $ticket->$action(MIMEObj => $ent,
-                               CcMessageTo => $cc,
-                               BccMessageTo => $bcc,
-                               TimeTaken => $changes{TimeWorked} || 0);
+my $cc = join ", ", @{ vsplit($changes{Cc} || '') };
+my $bcc = join ", ", @{ vsplit($changes{Bcc} || '') };
+my ($n, $s, $txn) = $ticket->$action(MIMEObj => $ent,
+                                     CcMessageTo => $cc,
+                                     BccMessageTo => $bcc,
+                                     TimeTaken => $changes{TimeWorked} || 0);
 $c = "# ".$s;
 if ($changes{Status}) {
    my  ($status_n, $status_s) = $ticket->SetStatus($changes{'Status'} );   
-- 
2.7.0.rc3
0002-Provide-a-helper-to-create-a-Transaction-CustomField.patch (text/x-patch, 1.4 KB)
From c20be27838b146fdf02d5d6fff14225a7bd6a39a Mon Sep 17 00:00:00 2001
From: Andrew Ruthven <[email protected]>
Date: Tue, 26 Jan 2016 15:51:22 +1300
Subject: [PATCH 2/3] Provide a helper to create a Transaction CustomField

Allow tests to easily create or reuse transaction custom fields on
a queue.
---
 lib/RT/Test.pm | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/lib/RT/Test.pm b/lib/RT/Test.pm
index 3d223e5..dcedba0 100644
--- a/lib/RT/Test.pm
+++ b/lib/RT/Test.pm
@@ -980,6 +980,33 @@ sub load_or_create_custom_field {
     return $obj;
 }
 
+=head2 load_or_create_txn_custom_field
+
+=cut
+
+sub load_or_create_txn_custom_field {
+    my $self = shift;
+    my %args = ( Disabled => 0, @_ );
+    my $obj = RT::CustomField->new( RT->SystemUser );
+    if ( $args{'Name'} ) {
+        $obj->LoadByName(
+            Name       => $args{'Name'},
+            LookupType => RT::Transaction->CustomFieldLookupType,
+            ObjectId   => $args{'Queue'}->id,
+        );
+    } else {
+        die "Name is required";
+    }
+    unless ( $obj->id ) {
+        $args{'LookupType'} = 'RT::Queue-RT::Ticket-RT::Transaction';
+        my $queue = delete $args{'Queue'};
+        my ($val, $msg) = $obj->Create( %args );
+        $obj->AddToObject($queue);
+    }
+
+    return $obj;
+}
+
 sub last_ticket {
     my $self = shift;
     my $current = shift;
-- 
2.7.0.rc3
0003-Allow-REST-1.0-API-to-set-Transaction-CustomFields.patch (text/x-patch, 6.7 KB)
From 35650ecb461ec6075299bc6dde00f993b180f337 Mon Sep 17 00:00:00 2001
From: Andrew Ruthven <[email protected]>
Date: Tue, 26 Jan 2016 15:52:42 +1300
Subject: [PATCH 3/3] Allow REST 1.0 API to set Transaction CustomFields.

---
 share/html/REST/1.0/Forms/ticket/comment |  33 ++++++++
 t/web/rest-txn-cf.t                      | 140 +++++++++++++++++++++++++++++++
 2 files changed, 173 insertions(+)
 create mode 100644 t/web/rest-txn-cf.t

diff --git a/share/html/REST/1.0/Forms/ticket/comment b/share/html/REST/1.0/Forms/ticket/comment
index c6d6779..de7f81e 100644
--- a/share/html/REST/1.0/Forms/ticket/comment
+++ b/share/html/REST/1.0/Forms/ticket/comment
@@ -55,6 +55,8 @@ $id
 use MIME::Entity;
 use RT::Interface::REST;
 
+my $cf_spec = RT::Interface::REST->custom_field_spec(1);
+
 $RT::Logger->debug("Got ticket id=$id for comment");
 $RT::Logger->debug("Got args @{[keys(%changes)]}.");
 
@@ -107,6 +109,20 @@ $ent->attach(
     }
 }
 
+{
+    my ($status, @msg) = $m->comp(
+        '/Elements/ValidateCustomFields',
+        CustomFields => $ticket->TransactionCustomFields,
+        Object => RT::Transaction->new( $session{'CurrentUser'} ),
+        ARGSRef => \%ARGS
+    );
+    unless ( $status ) {
+        $e = 1;
+        $c = "# " . join("\n# ", @msg);
+        goto OUTPUT;
+    }
+}
+
 unless ($ticket->CurrentUserHasRight('ModifyTicket') ||
         ($action eq "Comment" &&
          $ticket->CurrentUserHasRight("CommentOnTicket")) ||
@@ -130,6 +146,23 @@ if ($changes{Status}) {
   $c .= "\n# ".$status_s;
 }
 
+my %v;
+foreach my $k (keys %changes) {
+    if ($k =~ /^$cf_spec/) {
+        my $key = $1 || $2;
+
+        my $cf = $txn->LoadCustomFieldByIdentifier($key);
+
+        if (not $cf->id) {
+            $c .= "\n# Invalid custom field name ($key)";
+            delete $changes{$k};
+            next;
+        }
+        $v{"CustomField-".$cf->Id()} = delete $changes{$k};
+    }
+}
+$txn->UpdateCustomFields(%v);
+
 OUTPUT:
 
 return [ $c, $o, $k, $e ];
diff --git a/t/web/rest-txn-cf.t b/t/web/rest-txn-cf.t
new file mode 100644
index 0000000..2954bf0
--- /dev/null
+++ b/t/web/rest-txn-cf.t
@@ -0,0 +1,140 @@
+use strict;
+use warnings;
+use RT::Interface::REST;
+
+use RT::Test tests => 28;
+use Test::Warn;
+
+my ($baseurl, $m) = RT::Test->started_ok;
+
+my $queue = RT::Test->load_or_create_queue(Name => 'General');
+ok($queue->Id, "loaded the General queue");
+
+{
+    my $cf = RT::Test->load_or_create_txn_custom_field(
+        Name  => 'txn_cf',
+        Type  => 'FreeformSingle',
+        Queue => $queue,
+    );
+    ok($cf->Id, "created a CustomField: txn_cf");
+}
+
+my $other_queue = RT::Test->load_or_create_queue(Name => 'Other Queue');
+ok($other_queue->Id, "loaded the Other Queue queue");
+
+{
+    my $cf = RT::Test->load_or_create_txn_custom_field(
+        Name  => 'txn_other_queue_cf',
+        Type  => 'FreeformSingle',
+        Queue => $other_queue,
+    );
+    ok($cf->Id, "created a CustomField");
+}
+
+$m->post("$baseurl/REST/1.0/ticket/new", [
+    user    => 'root',
+    pass    => 'password',
+    format  => 'l',
+]);
+
+my $text = $m->content;
+my @lines = $text =~ m{.*}g;
+shift @lines; # header
+
+ok($text =~ s/Subject:\s*$/Subject: REST interface/m, "successfully replaced subject");
+
+$m->post("$baseurl/REST/1.0/ticket/edit", [
+    user    => 'root',
+    pass    => 'password',
+
+    content => $text,
+], Content_Type => 'form-data');
+
+my ($id) = $m->content =~ /Ticket (\d+) created/;
+ok($id, "got ticket #$id");
+
+$text = join("\n", ( "Ticket: $id", "Action: correspond", "Content-Type: text/plain" ));
+$m->post(
+    "$baseurl/REST/1.0/ticket/$id/comment",
+    [
+        user => 'root',
+        pass => 'password',
+        content => "$text\nText: Test with no CF",
+    ],
+    Content_Type => 'form-data'
+);
+like($m->content, qr{Correspondence added}, "correspondance added - no CF");
+
+my $with_valid_cf = $text . "\nText: Test with valid CF\nCF.{txn_cf}: valid cf";
+
+$m->post(
+    "$baseurl/REST/1.0/ticket/$id/comment",
+    [
+        user => 'root',
+        pass => 'password',
+        content => $with_valid_cf,
+    ],
+    Content_Type => 'form-data'
+);
+like($m->content, qr{Correspondence added}, "correspondance added - valid CF");
+unlike($m->content, qr{Invalid custom field name}, "no invalid custom field - valid CF");
+
+my $ticket = RT::Ticket->new(RT->SystemUser);
+$ticket->Load($id);
+is($ticket->Id, $id, "loaded the REST-created ticket");
+is($ticket->Subject, "REST interface", "subject successfully set");
+
+my $txn = $ticket->Transactions->Last;
+my ($msg, @attachments) = @{$txn->Attachments->ItemsArrayRef};
+like($msg->Content, qr/Test with valid CF/, "Transaction contains expected content - valid CF");
+
+is($txn->FirstCustomFieldValue('txn_cf'), "valid cf", "CF successfully set - valid CF");
+
+
+my $with_nonexistant_cf = $text . "\nText: Test with invalid CF\nCF.{other_cf}: invalid cf";
+
+$m->post(
+    "$baseurl/REST/1.0/ticket/$id/comment",
+    [
+        user => 'root',
+        pass => 'password',
+        content => $with_nonexistant_cf,
+    ],
+    Content_Type => 'form-data'
+);
+like($m->content, qr{Correspondence added}, "correspondance added - nonexistant CF");
+like($m->content, qr{Invalid custom field name}, "invalid custom field - nonexistant CF");
+
+$ticket->Load($id);
+is($ticket->Id, $id, "loaded the REST-created ticket");
+is($ticket->Subject, "REST interface", "subject successfully set");
+
+$txn = $ticket->Transactions->Last;
+($msg, @attachments) = @{$txn->Attachments->ItemsArrayRef};
+like($msg->Content, qr/Test with invalid CF/, "Transaction contains expected content - invalid CF");
+
+warning_like {$txn->FirstCustomFieldValue('other_cf')} qr"Couldn't load custom field by 'other_cf' identifier", "CF isn't set - invalid CF";
+
+my $with_other_queue_cf = $text . "\nText: Test with other queue CF\nCF.{txn_other_queue_cf}: invalid cf";
+
+$m->post(
+    "$baseurl/REST/1.0/ticket/$id/comment",
+    [
+        user => 'root',
+        pass => 'password',
+        content => $with_other_queue_cf,
+    ],
+    Content_Type => 'form-data'
+);
+like($m->content, qr{Correspondence added}, "correspondance added - other queue CF");
+like($m->content, qr{Invalid custom field name}, "invalid custom field - other queue CF");
+
+$ticket->Load($id);
+is($ticket->Id, $id, "loaded the REST-created ticket");
+is($ticket->Subject, "REST interface", "subject successfully set");
+
+$txn = $ticket->Transactions->Last;
+($msg, @attachments) = @{$txn->Attachments->ItemsArrayRef};
+like($msg->Content, qr/Test with other queue CF/, "Transaction contains expected content - other queue CF");
+
+warning_like {$txn->FirstCustomFieldValue('txn_other_queue_cf')} qr"Couldn't load custom field by 'txn_other_queue_cf' identifier", "CF isn't set - other queue CF";
-- 
2.7.0.rc3