[Bps-public-commit] RT-Extension-MandatoryOnTransition branch, must-and-must-not-be, created. 0.09_01-14-gbabe916

Jim Brandt jbrandt at bestpractical.com
Fri Jun 17 16:25:16 EDT 2016


The branch, must-and-must-not-be has been created
        at  babe91626fdd3c8463117ec31bcc2e10612ea23d (commit)

- Log -----------------------------------------------------------------
commit 2f48cf42006a6c6a6fdd1c4b62520531e52c8583
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Fri Jun 17 16:18:34 2016 -0400

    Remove config check for ARRAY before adding new HASH config option
    
    This check looks for a specific type of data in the config,
    but the new feature for checking specific values needs a
    hash reference. Adding hash would then make this check
    restrict almost nothing, so remove it.

diff --git a/lib/RT/Extension/MandatoryOnTransition.pm b/lib/RT/Extension/MandatoryOnTransition.pm
index 6222d17..5f6616c 100644
--- a/lib/RT/Extension/MandatoryOnTransition.pm
+++ b/lib/RT/Extension/MandatoryOnTransition.pm
@@ -165,31 +165,6 @@ If you're just using this module on your own RT instance, you should stop
 reading now.  You don't need to know about the implementation details unless
 you're writing a patch against this extension.
 
-=cut
-
-$RT::Config::META{'MandatoryOnTransition'} = {
-    Type            => 'HASH',
-    PostLoadCheck   => sub {
-        # Normalize field list to always be arrayref
-        my $self = shift;
-        my %config = $self->Get('MandatoryOnTransition');
-        for my $transitions (values %config) {
-            for (keys %$transitions) {
-                next if ref $transitions->{$_} eq 'ARRAY';
-
-                if (ref $transitions->{$_}) {
-                    RT->Logger->error("%MandatoryOnTransition definition '$_' must be a single field name or an array ref of field names.  Ignoring.");
-                    delete $transitions->{$_};
-                    next;
-                }
-
-                $transitions->{$_} = [ $transitions->{$_} ];
-            }
-        }
-        $self->Set(MandatoryOnTransition => %config);
-    },
-};
-
 =head2 Package variables
 
 =over 4

commit 022d05548787501a0ae3a6175c30ce4fccd2e247
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Fri Jun 17 16:22:13 2016 -0400

    Allow rules to require or restrict specific values on transition
    
    Add configuration and functionality to require specific values
    for transitions or restrict specific values.

diff --git a/README b/README
index f879b29..1ea76f0 100644
--- a/README
+++ b/README
@@ -106,6 +106,7 @@ CONFIGURATION
     The fallback for queues without specific rules is specified with '*'
     where the queue name would normally be.
 
+  Requiring Any Value
     Below is an example which requires 1) time worked and filling in a
     custom field named Resolution before resolving tickets in the Helpdesk
     queue and 2) a Category selection before resolving tickets in every
@@ -123,6 +124,28 @@ CONFIGURATION
     The transition syntax is similar to that found in RT's Lifecycles. See
     perldoc /opt/rt4/etc/RT_Config.pm.
 
+  Requiring or Restricting Specific Values
+    Sometimes you want to restrict a transition if a field has a specific
+    value (maybe a ticket can't be resolved if System Status = down) or
+    require a specific value to to allow a transition (ticket can't be
+    resolved unless System Status = normal). To enforce specific values, you
+    can add the following:
+
+        Set( %MandatoryOnTransition,
+            Helpdesk => {
+                '* -> resolved' => ['TimeWorked', 'CF.Some Field1', 'CF.Some Field2'],
+                'CF.Test Field2' => { transition => '* -> resolved', must_be => ['normal', 'restored'] },
+                'CF.Test Field3' => { transition => '* -> resolved', must_not_be => ['reduced', 'down']}
+            },
+        );
+
+    This will then enforce both that the value is set and that it either has
+    one of the required values on the configured transition or does not have
+    one of the restricted values.
+
+    Note that you need to specify the transition the rule applies to since a
+    given queue configuration could have multiple transition rules.
+
   $ShowAllCustomFieldsOnMandatoryUpdate
     By default, this extension shows only the mandatory fields on the update
     page to make it easy for users to fill them out when completing an
diff --git a/lib/RT/Extension/MandatoryOnTransition.pm b/lib/RT/Extension/MandatoryOnTransition.pm
index 5f6616c..b073ffd 100644
--- a/lib/RT/Extension/MandatoryOnTransition.pm
+++ b/lib/RT/Extension/MandatoryOnTransition.pm
@@ -134,6 +134,8 @@ status C<to>.
 The fallback for queues without specific rules is specified with C<'*'> where
 the queue name would normally be.
 
+=head2 Requiring Any Value
+
 Below is an example which requires 1) time worked and filling in a custom field
 named Resolution before resolving tickets in the Helpdesk queue and 2) a
 Category selection before resolving tickets in every other queue.
@@ -150,6 +152,29 @@ 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 or Restricting Specific Values
+
+Sometimes you want to restrict a transition if a field has a specific
+value (maybe a ticket can't be resolved if System Status = down) or
+require a specific value to to allow a transition (ticket can't be
+resolved unless System Status = normal). To enforce specific values, you
+can add the following:
+
+    Set( %MandatoryOnTransition,
+        Helpdesk => {
+            '* -> resolved' => ['TimeWorked', 'CF.Some Field1', 'CF.Some Field2'],
+            'CF.Test Field2' => { transition => '* -> resolved', must_be => ['normal', 'restored'] },
+            'CF.Test Field3' => { transition => '* -> resolved', must_not_be => ['reduced', 'down']}
+        },
+    );
+
+This will then enforce both that the value is set and that it either has
+one of the required values on the configured transition or does
+not have one of the restricted values.
+
+Note that you need to specify the transition the rule applies to
+since a given queue configuration could have multiple transition rules.
+
 =head2 C<$ShowAllCustomFieldsOnMandatoryUpdate>
 
 By default, this extension shows only the mandatory fields on the update page
@@ -272,7 +297,25 @@ sub RequiredFields {
     my @cfs  =  map { /^CF\.(.+)$/i; $1; }
                grep { /^CF\./i } @$required;
 
-    return (\@core, \@cfs);
+    # Pull out an must be or must not be rules
+    my %cf_must_values = ();
+    foreach my $cf (@cfs){
+        if ( $config{"CF.$cf"} ){
+            my $transition = $config{"CF.$cf"}->{'transition'};
+            unless ( $transition ){
+                RT->Logger->error("No transition defined in must be or must not be rules for $cf");
+                next;
+            }
+
+            if ( $transition eq "$from -> $to"
+                 || $transition eq "* -> $to"
+                 || $transition eq "$from -> *" ) {
+
+                $cf_must_values{$cf} = $config{"CF.$cf"};
+            }
+        }
+    }
+    return (\@core, \@cfs, \%cf_must_values);
 }
 
 =head3 CheckMandatoryFields
@@ -325,7 +368,7 @@ sub CheckMandatoryFields {
         return \@errors;
     }
 
-    my ($core, $cfs) = $self->RequiredFields(
+    my ($core, $cfs, $must_values) = $self->RequiredFields(
         Ticket  => $args{'Ticket'},
         Queue   => $args{'Queue'} ? $args{'Queue'}->Name : undef,
         From    => $args{'From'},
@@ -401,6 +444,39 @@ sub CheckMandatoryFields {
             $value = ($ARGSRef->{"${arg}s-Magic"} and exists $ARGSRef->{"${arg}s"}) ? $ARGSRef->{$arg . "s"} : $ARGSRef->{$arg};
         }
 
+        # Check for specific values
+        if ( exists $must_values->{$cf->Name} ){
+            my $cf_value = $value;
+            # Fetch the current value if we didn't receive a new one
+            $cf_value = $args{'Ticket'}->FirstCustomFieldValue($cf->Name)
+                unless defined $cf_value;
+
+            if ( exists $must_values->{$cf->Name}{'must_be'} ){
+                # OK if it's defined and is one of the specified values
+                next if defined $cf_value and grep { $cf_value eq $_ } @{$must_values->{$cf->Name}{'must_be'}};
+                my $valid_values = join ", ", @{$must_values->{$cf->Name}{'must_be'}};
+                my $one_of = '';
+                $one_of = " one of:" if @{$must_values->{$cf->Name}{'must_be'}} > 1;
+                push @errors,
+                  $CurrentUser->loc("[_1] must be$one_of [_3] when changing Status to [_2]",
+                                    $cf->Name, $CurrentUser->loc($ARGSRef->{Status}), $valid_values);
+                next;
+            }
+
+            if ( exists $must_values->{$cf->Name}{'must_not_be'} ){
+                # OK if it's defined and _not_ in the list
+                next if defined $cf_value and !grep { $cf_value eq $_ } @{$must_values->{$cf->Name}{'must_not_be'}};
+                my $valid_values = join ", ", @{$must_values->{$cf->Name}{'must_not_be'}};
+                my $one_of = '';
+                $one_of = " one of:" if @{$must_values->{$cf->Name}{'must_not_be'}} > 1;
+                push @errors,
+                  $CurrentUser->loc("[_1] must not be$one_of [_3] when changing Status to [_2]",
+                                    $cf->Name, $CurrentUser->loc($ARGSRef->{Status}), $valid_values);
+                next;
+            }
+        }
+
+        # Check for any value
         next if defined $value and length $value;
 
         # Is there a current value?  (Particularly important for Date/Datetime CFs

commit babe91626fdd3c8463117ec31bcc2e10612ea23d
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Fri Jun 17 16:23:50 2016 -0400

    Add tests for new must_be and must_not_be features

diff --git a/lib/RT/Extension/MandatoryOnTransition/Test.pm.in b/lib/RT/Extension/MandatoryOnTransition/Test.pm.in
index 41ae190..1757bb1 100644
--- a/lib/RT/Extension/MandatoryOnTransition/Test.pm.in
+++ b/lib/RT/Extension/MandatoryOnTransition/Test.pm.in
@@ -36,8 +36,9 @@ Set( %MandatoryOnTransition,
         'open -> resolved' => [qw(TimeWorked TimeTaken)]
     },
     'General' => {
-        '* -> resolved' => ['TimeWorked', 'TimeTaken', 'CF.Test Field']
-    },
+        '* -> resolved' => ['TimeWorked', 'TimeTaken', 'CF.Test Field', 'CF.Test Field3', 'CF.Test Field4'],
+        'CF.Test Field3' => { transition => '* -> resolved', must_be => ['normal', 'restored'] },
+        'CF.Test Field4' => { transition => '* -> resolved', must_not_be => ['down', 'reduced'] } },
 );
 CONFIG
 
diff --git a/xt/basic.t b/xt/basic.t
index 285eda6..3823fe6 100644
--- a/xt/basic.t
+++ b/xt/basic.t
@@ -42,6 +42,38 @@ ok( $id2, $msg );
 $cf2->AddValue( Name => 'blue' );
 $cf2->AddValue( Name => 'green' );
 
+my $cf3 = RT::CustomField->new($RT::SystemUser);
+my $id3;
+diag "Create custom field for must have values";
+( $id3, $msg ) = $cf3->Create(
+    Name      => 'Test Field3',
+    Type      => 'Select',
+    LookupType => 'RT::Queue-RT::Ticket',
+    MaxValues => '1',
+    Queue     => 'General',
+);
+
+ok( $id3, $msg );
+$cf3->AddValue( Name => 'normal' );
+$cf3->AddValue( Name => 'restored' );
+$cf3->AddValue( Name => 'other' );
+
+my $cf4 = RT::CustomField->new($RT::SystemUser);
+my $id4;
+diag "Create custom field for must not have values";
+( $id4, $msg ) = $cf4->Create(
+    Name      => 'Test Field4',
+    Type      => 'Select',
+    LookupType => 'RT::Queue-RT::Ticket',
+    MaxValues => '1',
+    Queue     => 'General',
+);
+
+ok( $id4, $msg );
+$cf4->AddValue( Name => 'normal' );
+$cf4->AddValue( Name => 'down' );
+$cf4->AddValue( Name => 'reduced' );
+
 diag "Try a resolve without TimeWorked";
 {
     my $t = RT::Test->create_ticket(
@@ -62,13 +94,30 @@ diag "Try a resolve without TimeWorked";
                           'Submit resolve with no Time Worked');
     $m->content_contains('Time Worked is required when changing Status to resolved');
     $m->content_contains('Test Field is required when changing Status to resolved');
+    $m->content_contains('Test Field3 must be one of: normal, restored when changing Status to resolved');
 
     $m->submit_form_ok( { form_name => 'TicketUpdate',
                           fields => { UpdateTimeWorked => 10,
-                                    'Object-RT::Ticket-' . $t->id . "-CustomField-$id-Values" => 'foo'},
+                                    'Object-RT::Ticket-' . $t->id . "-CustomField-$id-Values" => 'foo',
+                                    'Object-RT::Ticket-' . $t->id . "-CustomField-$id3-Values" => 'other',
+                                    'Object-RT::Ticket-' . $t->id . "-CustomField-$id4-Values" => 'down',},
+
                           button => 'SubmitTicket',
                         }, 'Submit resolve with Time Worked and Test Field');
 
+    $m->content_contains('Test Field3 must be one of: normal, restored when changing Status to resolved');
+    $m->content_contains('Test Field4 must not be one of: down, reduced when changing Status to resolved');
+
+    $m->submit_form_ok( { form_name => 'TicketUpdate',
+                          fields => { UpdateTimeWorked => 10,
+                                    'Object-RT::Ticket-' . $t->id . "-CustomField-$id-Values" => 'foo',
+                                    'Object-RT::Ticket-' . $t->id . "-CustomField-$id3-Values" => 'normal',
+                                    'Object-RT::Ticket-' . $t->id . "-CustomField-$id4-Values" => 'normal',},
+
+                          button => 'SubmitTicket',
+                        }, 'Submit resolve with Time Worked and Test Field');
+
+
     if ( $RT::VERSION =~ /^4\.0\.\d+/ ){
         $m->content_contains("TimeWorked changed from (no value) to '10'");
     }
@@ -106,10 +155,26 @@ diag "Try a resolve without TimeWorked in mobile interface";
 
     $m->content_contains('Time Worked is required when changing Status to resolved');
     $m->content_contains('Test Field is required when changing Status to resolved');
+    $m->content_contains('Test Field3 must be one of: normal, restored when changing Status to resolved');
 
     $m->submit_form_ok( { form_number => 1,
                           fields => { UpdateTimeWorked => 10,
-                                    'Object-RT::Ticket-' . $ticket_id . "-CustomField-$id-Values" => 'foo'},
+                                    'Object-RT::Ticket-' . $ticket_id . "-CustomField-$id-Values" => 'foo',
+                                    'Object-RT::Ticket-' . $ticket_id . "-CustomField-$id3-Values" => 'other',
+                                    'Object-RT::Ticket-' . $ticket_id . "-CustomField-$id4-Values" => 'down',},
+
+                          button => 'SubmitTicket',
+                        }, 'Submit resolve with Time Worked and Test Field');
+
+    $m->content_contains('Test Field3 must be one of: normal, restored when changing Status to resolved');
+    $m->content_contains('Test Field4 must not be one of: down, reduced when changing Status to resolved');
+
+    $m->submit_form_ok( { form_number => 1,
+                          fields => { UpdateTimeWorked => 10,
+                                    'Object-RT::Ticket-' . $ticket_id . "-CustomField-$id-Values" => 'foo',
+                                    'Object-RT::Ticket-' . $ticket_id . "-CustomField-$id3-Values" => 'normal',
+                                    'Object-RT::Ticket-' . $ticket_id . "-CustomField-$id4-Values" => 'normal',},
+
                           button => 'SubmitTicket',
                         }, 'Submit resolve with Time Worked and Test Field');
 
diff --git a/xt/required_fields.t b/xt/required_fields.t
index d5d7f63..3c7a91d 100644
--- a/xt/required_fields.t
+++ b/xt/required_fields.t
@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 
-use RT::Extension::MandatoryOnTransition::Test tests => 13;
+use RT::Extension::MandatoryOnTransition::Test tests => undef;
 
 use_ok('RT::Extension::MandatoryOnTransition');
 
@@ -13,7 +13,8 @@ diag "Test RequiredFields without a ticket";
                        );
     is( $core->[0], 'TimeWorked', 'Got TimeWorked for required core');
 
-    ($core, $cf) = RT::Extension::MandatoryOnTransition->RequiredFields(
+    my $must_values;
+    ($core, $cf, $must_values) = RT::Extension::MandatoryOnTransition->RequiredFields(
                            From => "''",
                            To   => 'resolved',
                            Queue => 'General',
@@ -21,6 +22,12 @@ diag "Test RequiredFields without a ticket";
 
     is( $core->[0], 'TimeWorked', 'Got TimeWorked for required core');
     is( $cf->[0], 'Test Field', 'Got Test Field for required custom field');
+
+    is( (ref $must_values->{'Test Field2'}), 'HASH', 'Got a hash for Test Field2 must values');
+    is( (ref $must_values->{'Test Field2'}{'must_be'}), 'ARRAY', 'Got an array for must be values');
+    is( (ref $must_values->{'Test Field2'}{'must_not_be'}), 'ARRAY', 'Got an array for must not be values');
+    is( $must_values->{'Test Field2'}{'must_be'}->[0], 'normal', "First must be value is 'normal'");
+    is( $must_values->{'Test Field2'}{'must_not_be'}->[0], 'down', "First must not be value is 'down'");
 }
 
 diag "Test RequiredFields with a ticket";
@@ -33,11 +40,19 @@ diag "Test RequiredFields with a ticket";
 
     ok( $t->id, 'Created test ticket: ' . $t->id);
 
-    my ($core, $cf) = RT::Extension::MandatoryOnTransition->RequiredFields(
+    my ($core, $cf, $must_values) = RT::Extension::MandatoryOnTransition->RequiredFields(
                            Ticket => $t,
                            To   => 'resolved',
                        );
 
     is( $core->[0], 'TimeWorked', 'Got TimeWorked for required core');
     is( $cf->[0], 'Test Field', 'Got Test Field for required custom field');
+
+    is( (ref $must_values->{'Test Field2'}), 'HASH', 'Got a hash for Test Field2 must values');
+    is( (ref $must_values->{'Test Field2'}{'must_be'}), 'ARRAY', 'Got an array for must be values');
+    is( (ref $must_values->{'Test Field2'}{'must_not_be'}), 'ARRAY', 'Got an array for must not be values');
+    is( $must_values->{'Test Field2'}{'must_be'}->[0], 'normal', "First must be value is 'normal'");
+    is( $must_values->{'Test Field2'}{'must_not_be'}->[0], 'down', "First must not be value is 'down'");
 }
+
+done_testing;

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


More information about the Bps-public-commit mailing list