[Bps-public-commit] RT-Extension-MandatoryOnTransition branch, roles-2, repushed

Craig Kaiser craig at bestpractical.com
Thu Feb 14 15:42:28 EST 2019


The branch roles-2 was deleted and repushed:
       was c9d1f69f82c05d0be3bc1d5d3509dfbbdf87ccda
       now 00fa5378376cd5e6696325a4cc5718ad24509111

1:  d3c735f ! 1:  84219d2 Support custom roles as manadatory on transition
    @@ -1,9 +1,9 @@
     Author: Craig Kaiser <craig at bestpractical.com>
     
    -    Support customroles for mandatoryontransition conditions
    +    Support custom roles as manadatory on transition
         
    -    Use the syntax 'CustomRole.MyRole' to set mandatory conditions for
    -    customrole values.
    +    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
     --- a/README
    @@ -49,19 +49,18 @@
          %obj_args = ( TicketObj => $Ticket );
      }
      
    -+my @roles;
    ++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($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;
    ++
    ++        ($ret, $msg) = $role_group->LoadRoleGroup(Object => $Ticket->QueueObj, Name => $role_object->GroupType);
    ++        RT::Logger->error("Could not load 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 $ret;
     +    }
     +}
    @@ -82,41 +81,22 @@
     +        &>
     +% }
     +% if ( @$roles ) {
    -+% my $count = 0;
     +%     foreach my $role (@roles) {
    -+%     next if $role->Name eq 'Owner';
     +        <tr>
    -+            <td class="label"><% $role->Name %>:</td>
    ++            <td class="label">Add <% $crs{$role->Name}->{Name} %>:</td>
     +            <td class="entry">
    -+            <input type='hidden' name="WatcherTypeEmail<% $count %>" value=<% $role->Name %>></input>
     +                <& /Elements/EmailInput,
    -+                    Name => grep ($role->Name eq $_, qw/Requestor AdminCc Cc/) ? "WatcherAddressEmail$count" : 'RT::CustomRole-' . $role->Id,
    ++                    Name               => 'RT::CustomRole-' . $crs{$role->Name}->{Id},
     +                    Autocomplete       => 1,
    -+                    AutocompleteNobody => 1,
    -+                    AutocompleteReturn => "Name",
    ++                    AutocompleteNobody => 0,
    ++                    AutocompleteReturn => "Email",
     +                    Size               => 20,
    ++                    Default            => $ARGS{'RT::CustomRole-' . $crs{$role->Name}->{Id}},
     +                &>
     +            </td>
     +        </tr>
    -+%     $count++;
     +%     }
     +% }
    -
    -diff --git a/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/BeforeUpdate b/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/BeforeUpdate
    ---- a/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/BeforeUpdate
    -+++ b/html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/BeforeUpdate
    -@@
    -     $$skip_update = 1;
    -     push @$results, @$errors_ref;
    - }
    -+
    -+# Allow mandatory roles to be set on update page
    -+foreach my $key (keys %{$ARGSRef}) {
    -+    if ( $key =~ /^WatcherAddressEmail(\d*)$/ && not $ARGSRef->{$key} ) {
    -+        $ARGSRef->{"WatcherTypeEmail$1"} = undef;
    -+    }
    -+}
    - </%init>
     
     diff --git a/lib/RT/Extension/MandatoryOnTransition.pm b/lib/RT/Extension/MandatoryOnTransition.pm
     --- a/lib/RT/Extension/MandatoryOnTransition.pm
    @@ -141,41 +121,11 @@
              '*' => {
                  '* -> resolved' => ['CF.Category'],
     @@
    - 
    - =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
    -@@
    -     my %config = ();
    -     %config = $self->Config($args{Queue});
    - 
    --    return ([], []) unless %config;
    -+    return ([], [], []) unless %config;
    - 
    -    $to ||= '';
    -    $from ||= '';
    -@@
          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\.|^AdminCc|^Cc|^Requestor|^Owner/i } @$required;
    ++               grep { /^CustomRole\./i } @$required;
      
          # Pull out any must_be or must_not_be rules
          my %cf_must_values = ();
    @@ -208,49 +158,12 @@
          my $transition =  ($args{'From'} ||'') ne ($args{'To'} || '') ? 'Status' : 'Queue';
      
     @@
    -             : $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?
    -@@
                  $label, $CurrentUser->loc($transition),  $CurrentUser->loc($field_label{$transition}));
          }
      
     +    if (@$roles) {
     +        foreach my $role (@$roles) {
    -+            my ( $role_values, $owner_value );
    ++            my $role_values;
     +            my ( $role_arg, $role_full ) = ( $role, $role );
     +
     +            if ( $role =~ s/^CustomRole\.//i ) {
    @@ -265,51 +178,13 @@
     +
     +                $role_values = $args{Ticket}->RoleGroup( $role_object->GroupType );
     +                push @errors, $CurrentUser->loc("Could not load current user") 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;
    -+            # Use this to keep track of input fields that use a count increment
    -+            # example: WatcherAddressEmail1 or WatcherAddressEmail2
    -+            my @row_input_num = grep $role_arg eq $ARGSRef->{$_}, keys %{$ARGSRef};
    -+            map { push @role_values, $ARGSRef->{$_} if $ARGSRef->{$_} } grep /$role_arg/, keys %{$ARGSRef};
    -+
    -+            # Grab our arguments for current role
    -+            if (@row_input_num) {
    -+                map { push @role_values, $ARGSRef->{"WatcherAddressEmail$_"}
    -+                      if $ARGSRef->{"WatcherAddressEmail$_"}
    -+                  }
    -+                map { $_ =~ /^WatcherTypeEmail(\d*)$/; $1 } @row_input_num;
    -+            }
    ++            if ( ref $role_values eq 'RT::Group' ) {
    ++                my @temp_array = $role_values->MemberEmailAddresses;
    ++                push @role_values, @temp_array if scalar @temp_array;
    ++            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]",
-:  ------- > 2:  c518436 Support making all core roles mandatory on transition
2:  61e8d23 ! 3:  6a85b1c Support requiring a role to be a member of a group
    @@ -60,19 +60,9 @@
              Queue   => $args{'Queue'} ? $args{'Queue'}->Name : undef,
              From    => $args{'From'},
     @@
    -                 map { $_ =~ /^WatcherTypeEmail(\d*)$/; $1 } @row_input_num;
                  }
    +             push @role_values, $ARGSRef->{$role_arg} if $ARGSRef->{$role_arg};
      
    --            if ( not scalar @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' ) {
    -+                my @temp_array = $role_values->MemberEmailAddresses;
    -+                push @role_values, @temp_array if scalar @temp_array;
    -+            }
    -+            else {
    -+                push @role_values, $role_values;
    -+            }
    -+
     +            # Check for mandatory group configuration, supports multiple groups where only
     +            # one true case needs to be found.
     +            if ( $role_group_values->{$role_full}->{group} ) {
    @@ -83,7 +73,8 @@
     +                    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;
    ++                    push @errors, $CurrentUser->loc("Failed to load group: $group_name") unless $ret;
    ++                    RT::Logger->error("Failed to load group: $group_name : $msg") unless $ret;
     +                    next unless $ret;
     +
     +                    my $user = RT::User->new( $args{Ticket}->CurrentUser );
    @@ -103,9 +94,7 @@
     +                    unless $has_valid_member;
     +                next unless $has_valid_member;
     +            }
    -+            if ( not scalar @role_values or ($role eq 'Owner' and !$owner_value or
    -+                ( $owner_value && $owner_value == $RT::Nobody->id) ) ) {
    +             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),
     
3:  ea168c4 ! 4:  5c7c3fd Create test file for mandatory roles feature
    @@ -1,6 +1,6 @@
     Author: Craig Kaiser <craig at bestpractical.com>
     
    -    Create test file for roles
    +    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
    @@ -161,6 +161,7 @@
     @@
     +use strict;
     +use warnings;
    ++use Test::Warn;
     +
     +use RT::Extension::MandatoryOnTransition::Test tests => undef, config => <<CONFIG
     +Set( %MandatoryOnTransition,
    @@ -304,6 +305,7 @@
     +        "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;
    @@ -325,6 +327,7 @@
     +        "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');
4:  322e2aa ! 5:  00fa537 Update README and Changelog
    @@ -107,4 +107,3 @@
      =head2 Restrictions on Queue Transitions
      
      The default behavior for C<MandatoryOnTransition> operates on status transitions,
    -
5:  c9d1f69 < -:  ------- Do not use 'WatcherType' style for adding watchers to ticket on update



More information about the Bps-public-commit mailing list