[Rt-commit] rt branch, 4.0/create-user-group-error-fix, created. rt-4.0.1rc1-126-g13a22f9
? sunnavy
sunnavy at bestpractical.com
Fri Jun 17 01:10:47 EDT 2011
The branch, 4.0/create-user-group-error-fix has been created
at 13a22f9c0290fee2401809a9ddfad0f760eecacf (commit)
- Log -----------------------------------------------------------------
commit ea90052ac841c7b56dfc55bab831e9dfeaa36e80
Author: sunnavy <sunnavy at bestpractical.com>
Date: Fri Jun 17 11:59:01 2011 +0800
show error with form with bad data filled when creating a user/group
it's not friendly if we only show the error without the form.
diff --git a/share/html/Admin/Groups/Modify.html b/share/html/Admin/Groups/Modify.html
index cb0ae9a..c022857 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="<%$ARGS{Name}||$Group->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="<%$ARGS{Description}||$Group->Description||''%>" size="60" /></td>
</tr>
% my $CFs = $Group->CustomFields;
% while (my $CF = $CFs->Next) {
@@ -75,15 +75,15 @@
<% loc($CF->Name) %>:
</td><td>
<& /Elements/EditCustomField, CustomField => $CF,
- Object => $Group,
+ %ARGS,
($Create ? (NamePrefix => 'Object-RT::Group--CustomField-')
- : () )&>
+ : (Object => $Group) )&>
</td></tr>
% }
<tr>
<td colspan="2">
<input type="hidden" class="hidden" name="SetEnabled" value="1" />
-<input type="checkbox" class="checkbox" name="Enabled" value="1" <%$EnabledChecked%> /> <&|/l&>Enabled (Unchecking this box disables this group)</&><br />
+<input type="checkbox" class="checkbox" name="Enabled" value="1" <%$EnabledChecked||''%> /> <&|/l&>Enabled (Unchecking this box disables this group)</&><br />
</td>
</tr>
% $m->callback( %ARGS, GroupObj => $Group, results => \@results );
@@ -105,19 +105,20 @@ 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');
}
- if ($id) {
- $title = loc("Modify the group [_1]", $Group->Name);
+ if ($Group->id) {
+ $title = loc("Modify the group [_1]", $Group->Name);
}
-
# If the create failed
else {
$title = loc("Create a new group");
@@ -125,7 +126,7 @@ if ($Create) {
}
}
-if ($id) {
+if ($Group->id) {
my @fields = qw(Description Name );
my @fieldresults = UpdateRecordObject ( AttributesRef => \@fields,
Object => $Group,
@@ -140,29 +141,40 @@ if ($id) {
if ($dupcheck->Count > 1) {
push @warnings, loc("There is more than one group with the name '[_1]'. This may cause inconsistency in parts of the admin interface, and therefore it's recommended you rename the conflicting groups.", $Group->Name);
}
-}
-#we're asking about enabled on the web page but really care about disabled.
-if (defined $Enabled && $Enabled == 1) {
- $Disabled = 0;
-} else {
- $Disabled = 1;
-}
-if ( ($SetEnabled) and ( $Disabled != $Group->Disabled) ) {
- my ($code, $msg) = $Group->SetDisabled($Disabled);
- push @results, $msg;
+ #we're asking about enabled on the web page but really care about disabled.
+ if (defined $Enabled && $Enabled == 1) {
+ $Disabled = 0;
+ } else {
+ $Disabled = 1;
+ }
+ if ( ($SetEnabled) and ( $Disabled != $Group->Disabled) ) {
+ my ($code, $msg) = $Group->SetDisabled($Disabled);
+ push @results, $msg;
+ }
}
# This code does automatic redirection if any updates happen.
-MaybeRedirectForResults(
- Actions => \@results,
- Arguments => { id => $Group->id },
-);
+# but if $Create is still true here, then there must be something wrong
+# and we need to show user the refilled form.
+if ( !$Create ) {
+ MaybeRedirectForResults(
+ Actions => \@results,
+ Arguments => { id => $Group->id },
+ );
+}
push @results, @warnings;
-unless ($Group->Disabled()) {
- $EnabledChecked ='checked="checked"';
+if ( $SetEnabled ) {
+ if ( $Enabled ) {
+ $EnabledChecked ='checked="checked"';
+ }
+}
+else {
+ unless ($Group->id && $Group->Disabled()) {
+ $EnabledChecked ='checked="checked"';
+ }
}
diff --git a/share/html/Admin/Users/Modify.html b/share/html/Admin/Users/Modify.html
index 26123e7..6d71762 100755
--- a/share/html/Admin/Users/Modify.html
+++ b/share/html/Admin/Users/Modify.html
@@ -66,44 +66,44 @@
<tr><td align="right">
<&|/l&>Username</&>:
</td><td>
-<input name="Name" value="<%$UserObj->Name||''%>" /> <strong><&|/l&>(required)</&></strong>
+<input name="Name" value="<% $ARGS{Name} || $UserObj->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="<%$ARGS{EmailAddress} || $UserObj->EmailAddress||''%>" />
</td></tr>
<tr><td align="right">
<&|/l&>Real Name</&>:
</td><td>
-<input name="RealName" value="<%$UserObj->RealName||''%>" />
+<input name="RealName" value="<%$ARGS{RealName} || $UserObj->RealName||''%>" />
</td></tr>
<tr><td align="right">
<&|/l&>Nickname</&>:
</td><td>
-<input name="NickName" value="<%$UserObj->NickName||''%>" />
+<input name="NickName" value="<%$ARGS{NickName}||$UserObj->NickName||''%>" />
</td></tr>
<tr><td align="right">
<&|/l&>Unix login</&>:
</td><td>
-<input name="Gecos" value="<%$UserObj->Gecos||''%>" />
+<input name="Gecos" value="<%$ARGS{Gecos}||$UserObj->Gecos||''%>" />
</td></tr>
<tr><td align="right">
<&|/l&>Language</&>:
</td><td>
-<& /Elements/SelectLang, Name => 'Lang', Default => $UserObj->Lang &>
+<& /Elements/SelectLang, Name => 'Lang', Default => $ARGS{Lang} || $UserObj->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"><%$ARGS{FreeformContactInfo}||$UserObj->FreeformContactInfo||''%></textarea>
</td></tr>
</table>
</&>
<br />
<&| /Widgets/TitleBox, title => loc('Access control') &>
<input type="hidden" class="hidden" name="SetEnabled" value="1" />
-<input type="checkbox" class="checkbox" name="Enabled" value="1" <%$EnabledChecked%> />
+<input type="checkbox" class="checkbox" name="Enabled" value="1" <%$EnabledChecked||''%> />
<&|/l&>Let this user access RT</&><br />
@@ -124,39 +124,39 @@
<tr><td align="right">
<&|/l&>Organization</&>:
</td><td>
-<input name="Organization" value="<%$UserObj->Organization||''%>" />
+<input name="Organization" value="<%$ARGS{Organization}||$UserObj->Organization||''%>" />
</td></tr>
<tr><td align="right">
<&|/l&>Address1</&>:
</td><td>
-<input name="Address1" value="<%$UserObj->Address1||''%>" />
+<input name="Address1" value="<%$ARGS{Address1}||$UserObj->Address1||''%>" />
</td></tr>
<tr><td align="right">
<&|/l&>Address2</&>:
</td><td>
-<input name="Address2" value="<%$UserObj->Address2||''%>" />
+<input name="Address2" value="<%$ARGS{Address2}||$UserObj->Address2||''%>" />
</td></tr>
<tr><td align="right">
<&|/l&>City</&>:
</td><td>
-<input name="City" value="<%$UserObj->City||''%>" size="14" />
+<input name="City" value="<%$ARGS{City}||$UserObj->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="<%$ARGS{State}||$UserObj->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="<%$ARGS{Zip}||$UserObj->Zip||''%>" size="9" />
</td></tr>
<tr><td align="right">
<&|/l&>Country</&>:
</td><td>
-<input name="Country" value="<%$UserObj->Country||''%>" />
+<input name="Country" value="<%$ARGS{Country}||$UserObj->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="<%$ARGS{HomePhone}||$UserObj->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="<%$ARGS{WorkPhone}||$UserObj->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="<%$ARGS{MobilePhone}||$UserObj->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="<%$ARGS{PaperPhone}||$UserObj->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"><%$ARGS{Comments}||$UserObj->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"><%$ARGS{Signature}||$UserObj->Signature||''%></textarea>
</&>
% }
@@ -359,19 +359,37 @@ if ( $UserObj->Id ) {
# Do some setup for the ui
-unless ( $UserObj->id && $UserObj->Disabled ) {
- $EnabledChecked = 'checked="checked"';
+if ( $SetEnabled ) {
+ if ( $Enabled ) {
+ $EnabledChecked = 'checked="checked"';
+ }
+}
+else {
+ unless ( $UserObj->id && $UserObj->Disabled ) {
+ $EnabledChecked = 'checked="checked"';
+ }
}
-if (!$Create && $UserObj->Privileged()) {
- $PrivilegedChecked = 'checked="checked"';
+if ( $SetPrivileged ) {
+ if ( $Privileged ) {
+ $PrivilegedChecked = 'checked="checked"';
+ }
+}
+else {
+ if (!$Create && $UserObj->Privileged()) {
+ $PrivilegedChecked = 'checked="checked"';
+ }
}
# This code does automatic redirection if any updates happen.
-MaybeRedirectForResults(
- Actions => \@results,
- Arguments => { id => $UserObj->Id },
-);
+# but if $Create is still true here, then there must be something wrong
+# and we need to show user the refilled form
+if ( !$Create ) {
+ MaybeRedirectForResults(
+ Actions => \@results,
+ Arguments => { id => $UserObj->Id },
+ );
+}
</%INIT>
commit 13a22f9c0290fee2401809a9ddfad0f760eecacf
Author: sunnavy <sunnavy at bestpractical.com>
Date: Fri Jun 17 12:14:53 2011 +0800
we don't abort directly if group is failed to create any more
diff --git a/t/web/admin_groups.t b/t/web/admin_groups.t
index e41c8d8..783b5ad 100644
--- a/t/web/admin_groups.t
+++ b/t/web/admin_groups.t
@@ -2,7 +2,7 @@
use strict;
use warnings;
-use RT::Test tests => 29;
+use RT::Test tests => 26;
my ( $url, $m ) = RT::Test->started_ok;
ok( $m->login(), 'logged in' );
@@ -39,10 +39,8 @@ ok( $m->login(), 'logged in' );
form_number => 3,
fields => { Name => 'test group' },
});
- $m->content_contains('RT Error', 'found error title');
$m->content_contains('Group could not be created', 'found results');
$m->content_like(qr/Group name .+? is already in use/, 'found message');
- $m->warning_like(qr/Group name .+? is already in use/, 'found warning');
}
{
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list