[Bps-public-commit] rt-extension-rest2 branch, update-custom-roles-on-correspond-and-comment, created. 1.09-13-g3fa8e15

Dianne Skoll dianne at bestpractical.com
Fri Jan 15 14:19:17 EST 2021


The branch, update-custom-roles-on-correspond-and-comment has been created
        at  3fa8e151b05ceac516c23d33d834ad0e1e9666e2 (commit)

- Log -----------------------------------------------------------------
commit e6c06b1a7eb5d19e225d48a9836038a8ad89152a
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Fri Jan 15 13:57:49 2021 -0500

    Move update_role_members Util.pm; implement fix_custom_role_ids

diff --git a/lib/RT/Extension/REST2/Resource/Record/Writable.pm b/lib/RT/Extension/REST2/Resource/Record/Writable.pm
index 3a4b763..d8e7068 100644
--- a/lib/RT/Extension/REST2/Resource/Record/Writable.pm
+++ b/lib/RT/Extension/REST2/Resource/Record/Writable.pm
@@ -5,7 +5,7 @@ use warnings;
 use Moose::Role;
 use namespace::autoclean;
 use JSON ();
-use RT::Extension::REST2::Util qw( deserialize_record error_as_json expand_uid update_custom_fields );
+use RT::Extension::REST2::Util qw( deserialize_record error_as_json expand_uid update_custom_fields update_role_members);
 use List::MoreUtils 'uniq';
 
 with 'RT::Extension::REST2::Resource::Role::RequestBodyIsJSON'
@@ -125,7 +125,7 @@ sub update_record {
     );
 
     push @results, update_custom_fields($self->record, $data->{CustomFields});
-    push @results, $self->_update_role_members($data);
+    push @results, update_role_members($self->record, $data);
     push @results, $self->_update_disabled($data->{Disabled})
       unless grep { $_ eq 'Disabled' } $self->record->WritableAttributes;
     push @results, $self->_update_privileged($data->{Privileged})
@@ -136,124 +136,6 @@ sub update_record {
     return @results;
 }
 
-sub _update_role_members {
-    my $self = shift;
-    my $data = shift;
-
-    my $record = $self->record;
-
-    return unless $record->DOES('RT::Record::Role::Roles');
-
-    my @results;
-
-    foreach my $role ($record->Roles) {
-        next unless exists $data->{$role};
-
-        # special case: RT::Ticket->Update already handles Owner for us
-        next if $role eq 'Owner' && $record->isa('RT::Ticket');
-
-        my $val = $data->{$role};
-
-        if ($record->Role($role)->{Single}) {
-            if (ref($val) eq 'ARRAY') {
-                $val = $val->[0];
-            }
-            elsif (ref($val)) {
-                die "Invalid value type for role $role";
-            }
-
-            my ($ok, $msg);
-            if ($record->can('AddWatcher')) {
-                ($ok, $msg) = $record->AddWatcher(
-                    Type => $role,
-                    User => $val,
-                );
-            } else {
-                ($ok, $msg) = $record->AddRoleMember(
-                    Type => $role,
-                    User => $val,
-                );
-            }
-            push @results, $msg;
-        }
-        else {
-            my %count;
-            my @vals;
-
-            for (ref($val) eq 'ARRAY' ? @$val : $val) {
-                my ($principal_id, $msg);
-
-                if (/^\d+$/) {
-                    $principal_id = $_;
-                }
-                elsif ($record->can('CanonicalizePrincipal')) {
-                    ((my $principal), $msg) = $record->CanonicalizePrincipal(User => $_);
-                    $principal_id = $principal->Id;
-                }
-                else {
-                    my $user = RT::User->new($record->CurrentUser);
-                    if (/@/) {
-                        ((my $ok), $msg) = $user->LoadOrCreateByEmail( $_ );
-                    } else {
-                        ((my $ok), $msg) = $user->Load( $_ );
-                    }
-                    $principal_id = $user->PrincipalId;
-                }
-
-                if (!$principal_id) {
-                    push @results, $msg;
-                    next;
-                }
-
-                push @vals, $principal_id;
-                $count{$principal_id}++;
-            }
-
-            my $group = $record->RoleGroup($role);
-            my $members = $group->MembersObj;
-            while (my $member = $members->Next) {
-                $count{$member->MemberId}--;
-            }
-
-            # RT::Ticket has specialized methods
-            my $add_method = $record->can('AddWatcher') ? 'AddWatcher' : 'AddRoleMember';
-            my $del_method = $record->can('DeleteWatcher') ? 'DeleteWatcher' : 'DeleteRoleMember';
-
-            # we want to provide a stable order, so first go by the order
-            # provided in the argument list, and then for any role members
-            # that are being removed, remove in sorted order
-            for my $id (uniq(@vals, sort keys %count)) {
-                my $count = $count{$id};
-                if ($count == 0) {
-                    # new == old, no change needed
-                }
-                elsif ($count > 0) {
-                    # new > old, need to add new
-                    while ($count-- > 0) {
-                        my ($ok, $msg) = $record->$add_method(
-                            Type        => $role,
-                            PrincipalId => $id,
-                        );
-                        push @results, $msg;
-                    }
-                }
-                elsif ($count < 0) {
-                    # old > new, need to remove old
-                    while ($count++ < 0) {
-                        my ($ok, $msg) = $record->$del_method(
-                            Type        => $role,
-                            PrincipalId => $id,
-                        );
-                        push @results, $msg;
-                    }
-                }
-            }
-        }
-    }
-
-    return @results;
-}
-
 sub _update_disabled {
     my $self = shift;
     my $data = shift;
diff --git a/lib/RT/Extension/REST2/Util.pm b/lib/RT/Extension/REST2/Util.pm
index 6a2ba19..55e05f8 100644
--- a/lib/RT/Extension/REST2/Util.pm
+++ b/lib/RT/Extension/REST2/Util.pm
@@ -21,6 +21,8 @@ use Sub::Exporter -setup => {
         custom_fields_for
         format_datetime
         update_custom_fields
+        update_role_members
+        fix_custom_role_ids
     ]]
 };
 
@@ -396,5 +398,154 @@ sub update_custom_fields {
     return @results;
 }
 
+sub update_role_members {
+    my $record = shift;
+    my $data = shift;
+
+    return unless $record->DOES('RT::Record::Role::Roles');
+
+    my @results;
+
+    foreach my $role ($record->Roles) {
+        next unless exists $data->{$role};
+
+        # special case: RT::Ticket->Update already handles Owner for us
+        next if $role eq 'Owner' && $record->isa('RT::Ticket');
+
+        my $val = $data->{$role};
+
+        if ($record->Role($role)->{Single}) {
+            if (ref($val) eq 'ARRAY') {
+                $val = $val->[0];
+            }
+            elsif (ref($val)) {
+                die "Invalid value type for role $role";
+            }
+
+            my ($ok, $msg);
+            if ($record->can('AddWatcher')) {
+                ($ok, $msg) = $record->AddWatcher(
+                    Type => $role,
+                    User => $val,
+                );
+            } else {
+                ($ok, $msg) = $record->AddRoleMember(
+                    Type => $role,
+                    User => $val,
+                );
+            }
+            push @results, $msg;
+        }
+        else {
+            my %count;
+            my @vals;
+
+            for (ref($val) eq 'ARRAY' ? @$val : $val) {
+                my ($principal_id, $msg);
+
+                if (/^\d+$/) {
+                    $principal_id = $_;
+                }
+                elsif ($record->can('CanonicalizePrincipal')) {
+                    ((my $principal), $msg) = $record->CanonicalizePrincipal(User => $_);
+                    $principal_id = $principal->Id;
+                }
+                else {
+                    my $user = RT::User->new($record->CurrentUser);
+                    if (/@/) {
+                        ((my $ok), $msg) = $user->LoadOrCreateByEmail( $_ );
+                    } else {
+                        ((my $ok), $msg) = $user->Load( $_ );
+                    }
+                    $principal_id = $user->PrincipalId;
+                }
+
+                if (!$principal_id) {
+                    push @results, $msg;
+                    next;
+                }
+
+                push @vals, $principal_id;
+                $count{$principal_id}++;
+            }
+
+            my $group = $record->RoleGroup($role);
+            my $members = $group->MembersObj;
+            while (my $member = $members->Next) {
+                $count{$member->MemberId}--;
+            }
+
+            # RT::Ticket has specialized methods
+            my $add_method = $record->can('AddWatcher') ? 'AddWatcher' : 'AddRoleMember';
+            my $del_method = $record->can('DeleteWatcher') ? 'DeleteWatcher' : 'DeleteRoleMember';
+
+            # we want to provide a stable order, so first go by the order
+            # provided in the argument list, and then for any role members
+            # that are being removed, remove in sorted order
+            for my $id (uniq(@vals, sort keys %count)) {
+                my $count = $count{$id};
+                if ($count == 0) {
+                    # new == old, no change needed
+                }
+                elsif ($count > 0) {
+                    # new > old, need to add new
+                    while ($count-- > 0) {
+                        my ($ok, $msg) = $record->$add_method(
+                            Type        => $role,
+                            PrincipalId => $id,
+                        );
+                        push @results, $msg;
+                    }
+                }
+                elsif ($count < 0) {
+                    # old > new, need to remove old
+                    while ($count++ < 0) {
+                        my ($ok, $msg) = $record->$del_method(
+                            Type        => $role,
+                            PrincipalId => $id,
+                        );
+                        push @results, $msg;
+                    }
+                }
+            }
+        }
+    }
+
+    return @results;
+}
+
+=head2 fix_custom_role_ids ( $record, $custom_roles )
+
+$record is the RT object (eg, an RT::Ticket) associated
+with custom roles.
+
+$custom_roles is a hashref where the keys are custom role
+IDs, names or email addresses and the values can be
+anything.  Returns a new hashref where all the keys
+are replaced with "RT::CustomRole-ID" if they were
+not originally in that form, and the values are kept
+the same.
+
+=cut
+
+sub fix_custom_role_ids
+{
+    my ($record, $custom_roles) = @_;
+    my $ret = {};
+    return $ret unless $custom_roles;
+
+    foreach my $key (keys(%$custom_roles)) {
+        if ($key =~ /^RT::CustomRole-\d+$/) {
+            # Already in the correct form
+            $ret->{$key} = $custom_roles->{$key};
+            next;
+        }
+
+        my $cr = RT::CustomRole->new($record->CurrentUser);
+        next unless $cr->Load($key);
+        $ret->{'RT::CustomRole-' . $cr->Id} = $custom_roles->{$key};
+    }
+    return $ret;
+}
 
 1;

commit 4350694ff015f8f6c84085bbf2e820632f847a9b
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Fri Jan 15 13:59:05 2021 -0500

    Allow updating of custom roles on ticket comment/correspond

diff --git a/lib/RT/Extension/REST2/Resource/Message.pm b/lib/RT/Extension/REST2/Resource/Message.pm
index f15d996..2d39e33 100644
--- a/lib/RT/Extension/REST2/Resource/Message.pm
+++ b/lib/RT/Extension/REST2/Resource/Message.pm
@@ -7,7 +7,7 @@ use namespace::autoclean;
 use MIME::Base64;
 
 extends 'RT::Extension::REST2::Resource';
-use RT::Extension::REST2::Util qw( error_as_json update_custom_fields );
+use RT::Extension::REST2::Util qw( error_as_json update_custom_fields update_role_members fix_custom_role_ids);
 
 sub dispatch_rules {
     Path::Dispatcher::Rule::Regex->new(
@@ -150,6 +150,11 @@ sub add_message {
 
     push @results, $msg;
     push @results, update_custom_fields($self->record, $args{CustomFields});
+
+    # update_role_members wants custom role IDs (like RT::CustomRole-ID)
+    # rather than role names.
+    my $renamed_custom_roles = fix_custom_role_ids($self->record, $args{CustomRoles});
+    push @results, update_role_members($self->record, $renamed_custom_roles);
     push @results, $self->_update_txn_custom_fields( $TransObj, $args{TxnCustomFields} || $args{TransactionCustomFields} );
 
     # Set ticket status if we were passed a "Status":"foo" argument

commit 287a7c8a06ebb7f399f7de0840fac2ff2790fe48
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Fri Jan 15 14:01:05 2021 -0500

    Document CustomFields, TxnCustomFields and CustomRoles data members

diff --git a/lib/RT/Extension/REST2.pm b/lib/RT/Extension/REST2.pm
index e11acfb..fe4622a 100644
--- a/lib/RT/Extension/REST2.pm
+++ b/lib/RT/Extension/REST2.pm
@@ -275,6 +275,31 @@ The new status (for example, "open", "rejected", etc.) to set the
 ticket to.  The Status value must be a valid status based on the
 lifecycle of the ticket's current queue.
 
+=item C<CustomRoles>
+
+A hash whose keys are custom role names and values are as described below:
+
+For a single-value custom role, the value must be a string representing an
+email address or user name; the custom role is set to the user with
+that email address or user name.
+
+For a multi-value custom role, the value can be a string representing
+an email address or user name, or can be an array of email addresses
+or user names; in either case, the members of the custom role are set
+to the corresponding users.
+
+=item C<CustomFields>
+
+A hash similar to the C<CustomRoles> hash, but whose keys are custom
+field names that apply to the Ticket; those fields are set to the
+supplied values.
+
+=item C<TxnCustomFields>
+
+A hash similar to the C<CustomRoles> hash, but whose keys are custom
+field names that apply to the Transaction; those fields are set
+to the supplied values.
+
 =back
 
 =head3 Add Attachments

commit 3fa8e151b05ceac516c23d33d834ad0e1e9666e2
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Fri Jan 15 14:03:31 2021 -0500

    Add unit test for updating custom roles on ticket comment/correspond

diff --git a/lib/RT/Extension/REST2/Test.pm.in b/lib/RT/Extension/REST2/Test.pm.in
index 3ff37c7..fb1cdaa 100644
--- a/lib/RT/Extension/REST2/Test.pm.in
+++ b/lib/RT/Extension/REST2/Test.pm.in
@@ -46,6 +46,7 @@ sub mech { RT::Extension::REST2::Test::Mechanize->new }
         $u->Create(
             Name => 'test',
             Password => 'password',
+            EmailAddress => 'test at rt.example',
             Privileged => 1,
         );
         return $u;
diff --git a/ticket-correspond-update-customroles.t b/ticket-correspond-update-customroles.t
new file mode 100644
index 0000000..f3462b7
--- /dev/null
+++ b/ticket-correspond-update-customroles.t
@@ -0,0 +1,239 @@
+use strict;
+use warnings;
+use RT::Extension::REST2::Test tests => undef;
+use Test::Deep;
+
+BEGIN {
+    plan skip_all => 'RT 4.4 required'
+        unless RT::Handle::cmp_version($RT::VERSION, '4.4.0') >= 0;
+}
+
+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;
+
+# Set up a couple of custom roles
+my $queue = RT::Test->load_or_create_queue( Name => "General" );
+
+my $single = RT::CustomRole->new(RT->SystemUser);
+my ($ok, $msg) = $single->Create(Name => 'Single Member', MaxValues => 1);
+ok($ok, $msg);
+my $single_id = $single->Id;
+
+($ok, $msg) = $single->AddToObject($queue->id);
+ok($ok, $msg);
+
+my $multi = RT::CustomRole->new(RT->SystemUser);
+($ok, $msg) = $multi->Create(Name => 'Multi Member');
+ok($ok, $msg);
+my $multi_id = $multi->Id;
+
+($ok, $msg) = $multi->AddToObject($queue->id);
+ok($ok, $msg);
+
+#for my $email (qw/multi at example.com test at localhost multi2 at example.com single2 at example.com/) {
+#    my $user = RT::User->new(RT->SystemUser);
+#    my ($ok, $msg) = $user->Create(Name => $email, EmailAddress => $email);
+#    ok($ok, $msg);
+#}
+
+$user->PrincipalObj->GrantRight( Right => $_ )
+    for qw/CreateTicket ShowTicket ModifyTicket OwnTicket AdminUsers SeeGroup SeeQueue/;
+
+# Ticket Creation
+my ($ticket_url, $ticket_id);
+{
+    my $payload = {
+        Subject => 'Ticket creation using REST',
+        Queue   => 'General',
+        Content => 'Testing ticket creation using REST API.',
+    };
+
+    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+)]);
+}
+
+# Ticket Display
+{
+    $user->PrincipalObj->GrantRight( Right => 'ShowTicket' );
+
+    my $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}, '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);
+}
+
+# Ticket Reply
+{
+    $user->PrincipalObj->GrantRight( Right => 'ReplyToTicket' );
+
+    my $res = $mech->get($ticket_url,
+        'Authorization' => $auth,
+    );
+    is($res->code, 200);
+    my $content = $mech->json_response;
+
+    my ($hypermedia) = grep { $_->{ref} eq 'correspond' } @{ $content->{_hyperlinks} };
+    ok($hypermedia, 'got correspond hypermedia');
+    like($hypermedia->{_url}, qr[$rest_base_path/ticket/$ticket_id/correspond$]);
+
+    my $correspond_url = $mech->url_for_hypermedia('correspond');
+    my $comment_url = $correspond_url;
+    $comment_url =~ s/correspond/comment/;
+
+    $res = $mech->post_json($correspond_url,
+        {
+            Content => 'Hello from hypermedia!',
+            ContentType => 'text/plain',
+            CustomRoles => {
+                'Single Member' => 'foo at bar.example',
+                'Multi Member' => 'quux at cabbage.example',
+            },
+        },
+        'Authorization' => $auth,
+    );
+    is($res->code, 201);
+    $content = $mech->json_response;
+
+    # Because CustomRoles are set in an unpredictable order, sort the
+    # responses so we have a predictable order.
+    @$content = sort { $a cmp $b } (@$content);
+    cmp_deeply($content, ['Added quux at cabbage.example as Multi Member for this ticket', re(qr/Correspondence added|Message recorded/), 'Single Member changed from Nobody to foo at bar.example']);
+    like($res->header('Location'), qr{$rest_base_path/transaction/\d+$});
+    $res = $mech->get($res->header('Location'),
+        'Authorization' => $auth,
+    );
+    is($res->code, 200);
+    $content = $mech->json_response;
+    is($content->{Type}, 'Correspond');
+    is($content->{TimeTaken}, 0);
+    is($content->{Object}{type}, 'ticket');
+    is($content->{Object}{id}, $ticket_id);
+
+    $res = $mech->get($mech->url_for_hypermedia('attachment'),
+        'Authorization' => $auth,
+    );
+    is($res->code, 200);
+    $content = $mech->json_response;
+    is($content->{Content}, 'Hello from hypermedia!');
+    is($content->{ContentType}, 'text/plain');
+
+    # Load the ticket and check the custom roles
+    my $ticket = RT::Ticket->new($user);
+    $ticket->Load($ticket_id);
+
+    is($ticket->RoleAddresses("RT::CustomRole-$single_id"), 'foo at bar.example',
+       "Single Member role set correctly");
+    is($ticket->RoleAddresses("RT::CustomRole-$multi_id"), 'quux at cabbage.example',
+       "Multi Member role set correctly");
+
+    # Update again (this time as a comment)
+    $res = $mech->post_json($comment_url,
+        {
+            Content => 'Hello from hypermedia!',
+            ContentType => 'text/plain',
+            CustomRoles => {
+                'Single Member' => 'foo-new at bar.example',
+                'Multi Member' => 'quux-new at cabbage.example',
+            },
+        },
+        'Authorization' => $auth,
+    );
+    is($res->code, 201);
+
+    is($ticket->RoleAddresses("RT::CustomRole-$single_id"), 'foo-new at bar.example',
+       "Single Member role set correctly");
+    is($ticket->RoleAddresses("RT::CustomRole-$multi_id"), 'quux-new at cabbage.example',
+       "Multi Member role updated correctly");
+
+    # Supply an array for multi-member role
+    $res = $mech->post_json($correspond_url,
+        {
+            Content => 'Hello from hypermedia!',
+            ContentType => 'text/plain',
+            CustomRoles => {
+                'Multi Member' => ['abacus at example.com', 'quux-new at cabbage.example'],
+            },
+        },
+        'Authorization' => $auth,
+    );
+    is($res->code, 201);
+
+    is($ticket->RoleAddresses("RT::CustomRole-$single_id"), 'foo-new at bar.example',
+       "Single Member role unchanged");
+    is($ticket->RoleAddresses("RT::CustomRole-$multi_id"), 'abacus at example.com, quux-new at cabbage.example',
+       "Multi Member role set correctly");
+
+    # Add an existing user to multi-member role
+    $res = $mech->post_json($correspond_url,
+        {
+            Content => 'Hello from hypermedia!',
+            ContentType => 'text/plain',
+            CustomRoles => {
+                'Multi Member' => 'abacus at example.com',
+            },
+        },
+        'Authorization' => $auth,
+    );
+    is($res->code, 201);
+
+    is($ticket->RoleAddresses("RT::CustomRole-$single_id"), 'foo-new at bar.example',
+       "Single Member role unchanged");
+    is($ticket->RoleAddresses("RT::CustomRole-$multi_id"), 'abacus at example.com',
+       "Multi Member role unchanged");
+
+    # Supply an array for single-member role
+    $res = $mech->post_json($correspond_url,
+        {
+            Content => 'Hello from hypermedia!',
+            ContentType => 'text/plain',
+            CustomRoles => {
+                'Single Member' => ['abacus at example.com', 'quux-new at cabbage.example'],
+            },
+        },
+        'Authorization' => $auth,
+    );
+    is($res->code, 201);
+    $content = $mech->json_response;
+    cmp_deeply($content, ['Correspondence added', 'Single Member changed from foo-new at bar.example to abacus at example.com'], "Got expected respose");
+    is($ticket->RoleAddresses("RT::CustomRole-$single_id"), 'abacus at example.com',
+       "Single Member role changed to first member of array");
+
+    # Try using a username instead of password
+    $res = $mech->post_json($correspond_url,
+        {
+            Content => 'Hello from hypermedia!',
+            ContentType => 'text/plain',
+            CustomRoles => {
+                'Single Member' => 'test',
+            },
+        },
+        'Authorization' => $auth,
+    );
+    is($res->code, 201);
+    $content = $mech->json_response;
+    cmp_deeply($content, ['Correspondence added', 'Single Member changed from abacus at example.com to test'], "Got expected respose");
+    is($ticket->RoleAddresses("RT::CustomRole-$single_id"), 'test at rt.example',
+       "Single Member role changed");
+}
+
+done_testing;

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


More information about the Bps-public-commit mailing list