[Bps-public-commit] RT-Extension-MandatoryOnTransition branch, support-coreroles-and-customroles, updated. 0.17-12-gffc1664

Craig Kaiser craig at bestpractical.com
Tue Jan 22 17:46:59 EST 2019


The branch, support-coreroles-and-customroles has been updated
       via  ffc1664001d998b78a508285c9644a5042d4b416 (commit)
       via  30ce0c9b0a44a2a1a59d5f58214b29ea7d4e5054 (commit)
       via  8d236800fc0c74207227aeab9f07178eca4fbe26 (commit)
       via  e8b66d7d9a5ffb9918a1b0f30ba399b3b3f29792 (commit)
       via  a63da469910f439a3e587506a659c46588e502e7 (commit)
      from  1d64e7543e6993a9ca3494c65fc1b4f48f6ec929 (commit)

Summary of changes:
 README                                             | 32 +++++----
 .../Ticket/Update.html/AfterWorked                 | 53 +++++++++++---
 lib/RT/Extension/MandatoryOnTransition.pm          | 84 ++++++++++++----------
 xt/roles.t                                         |  6 +-
 4 files changed, 111 insertions(+), 64 deletions(-)

- Log -----------------------------------------------------------------
commit a63da469910f439a3e587506a659c46588e502e7
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Tue Jan 22 11:26:59 2019 -0500

    Update README

diff --git a/README b/README
index 80db750..9fa5c66 100644
--- a/README
+++ b/README
@@ -36,10 +36,6 @@ DESCRIPTION
     TimeTaken
         Requires that the Worked field on the update page is non-zero.
 
-    Owner
-        Requires that the ticket has a real Owner or the real Owner will be
-        set on the update page.
-
     A larger set of basic fields may be supported in future releases. If
     you'd like to see additional fields added, please email your request to
     the bug address at the bottom of this documentation.
@@ -126,16 +122,24 @@ CONFIGURATION
     perldoc /opt/rt4/etc/RT_Config.pm.
 
   Requiring role values
-    You can require any core or custom role on a RT::Ticket object, further
-    more you can require a 'group' category to check that at least one value
-    for that role is a member of the required group. In the below example we
-    require that the custom role 'customer' have a user value who is a
-    member of the group 'Customers' and the owner of the ticket must be a
-    member of the group 'Helpdesk'.
-
-    Set( %MandatoryOnTransition, 'General' => { 'CustomRole.customer' => {
-    transition => 'open -> *', group => 'Customers' }, 'Owner' => {
-    transition => 'open -> *', group => 'Helpdesk' }, }, );
+    You can require any core or custom role on a RT::Ticket object, below is
+    an example of requiring a customrole "customer" be set on transition
+    from open and the owner also be set for the ticket on transition from a
+    status of open.
+
+    Set( %MandatoryOnTransition, 'General' => { '*' =>
+    ['CustomRole.customer', 'Owner'] 'CustomRole.customer' => { transition
+    => 'open -> *' }, 'Owner' => { transition => 'open -> *' }, }, );
+
+  Role Membership in a Group
+    Roles can require the members of the role to also be a member of a group
+    before satisfying to mandatory condition. Below we require that the
+    Owner role be set and that the member it is set to is a member of the
+    group 'SupportReps' or 'Admins'.
+
+    Set( %MandatoryOnTransition, 'General' => { 'open -> *' => ['Owner'],
+    'Owner' => { transition => 'open -> *', group => ['SupportReps',
+    'Admins'] } } );
 
   Restrictions on Queue Transitions
     The default behavior for MandatoryOnTransition operates on status
diff --git a/lib/RT/Extension/MandatoryOnTransition.pm b/lib/RT/Extension/MandatoryOnTransition.pm
index 46e6167..b40594d 100644
--- a/lib/RT/Extension/MandatoryOnTransition.pm
+++ b/lib/RT/Extension/MandatoryOnTransition.pm
@@ -50,11 +50,6 @@ field on the update page.
 
 Requires that the Worked field on the update page is non-zero.
 
-=item Owner
-
-Requires that the ticket has a real Owner or the real Owner will be set on
-the update page.
-
 =back
 
 A larger set of basic fields may be supported in future releases.  If you'd
@@ -158,19 +153,32 @@ C<perldoc /opt/rt4/etc/RT_Config.pm>.
 
 =head2 Requiring role values
 
-You can require any core or custom role on a RT::Ticket object, further more
-you can require a 'group' category to check that at least one value for that
-role is a member of the required group. In the below example we require that
-the custom role 'customer' have a user value who is a member of the group 'Customers'
-and the owner of the ticket must be a member of the group 'Helpdesk'.
+You can require any core or custom role on a RT::Ticket object, below is an
+example of requiring a customrole "customer" be set on transition from open
+and the owner also be set for the ticket on transition from a status of open.
 
 Set( %MandatoryOnTransition,
     'General' => {
-        'CustomRole.customer' => { transition => 'open -> *', group => 'Customers' },
-        'Owner' => { transition => 'open -> *', group => 'Helpdesk' },
+        '*' => ['CustomRole.customer', 'Owner']
+        'CustomRole.customer' => { transition => 'open -> *' },
+        'Owner' => { transition => 'open -> *' },
     },
 );
 
+=head2 Role Membership in a Group
+
+Roles can require the members of the role to also be a member of a group
+before satisfying to mandatory condition. Below we require that the Owner
+role be set and that the member it is set to is a member of the group
+'SupportReps' or 'Admins'.
+
+Set( %MandatoryOnTransition,
+    'General' => {
+        'open -> *' => ['Owner'],
+        'Owner' => { transition => 'open -> *', group => ['SupportReps', 'Admins'] }
+    }
+);
+
 =head2 Restrictions on Queue Transitions
 
 The default behavior for C<MandatoryOnTransition> operates on status transitions,

commit e8b66d7d9a5ffb9918a1b0f30ba399b3b3f29792
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Tue Jan 22 11:28:18 2019 -0500

    Update group message

diff --git a/lib/RT/Extension/MandatoryOnTransition.pm b/lib/RT/Extension/MandatoryOnTransition.pm
index b40594d..d73ed51 100644
--- a/lib/RT/Extension/MandatoryOnTransition.pm
+++ b/lib/RT/Extension/MandatoryOnTransition.pm
@@ -591,6 +591,8 @@ sub CheckMandatoryFields {
                         $role, $CurrentUser->loc("$transition"), $CurrentUser->loc($args{To}));
                     return \@errors;
             } elsif ( $role_group_values->{$role_full}->{group} ) {
+                use Data::Printer;
+                p(%role_group_values);
                 my $group_name = $role_group_values->{$role_full}->{group};
                 my $group = RT::Group->new($args{Ticket}->CurrentUser);
 
@@ -609,7 +611,7 @@ sub CheckMandatoryFields {
 
                     $has_valid_member = $group->HasMemberRecursively($user->Id);
                 }
-                push @errors, $CurrentUser->loc("User who is a member of $group_name is required for role: $role")
+                push @errors, $CurrentUser->loc("A member of group $group_name is required for role: $role")
                     unless $has_valid_member;
                 return \@errors unless $has_valid_member;
             }

commit 8d236800fc0c74207227aeab9f07178eca4fbe26
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Tue Jan 22 12:07:00 2019 -0500

    Make minor change to get owner working

diff --git a/lib/RT/Extension/MandatoryOnTransition.pm b/lib/RT/Extension/MandatoryOnTransition.pm
index d73ed51..cfe4adb 100644
--- a/lib/RT/Extension/MandatoryOnTransition.pm
+++ b/lib/RT/Extension/MandatoryOnTransition.pm
@@ -551,17 +551,20 @@ sub CheckMandatoryFields {
                 } else {
                     $value = $ARGSRef->{$role};
                 }
-                if (($value || $args{'Ticket'}->$role()) == $RT::Nobody->id) {
+                if (($value || $args{'Ticket'}->$role) == $RT::Nobody->id) {
                     push @errors,
                     $CurrentUser->loc(
                         "[_1] is required when changing [_2] to [_3]",
                         $role,
                         $CurrentUser->loc($transition),
-                        $CurrentUser->loc($args{To})
+                        $CurrentUser->loc($args{'To'})
                     );
                 } else {
                     my $user = RT::User->new($args{Ticket}->CurrentUser);
+                    $value = $args{Ticket}->Owner unless $value;
+
                     my ($ret, $msg) = $user->Load($value);
+
                     push @errors, $CurrentUser->loc("Could not load user: $value") unless $ret;
                     return \@errors unless $ret;
 
@@ -570,6 +573,7 @@ sub CheckMandatoryFields {
             } else {
                 $role_values = RT::Group->new($args{Ticket}->CurrentUser);
                 my ($ret, $msg) = $role_values->LoadRoleGroup(Object => $args{Ticket}, Name => $role);
+
                 push @errors, $CurrentUser->loc("Failed to load role $role for ticket: $msg") unless $ret;
                 return \@errors unless $ret;
             }
@@ -591,8 +595,6 @@ sub CheckMandatoryFields {
                         $role, $CurrentUser->loc("$transition"), $CurrentUser->loc($args{To}));
                     return \@errors;
             } elsif ( $role_group_values->{$role_full}->{group} ) {
-                use Data::Printer;
-                p(%role_group_values);
                 my $group_name = $role_group_values->{$role_full}->{group};
                 my $group = RT::Group->new($args{Ticket}->CurrentUser);
 
@@ -616,7 +618,6 @@ sub CheckMandatoryFields {
                 return \@errors unless $has_valid_member;
             }
         }
-        return \@errors if scalar @errors;
     }
 
     return \@errors unless @$cfs;

commit 30ce0c9b0a44a2a1a59d5f58214b29ea7d4e5054
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Tue Jan 22 12:21:27 2019 -0500

    Support roles arg as a array

diff --git a/lib/RT/Extension/MandatoryOnTransition.pm b/lib/RT/Extension/MandatoryOnTransition.pm
index cfe4adb..9e62881 100644
--- a/lib/RT/Extension/MandatoryOnTransition.pm
+++ b/lib/RT/Extension/MandatoryOnTransition.pm
@@ -575,7 +575,6 @@ sub CheckMandatoryFields {
                 my ($ret, $msg) = $role_values->LoadRoleGroup(Object => $args{Ticket}, Name => $role);
 
                 push @errors, $CurrentUser->loc("Failed to load role $role for ticket: $msg") unless $ret;
-                return \@errors unless $ret;
             }
             my @role_values;
             my @row_input_id = grep $role_arg eq $ARGSRef->{$_}, keys %{$ARGSRef};
@@ -593,29 +592,29 @@ sub CheckMandatoryFields {
             if ( not scalar @role_values ) {
                     push @errors, $CurrentUser->loc("[_1] is required when changing [_2] to [_3]",
                         $role, $CurrentUser->loc("$transition"), $CurrentUser->loc($args{To}));
-                    return \@errors;
             } elsif ( $role_group_values->{$role_full}->{group} ) {
-                my $group_name = $role_group_values->{$role_full}->{group};
-                my $group = RT::Group->new($args{Ticket}->CurrentUser);
-
-                my ($ret, $msg) = $group->LoadUserDefinedGroup($group_name);
-                push @errors, $CurrentUser->loc("Failed to load group: '$group_name' $msg") unless $ret;
-                return \@errors unless $ret;
-
                 my $has_valid_member;
-                my $user = RT::User->new($args{Ticket}->CurrentUser);
 
-                foreach my $member (@role_values) {
+                foreach my $group_name (@{$role_group_values->{$role_full}->{group}}) {
                     next if $has_valid_member;
-                    ($ret, $msg) = $user->LoadByEmail($member);
-                    RT::Logger->error($msg) unless $ret;
+                    my $group = RT::Group->new($args{Ticket}->CurrentUser);
+
+                    my ($ret, $msg) = $group->LoadUserDefinedGroup($group_name);
+                    push @errors, $CurrentUser->loc("Failed to load group: '$group_name' $msg") unless $ret;
                     next unless $ret;
 
-                    $has_valid_member = $group->HasMemberRecursively($user->Id);
+                    my $user = RT::User->new($args{Ticket}->CurrentUser);
+                    foreach my $member (@role_values) {
+                        ($ret, $msg) = $user->LoadByEmail($member);
+                        RT::Logger->error($msg) unless $ret;
+                        next unless $ret;
+
+                        $has_valid_member = $group->HasMemberRecursively($user->Id);
+                    }
                 }
-                push @errors, $CurrentUser->loc("A member of group $group_name is required for role: $role")
+                my $roles = join(' or ', @{$role_group_values->{$role_full}->{group}});
+                push @errors, $CurrentUser->loc("A member of $roles is required for role: $role")
                     unless $has_valid_member;
-                return \@errors unless $has_valid_member;
             }
         }
     }

commit ffc1664001d998b78a508285c9644a5042d4b416
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Tue Jan 22 17:20:31 2019 -0500

    tmp

diff --git a/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/AfterWorked b/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/AfterWorked
index b00e7d7..b3238ea 100644
--- a/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/AfterWorked
+++ b/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/AfterWorked
@@ -2,11 +2,11 @@
 $Ticket
 </%args>
 <%init>
-my ($core, $cfs) = RT::Extension::MandatoryOnTransition->RequiredFields(
+my ($core, $cfs, $roles) = RT::Extension::MandatoryOnTransition->RequiredFields(
     Ticket  => $Ticket,
     To      => $ARGS{'Status'} || $ARGS{'DefaultStatus'},
 );
-return unless @$cfs;
+return unless @$cfs or @$roles;
 
 my $comp = '/Elements/EditCustomFields';
 my %obj_args = ( Object => $Ticket );
@@ -17,11 +17,48 @@ if (!$m->comp_exists('/Elements/EditCustomFields')) {
     %obj_args = ( TicketObj => $Ticket );
 }
 
+my @roles;
+foreach my $role (@{$roles}) {
+    if ( $role =~ 'CustomRole.' ) {
+        $role =~ qr/^CustomRole\.(.+)$/i;
+        $role = $1;
+        my $role_object = RT::CustomRole->new($session{CurrentUser});
+        my ($ret, $msg) = $role_object->Load($role);
+
+        RT::Logger->error($msg) unless $ret;
+        push @roles, $role_object if $ret;
+    } else {
+        my $role_group = RT::Group->new($session{CurrentUser});
+        my ($ret, $msg) = $role_group->LoadRoleGroup(Object => $Ticket, Name => $role);
+
+        RT::Logger->error($msg) unless $ret;
+        push @roles, $role_group if $ret;
+    }
+}
 </%init>
 %# 'Named' is handled by this extension in the MassageCustomFields callback
-<& $comp,
-    %ARGS,
-    %obj_args,
-    InTable     => 1,
-    Named       => $cfs,
-    &>
+% if ( @$cfs ) {
+    <& $comp,
+        %ARGS,
+        %obj_args,
+        InTable     => 1,
+        Named       => $cfs,
+        &>
+% }
+% if ( @$roles ) {
+%     foreach my $role (@roles) {
+%     next if $role->Name eq 'Owner';
+        <tr>
+            <td class="label"><% $role->Name %>:</td>
+            <td class="entry">
+                <& /Elements/EmailInput,
+                    Name => grep (/$role->Name/, qw/Requestor AdminCc/) ? $role->Name : 'RT::CustomRole-' . $role->Id,
+                    Autocomplete       => 1,
+                    AutocompleteNobody => 1,
+                    AutocompleteReturn => "Name",
+                    Size               => 10,
+                &>
+            </td>
+        </tr>
+%     }
+% }
\ No newline at end of file
diff --git a/lib/RT/Extension/MandatoryOnTransition.pm b/lib/RT/Extension/MandatoryOnTransition.pm
index 9e62881..837c907 100644
--- a/lib/RT/Extension/MandatoryOnTransition.pm
+++ b/lib/RT/Extension/MandatoryOnTransition.pm
@@ -476,7 +476,7 @@ sub CheckMandatoryFields {
         To      => $args{'To'},
         NewQueue => $$ARGSRef{'Queue'},
     );
-    return \@errors unless @$core or @$cfs or $roles;
+    return \@errors unless @$core or @$cfs or @$roles;
 
     my $transition =  ($args{'From'} ||'') ne ($args{'To'} || '') ? 'Status' : 'Queue';
 
@@ -551,7 +551,9 @@ sub CheckMandatoryFields {
                 } else {
                     $value = $ARGSRef->{$role};
                 }
-                if (($value || $args{'Ticket'}->$role) == $RT::Nobody->id) {
+
+                $value = $args{Ticket}->Owner unless $value;
+                if ( $value == $RT::Nobody->id ) {
                     push @errors,
                     $CurrentUser->loc(
                         "[_1] is required when changing [_2] to [_3]",
@@ -561,13 +563,9 @@ sub CheckMandatoryFields {
                     );
                 } else {
                     my $user = RT::User->new($args{Ticket}->CurrentUser);
-                    $value = $args{Ticket}->Owner unless $value;
-
                     my ($ret, $msg) = $user->Load($value);
 
                     push @errors, $CurrentUser->loc("Could not load user: $value") unless $ret;
-                    return \@errors unless $ret;
-
                     $role_values = $user->EmailAddress;
                 }
             } else {
@@ -578,6 +576,8 @@ sub CheckMandatoryFields {
             }
             my @role_values;
             my @row_input_id = grep $role_arg eq $ARGSRef->{$_}, keys %{$ARGSRef};
+
+            map { push @role_values, $_} grep /$role_arg/, keys %{$ARGSRef};
             if ( @row_input_id ) {
                 map {push @role_values, $ARGSRef->{"WatcherAddressEmail$_"} } grep { $_ =~ qr/WatcherTypeEmail(.+)/ } @row_input_id;
             }
@@ -588,11 +588,7 @@ sub CheckMandatoryFields {
             } else {
                 push @role_values, $role_values;
             }
-
-            if ( not scalar @role_values ) {
-                    push @errors, $CurrentUser->loc("[_1] is required when changing [_2] to [_3]",
-                        $role, $CurrentUser->loc("$transition"), $CurrentUser->loc($args{To}));
-            } elsif ( $role_group_values->{$role_full}->{group} ) {
+            if ( $role_group_values->{$role_full}->{group} ) {
                 my $has_valid_member;
 
                 foreach my $group_name (@{$role_group_values->{$role_full}->{group}}) {
@@ -613,7 +609,7 @@ sub CheckMandatoryFields {
                     }
                 }
                 my $roles = join(' or ', @{$role_group_values->{$role_full}->{group}});
-                push @errors, $CurrentUser->loc("A member of $roles is required for role: $role")
+                push @errors, $CurrentUser->loc("A member of group $roles is required for role: $role")
                     unless $has_valid_member;
             }
         }
diff --git a/xt/roles.t b/xt/roles.t
index 71a975e..0a00b63 100644
--- a/xt/roles.t
+++ b/xt/roles.t
@@ -4,10 +4,10 @@ use warnings;
 use RT::Extension::MandatoryOnTransition::Test tests => undef, config => <<CONFIG
 Set( %MandatoryOnTransition,
      '*' => {
-         '* -> resolved'  => ['Owner',],
+         '* -> resolved'  => ['Owner'],
          '* -> stalled'   => ['AdminCc'],
          '* -> deleted'   => ['CustomRole.vip'],
-         'AdminCc'        => { transition => '* -> stalled', group => 'Admins' },
+         'AdminCc'        => { transition => '* -> stalled', group => ['Admins'] },
          'CustomRole.vip' => { transition => '* -> deleted' },
         }
     );
@@ -179,7 +179,7 @@ diag "Test core role fields";
         },
         "Try to stall ticket with no $role but not a member of required group"
     );
-    $m->text_contains("User who is a member of Admins is required for role: $role");
+    $m->text_contains("A member of group $role is required for role: $role");
 
     ($ret, $msg) = $group->AddMember($root->PrincipalId);
     ok $ret, $msg;

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


More information about the Bps-public-commit mailing list