[Rt-commit] rt branch, 4.0/user-group-creation-errors, created. rt-4.0.1rc1-136-g6fd2cb6

Alex Vandiver alexmv at bestpractical.com
Tue Jun 21 15:16:03 EDT 2011


The branch, 4.0/user-group-creation-errors has been created
        at  6fd2cb6c9d25fb4e761b439065837b23c7d1f5a9 (commit)

- Log -----------------------------------------------------------------
commit 24e10893b834a15675a63757219ad03a3a51eb15
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Jun 21 14:14:03 2011 -0400

    Only redirect if we have a valid object (creation was successful)
    
    Otherwise, we fail to preserve the Create=1, and display a "Couldn't
    load user/group ''" message, which is most unhelpful.

diff --git a/share/html/Admin/Groups/Modify.html b/share/html/Admin/Groups/Modify.html
index cb0ae9a..0cf9853 100755
--- a/share/html/Admin/Groups/Modify.html
+++ b/share/html/Admin/Groups/Modify.html
@@ -157,7 +157,7 @@ if  ( ($SetEnabled) and ( $Disabled != $Group->Disabled) ) {
 MaybeRedirectForResults(
     Actions     => \@results,
     Arguments   => { id => $Group->id },
-);
+) if $Group->Id;
 
 push @results, @warnings;
 
diff --git a/share/html/Admin/Users/Modify.html b/share/html/Admin/Users/Modify.html
index 26123e7..54f49bd 100755
--- a/share/html/Admin/Users/Modify.html
+++ b/share/html/Admin/Users/Modify.html
@@ -371,7 +371,7 @@ if (!$Create && $UserObj->Privileged()) {
 MaybeRedirectForResults(
     Actions   => \@results,
     Arguments => { id => $UserObj->Id },
-);
+) if $UserObj->Id;
 
 </%INIT>
 

commit e212b4ac8a7d901bb5baccfb2d7750e303f176ea
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Jun 21 15:11:01 2011 -0400

    Rather than aborting and leaving an empty page, use @results to show the error

diff --git a/share/html/Admin/Groups/Modify.html b/share/html/Admin/Groups/Modify.html
index 0cf9853..1347102 100755
--- a/share/html/Admin/Groups/Modify.html
+++ b/share/html/Admin/Groups/Modify.html
@@ -105,11 +105,12 @@ if ($Create) {
 } else {
     if ($id eq 'new' ) {
 	my ($create_id, $create_msg) = $Group->CreateUserDefinedGroup(Name => $Name );
-        unless ($create_id) {
-            Abort (loc("Group could not be created: [_1]", $create_msg));
+        if ($create_id) {
+            $id = $Group->Id;
+            push @results, $create_msg;
+        } else {
+            push @results, loc("Group could not be created: [_1]", $create_msg);
         }
-	$id = $Group->Id;
-        push @results, $create_msg;
     } else {
 	$Group->Load($id) || Abort('Could not load group');
     }

commit 8268325664c28bcc1fd4dc5bb5b45b95f376ee7a
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Jun 21 15:11:22 2011 -0400

    Use $Group->Id to properly handle when creation failed, now that it can

diff --git a/share/html/Admin/Groups/Modify.html b/share/html/Admin/Groups/Modify.html
index 1347102..431fa0b 100755
--- a/share/html/Admin/Groups/Modify.html
+++ b/share/html/Admin/Groups/Modify.html
@@ -115,7 +115,7 @@ if ($Create) {
 	$Group->Load($id) || Abort('Could not load group');
     }
 
-    if ($id) {
+    if ($Group->Id) {
 	$title = loc("Modify the group [_1]", $Group->Name);
     }
 
@@ -126,7 +126,7 @@ if ($Create) {
     }
 }
 
-if ($id) {
+if ($Group->Id) {
     my @fields = qw(Description Name );
     my @fieldresults = UpdateRecordObject ( AttributesRef => \@fields,
 					    Object => $Group,
@@ -149,7 +149,7 @@ if (defined $Enabled && $Enabled == 1) {
 } else {
     $Disabled = 1;
 }
-if  ( ($SetEnabled) and ( $Disabled != $Group->Disabled) ) { 
+if  ( $Group->Id and ($SetEnabled) and ( $Disabled != $Group->Disabled) ) { 
     my  ($code, $msg) = $Group->SetDisabled($Disabled);
     push @results, $msg;
 }

commit 6fd2cb6c9d25fb4e761b439065837b23c7d1f5a9
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Jun 21 14:16:53 2011 -0400

    During user and group creation, keep submitted values on error
    
    If the user creation fails, pull the defaults for the form from the
    values submitted previously.  This is not entirely possible for
    checkboxes, but this provides simple best-effort towards not losing
    user-input data.
    
    We intentionally insert $Object->Field||$Field, and not the other way
    around, such that invalid values are preserved on create (to reduce
    information loss) but are reset on update (to prevent the user from
    assuming that they succeeded).  Since field updates are orthogonal
    during update (a failure to update email doesn't prevent phone from
    updating), this is not a significant difference, however.

diff --git a/share/html/Admin/Groups/Modify.html b/share/html/Admin/Groups/Modify.html
index 431fa0b..b5b0aad 100755
--- a/share/html/Admin/Groups/Modify.html
+++ b/share/html/Admin/Groups/Modify.html
@@ -63,11 +63,11 @@
 <tr><td align="right">
 <&|/l&>Name</&>:
 </td>
-<td><input name="Name" value="<%$Group->Name||''%>" /></td>
+<td><input name="Name" value="<%$Group->Name||$Name||''%>" /></td>
 </tr>
 <tr>
 <td align="right">
-<&|/l&>Description</&>:</td><td colspan="3"><input name="Description" value="<%$Group->Description||''%>" size="60" /></td>
+<&|/l&>Description</&>:</td><td colspan="3"><input name="Description" value="<%$Group->Description||$Description||''%>" size="60" /></td>
 </tr>
 % my $CFs = $Group->CustomFields;
 % while (my $CF = $CFs->Next) {
@@ -149,7 +149,7 @@ if (defined $Enabled && $Enabled == 1) {
 } else {
     $Disabled = 1;
 }
-if  ( $Group->Id and ($SetEnabled) and ( $Disabled != $Group->Disabled) ) { 
+if  ( $Group->Id and ($SetEnabled) and ( $Disabled != $Group->Disabled) ) {
     my  ($code, $msg) = $Group->SetDisabled($Disabled);
     push @results, $msg;
 }
diff --git a/share/html/Admin/Users/Modify.html b/share/html/Admin/Users/Modify.html
index 54f49bd..d189799 100755
--- a/share/html/Admin/Users/Modify.html
+++ b/share/html/Admin/Users/Modify.html
@@ -66,37 +66,37 @@
 <tr><td align="right">
 <&|/l&>Username</&>:
 </td><td>
-<input name="Name" value="<%$UserObj->Name||''%>" /> <strong><&|/l&>(required)</&></strong>
+<input name="Name" value="<%$UserObj->Name||$Name||''%>" /> <strong><&|/l&>(required)</&></strong>
 </td></tr>
 <tr><td align="right">
 <&|/l&>Email</&>:
 </td><td>
-<input name="EmailAddress" value="<%$UserObj->EmailAddress||''%>" />
+<input name="EmailAddress" value="<%$UserObj->EmailAddress||$EmailAddress||''%>" />
 </td></tr>
 <tr><td align="right">
 <&|/l&>Real Name</&>: 
 </td><td>
-<input name="RealName" value="<%$UserObj->RealName||''%>" />
+<input name="RealName" value="<%$UserObj->RealName||$RealName||''%>" />
 </td></tr>
 <tr><td align="right">
 <&|/l&>Nickname</&>: 
 </td><td>
-<input name="NickName" value="<%$UserObj->NickName||''%>" />
+<input name="NickName" value="<%$UserObj->NickName||$NickName||''%>" />
 </td></tr>
 <tr><td align="right">
 <&|/l&>Unix login</&>: 
 </td><td>
-<input name="Gecos" value="<%$UserObj->Gecos||''%>" />
+<input name="Gecos" value="<%$UserObj->Gecos||$Gecos||''%>" />
 </td></tr>
 <tr><td align="right">
 <&|/l&>Language</&>: 
 </td><td>
-<& /Elements/SelectLang, Name => 'Lang', Default => $UserObj->Lang &>
+<& /Elements/SelectLang, Name => 'Lang', Default => $UserObj->Lang||$Lang &>
 </td></tr>
 <tr><td align="right">
 <&|/l&>Extra info</&>: 
 </td><td>
-<textarea name="FreeformContactInfo" cols="20" rows="5"><%$UserObj->FreeformContactInfo||''%></textarea>
+<textarea name="FreeformContactInfo" cols="20" rows="5"><%$UserObj->FreeformContactInfo||$FreeformContactInfo||''%></textarea>
 </td></tr>
 </table>
 </&>
@@ -124,39 +124,39 @@
 <tr><td align="right">
 <&|/l&>Organization</&>: 
 </td><td>
-<input name="Organization" value="<%$UserObj->Organization||''%>" />
+<input name="Organization" value="<%$UserObj->Organization||$Organization||''%>" />
 </td></tr>
 <tr><td align="right">
 <&|/l&>Address1</&>: 
 </td><td>
-<input name="Address1" value="<%$UserObj->Address1||''%>" />
+<input name="Address1" value="<%$UserObj->Address1||$Address1||''%>" />
 </td></tr>
 <tr><td align="right">
 <&|/l&>Address2</&>: 
 </td><td>
-<input name="Address2" value="<%$UserObj->Address2||''%>" />
+<input name="Address2" value="<%$UserObj->Address2||$Address2||''%>" />
 </td></tr>
 <tr><td align="right">
 <&|/l&>City</&>: 
 </td><td>
-<input name="City" value="<%$UserObj->City||''%>" size="14" />
+<input name="City" value="<%$UserObj->City||$City||''%>" size="14" />
 
 </td></tr>
 <tr><td align="right">
 <&|/l&>State</&>: 
 </td><td>
-<input name="State" value="<%$UserObj->State||''%>" size="3" />
+<input name="State" value="<%$UserObj->State||$State||''%>" size="3" />
 
 </td></tr>
 <tr><td align="right">
 <&|/l&>Zip</&>: 
 </td><td>
-<input name="Zip" value="<%$UserObj->Zip||''%>" size="9" />
+<input name="Zip" value="<%$UserObj->Zip||$Zip||''%>" size="9" />
 </td></tr>
 <tr><td align="right">
 <&|/l&>Country</&>: 
 </td><td>
-<input name="Country" value="<%$UserObj->Country||''%>" />
+<input name="Country" value="<%$UserObj->Country||$Country||''%>" />
 </td></tr>
 </table>
 </&>
@@ -166,22 +166,22 @@
 <tr><td align="right">
 <&|/l&>Residence</&>: 
 </td><td>
-<input name="HomePhone" value="<%$UserObj->HomePhone||''%>" size="13" /><br />
+<input name="HomePhone" value="<%$UserObj->HomePhone||$HomePhone||''%>" size="13" /><br />
 </td></tr>
 <tr><td align="right">
 <&|/l&>Work</&>: 
 </td><td>
-<input name="WorkPhone" value="<%$UserObj->WorkPhone||''%>" size="13" /><br />
+<input name="WorkPhone" value="<%$UserObj->WorkPhone||$WorkPhone||''%>" size="13" /><br />
 </td></tr>
 <tr><td align="right">
 <&|/l&>Mobile</&>: 
 </td><td>
-<input name="MobilePhone" value="<%$UserObj->MobilePhone||''%>" size="13" /><br />
+<input name="MobilePhone" value="<%$UserObj->MobilePhone||$MobilePhone||''%>" size="13" /><br />
 </td></tr>
 <tr><td align="right">
 <&|/l&>Pager</&>: 
 </td><td>
-<input name="PagerPhone" value="<%$UserObj->PagerPhone||''%>" size="13" /><br />
+<input name="PagerPhone" value="<%$UserObj->PagerPhone||$PagerPhone||''%>" size="13" /><br />
 </td>
 </tr>
 </table>
@@ -208,12 +208,12 @@
 <tr>
 <td colspan="2">
 <&| /Widgets/TitleBox, title => loc('Comments about this user') &>
-<textarea class="comments" name="Comments" cols="80" rows="5" wrap="virtual"><%$UserObj->Comments||''%></textarea>
+<textarea class="comments" name="Comments" cols="80" rows="5" wrap="virtual"><%$UserObj->Comments||$Comments||''%></textarea>
 </&>
 %if (!$Create && $UserObj->Privileged) {
 <br />
 <&| /Widgets/TitleBox, title => loc('Signature') &>
-<textarea class="signature" cols="80" rows="5" name="Signature" wrap="hard"><%$UserObj->Signature||''%></textarea>
+<textarea class="signature" cols="80" rows="5" name="Signature" wrap="hard"><%$UserObj->Signature||$Signature||''%></textarea>
 </&>
 % }
 
@@ -363,7 +363,7 @@ unless ( $UserObj->id && $UserObj->Disabled ) {
     $EnabledChecked = 'checked="checked"';
 }
 
-if (!$Create && $UserObj->Privileged()) {  
+if ((!$Create && $UserObj->Privileged()) or (!$UserObj->Id and $Privileged)) {  
     $PrivilegedChecked = 'checked="checked"';
 }
 

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


More information about the Rt-commit mailing list