[Rt-commit] rt branch, 4.0/numeric-field-errors, created. rt-4.0.1-232-g3708f36

Jason May jasonmay at bestpractical.com
Thu Aug 4 18:31:11 EDT 2011


The branch, 4.0/numeric-field-errors has been created
        at  3708f36018453cc6c4fe509c3e867f26a7490190 (commit)

- Log -----------------------------------------------------------------
commit ad527db523bebdf2d51421dee5c741fe249705b2
Author: Jason May <jasonmay at bestpractical.com>
Date:   Thu Aug 4 12:35:40 2011 -0400

    Show results on the same page instead of redirecting to an error page
    
    http://issues.bestpractical.com/Ticket/Display.html?id=18092
    
    Since we don't redirect anymore, the field values had to be preserved
    among form submissions -- for rationale, see Alex's description in
    commit 6fd2cb6c.

diff --git a/share/html/Admin/CustomFields/Modify.html b/share/html/Admin/CustomFields/Modify.html
index 4558bc6..2fae7d3 100644
--- a/share/html/Admin/CustomFields/Modify.html
+++ b/share/html/Admin/CustomFields/Modify.html
@@ -56,10 +56,10 @@
 <table>
 
 <tr><td class="label"><&|/l&>Name</&></td>
-<td><input name="Name" value="<% $CustomFieldObj->Name || '' %>" size="20" /></td></tr>
+<td><input name="Name" value="<% $CustomFieldObj->Name || $Name || '' %>" size="20" /></td></tr>
 
 <tr><td class="label"><&|/l&>Description</&></td>
-<td><input name="Description" value="<% $CustomFieldObj->Description || '' %>" size="80" /></td></tr>
+<td><input name="Description" value="<% $CustomFieldObj->Description || $Description || '' %>" size="80" /></td></tr>
 
 <tr><td class="label"><&|/l&>Type</&></td>
 <td><& /Admin/Elements/SelectCustomFieldType, 
@@ -95,13 +95,13 @@
 <tr class="edit_validation"><td class="label"><&|/l&>Validation</&></td>
 <td><& /Widgets/ComboBox,
     Name    => 'Pattern',
-    Default => $CustomFieldObj->Pattern,
+    Default => $CustomFieldObj->Pattern || $Pattern,
     Size    => 20,
     Values  => \@CFvalidations,
 &></td></tr>
 
 <tr><td class="label"><&|/l&>Link values to</&></td><td>
-<input size="60" name="LinkValueTo"  value="<% $CustomFieldObj->LinkValueTo || '' %>" />
+<input size="60" name="LinkValueTo"  value="<% $CustomFieldObj->LinkValueTo || $LinkValueTo || '' %>" />
 <div class="hints">
 <&|/l&>RT can make this custom field's values into hyperlinks to another service.</&>
 <&|/l&>Fill in this field with a URL.</&>
@@ -109,7 +109,7 @@
 </div></td></tr>
 
 <tr><td class="label"><&|/l&>Include page</&></td><td>
-<input size="60" name="IncludeContentForValue" value="<% $CustomFieldObj->IncludeContentForValue || '' %>" />
+<input size="60" name="IncludeContentForValue" value="<% $CustomFieldObj->IncludeContentForValue || $IncludeContentForValue || '' %>" />
 <div class="hints">
 <&|/l&>RT can include content from another web service when showing this custom field.</&>
 <&|/l&>Fill in this field with a URL.</&>
@@ -122,7 +122,7 @@
 <& /Admin/Elements/SelectCustomField,
     Name => "BasedOn",
     LookupType => $CustomFieldObj->LookupType,
-    Default => $CustomFieldObj->BasedOnObj,
+    Default => $CustomFieldObj->BasedOnObj || $BasedOn,
     Not => $CustomFieldObj->id,
 &>
 </td></tr>
@@ -175,11 +175,16 @@ else {
             BasedOn       => $BasedOn,
             Disabled      => !$Enabled,
         );
-        $m->comp( "/Elements/Error", Why => loc( "Could not create CustomField", $msg ) ) unless $val;
-        push @results, $msg;
-        $title = loc( 'Created CustomField [_1]', $CustomFieldObj->Name );
+        if (!$val) {
+            push @results, loc("Could not create CustomField: [_1]", $msg);
+            $title = loc( 'Create a CustomField');
+        }
+        else {
+            push @results, loc("Object created");
+            $title = loc( 'Created CustomField [_1]', $CustomFieldObj->Name );
+        }
     } else {
-        $CustomFieldObj->Load( $id ) || $m->comp("/Elements/Error", Why => loc('No CustomField') );
+        $CustomFieldObj->Load( $id ) || push @results, loc('No CustomField');
         $title = loc( 'Editing CustomField [_1]', $CustomFieldObj->Name );
     }
 }
@@ -272,12 +277,6 @@ CustomFieldObj => $CustomFieldObj, CustomFieldValueObj => $cfv, ARGSRef => \%ARG
 $id = $CustomFieldObj->id if $CustomFieldObj->id;
 
 
-# This code does automatic redirection if any updates happen.
-MaybeRedirectForResults(
-    Actions     => \@results,
-    Arguments   => { id => $id },
-);
-
 my $EnabledChecked = qq[checked="checked"];
 $EnabledChecked = '' if $CustomFieldObj->Disabled;
 

commit 4c20b424e8d71ce7a8ee40d318ce0d2e0674a694
Author: Jason May <jasonmay at bestpractical.com>
Date:   Thu Aug 4 13:24:26 2011 -0400

    Rearrange the posititon of the condition in the syntax

diff --git a/share/html/Admin/CustomFields/Modify.html b/share/html/Admin/CustomFields/Modify.html
index 2fae7d3..167f48e 100644
--- a/share/html/Admin/CustomFields/Modify.html
+++ b/share/html/Admin/CustomFields/Modify.html
@@ -184,7 +184,9 @@ else {
             $title = loc( 'Created CustomField [_1]', $CustomFieldObj->Name );
         }
     } else {
-        $CustomFieldObj->Load( $id ) || push @results, loc('No CustomField');
+        push @results, loc('No CustomField')
+            unless  $CustomFieldObj->Load( $id );
+
         $title = loc( 'Editing CustomField [_1]', $CustomFieldObj->Name );
     }
 }

commit 6a06cb90770fbb915972d3e6dfcd51e8d06708f3
Author: Jason May <jasonmay at bestpractical.com>
Date:   Thu Aug 4 14:26:35 2011 -0400

    Only redirect if CF creation was successful to prevent multi-posting

diff --git a/share/html/Admin/CustomFields/Modify.html b/share/html/Admin/CustomFields/Modify.html
index 167f48e..70de38e 100644
--- a/share/html/Admin/CustomFields/Modify.html
+++ b/share/html/Admin/CustomFields/Modify.html
@@ -273,11 +273,14 @@ CustomFieldObj => $CustomFieldObj, CustomFieldValueObj => $cfv, ARGSRef => \%ARG
     }
 }
 
-
-
-
 $id = $CustomFieldObj->id if $CustomFieldObj->id;
 
+# This code does automatic redirection if any updates happen.
+MaybeRedirectForResults(
+    Actions     => \@results,
+    Arguments   => { id => $id },
+) if $CustomFieldObj->id;
+
 
 my $EnabledChecked = qq[checked="checked"];
 $EnabledChecked = '' if $CustomFieldObj->Disabled;

commit 29b1120432a79211a4337bd75de338a0a7b0045f
Author: Jason May <jasonmay at bestpractical.com>
Date:   Thu Aug 4 17:18:37 2011 -0400

    Make the create-queue error more correct (and detailed)
    
    Did:
    Numeric names triggered 'Queue already exists'
    
    Does:
    Pre-existing queues trigger the right error.
    Numeric names trigger "That's not a valid name."

diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index 50fc5dc..73733fc 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -404,8 +404,12 @@ sub Create {
         return ( 0, $self->loc("No permission to create queues") );
     }
 
-    unless ( $self->ValidateName( $args{'Name'} ) ) {
-        return ( 0, $self->loc('Queue already exists') );
+    {
+        my ($val, $msg) = $self->ValidateName( $args{'Name'} );
+
+        if (!$val) {
+            return ($val, $self->loc($msg));
+        }
     }
 
     if ( $args{'Lifecycle'} && $args{'Lifecycle'} ne 'default' ) {
@@ -539,12 +543,15 @@ sub ValidateName {
 
     #If this queue exists, return undef
     if ( $tempqueue->Name() && $tempqueue->id != $self->id)  {
-        return (undef);
+        return (undef, "Queue already exists.");
     }
 
     #If the queue doesn't exist, return 1
+    elsif (my $q = $self->SUPER::ValidateName($name)) {
+        return ($q);
+    }
     else {
-        return ($self->SUPER::ValidateName($name));
+        return (undef, "That's not a valid name.");
     }
 
 }

commit 8e1b1c8071ee5653eb0586f84eff0e88ccfee8f9
Author: Jason May <jasonmay at bestpractical.com>
Date:   Thu Aug 4 17:22:13 2011 -0400

    Don't abort upon error; resubmit to itself and show errors instead
    
    Form fields are also populated with what was entered the previous
    time for usability. The rationale for this is explained in 6fd2cb6c9d2.

diff --git a/share/html/Admin/Queues/Modify.html b/share/html/Admin/Queues/Modify.html
index 011678b..7fb4c56 100755
--- a/share/html/Admin/Queues/Modify.html
+++ b/share/html/Admin/Queues/Modify.html
@@ -57,18 +57,18 @@
 
 <table>
 <tr><td align="right"><&|/l&>Queue Name</&>:</td>
-<td colspan="3"><input name="Name" value="<% ($Create) ? "" : $QueueObj->Name %>" /></td>
+<td colspan="3"><input name="Name" value="<% $Create ? "" : $QueueObj->Name || $Name %>" /></td>
 </tr>
 
 <tr><td align="right"><&|/l&>Description</&>:</td>
-<td colspan="3"><input name="Description" value="<% $Create ? "" : $QueueObj->Description || '' %>" size="60" /></td>
+<td colspan="3"><input name="Description" value="<% $Create ? "" : $QueueObj->Description || $Description || '' %>" size="60" /></td>
 </tr>
 
 <tr><td align="right"><&|/l&>Lifecycle</&>:</td>
 <td colspan="3"><& /Widgets/Form/Select:InputOnly,
     Name         => 'Lifecycle',
     Values       => [ sort { loc($a) cmp loc($b) } RT::Lifecycle->List ],
-    CurrentValue => $Create ? "default" : $QueueObj->Lifecycle,
+    CurrentValue => $Create ? "default" : $QueueObj->Lifecycle || $ARGS{'Lifecycle'},
     Default      => 0,
 &></td>
 </tr>
@@ -78,27 +78,27 @@
 </tr>
 
 <tr><td align="right"><&|/l&>Reply Address</&>:</td>
-<td><input name="CorrespondAddress" value="<% $Create ? "" : $QueueObj->CorrespondAddress || '' %>" />
+<td><input name="CorrespondAddress" value="<% $Create ? "" : $QueueObj->CorrespondAddress || $CorrespondAddress || '' %>" />
 <br /><span><em><&|/l , RT->Config->Get('CorrespondAddress')&>(If left blank, will default to [_1])</&></em></span></td>
 <td align="right"><&|/l&>Comment Address</&>:</td>
-<td><input name="CommentAddress" value="<% $Create ? "" : $QueueObj->CommentAddress || '' %>" />
+<td><input name="CommentAddress" value="<% $Create ? "" : $QueueObj->CommentAddress || $CommentAddress || '' %>" />
 <br /><span><em><&|/l , RT->Config->Get('CommentAddress')&>(If left blank, will default to [_1])</&></em></span></td>
 </tr>
 
 <tr><td align="right"><&|/l&>Priority starts at</&>:</td>
 <td><& /Elements/SelectPriority,
     Name => "InitialPriority",
-    Default => $Create? 0: $QueueObj->InitialPriority,
+    Default => $Create? 0: $QueueObj->InitialPriority || $InitialPriority,
 &></td>
 <td align="right"><&|/l&>Over time, priority moves toward</&>:</td>
 <td><& /Elements/SelectPriority,
     Name => "FinalPriority",
-    Default => $Create? 0: $QueueObj->FinalPriority,
+    Default => $Create? 0: $QueueObj->FinalPriority || $FinalPriority,
 &><br /><span><em><&|/l&>requires running rt-crontool</&></em></span></td>
 </tr>
 
 <tr><td align="right"><&|/l&>Requests should be due in</&>:</td>
-<td colspan="3"><input name="DefaultDueIn" value="<% ($Create) ? "" : $QueueObj->DefaultDueIn%>" /> <&|/l&>days</&>.</td>
+<td colspan="3"><input name="DefaultDueIn" value="<% ($Create) ? "" : $QueueObj->DefaultDueIn || $DefaultDueIn %>" /> <&|/l&>days</&>.</td>
 </tr>
 
 % my $CFs = $QueueObj->CustomFields;
@@ -171,8 +171,10 @@ if ($Create) {
 } else {
     if ( defined $id && $id eq 'new' ) {
         my ($val, $msg) = $QueueObj->Create( Name => $Name );
-        Abort("$msg") unless $val;
-	push @results, $msg;
+        if ($val) {
+            $Create = 1; # Create failed, so bring us back to step 1
+        }
+        push @results, $msg;
     }
     else {
         $QueueObj->Load($id) || $QueueObj->Load($Name) || Abort(loc("Couldn't load queue '[_1]'", $Name));
@@ -230,7 +232,7 @@ if ( $QueueObj->Id ) {
 MaybeRedirectForResults(
     Actions   => \@results,
     Arguments => { id => $QueueObj->Id },
-);
+) if $QueueObj->id;
 
 push @results, @no_redirect_results;
 </%INIT>

commit 3708f36018453cc6c4fe509c3e867f26a7490190
Author: Jason May <jasonmay at bestpractical.com>
Date:   Thu Aug 4 18:16:20 2011 -0400

    Fall back to an empty string for DefaultDueIn
    
    This silence warnings in tests

diff --git a/share/html/Admin/Queues/Modify.html b/share/html/Admin/Queues/Modify.html
index 7fb4c56..767ea62 100755
--- a/share/html/Admin/Queues/Modify.html
+++ b/share/html/Admin/Queues/Modify.html
@@ -98,7 +98,7 @@
 </tr>
 
 <tr><td align="right"><&|/l&>Requests should be due in</&>:</td>
-<td colspan="3"><input name="DefaultDueIn" value="<% ($Create) ? "" : $QueueObj->DefaultDueIn || $DefaultDueIn %>" /> <&|/l&>days</&>.</td>
+<td colspan="3"><input name="DefaultDueIn" value="<% ($Create) ? "" : $QueueObj->DefaultDueIn || $DefaultDueIn || "" %>" /> <&|/l&>days</&>.</td>
 </tr>
 
 % my $CFs = $QueueObj->CustomFields;

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


More information about the Rt-commit mailing list