Add mandatory checks on create and refactor for update

Move the mandatory checking logic into a new method CheckMandatoryFields
in the main module to make it available from both create and update.

Also make CheckMandatoryFields work with both update where a ticket
already exists and create where it hasn't yet been created.

html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Create.html/BeforeCreate

+<%args>
+$ARGSRef
+$skip_create
+$results => []
+</%args>
+<%init>
+
+my $Queue = $ARGSRef->{Queue};
+my $QueueObj = RT::Queue->new($session{'CurrentUser'});
+$QueueObj->Load($Queue);
+
+my $errors_ref = RT::Extension::MandatoryOnTransition->CheckMandatoryFields(
+    ARGSRef => $ARGSRef,
+    From    => "''",
+    To      => $ARGSRef->{'Status'},
+    Queue   => $QueueObj,
+);
+
+if (@$errors_ref) {
+    RT->Logger->debug("Preventing create because of missing mandatory fields");
+    $$skip_create = 1;
+    push @$results, @$errors_ref;
+}
+</%init>

html/Callbacks/RT-Extension-MandatoryOnTransition/Ticket/Update.html/BeforeUpdate

@@ -5,66 +5,16 @@ $skip_update
 $results => []
 </%args>
 <%init>
-my ($core, $cfs) = RT::Extension::MandatoryOnTransition->RequiredFields(
+
+my $errors_ref = RT::Extension::MandatoryOnTransition->CheckMandatoryFields(
+    ARGSRef => $ARGSRef,
     Ticket  => $TicketObj,
     To      => $ARGSRef->{'Status'},
 );
-return unless @$core or @$cfs;
-
-my @errors;
-
-# Check core fields, after canonicalization for update
-for my $field (@$core) {
-    # Will we have a value on update?
-    my $arg = $RT::Extension::MandatoryOnTransition::CORE_FOR_UPDATE{$field} || $field;
-    next if defined $ARGSRef->{$arg} and length $ARGSRef->{$arg};
-
-    # Do we have a value currently?
-    next if grep { $_ eq $field } @RT::Extension::MandatoryOnTransition::CORE_TICKET
-        and $TicketObj->$field();
-
-    (my $label = $field) =~ s/(?<=[a-z])(?=[A-Z])/ /g;  # /
-    push @errors, loc("[_1] is required when changing Status to [_2]", $label, $ARGSRef->{Status});
-}
-
-# Find the CFs we want
-my $CFs = $TicketObj->CustomFields;
-$CFs->Limit( FIELD => 'Name', VALUE => $_, SUBCLAUSE => 'names', ENTRYAGGREGRATOR => 'OR' )
-    for @$cfs;
-
-# Validate them
-my $ValidCFs = $m->comp(
-    '/Elements/ValidateCustomFields',
-    CustomFields => $CFs,
-    NamePrefix => "Object-RT::Ticket-".$TicketObj->Id."-CustomField-",
-    ARGSRef => $ARGSRef
-);
-
-# Check validation results and mandatory-ness
-while (my $cf = $CFs->Next) {
-    # Is there a validation error?
-    if ( not $ValidCFs and my $msg = $m->notes('InvalidField-' . $cf->Id)) {
-        push @errors, loc($cf->Name) . ': ' . $msg;
-        next;
-    }
-
-    # Do we have a submitted value for update?
-    my $arg   = "Object-RT::Ticket-".$TicketObj->Id."-CustomField-".$cf->Id."-Value";
-    my $value = ($ARGSRef->{"${arg}s-Magic"} and exists $ARGSRef->{"${arg}s"})
-                    ? $ARGSRef->{$arg . "s"}
-                    : $ARGSRef->{$arg};
-    next if defined $value and length $value;
-
-    # Is there a current value?  (Particularly important for Date/Datetime CFs
-    # since they don't submit a value on update.)
-    next if $cf->ValuesForObject($TicketObj)->Count;
-
-    push @errors, loc("[_1] is required when changing Status to [_2]", $cf->Name, $ARGSRef->{Status});
-}
 
-if (@errors) {
+if (@$errors_ref) {
     RT->Logger->debug("Preventing update because of missing mandatory fields");
     $$skip_update = 1;
-    push @$results, @errors;
+    push @$results, @$errors_ref;
 }
 </%init>

lib/RT/Extension/MandatoryOnTransition.pm

@@ -2,7 +2,7 @@ use strict;
 use warnings;
 package RT::Extension::MandatoryOnTransition;
 
-our $VERSION = '0.01';
+our $VERSION = '0.02';
 
 =head1 NAME
 
@@ -251,6 +251,123 @@ sub RequiredFields {
     return (\@core, \@cfs);
 }
 
+=head3 CheckMandatoryFields
+
+Pulls core and custom mandatory fields from the configuration and
+checks that they have a value set before transitioning to the
+requested status.
+
+Accepts a paramhash of values:
+    ARGSRef => Reference to Mason ARGS
+    Ticket => ticket object being updated
+    Queue  => Queue object for the queue in which a new ticket is being created
+    From   => Ticket status transitioning from
+    To     => Ticket status transitioning to
+
+Works for both create, where no ticket exists yet, and update on an
+existing ticket. ARGSRef is required for both.
+
+For create, you must also pass Queue, From, and To.
+
+Update requires only Ticket and To since From can be fetched from the
+ticket object.
+
+=cut
+
+sub CheckMandatoryFields {
+    my $self = shift;
+    my %args  = (
+        Ticket  => undef,
+        Queue   => undef,
+        From    => undef,
+        To      => undef,
+        @_,
+    );
+    my $ARGSRef = $args{'ARGSRef'};
+
+    my @errors;
+
+    my ($core, $cfs) = RT::Extension::MandatoryOnTransition->RequiredFields(
+        Ticket  => $args{'Ticket'},
+        Queue   => $args{'Queue'} ? $args{'Queue'}->Name : undef,
+        From    => $args{'From'},
+        To      => $args{'To'},
+    );
+
+    return \@errors unless @$core or @$cfs;
+
+    # Check core fields, after canonicalization for update
+    for my $field (@$core) {
+        # Will we have a value on update?
+        # If we have a Ticket, it's an update, so use the CORE_FOR_UPDATE values
+        # otherwise it's a create so use raw field value with no UPDATE prefix
+        my $arg = $args{'Ticket'} ? $RT::Extension::MandatoryOnTransition::CORE_FOR_UPDATE{$field} || $field
+                                  : $field;
+        next if defined $ARGSRef->{$arg} and length $ARGSRef->{$arg};
+
+        # Do we have a value currently?
+        # In Create the ticket hasn't been created yet.
+        next if grep { $_ eq $field } @RT::Extension::MandatoryOnTransition::CORE_TICKET
+          and ($args{'Ticket'} && $args{'Ticket'}->$field());
+
+        (my $label = $field) =~ s/(?<=[a-z])(?=[A-Z])/ /g; # /
+        push @errors,
+          HTML::Mason::Commands::loc("[_1] is required when changing Status to [_2]",
+                                     $label, $ARGSRef->{Status});
+    }
+
+    # Find the CFs we want
+    my $CFs = $args{'Ticket'} ? $args{'Ticket'}->CustomFields
+      : $args{'Queue'}->TicketCustomFields();
+
+    if ( not $CFs ){
+        $RT::Logger->error("Custom Fields object required to process mandatory custom fields");
+        return \@errors;
+    }
+
+    $CFs->Limit( FIELD => 'Name', VALUE => $_, SUBCLAUSE => 'names', ENTRYAGGREGRATOR => 'OR' )
+      for @$cfs;
+
+    # For constructing NamePrefix for both update and create
+    my $TicketId = $args{'Ticket'} ? $args{'Ticket'}->Id : '';
+
+    # Validate them
+    my $ValidCFs = $HTML::Mason::Commands::m->comp(
+                            '/Elements/ValidateCustomFields',
+                            CustomFields => $CFs,
+                            NamePrefix => "Object-RT::Ticket-".$TicketId."-CustomField-",
+                            ARGSRef => $ARGSRef
+                           );
+
+    # Check validation results and mandatory-ness
+    while (my $cf = $CFs->Next) {
+        # Is there a validation error?
+        if ( not $ValidCFs
+             and my $msg = $HTML::Mason::Commands::m->notes('InvalidField-' . $cf->Id)) {
+            push @errors, loc($cf->Name) . ': ' . $msg;
+            next;
+        }
+
+        # Do we have a submitted value for update?
+        my $arg   = "Object-RT::Ticket-".$TicketId."-CustomField-".$cf->Id."-Value";
+        my $value = ($ARGSRef->{"${arg}s-Magic"} and exists $ARGSRef->{"${arg}s"})
+          ? $ARGSRef->{$arg . "s"}
+            : $ARGSRef->{$arg};
+
+        next if defined $value and length $value;
+
+        # Is there a current value?  (Particularly important for Date/Datetime CFs
+        # since they don't submit a value on update.)
+        next if $args{'Ticket'} && $cf->ValuesForObject($args{'Ticket'})->Count;
+
+        push @errors,
+          HTML::Mason::Commands::loc("[_1] is required when changing Status to [_2]",
+                                     $cf->Name, $ARGSRef->{Status});
+    }
+
+    return \@errors;
+}
+
 =head3 Config
 
 Takes a queue name.  Returns a hashref for the given queue (possibly using the

lib/RT/Extension/MandatoryOnTransition/Test.pm.in

@@ -35,6 +35,9 @@ Set( %MandatoryOnTransition,
     '*' => {
         'open -> resolved' => [qw(TimeWorked)]
     },
+    'General' => {
+        '* -> resolved' => ['TimeWorked', 'CF.Test Field']
+    },
 );
 CONFIG
 

t/mandatory_on_create.t

+use strict;
+use warnings;
+
+use RT::Extension::MandatoryOnTransition::Test tests => 19;
+
+use_ok('RT::Extension::MandatoryOnTransition');
+
+my ( $baseurl, $m ) = RT::Test->started_ok();
+
+ok( $m->login( 'root', 'password' ), 'logged in' );
+
+my $cf = RT::CustomField->new($RT::SystemUser);
+my ( $id, $ret, $msg );
+
+diag "Create custom field";
+( $id, $msg ) = $cf->Create(
+    Name      => 'Test Field',
+    Type      => 'Select',
+    LookupType => 'RT::Queue-RT::Ticket',
+    MaxValues => '1',
+    Queue     => 'General',
+);
+
+ok( $id, $msg );
+$cf->AddValue( Name => 'foo' );
+$cf->AddValue( Name => 'bar' );
+
+diag "Test mandatory fields on create";
+{
+    $m->get_ok($m->rt_base_url);
+    $m->submit_form_ok( { form_name => 'CreateTicketInQueue',
+                          fields => { Queue => 'General',},
+                        }, 'Click button to create ticket');
+
+    $m->title_is('Create a new ticket');
+
+    $m->submit_form_ok( { form_name => 'TicketCreate',
+                          fields => { Status => 'resolved' },
+                        }, 'Submit with resolved status');
+
+    $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->submit_form_ok( { form_name => 'TicketCreate',
+                          fields => { Status => 'resolved',
+                                    'Object-RT::Ticket--CustomField-1-Values' => 'foo'},
+                        }, 'Submit with resolved status');
+
+    $m->content_contains('Time Worked is required when changing Status to resolved');
+
+    $m->submit_form_ok( { form_name => 'TicketCreate',
+                          fields => { Status => 'resolved',
+                                      'Object-RT::Ticket--CustomField-1-Values' => 'foo',
+                                      'TimeWorked' => '10', },
+                        }, 'Submit with resolved status');
+
+    $m->content_contains("Ticket 1 created in queue 'General'");
+}

t/required_fields.t

+use strict;
+use warnings;
+
+use RT::Extension::MandatoryOnTransition::Test tests => 13;
+
+use_ok('RT::Extension::MandatoryOnTransition');
+
+diag "Test RequiredFields without a ticket";
+{
+    my ($core, $cf) = RT::Extension::MandatoryOnTransition->RequiredFields(
+                           From => 'open',
+                           To   => 'resolved',
+                       );
+    is( $core->[0], 'TimeWorked', 'Got TimeWorked for required core');
+
+    ($core, $cf) = RT::Extension::MandatoryOnTransition->RequiredFields(
+                           From => "''",
+                           To   => 'resolved',
+                           Queue => 'General',
+                       );
+
+    is( $core->[0], 'TimeWorked', 'Got TimeWorked for required core');
+    is( $cf->[0], 'Test Field', 'Got Test Field for required custom field');
+}
+
+diag "Test RequiredFields with a ticket";
+{
+    my $t = RT::Test->create_ticket(
+         Queue => 'General',
+         Subject => 'Test Mandatory On Resolve',
+         Content => 'Testing',
+         );
+
+    ok( $t->id, 'Created test ticket: ' . $t->id);
+
+    my ($core, $cf) = 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');
+}