improve mandatory fields, #9260

14 files changed, 174 insertions, 77 deletions

diff --git a/rt/FREESIDE_MODIFIED b/rt/FREESIDE_MODIFIED
index aa74d86..d18022f 100644
--- a/rt/FREESIDE_MODIFIED
+++ b/rt/FREESIDE_MODIFIED
@@ -10,8 +10,9 @@ etc/initialdata
 
 lib/RT/Attribute_Overlay.pm #bugfix
  lib/RT/Config.pm
-lib/RT/CustomField.pm #CheckMandatoryFields
+lib/RT/CustomField.pm #reverted
 lib/RT/CustomField_Overlay.pm #customfield date patch #timeworked custom fields
+lib/RT/CustomField_Vendor.pm #mandatory fields
 lib/RT/Interface/Web.pm #customfield date patch
                         #fix transaction custom fields
                         #fix Web.pm Overlay/Vendor/Local inclusion
@@ -35,7 +36,7 @@ lib/RT/SearchBuilder.pm #need DBIx::SearchBuilder >= 1.36 for Pg 8.1+
 lib/RT/Transaction_Overlay.pm #fix transaction custom fields
 lib/RT/Tickets_Overlay.pm #customfield date patch #SearchCustomerFields #this-month condition 
  lib/RT/Ticket_Overlay.pm #fix transaction custom fields
-lib/RT/Ticket_Vendor.pm #bulk increment priority
+lib/RT/Ticket_Vendor.pm #bulk increment priority #mandatory fields
  lib/RT/Users_Overlay.pm
  lib/RT/Groups_Overlay.pm
 lib/RT/Date.pm #this-month condition
@@ -86,16 +87,18 @@ share/html/Elements/ShowLink_Checklist
  share/html/Elements/SelectCustomerClass #SearchCustomerFields
  share/html/Elements/SelectCustomerTag #SearchCustomerFields
 share/html/Prefs/SavedSearches.html #saved searches
-share/html/Search/Bulk.html #bulk increment priority
+share/html/Search/Bulk.html #bulk increment priority #mandatory fields
  share/html/Search/Build.html
 share/html/Search/Results.tsv #content-type bug fix
  share/html/Search/Elements/BuildFormatString
  share/html/Search/Elements/PickCFs #customfield date patch
 share/html/Ticket/Checklist.html
- share/html/Ticket/Display.html #timeworked custom fields
+share/html/Ticket/Display.html #timeworked custom fields #mandatory fields
+share/html/Ticket/Modify.html #mandatory fields
+share/html/Ticket/ModifyAll.html #mandatory fields
 share/html/Ticket/Elements/AddCustomers
- share/html/Ticket/Elements/CheckMandatoryFields
 share/html/Ticket/Elements/EditCustomers
+share/html/Ticket/Elements/EditCustomFields #mandatory fields
 share/html/Ticket/Elements/EditTransactionCustomFields #timeworked custom fields
 share/html/Ticket/Elements/ShowCustomers
 share/html/Ticket/Elements/ShowMembers_Checklist
@@ -123,7 +126,7 @@ share/html/Elements/EditCustomers
 
 share/html/Callbacks/RTx-Checklist/*
 
-share/html/Callbacks/CheckMandatoryFields/*
+share/html/Callbacks/CheckMandatoryFields/* #removed
 
 share/html/Callbacks/TimeToResolve/*
 
diff --git a/rt/lib/RT/CustomField.pm b/rt/lib/RT/CustomField.pm
index c018356..0582edd 100644
--- a/rt/lib/RT/CustomField.pm
+++ b/rt/lib/RT/CustomField.pm
@@ -122,7 +122,6 @@ sub Create {
                 Disabled => '0',
                 LinkToValue => '',
                 IncludeContentForValue => '',
-                Required => '0',
 
                   @_);
     $self->SUPER::Create(
@@ -382,9 +381,6 @@ sub _CoreAccessible {
 		{read => 1, auto => 1, sql_type => 11, length => 0,  is_blob => 0,  is_numeric => 0,  type => 'datetime', default => ''},
         Disabled => 
 		{read => 1, write => 1, sql_type => 5, length => 6,  is_blob => 0,  is_numeric => 1,  type => 'smallint(6)', default => '0'},
-        Required =>
-                {read => 1, write => 1, sql_type => 5, length => 6,  is_blob => 0,  is_numeric => 1,  type => 'smallint(6)', default => '0'},
-
  }
 };
 
diff --git a/rt/lib/RT/CustomField_Vendor.pm b/rt/lib/RT/CustomField_Vendor.pm
new file mode 100644
index 0000000..9f55c9a
--- /dev/null
+++ b/rt/lib/RT/CustomField_Vendor.pm
@@ -0,0 +1,12 @@
+package RT::CustomField;
+use strict;
+no warnings 'redefine';
+
+sub _VendorAccessible {
+    {
+        Required => 
+                {read => 1, write => 1, sql_type => 5, length => 6, is_blob => 0, is_numeric => 1, type => 'smallint(6)', default => '0'},
+    },
+};
+
+1;
diff --git a/rt/lib/RT/Interface/Web_Vendor.pm b/rt/lib/RT/Interface/Web_Vendor.pm
index 1999096..c79222b 100644
--- a/rt/lib/RT/Interface/Web_Vendor.pm
+++ b/rt/lib/RT/Interface/Web_Vendor.pm
@@ -34,6 +34,7 @@ use_ok(RT::Interface::Web_Vendor);
 
 package HTML::Mason::Commands;
 use strict;
+no warnings qw(redefine);
 
 =head2 ProcessTicketCustomers 
 
@@ -197,5 +198,105 @@ sub ProcessObjectCustomers {
 
 }
 
+=head2 ProcessTicketBasics ( TicketObj => $Ticket, ARGSRef => \%ARGS );
+
+Updates all core ticket fields except Status, and returns an array of results
+messages.
+
+=cut
+
+sub ProcessTicketBasics {
+
+    my %args = (
+        TicketObj => undef,
+        ARGSRef   => undef,
+        @_
+    );
+
+    my $TicketObj = $args{'TicketObj'};
+    my $ARGSRef   = $args{'ARGSRef'};
+
+    # {{{ Set basic fields
+    my @attribs = qw(
+        Subject
+        FinalPriority
+        Priority
+        TimeEstimated
+        TimeWorked
+        TimeLeft
+        Type
+        Queue
+    );
+
+    if ( $ARGSRef->{'Queue'} and ( $ARGSRef->{'Queue'} !~ /^(\d+)$/ ) ) {
+        my $tempqueue = RT::Queue->new($RT::SystemUser);
+        $tempqueue->Load( $ARGSRef->{'Queue'} );
+        if ( $tempqueue->id ) {
+            $ARGSRef->{'Queue'} = $tempqueue->id;
+        }
+    }
+
+    my @results = UpdateRecordObject(
+        AttributesRef => \@attribs,
+        Object        => $TicketObj,
+        ARGSRef       => $ARGSRef,
+    );
+
+    # We special case owner changing, so we can use ForceOwnerChange
+    if ( $ARGSRef->{'Owner'} && ( $TicketObj->Owner != $ARGSRef->{'Owner'} ) ) {
+        my ($ChownType);
+        if ( $ARGSRef->{'ForceOwnerChange'} ) {
+            $ChownType = "Force";
+        } else {
+            $ChownType = "Give";
+        }
+
+        my ( $val, $msg ) = $TicketObj->SetOwner( $ARGSRef->{'Owner'}, $ChownType );
+        push( @results, $msg );
+    }
+
+    # }}}
+
+    return (@results);
+}
+
+=head2 ProcessTicketStatus (TicketObj => RT::Ticket, ARGSRef => {})
+
+Process updates to the 'Status' field of the ticket.  If the new value 
+of Status is 'resolved', this will check required custom fields before 
+allowing the update.
+
+=cut
+
+sub ProcessTicketStatus {
+    my %args = (
+        TicketObj => undef,
+        ARGSRef   => undef,
+        @_
+    );
+
+    my $TicketObj = $args{'TicketObj'};
+    my $ARGSRef   = $args{'ARGSRef'};
+    my @results;
+
+    return () if !$ARGSRef->{'Status'};
+
+    if ( lc( $ARGSRef->{'Status'} ) eq 'resolved' ) {
+        foreach my $field ( $TicketObj->MissingRequiredFields ) {
+            push @results, loc('Missing required field: [_1]', $field->Name);
+        }
+    }
+    if ( @results ) {
+        $m->notes('RedirectToBasics' => 1);
+        return @results;
+    }
+
+    return UpdateRecordObject(
+        AttributesRef => [ 'Status' ],
+        Object        => $TicketObj,
+        ARGSRef       => $ARGSRef,
+    );
+}
+
 1;
 
diff --git a/rt/lib/RT/Ticket_Vendor.pm b/rt/lib/RT/Ticket_Vendor.pm
index 604a84a..2039f3e 100644
--- a/rt/lib/RT/Ticket_Vendor.pm
+++ b/rt/lib/RT/Ticket_Vendor.pm
@@ -13,4 +13,24 @@ sub SetPriority {
   $Ticket->SUPER::SetPriority($value);
 }
 
+=head2 MissingRequiredFields {
+
+Return all custom fields with the Required flag set for which this object
+doesn't have any non-empty values.
+
+=cut
+
+sub MissingRequiredFields {
+    my $self = shift;
+    my $CustomFields = $self->CustomFields;
+    my @results;
+    while ( my $CF = $CustomFields->Next ) {
+        next if !$CF->Required;
+        if ( !length($self->FirstCustomFieldValue($CF->Id) || '') )  {
+            push @results, $CF;
+        }
+    }
+    return @results;
+}
+
 1;
diff --git a/rt/share/html/Callbacks/CheckMandatoryFields/Ticket/Elements/Tabs/Default b/rt/share/html/Callbacks/CheckMandatoryFields/Ticket/Elements/Tabs/Default
deleted file mode 100644
index 2c0698e..0000000
--- a/rt/share/html/Callbacks/CheckMandatoryFields/Ticket/Elements/Tabs/Default
+++ /dev/null
@@ -1,12 +0,0 @@
-<%doc>
-If mandatory fields aren't set yet, point the "Resolve" link back
-to "Ticket Basics".
-</%doc>
-<%init>
-my $TicketObj = delete($ARGS{'Ticket'});
-my $actions = $ARGS{'actions'};
-if( $m->comp('/Ticket/Elements/CheckMandatoryFields', Ticket => $TicketObj) 
-  ) {
-  $actions->{'G'}->{'path'} = 'Ticket/Modify.html?id='.$TicketObj->Id.'&resolve=1';
-}
-</%init>
diff --git a/rt/share/html/Callbacks/CheckMandatoryFields/Ticket/Modify.html/BeforeActionList b/rt/share/html/Callbacks/CheckMandatoryFields/Ticket/Modify.html/BeforeActionList
deleted file mode 100644
index 4779411..0000000
--- a/rt/share/html/Callbacks/CheckMandatoryFields/Ticket/Modify.html/BeforeActionList
+++ /dev/null
@@ -1,15 +0,0 @@
-<%init>
-use Data::Dumper;
-my $ARGSRef = $ARGS{'ARGSRef'};
-my $TicketObj = $ARGS{'Ticket'};
-my $results = $ARGS{'Actions'};
-if(defined($ARGSRef->{'resolve'})) {
-  my @errors =  
-    $m->comp('/Ticket/Elements/CheckMandatoryFields', Ticket => $TicketObj);
-  return if !@errors;
-  my $msg = 'Missing required field'.(@errors > 1 ? 's' : '').': ' .
-            join(', ', map { $_->Name } @errors);
-  $m->notes( ('InvalidField-' . $_->Id) => 'Required' ) foreach @errors;
-  push @$results, $msg;
-}
-</%init>
diff --git a/rt/share/html/Callbacks/CheckMandatoryFields/Ticket/Update.html/BeforeDisplay b/rt/share/html/Callbacks/CheckMandatoryFields/Ticket/Update.html/BeforeDisplay
deleted file mode 100644
index 0d69bc2..0000000
--- a/rt/share/html/Callbacks/CheckMandatoryFields/Ticket/Update.html/BeforeDisplay
+++ /dev/null
@@ -1,24 +0,0 @@
-<%doc>
-When the user tries to change a ticket's status to "resolved" through 
-the Update interface, check mandatory fields.  If they aren't all set, 
-redirect to Ticket Basics instead of updating.  Note that this will 
-lose any comments/time/other information the user has entered.
-</%doc>
-
-<%init>
-my $TicketObj = $ARGS{'Ticket'};
-my $ARGSRef = $ARGS{'ARGSRef'};
-my $oldStatus = $TicketObj->Status();
-my $newStatus = $ARGSRef->{'Status'} || $ARGSRef->{'DefaultStatus'};
-if( $oldStatus ne 'resolved' and 
-    $newStatus eq 'resolved' and
-    $m->comp('/Ticket/Elements/CheckMandatoryFields', 
-              Ticket => $TicketObj
-              ) ) {
-    $m->clear_buffer;
-    RT::Interface::Web::Redirect( 
-      RT->Config->Get('WebURL')."Ticket/Modify.html?id=".$TicketObj->Id."&resolve=1"
-    );
-    $m->abort;
-}
-</%init>
diff --git a/rt/share/html/Search/Bulk.html b/rt/share/html/Search/Bulk.html
index 84f2c82..4a510ce 100755
--- a/rt/share/html/Search/Bulk.html
+++ b/rt/share/html/Search/Bulk.html
@@ -441,13 +441,20 @@ unless ( $ARGS{'AddMoreAttach'} ) {
                 }
             }
         }
-        my @tempresults = (
+        my @statusresults =
+          ProcessTicketStatus( TicketObj => $Ticket, ARGSRef => \%ARGS );
+
+          my @tempresults = (
             @watchresults,  @basicresults, @dateresults,
-            @updateresults, @linkresults,  @cfresults
+            @updateresults, @linkresults,  @cfresults,
+            @statusresults
         );
 
         @tempresults =
-          map { loc( "Ticket [_1]: [_2]", $Ticket->Id, $_ ) } @tempresults;
+          map { 
+              $_ =~ /^Ticket \d+:/ ?  $_ : 
+              loc( "Ticket [_1]: [_2]", $Ticket->Id, $_ ) 
+            } @tempresults;
 
         @results = ( @results, @tempresults );
     }
diff --git a/rt/share/html/Ticket/Display.html b/rt/share/html/Ticket/Display.html
index a6850bb..59adbd6 100755
--- a/rt/share/html/Ticket/Display.html
+++ b/rt/share/html/Ticket/Display.html
@@ -164,9 +164,12 @@ if ($ARGS{'id'} eq 'new') {
     push @Actions, ProcessTicketLinks(   ARGSRef => \%ARGS, TicketObj => $TicketObj );
     push @Actions, ProcessTicketDates(   ARGSRef => \%ARGS, TicketObj => $TicketObj );
     push @Actions, ProcessTicketCustomFieldUpdates(ARGSRef => \%ARGS, TicketObj => $TicketObj );
+    # If this fails due to required fields being empty, it will set
+    # notes('RedirectToBasics').
+    push @Actions, ProcessTicketStatus(  ARGSRef => \%ARGS, TicketObj => $TicketObj );
 
-    # XXX: we shouldn't block actions here if user has no right to see the ticket,
-    # but we should allow him to see actions he has done
+    # XXX: we shouldn't block actions here if user has no right to see the 
+    # ticket, but we should allow him to see actions he has done
     unless ($TicketObj->CurrentUserHasRight('ShowTicket')) {
         Abort("No permission to view ticket");
     }
@@ -197,7 +200,14 @@ if (@Actions) {
     my $key = Digest::MD5::md5_hex( rand(1024) );
     push @{ $session{"Actions"}->{$key} ||= [] }, @Actions;
     $session{'i'}++;
-    my $url = RT->Config->Get('WebURL') . "Ticket/Display.html?id=" . $TicketObj->id . "&results=" . $key;
+    my $url = RT->Config->Get('WebURL');
+    if ( $m->notes('RedirectToBasics') ) {
+        $url .= 'Ticket/Modify.html';
+    }
+    else {
+        $url .= 'Ticket/Display.html';
+    }
+    $url .= '?id=' . $TicketObj->id . "&results=" . $key;
     $url .= '#' . $ARGS{Anchor} if $ARGS{Anchor};
     RT::Interface::Web::Redirect($url);
 }
diff --git a/rt/share/html/Ticket/Elements/CheckMandatoryFields b/rt/share/html/Ticket/Elements/CheckMandatoryFields
deleted file mode 100644
index 3d0324f..0000000
--- a/rt/share/html/Ticket/Elements/CheckMandatoryFields
+++ /dev/null
@@ -1,9 +0,0 @@
-<%init>
-
-my $TicketObj = $ARGS{'Ticket'} or return ();
-my $ARGSRef = $ARGS{'ARGSRef'};
-my @fields = grep { $_->Required } 
-             @{ $TicketObj->CustomFields->ItemsArrayRef };
-return grep { !defined($TicketObj->FirstCustomFieldValue($_->id)) } @fields;
-
-</%init>
diff --git a/rt/share/html/Ticket/Elements/EditCustomFields b/rt/share/html/Ticket/Elements/EditCustomFields
index 918f4d4..9431706 100755
--- a/rt/share/html/Ticket/Elements/EditCustomFields
+++ b/rt/share/html/Ticket/Elements/EditCustomFields
@@ -105,6 +105,11 @@ $m->callback( %ARGS, CallbackName => 'MassageCustomFields', CustomFields => $Cus
 
 my $single_column = RT->Config->Get('EditCustomFieldsSingleColumn');
 
+# show hints for missing required fields
+foreach my $field ( $TicketObj->MissingRequiredFields ) {
+    $m->notes('InvalidField-' . $field->Id => 'Required to resolve');
+}
+
 </%INIT>
 <%ARGS>
 $NamePrefix => ''
diff --git a/rt/share/html/Ticket/Modify.html b/rt/share/html/Ticket/Modify.html
index b80d86f..9f1a959 100755
--- a/rt/share/html/Ticket/Modify.html
+++ b/rt/share/html/Ticket/Modify.html
@@ -79,8 +79,10 @@ $m->comp(
 # Now let callbacks have a chance at editing %ARGS
 $m->callback( TicketObj => $TicketObj, CustomFields => $CustomFields, ARGSRef => \%ARGS );
 
-my @results = ProcessTicketBasics(TicketObj => $TicketObj, ARGSRef => \%ARGS);
+my @results;
+push @results, ProcessTicketBasics(TicketObj => $TicketObj, ARGSRef => \%ARGS);
 push @results, ProcessObjectCustomFieldUpdates(Object => $TicketObj, ARGSRef => \%ARGS);
+push @results, ProcessTicketStatus(TicketObj => $TicketObj, ARGSRef => \%ARGS);
 
 $TicketObj->ApplyTransactionBatch;
 
diff --git a/rt/share/html/Ticket/ModifyAll.html b/rt/share/html/Ticket/ModifyAll.html
index 1776c40..cd9bb38 100755
--- a/rt/share/html/Ticket/ModifyAll.html
+++ b/rt/share/html/Ticket/ModifyAll.html
@@ -236,6 +236,7 @@ unless ($OnlySearchForPeople or $OnlySearchForGroup or $ARGS{'AddMoreAttach'} )
     push @results, ProcessTicketBasics( TicketObj => $Ticket, ARGSRef => \%ARGS );
     push @results, ProcessTicketLinks( TicketObj => $Ticket, ARGSRef => \%ARGS);
 }
+    push @results, ProcessTicketStatus( TicketObj => $Ticket, ARGSRef => \%ARGS );
 
 $Ticket->ApplyTransactionBatch;