[Bps-public-commit] RT-Extension-MandatoryOnTransition branch, roles-2, created. 0.17-5-gb6c7f37

Jim Brandt jbrandt at bestpractical.com
Fri Feb 15 16:15:05 EST 2019


The branch, roles-2 has been created
        at  b6c7f37dfcae6b21ad1c63722412ade6c99ac783 (commit)

- Log -----------------------------------------------------------------
commit b52c9b90275eb3f652d83c2b8a6f3d4dd8074598
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Thu Feb 14 15:29:39 2019 -0500

    Support custom roles as manadatory on transition
    
    Can specify if a custom role needs to have a value in order for a
    ticket to transition from one status to another.

diff --git a/README b/README
index ad7e034..10ec6f6 100644
--- a/README
+++ b/README
@@ -96,7 +96,7 @@ CONFIGURATION
 
         Set( %MandatoryOnTransition,
             'QueueName' => {
-                'from -> to' => [ 'BasicField', 'CF.MyField', ],
+                'from -> to' => [ 'BasicField', 'CF.MyField', 'CustomRole.MyRole' ],
             },
         );
 
@@ -115,7 +115,8 @@ CONFIGURATION
 
         Set( %MandatoryOnTransition,
             Helpdesk => {
-                '* -> resolved' => ['TimeWorked', 'CF.Resolution'],
+                '* -> resolved'      => ['TimeWorked', 'CF.Resolution', 'CustomRole.Analyst'],
+                'CustomRole.Analyst' => {transition => '* -> open', group => 'Engineering'},
             },
             '*' => {
                 '* -> resolved' => ['CF.Category'],
diff --git a/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/AfterWorked b/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/AfterWorked
index b00e7d7..2f6f0a2 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,47 @@ if (!$m->comp_exists('/Elements/EditCustomFields')) {
     %obj_args = ( TicketObj => $Ticket );
 }
 
+my (@roles, %crs);
+foreach my $role (@{$roles}) {
+    my $role_group = RT::Group->new($session{CurrentUser});
+    if ( $role =~ s/^CustomRole\.//i ) {
+        my $role_object = RT::CustomRole->new($session{CurrentUser});
+        my ($ret, $msg) = $role_object->Load($role);
+        RT::Logger->error("Unable to load custom role $role: $msg") unless $ret;
+
+        ($ret, $msg) = $role_group->LoadRoleGroup(
+            Object => $Ticket->QueueObj, Name => $role_object->GroupType);
+        RT::Logger->error("Unable to load role group: " . $role_object->GroupType . " $msg")
+            unless $role_group->Id;
+        $crs{$role_group->Name} = { Id => $role_object->Id, Name => $role_object->Name };
+
+        push @roles, $role_group if $role_group->Id;
+    }
+}
 </%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) {
+        <tr>
+            <td class="label">Add <% $crs{$role->Name}->{Name} %>:</td>
+            <td class="entry">
+                <& /Elements/EmailInput,
+                    Name               => 'RT::CustomRole-' . $crs{$role->Name}->{Id},
+                    Autocomplete       => 1,
+                    AutocompleteNobody => 0,
+                    AutocompleteReturn => "Email",
+                    Size               => 20,
+                    Default            => $ARGS{'RT::CustomRole-' . $crs{$role->Name}->{Id}},
+                &>
+            </td>
+        </tr>
+%     }
+% }
diff --git a/lib/RT/Extension/MandatoryOnTransition.pm b/lib/RT/Extension/MandatoryOnTransition.pm
index d238c18..8cfd4e3 100644
--- a/lib/RT/Extension/MandatoryOnTransition.pm
+++ b/lib/RT/Extension/MandatoryOnTransition.pm
@@ -127,7 +127,7 @@ config option.  This option takes the generic form of:
 
     Set( %MandatoryOnTransition,
         'QueueName' => {
-            'from -> to' => [ 'BasicField', 'CF.MyField', ],
+            'from -> to' => [ 'BasicField', 'CF.MyField', 'CustomRole.MyRole' ],
         },
     );
 
@@ -146,7 +146,8 @@ Category selection before resolving tickets in every other queue.
 
     Set( %MandatoryOnTransition,
         Helpdesk => {
-            '* -> resolved' => ['TimeWorked', 'CF.Resolution'],
+            '* -> resolved'      => ['TimeWorked', 'CF.Resolution', 'CustomRole.Analyst'],
+            'CustomRole.Analyst' => {transition => '* -> open', group => 'Engineering'},
         },
         '*' => {
             '* -> resolved' => ['CF.Category'],
@@ -275,9 +276,10 @@ our %CORE_FOR_CREATE = (
 
 =head3 RequiredFields
 
-Returns two array refs of required fields for the described status transition.
-The first is core fields, the second is CF names.  Returns empty array refs
-on error or if nothing is required.
+Returns three array refs of required fields for the described status transition.
+The first is core fields, the second is CF names, the third is roles.  Returns
+empty array refs on error or if nothing is required. A forth parameter is a
+hashref of must-have values for custom fields.
 
 Takes a paramhash with the keys Ticket, Queue, From, and To.  Ticket should be
 an object.  Queue should be a name.  From and To should be statuses.  If you
@@ -340,6 +342,8 @@ sub RequiredFields {
     my @core = grep { !/^CF\./i && $core_supported{$_} } @$required;
     my @cfs  =  map { /^CF\.(.+)$/i; $1; }
                grep { /^CF\./i } @$required;
+    my @roles = map { /^(:?[CustomRole\.]?.+)$/i; $1; }
+               grep { /^CustomRole\./i } @$required;
 
     # Pull out any must_be or must_not_be rules
     my %cf_must_values = ();
@@ -359,7 +363,8 @@ sub RequiredFields {
             }
         }
     }
-    return (\@core, \@cfs, \%cf_must_values);
+
+    return (\@core, \@cfs, \@roles, \%cf_must_values);
 }
 
 =head3 CheckMandatoryFields
@@ -426,15 +431,14 @@ sub CheckMandatoryFields {
         return \@errors;
     }
 
-    my ($core, $cfs, $must_values) = $self->RequiredFields(
+    my ($core, $cfs, $roles, $must_values) = $self->RequiredFields(
         Ticket  => $args{'Ticket'},
         Queue   => $args{'Queue'} ? $args{'Queue'}->Name : undef,
         From    => $args{'From'},
         To      => $args{'To'},
         NewQueue => $$ARGSRef{'Queue'},
     );
-
-    return \@errors unless @$core or @$cfs;
+    return \@errors unless @$core or @$cfs or @$roles;
 
     my $transition =  ($args{'From'} ||'') ne ($args{'To'} || '') ? 'Status' : 'Queue';
 
@@ -500,6 +504,44 @@ sub CheckMandatoryFields {
             $label, $CurrentUser->loc($transition),  $CurrentUser->loc($field_label{$transition}));
     }
 
+    if (@$roles and $args{'To'}) {
+        foreach my $role (@$roles) {
+            my $role_values;
+            my ( $role_arg, $role_full ) = ( $role, $role );
+
+            if ( $role =~ s/^CustomRole\.//i ) {
+                my $role_object = RT::CustomRole->new( $args{Ticket}->CurrentUser );
+
+                my ( $ret, $msg ) = $role_object->Load($role);
+                push @errors, $CurrentUser->loc("Could not load object for [_1]", $role) unless $ret;
+                RT::Logger->error("Unable to load custom role $role: $msg") unless $ret;
+                next unless $role_object->Id;
+
+                $role_arg = 'RT::CustomRole-' . $role_object->Id;
+
+                $role_values = $args{Ticket}->RoleGroup( $role_object->GroupType );
+                RT::Logger->error("Unable to load role group for " . $role_object->GroupType)
+                    unless $role_values;
+            }
+
+            my @role_values;
+            if ( ref $role_values eq 'RT::Group' ) {
+                push @role_values, $role_values->MemberEmailAddresses
+                    if $role_values->MemberEmailAddresses;
+            }
+            push @role_values, $ARGSRef->{$role_arg} if $ARGSRef->{$role_arg};
+
+            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'} )
+                );
+                next;
+            }
+        }
+    }
+
     return \@errors unless @$cfs;
 
     if ( not $CFs ){

commit 673f20a569617cbed8ca0220660a441a20abe93c
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Fri Feb 15 12:02:09 2019 -0500

    Support making all core roles mandatory on transition

diff --git a/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/AfterWorked b/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/AfterWorked
index 2f6f0a2..ef1a203 100644
--- a/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/AfterWorked
+++ b/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/AfterWorked
@@ -32,6 +32,11 @@ foreach my $role (@{$roles}) {
         $crs{$role_group->Name} = { Id => $role_object->Id, Name => $role_object->Name };
 
         push @roles, $role_group if $role_group->Id;
+    } else {
+        # Handle core roles like Requestor, Cc, AdminCc
+        my ($ret, $msg) = $role_group->LoadRoleGroup(Object => $Ticket, Name => $role);
+        RT::Logger->error("Unable to load role $role: $msg") unless $ret;
+        push @roles, $role_group if $ret;
     }
 }
 </%init>
@@ -46,16 +51,17 @@ foreach my $role (@{$roles}) {
 % }
 % if ( @$roles ) {
 %     foreach my $role (@roles) {
+%     next if $role->Name eq 'Owner';
         <tr>
-            <td class="label">Add <% $crs{$role->Name}->{Name} %>:</td>
+            <td class="label">Add <% $role->Name =~ /CustomRole/ ? $crs{$role->Name}->{Name} : $role->Name %>:</td>
             <td class="entry">
                 <& /Elements/EmailInput,
-                    Name               => 'RT::CustomRole-' . $crs{$role->Name}->{Id},
+                    Name => grep ($role->Name eq $_, qw/Requestor AdminCc Cc/) ? $role->Name : 'RT::CustomRole-' . $crs{$role->Name}->{Id},
                     Autocomplete       => 1,
                     AutocompleteNobody => 0,
                     AutocompleteReturn => "Email",
                     Size               => 20,
-                    Default            => $ARGS{'RT::CustomRole-' . $crs{$role->Name}->{Id}},
+                    Default            => grep ($role->Name eq $_, qw/Requestor AdminCc Cc/) ? $ARGS{$role->Name} : $ARGS{'RT::CustomRole-' . $crs{$role->Name}->{Id}},
                 &>
             </td>
         </tr>
diff --git a/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/BeforeUpdate b/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/BeforeUpdate
index 171893f..3c5a7d9 100644
--- a/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/BeforeUpdate
+++ b/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/BeforeUpdate
@@ -15,5 +15,22 @@ if (@$errors_ref) {
     RT->Logger->debug("Preventing update because of missing mandatory fields");
     $$skip_update = 1;
     push @$results, @$errors_ref;
+} else {
+    # The update page does not look for core roles, this allows us to
+    # set them if they are present. We only want to set them if no errors
+    # were reported.
+
+    my $role_group = RT::Group->new($session{CurrentUser});
+    foreach my $role (qw/Requestor AdminCc Cc/) {
+        next unless $ARGSRef->{$role};
+
+        if ( $TicketObj->IsWatcher(Type => $role, Email => $ARGSRef->{$role}) ) {
+            RT::Logger->debug($ARGSRef->{$role} . " is already a watcher for role: $role");
+        } else {
+            my ($ret, $msg) = $TicketObj->AddWatcher(Type => $role, Email => $ARGSRef->{$role});
+            RT::Logger->error("Unable to add " . $ARGSRef->{$role} . " to role $role: $msg")
+                unless $ret;
+        }
+    }
 }
 </%init>
diff --git a/lib/RT/Extension/MandatoryOnTransition.pm b/lib/RT/Extension/MandatoryOnTransition.pm
index 8cfd4e3..c2395c1 100644
--- a/lib/RT/Extension/MandatoryOnTransition.pm
+++ b/lib/RT/Extension/MandatoryOnTransition.pm
@@ -258,18 +258,16 @@ pair to %CORE_FOR_UPDATE and/or %CORE_FOR_CREATE.
 
 =cut
 
-our @CORE_SUPPORTED  = qw(Content TimeWorked TimeTaken Owner);
-our @CORE_TICKET     = qw(TimeWorked Owner);
+our @CORE_SUPPORTED  = qw(Content TimeWorked TimeTaken);
+our @CORE_TICKET     = qw(TimeWorked);
 our %CORE_FOR_UPDATE = (
     TimeWorked  => 'UpdateTimeWorked',
     TimeTaken   => 'UpdateTimeWorked',
     Content     => 'UpdateContent',
-    Owner       => 'Owner',
 );
 our %CORE_FOR_CREATE = (
     TimeWorked  => 'TimeWorked',
     Content     => 'Content',
-    Owner       => 'Owner',
 );
 
 =head2 Methods
@@ -325,7 +323,7 @@ sub RequiredFields {
     my %config = ();
     %config = $self->Config($args{Queue});
 
-    return ([], []) unless %config;
+    return ([], [], []) unless %config;
 
    $to ||= '';
    $from ||= '';
@@ -343,7 +341,7 @@ sub RequiredFields {
     my @cfs  =  map { /^CF\.(.+)$/i; $1; }
                grep { /^CF\./i } @$required;
     my @roles = map { /^(:?[CustomRole\.]?.+)$/i; $1; }
-               grep { /^CustomRole\./i } @$required;
+               grep { /^CustomRole\.|^AdminCc|^Cc|^Requestor|^Owner/i } @$required;
 
     # Pull out any must_be or must_not_be rules
     my %cf_must_values = ();
@@ -461,36 +459,6 @@ sub CheckMandatoryFields {
             : $CORE_FOR_CREATE{$field};
         next unless $arg;
 
-        if ($field eq 'Owner') {
-            my $value;
-
-            # There are 2 Owner fields on Jumbo page, copied the same handling from it.
-            if (ref $ARGSRef->{$arg}) {
-                foreach my $owner (@{$ARGSRef->{$arg}}) {
-                    if (defined($owner) && $owner =~ /\D/) {
-                        $value = $owner unless ($args{'Ticket'}->OwnerObj->Name eq $owner);
-                    }
-                    elsif (length $owner) {
-                        $value = $owner unless ($args{'Ticket'}->OwnerObj->id == $owner);
-                    }
-                }
-            }
-            else {
-                $value = $ARGSRef->{$arg};
-            }
-
-            if (($value || $args{'Ticket'}->$field()) == $RT::Nobody->id) {
-                push @errors,
-                  $CurrentUser->loc(
-                    "[_1] is required when changing [_2] to [_3]",
-                    $field,
-                    $CurrentUser->loc($transition),
-                    $CurrentUser->loc($field_label{$transition})
-                  );
-                next;
-            }
-        }
-
         next if defined $ARGSRef->{$arg} and length $ARGSRef->{$arg};
 
         # Do we have a value currently?
@@ -506,7 +474,7 @@ sub CheckMandatoryFields {
 
     if (@$roles and $args{'To'}) {
         foreach my $role (@$roles) {
-            my $role_values;
+            my ( $role_values, $owner_value );
             my ( $role_arg, $role_full ) = ( $role, $role );
 
             if ( $role =~ s/^CustomRole\.//i ) {
@@ -522,16 +490,52 @@ sub CheckMandatoryFields {
                 $role_values = $args{Ticket}->RoleGroup( $role_object->GroupType );
                 RT::Logger->error("Unable to load role group for " . $role_object->GroupType)
                     unless $role_values;
+            } elsif ( $role eq 'Owner' ) {
+                # There are 2 Owner fields on Jumbo page, copied the same handling from it.
+                if ( ref $ARGSRef->{$role} ) {
+                    foreach my $owner ( @{ $ARGSRef->{$role} } ) {
+                        if ( defined($owner) && $owner =~ /\D/ ) {
+                            $owner_value = $owner unless ( $args{'Ticket'}->OwnerObj->Name eq $owner );
+                        }
+                        elsif ( length $owner ) {
+                            $owner_value = $owner unless ($args{'Ticket'}->OwnerObj->id == $owner );
+                        }
+                    }
+                }
+                else {
+                    $owner_value = $ARGSRef->{$role};
+                }
+                if ( $owner_value ) {
+                    my $user = RT::User->new( $args{Ticket}->CurrentUser );
+                    $user->Load($owner_value);
+                    push @errors, $CurrentUser->loc("Could not load user: $owner_value") unless $user->Id;
+                    $role_values = $user->EmailAddress if $user->Id;
+                }
+            }
+            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") unless $ret;
+                RT::Logger->error($msg) unless $ret;
             }
 
             my @role_values;
+            # We could end up with a RT::Group option or a single value in the case of Owner
             if ( ref $role_values eq 'RT::Group' ) {
                 push @role_values, $role_values->MemberEmailAddresses
                     if $role_values->MemberEmailAddresses;
             }
+            else {
+                push @role_values, $role_values;
+            }
             push @role_values, $ARGSRef->{$role_arg} if $ARGSRef->{$role_arg};
 
-            if ( not scalar @role_values ) {
+            if ( not scalar @role_values
+                    or ($role eq 'Owner' and !$owner_value or
+                    ( $owner_value && $owner_value == $RT::Nobody->id) ) ) {
                 push @errors, $CurrentUser->loc("[_1] is required when changing [_2] to [_3]",
                     $role,
                     $CurrentUser->loc($transition),

commit 9f0ed361b1e4d64075d11e00e726dbee97f13c2a
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Fri Feb 15 13:22:26 2019 -0500

    Support requiring a role to be a member of a group
    
    Provide a group for the role specific key in the
    configuration hash to require a role to have
    a member in at least one of the groups.

diff --git a/lib/RT/Extension/MandatoryOnTransition.pm b/lib/RT/Extension/MandatoryOnTransition.pm
index c2395c1..bd30136 100644
--- a/lib/RT/Extension/MandatoryOnTransition.pm
+++ b/lib/RT/Extension/MandatoryOnTransition.pm
@@ -276,8 +276,11 @@ our %CORE_FOR_CREATE = (
 
 Returns three array refs of required fields for the described status transition.
 The first is core fields, the second is CF names, the third is roles.  Returns
-empty array refs on error or if nothing is required. A forth parameter is a
-hashref of must-have values for custom fields.
+empty array refs on error or if nothing is required.
+
+A fourth returned parameter is a hashref of must-have values for custom fields.
+
+The fifth parameter is a hashref of groups a role member must be in.
 
 Takes a paramhash with the keys Ticket, Queue, From, and To.  Ticket should be
 an object.  Queue should be a name.  From and To should be statuses.  If you
@@ -362,7 +365,38 @@ sub RequiredFields {
         }
     }
 
-    return (\@core, \@cfs, \@roles, \%cf_must_values);
+    my %role_group_values;
+    my $cr = RT::CustomRole->new(RT->SystemUser);
+    foreach my $role (@roles){
+        if ( $role =~ /^CustomRole\.(.*)/ ) {
+            my $role_name = $1;
+            my ($ret, $msg) = $cr->Load($role_name);
+            if ( not $cr and $cr->Id ) {
+                RT::Logger->error("Could not load Custom role $role_name: $msg");
+                @roles = grep/$_ ne $role_name/, at roles;
+                next;
+            } elsif ( not $cr->IsAdded($args{Ticket}->QueueObj->Id) or $cr->Disabled ) {
+                RT::Logger->error("Custom role $role_name is not applied to: " . $args{Ticket}->QueueObj->Name );
+                @roles = grep/$_ ne $role_name/, at roles;
+                next;
+            }
+        }
+        if ( $config{$role} ){
+            my $transition = $config{$role}->{'transition'};
+            unless ( $transition ){
+                RT->Logger->error("No transition defined in group rules for $role");
+                next;
+            }
+
+            if ( $transition eq "$from -> $to"
+                || $transition eq "* -> $to"
+                || $transition eq "$from -> *" ) {
+
+                $role_group_values{$role} = $config{$role};
+            }
+        }
+    }
+    return (\@core, \@cfs, \@roles, \%cf_must_values, \%role_group_values);
 }
 
 =head3 CheckMandatoryFields
@@ -429,7 +463,7 @@ sub CheckMandatoryFields {
         return \@errors;
     }
 
-    my ($core, $cfs, $roles, $must_values) = $self->RequiredFields(
+    my ($core, $cfs, $roles, $must_values, $role_group_values) = $self->RequiredFields(
         Ticket  => $args{'Ticket'},
         Queue   => $args{'Queue'} ? $args{'Queue'}->Name : undef,
         From    => $args{'From'},
@@ -525,14 +559,44 @@ sub CheckMandatoryFields {
             my @role_values;
             # We could end up with a RT::Group option or a single value in the case of Owner
             if ( ref $role_values eq 'RT::Group' ) {
-                push @role_values, $role_values->MemberEmailAddresses
-                    if $role_values->MemberEmailAddresses;
+                my @member_emails = $role_values->MemberEmailAddresses;
+                push @role_values, @member_emails if @member_emails;
             }
             else {
                 push @role_values, $role_values;
             }
             push @role_values, $ARGSRef->{$role_arg} if $ARGSRef->{$role_arg};
 
+            # Check for mandatory group configuration, supports multiple groups where only
+            # one true case needs to be found.
+            if ( $role_group_values->{$role_full}->{group} ) {
+                my $has_valid_member;
+
+                foreach my $group_name ( @{ $role_group_values->{$role_full}->{group} } ) {
+                    my $group = RT::Group->new( $args{Ticket}->CurrentUser );
+
+                    my ( $ret, $msg ) = $group->LoadUserDefinedGroup($group_name);
+                    RT::Logger->error("Failed to load group: $group_name : $msg") unless $ret;
+                    next unless $ret;
+
+                    foreach my $member (@role_values) {
+                        next unless $member;
+
+                        my $user = RT::User->new( $args{Ticket}->CurrentUser );
+                        $user->Load($member);
+                        $user->LoadByEmail($member) unless $user->Id;
+                        RT::Logger->error("Failed to load: $member") unless $user->Id;
+                        next unless $user->Id;
+
+                        $has_valid_member = $group->HasMemberRecursively( $user->Id );
+                        last if $has_valid_member;
+                    }
+                    last if $has_valid_member;
+                }
+                my $roles = join( ' or ', @{ $role_group_values->{$role_full}->{group} } );
+                push @errors, $CurrentUser->loc("A member of group $roles is required for role: $role")
+                    unless $has_valid_member;
+            }
             if ( not scalar @role_values
                     or ($role eq 'Owner' and !$owner_value or
                     ( $owner_value && $owner_value == $RT::Nobody->id) ) ) {

commit ec60133c9d712289d88ce3cd788c9db93d983c0d
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Fri Jan 11 16:59:07 2019 -0500

    Create test file for mandatory roles feature

diff --git a/xt/require_owner_for_resolve.t b/xt/require_owner_for_resolve.t
deleted file mode 100644
index 20fd377..0000000
--- a/xt/require_owner_for_resolve.t
+++ /dev/null
@@ -1,122 +0,0 @@
-use strict;
-use warnings;
-
-use RT::Extension::MandatoryOnTransition::Test tests => undef, config => <<CONFIG
-Set( %MandatoryOnTransition,
-     '*' => {
-         '* -> resolved' => ['Owner',],
-     }
-    );
-CONFIG
-  ;
-
-use_ok('RT::Extension::MandatoryOnTransition');
-
-my ($baseurl, $m) = RT::Test->started_ok();
-
-ok($m->login('root', 'password'), 'logged in');
-$m->get_ok($m->rt_base_url);
-
-my $root = RT::User->new(RT->SystemUser);
-$root->Load('root');
-ok($root->id, 'Loaded root');
-
-diag "Resolve ticket through Update with required Owner";
-{
-    my $t = RT::Test->create_ticket(
-        Queue   => 'General',
-        Subject => 'Test Mandatory Owner On Resolve',
-        Content => 'Testing',
-    );
-
-    ok($t->id, 'Created test ticket: ' . $t->id);
-
-    $m->goto_ticket($t->id);
-
-    $m->follow_link_ok({ text => 'Resolve' }, 'Try to resolve ticket');
-    $m->text_contains('Test Mandatory Owner On Resolve');
-    $m->submit_form_ok(
-        {   form_name => 'TicketUpdate',
-            button    => 'SubmitTicket',
-        },
-        'Submit resolve with no Owner set'
-    );
-    $m->text_contains('Owner is required when changing Status to resolved');
-    $m->submit_form_ok(
-        {   form_name => 'TicketUpdate',
-            button    => 'SubmitTicket',
-            fields    => { Owner => $root->id }
-        },
-        'Submit resolve with Owner set'
-    );
-    $m->text_lacks('Owner is required when changing Status to resolved');
-    $m->text_contains('Owner changed from Nobody to root');
-    $m->text_contains("Status changed from 'new' to 'resolved'");
-}
-
-diag "Resolve ticket through Basics with required Owner";
-{
-    my $t = RT::Test->create_ticket(
-        Queue   => 'General',
-        Subject => 'Test Mandatory Owner On Resolve',
-        Content => 'Testing',
-    );
-    ok($t->id, 'Created ticket to resolve through Modify.html: ' . $t->id);
-
-    $m->goto_ticket($t->id);
-    $m->follow_link_ok({ text => 'Basics' }, 'Get Modify.html of ticket');
-    $m->submit_form_ok(
-        {   form_name => 'TicketModify',
-            fields    => { Status => 'resolved', },
-            button    => 'SubmitTicket',
-        },
-        'Submit resolve with no Owner set'
-    );
-    $m->text_contains('Owner is required when changing Status to resolved');
-
-    $m->submit_form_ok(
-        {   form_name => 'TicketModify',
-            fields    => { Status => 'resolved', Owner => $root->id, },
-            button    => 'SubmitTicket',
-        },
-        'Submit resolve with no Owner set'
-    );
-    $m->text_lacks('Owner is required when changing Status to resolved');
-    $m->text_contains('Owner changed from Nobody to root');
-    $m->text_contains("Status changed from 'new' to 'resolved'");
-}
-
-diag "Resolve ticket through Jumbo with required Owner";
-{
-    my $t = RT::Test->create_ticket(
-        Queue   => 'General',
-        Subject => 'Test Mandatory Owner On Resolve',
-        Content => 'Testing',
-    );
-    ok($t->id, 'Created ticket to resolve through ModifyAll.html: ' . $t->id);
-
-    $m->goto_ticket($t->id);
-    $m->follow_link_ok({ text => 'Jumbo' }, 'Get ModifyAll.html of ticket');
-    $m->submit_form_ok(
-        {   form_name => 'TicketModifyAll',
-            fields    => { Status => 'resolved', },
-            button    => 'SubmitTicket',
-        },
-        'Submit resolve with no Owner set'
-    );
-    $m->text_contains('Owner is required when changing Status to resolved');
-
-    $m->submit_form_ok(
-        {   form_name => 'TicketModifyAll',
-            fields    => { Status => 'resolved', Owner => $root->id, },
-            button    => 'SubmitTicket',
-        },
-        'Submit resolve with no Owner set'
-    );
-    $m->text_lacks('Owner is required when changing Status to resolved');
-    $m->text_contains('Owner changed from Nobody to root');
-    $m->text_contains("Status changed from 'new' to 'resolved'");
-}
-
-undef $m;
-done_testing;
diff --git a/xt/required_fields.t b/xt/required_fields.t
index 4f2135c..cd1fda7 100644
--- a/xt/required_fields.t
+++ b/xt/required_fields.t
@@ -14,8 +14,8 @@ diag "Test RequiredFields without a ticket";
                        );
     is( $core->[0], 'TimeWorked', 'Got TimeWorked for required core');
 
-    my $must_values;
-    ($core, $cf, $must_values) = RT::Extension::MandatoryOnTransition->RequiredFields(
+    my ($must_values, $role, $role_group_values);
+    ($core, $cf, $role, $must_values, $role_group_values) = RT::Extension::MandatoryOnTransition->RequiredFields(
                            From => "''",
                            To   => 'resolved',
                            Queue => 'General',
@@ -41,7 +41,7 @@ diag "Test RequiredFields with a ticket";
 
     ok( $t->id, 'Created test ticket: ' . $t->id);
 
-    my ($core, $cf, $must_values) = RT::Extension::MandatoryOnTransition->RequiredFields(
+    my ($core, $cf, $roles, $must_values, $role_group_values) = RT::Extension::MandatoryOnTransition->RequiredFields(
                            Ticket => $t,
                            To   => 'resolved',
                        );
diff --git a/xt/roles.t b/xt/roles.t
new file mode 100644
index 0000000..f1714f8
--- /dev/null
+++ b/xt/roles.t
@@ -0,0 +1,251 @@
+use strict;
+use warnings;
+use Test::Warn;
+
+use RT::Extension::MandatoryOnTransition::Test tests => undef, config => <<CONFIG
+Set( %MandatoryOnTransition,
+     '*' => {
+         '* -> resolved'  => ['Owner'],
+         '* -> stalled'   => ['AdminCc'],
+         '* -> deleted'   => ['CustomRole.vip'],
+         'AdminCc'        => { transition => '* -> stalled', group => ['Admins'] },
+         'CustomRole.vip' => { transition => '* -> deleted' },
+        }
+    );
+CONFIG
+  ;
+
+use_ok('RT::Extension::MandatoryOnTransition');
+
+my ($baseurl, $m) = RT::Test->started_ok();
+
+ok($m->login('root', 'password'), 'logged in');
+$m->get_ok($m->rt_base_url);
+
+my $root = RT::User->new(RT->SystemUser);
+$root->Load('root');
+ok($root->id, 'Loaded root');
+
+diag "Resolve ticket through Update with required Owner";
+{
+    my $t = RT::Test->create_ticket(
+        Queue   => 'General',
+        Subject => 'Test Mandatory Owner On Resolve',
+        Content => 'Testing',
+    );
+
+    ok($t->id, 'Created test ticket: ' . $t->id);
+
+    $m->goto_ticket($t->id);
+
+    $m->follow_link_ok({ text => 'Resolve' }, 'Try to resolve ticket');
+    $m->submit_form_ok(
+        {   form_name => 'TicketUpdate',
+            button    => 'SubmitTicket',
+        },
+        'Submit resolve with no Owner set'
+    );
+    $m->text_contains('Owner is required when changing Status to resolved');
+    $m->submit_form_ok(
+        {   form_name => 'TicketUpdate',
+            button    => 'SubmitTicket',
+            fields    => { Owner => $root->id }
+        },
+        'Submit resolve with Owner set'
+    );
+    $m->text_lacks('Owner is required when changing Status to resolved');
+    $m->text_contains('Owner changed from Nobody to root');
+    $m->text_contains("Status changed from 'new' to 'resolved'");
+}
+
+diag "Resolve ticket through Basics with required Owner";
+{
+    my $t = RT::Test->create_ticket(
+        Queue   => 'General',
+        Subject => 'Test Mandatory Owner On Resolve',
+        Content => 'Testing',
+    );
+    ok($t->id, 'Created ticket to resolve through Modify.html: ' . $t->id);
+
+    $m->goto_ticket($t->id);
+    $m->follow_link_ok({ text => 'Basics' }, 'Get Modify.html of ticket');
+    $m->submit_form_ok(
+        {   form_name => 'TicketModify',
+            fields    => { Status => 'resolved', },
+            button    => 'SubmitTicket',
+        },
+        'Submit resolve with no Owner set'
+    );
+    $m->text_contains('Owner is required when changing Status to resolved');
+
+    $m->submit_form_ok(
+        {   form_name => 'TicketModify',
+            fields    => { Status => 'resolved', Owner => $root->id, },
+            button    => 'SubmitTicket',
+        },
+        'Submit resolve with no Owner set'
+    );
+    $m->text_lacks('Owner is required when changing Status to resolved');
+    $m->text_contains('Owner changed from Nobody to root');
+    $m->text_contains("Status changed from 'new' to 'resolved'");
+}
+
+diag "Resolve ticket through Jumbo with required Owner";
+{
+    my $t = RT::Test->create_ticket(
+        Queue   => 'General',
+        Subject => 'Test Mandatory Owner On Resolve',
+        Content => 'Testing',
+    );
+    ok($t->id, 'Created ticket to resolve through ModifyAll.html: ' . $t->id);
+
+    $m->goto_ticket($t->id);
+    $m->follow_link_ok({ text => 'Jumbo' }, 'Get ModifyAll.html of ticket');
+    $m->submit_form_ok(
+        {   form_name => 'TicketModifyAll',
+            fields    => { Status => 'resolved', },
+            button    => 'SubmitTicket',
+        },
+        'Submit resolve with no Owner set'
+    );
+    $m->text_contains('Owner is required when changing Status to resolved');
+
+    $m->submit_form_ok(
+        {   form_name => 'TicketModifyAll',
+            fields    => { Status => 'resolved', Owner => $root->id, },
+            button    => 'SubmitTicket',
+        },
+        'Submit resolve with no Owner set'
+    );
+    $m->text_lacks('Owner is required when changing Status to resolved');
+    $m->text_contains('Owner changed from Nobody to root');
+    $m->text_contains("Status changed from 'new' to 'resolved'");
+}
+
+diag "Test core role fields";
+{
+    my $role = qw/AdminCc/;
+    my $t = RT::Test->create_ticket(
+        Queue   => 'General',
+        Subject => 'Test Mandatory Owner On Resolve',
+        Content => 'Testing',
+    );
+    ok($t->id, 'Created test ticket: ' . $t->id);
+    my ($ret, $msg) = $t->SetStatus('open');
+    ok $ret, $msg;
+
+    $m->goto_ticket($t->id);
+    $m->follow_link_ok({ text => 'Basics' }, 'Get Modify.html of ticket');
+    $m->submit_form_ok(
+        {   form_name => 'TicketModify',
+            fields    => { Status => 'stalled', },
+            button    => 'SubmitTicket',
+        },
+        "Submit stalled with no $role member"
+    );
+    $m->text_contains("A member of group Admins is required for role: $role");
+    $m->warning_like(qr/Failed to load group: Admins : Couldn't find row/);
+
+    my $role_group = $t->$role;
+    ok $role_group->Id;
+
+    my $root = RT::User->new(RT->SystemUser);
+    $root->Load('root');
+    ok($root->id, 'Loaded root');
+
+    ($ret, $msg) = $role_group->AddMember($root->PrincipalId);
+    ok $ret, $msg;
+
+    $m->goto_ticket($t->id);
+    $m->follow_link_ok({ text => 'Basics' }, 'Get Modify.html of ticket');
+    $m->submit_form_ok(
+        {   form_name => 'TicketModify',
+            fields    => { Status => 'stalled', },
+            button    => 'SubmitTicket',
+        },
+        "Try to stall ticket with no Admins group created"
+    );
+    $m->text_contains("A member of group Admins is required for role: $role");
+    $m->warning_like(qr/Failed to load group: Admins : Couldn't find row/);
+
+    my $group = RT::Group->new(RT->SystemUser);
+    ($ret, $msg) = $group->CreateUserDefinedGroup(Name => 'Admins');
+    ok $ret, "Failed to create Admins group: $msg";
+
+    $m->goto_ticket($t->id);
+    $m->follow_link_ok({ text => 'Basics' }, 'Get Modify.html of ticket');
+    $m->submit_form_ok(
+        {   form_name => 'TicketModify',
+            fields    => { Status => 'stalled', },
+            button    => 'SubmitTicket',
+        },
+        "Try to stall ticket with no $role but not a member of required group"
+    );
+    $m->text_contains("A member of group Admins is required for role: $role");
+
+    ($ret, $msg) = $group->AddMember($root->PrincipalId);
+    ok $ret, $msg;
+
+    $m->goto_ticket($t->id);
+    $m->follow_link_ok({ text => 'Basics' }, 'Get Modify.html of ticket');
+    $m->submit_form_ok(
+        {   form_name => 'TicketModify',
+            fields    => { Status => 'stalled', },
+            button    => 'SubmitTicket',
+        },
+        "Try to stall ticket with $role and group required"
+    );
+    $m->text_contains("Status changed from 'open' to 'stalled'");
+}
+
+diag "Test custom role mandatory fields";
+{
+    my $t = RT::Test->create_ticket(
+        Queue   => 'General',
+        Subject => 'Test Mandatory Owner On Resolve',
+        Content => 'Testing',
+    );
+    ok($t->id, 'Created test ticket: ' . $t->id);
+    my ($ret, $msg) = $t->SetStatus('open');
+    ok $ret, $msg;
+
+    my $id = $t->id;
+
+    my $customrole = RT::CustomRole->new(RT->SystemUser);
+    ($ret, $msg) = $customrole->Create(Name => 'vip');
+    ok $ret, $msg;
+
+    ($ret, $msg) = $customrole->Load('vip');
+    ok $ret, $msg;
+
+    ($ret, $msg) = $customrole->AddToObject($t->Queue);
+    ok $ret, $msg;
+
+    $m->goto_ticket($t->id);
+    $m->follow_link_ok({ text => 'Basics' }, 'Get Modify.html of ticket');
+    $m->submit_form_ok(
+        {   form_name => 'TicketModify',
+            fields    => { Status => 'deleted', },
+            button    => 'SubmitTicket',
+        },
+        "Submit deleted with no value for required custom role"
+    );
+    $m->text_contains("vip is required when changing Status to deleted");
+
+    ($ret, $msg) = $t->AddWatcher(Type => $customrole->GroupType, Principal => $root->PrincipalObj);
+    ok $ret, $msg;
+
+    $m->goto_ticket($t->id);
+    $m->follow_link_ok({ text => 'Basics' }, 'Get Modify.html of ticket');
+    $m->submit_form_ok(
+        {   form_name => 'TicketModify',
+            fields    => { Status => 'deleted', },
+            button    => 'SubmitTicket',
+        },
+        "Submit deleted with manatory custom role requirements met."
+    );
+    $m->text_contains("Ticket deleted");
+}
+
+undef $m;
+done_testing;

commit b6c7f37dfcae6b21ad1c63722412ade6c99ac783
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Fri Jan 11 11:37:10 2019 -0500

    Update README and Changelog

diff --git a/Changes b/Changes
index 5eb32fc..3f54ca7 100644
--- a/Changes
+++ b/Changes
@@ -1,3 +1,8 @@
+0.17 2019-01-23
+ - Add support for custom roles as a mandatory field
+ - Add support for all RT core roles as a mandatory field
+ - Add support for requiring group membership for roles
+
 0.17 2018-10-26
  - Add support for Owner as a mandatory field
 
diff --git a/README b/README
index 10ec6f6..45c8fa4 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,6 +122,26 @@ CONFIGURATION
     The transition syntax is similar to that found in RT's Lifecycles. See
     perldoc /opt/rt4/etc/RT_Config.pm.
 
+  Requiring role values
+    You can require any core or custom role on a RT::Ticket object, below is
+    an example of requiring a custom role "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' => { '* -> resolved' =>
+    ['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
     transitions, so a change from new to open or from open to resolved. It
diff --git a/lib/RT/Extension/MandatoryOnTransition.pm b/lib/RT/Extension/MandatoryOnTransition.pm
index bd30136..3691eb3 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
@@ -157,6 +152,34 @@ Category selection before resolving tickets in every other queue.
 The transition syntax is similar to that found in RT's Lifecycles.  See
 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, below is an
+example of requiring a custom role "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' => {
+        '* -> resolved' => ['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,

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


More information about the Bps-public-commit mailing list