From e02e5448d8fdad322dbe5562f92d9623e5d6f0dd Mon Sep 17 00:00:00 2001 From: mark Date: Tue, 31 May 2011 23:30:13 +0000 Subject: [PATCH] improve mandatory fields, #9260 --- rt/FREESIDE_MODIFIED | 15 +-- rt/lib/RT/CustomField.pm | 4 - rt/lib/RT/CustomField_Vendor.pm | 12 +++ rt/lib/RT/Interface/Web_Vendor.pm | 101 +++++++++++++++++++++ rt/lib/RT/Ticket_Vendor.pm | 20 ++++ .../Ticket/Elements/Tabs/Default | 12 --- .../Ticket/Modify.html/BeforeActionList | 15 --- .../Ticket/Update.html/BeforeDisplay | 24 ----- rt/share/html/Search/Bulk.html | 13 ++- rt/share/html/Ticket/Display.html | 16 +++- rt/share/html/Ticket/Elements/CheckMandatoryFields | 9 -- rt/share/html/Ticket/Elements/EditCustomFields | 5 + rt/share/html/Ticket/Modify.html | 4 +- rt/share/html/Ticket/ModifyAll.html | 1 + 14 files changed, 174 insertions(+), 77 deletions(-) create mode 100644 rt/lib/RT/CustomField_Vendor.pm delete mode 100644 rt/share/html/Callbacks/CheckMandatoryFields/Ticket/Elements/Tabs/Default delete mode 100644 rt/share/html/Callbacks/CheckMandatoryFields/Ticket/Modify.html/BeforeActionList delete mode 100644 rt/share/html/Callbacks/CheckMandatoryFields/Ticket/Update.html/BeforeDisplay delete mode 100644 rt/share/html/Ticket/Elements/CheckMandatoryFields diff --git a/rt/FREESIDE_MODIFIED b/rt/FREESIDE_MODIFIED index aa74d86af..d18022f7b 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 c018356b2..0582edd5b 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 000000000..9f55c9aa4 --- /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 1999096a7..c79222be5 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 604a84aa2..2039f3e2d 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 2c0698ec2..000000000 --- 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". - -<%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'; -} - 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 4779411ce..000000000 --- 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; -} - 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 0d69bc27b..000000000 --- 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. - - -<%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; -} - diff --git a/rt/share/html/Search/Bulk.html b/rt/share/html/Search/Bulk.html index 84f2c8230..4a510ce97 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 a6850bb1e..59adbd68d 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 3d0324f98..000000000 --- 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; - - diff --git a/rt/share/html/Ticket/Elements/EditCustomFields b/rt/share/html/Ticket/Elements/EditCustomFields index 918f4d4f5..943170677 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'); +} + <%ARGS> $NamePrefix => '' diff --git a/rt/share/html/Ticket/Modify.html b/rt/share/html/Ticket/Modify.html index b80d86f68..9f1a95932 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 1776c40d8..cd9bb3891 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; -- 2.11.0