[Rt-commit] rt branch, 4.0/disallow-blank-field-names, updated. rt-4.0.2-83-gcf76e47
Jason May
jasonmay at bestpractical.com
Tue Sep 27 18:53:41 EDT 2011
The branch, 4.0/disallow-blank-field-names has been updated
via cf76e473f879a20cb28fd0b3aab9338d3b7dccbd (commit)
via 8b41d7f80b931c36372c735b9b121fd1e418e5e2 (commit)
via 6da6bebc9227aa015cc8731d734e1c8cb8ee8a77 (commit)
via 95bdea64674c2c1e87d8b7747591b9f31b423d01 (commit)
from c7c6de2632f901209f01d70de14a2b2bdf60d1f0 (commit)
Summary of changes:
lib/RT/Group.pm | 21 ++++++++++++---------
lib/RT/Queue.pm | 10 +++++-----
share/html/Admin/Groups/Modify.html | 5 ++---
t/web/group_create.t | 10 +++++++++-
4 files changed, 28 insertions(+), 18 deletions(-)
- Log -----------------------------------------------------------------
commit 95bdea64674c2c1e87d8b7747591b9f31b423d01
Author: Jason May <jasonmay at bestpractical.com>
Date: Fri Sep 16 14:46:30 2011 -0400
Validate group names on update
We validate group names on 'create', which is well and good, but
not having constraints on 'update' as well defeats the purpose.
diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index 16c9cb3..b0a7bd9 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -506,10 +506,10 @@ Enforces unique user defined group names when updating
=cut
sub ValidateName {
- my ($self, $value) = @_;
+ my ($self, $value, %args) = @_;
if ($self->Domain and $self->Domain eq 'UserDefined') {
- my ($ok, $msg) = $self->_ValidateUserDefinedName($value);
+ my ($ok, $msg) = $self->_ValidateUserDefinedName($value, %args);
# It's really too bad we can't pass along the actual error
return 0 if not $ok;
}
@@ -523,12 +523,14 @@ Returns true if the user defined group name isn't in use, false otherwise.
=cut
sub _ValidateUserDefinedName {
- my ($self, $value) = @_;
- my $dupcheck = RT::Group->new(RT->SystemUser);
- $dupcheck->LoadUserDefinedGroup($value);
+ my ($self, $value, %args) = @_;
return (0, $self->loc("A group name is required")) unless $value;
- return (0, $self->loc("Group name '[_1]' is already in use", $value))
- if $dupcheck->id;
+ unless ($args{for_update_only}) {
+ my $dupcheck = RT::Group->new(RT->SystemUser);
+ $dupcheck->LoadUserDefinedGroup($value);
+ return (0, $self->loc("Group name '[_1]' is already in use", $value))
+ if $dupcheck->id;
+ }
return 1;
}
diff --git a/share/html/Admin/Groups/Modify.html b/share/html/Admin/Groups/Modify.html
index 9ad48e8..86ed7e4 100755
--- a/share/html/Admin/Groups/Modify.html
+++ b/share/html/Admin/Groups/Modify.html
@@ -128,19 +128,26 @@ if ($Create) {
if ($Group->Id) {
my @fields = qw(Description Name );
- my @fieldresults = UpdateRecordObject ( AttributesRef => \@fields,
- Object => $Group,
- ARGSRef => \%ARGS );
- push (@results, at fieldresults);
- push @results, ProcessObjectCustomFieldUpdates( ARGSRef => \%ARGS, Object => $Group );
-
- # Warn about duplicate groups
- my $dupcheck = RT::Groups->new(RT->SystemUser);
- $dupcheck->LimitToUserDefinedGroups();
- $dupcheck->Limit( FIELD => 'Name', VALUE => $Group->Name );
- 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);
+ my ($val_ok, $val_msg) = $Group->ValidateName($Name, for_update_only => 1);
+ if ($val_ok) {
+ my @fieldresults = UpdateRecordObject ( AttributesRef => \@fields,
+ Object => $Group,
+ ARGSRef => \%ARGS );
+ push (@results, at fieldresults);
+ push @results, ProcessObjectCustomFieldUpdates( ARGSRef => \%ARGS, Object => $Group );
+ # Warn about duplicate groups
+ my $dupcheck = RT::Groups->new(RT->SystemUser);
+ $dupcheck->LimitToUserDefinedGroups();
+ $dupcheck->Limit( FIELD => 'Name', VALUE => $Group->Name );
+ 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);
+ }
+ }
+ else {
+ # Not sure why we can't pass a message
+ push @warnings, $val_msg || 'The group name you supplied is invalid.';
}
+
}
#we're asking about enabled on the web page but really care about disabled.
diff --git a/t/web/group_create.t b/t/web/group_create.t
index 3f69c6a..9c9e431 100644
--- a/t/web/group_create.t
+++ b/t/web/group_create.t
@@ -3,7 +3,7 @@
use strict;
use warnings;
-use RT::Test tests => 9;
+use RT::Test tests => 10;
my ( $baseurl, $m ) = RT::Test->started_ok;
ok $m->login, 'logged in as root';
@@ -41,5 +41,13 @@ diag "Create a group";
$m->content_contains('Group created', 'created group sucessfully' );
$group_id = $m->form_name('ModifyGroup')->value('id');
ok $group_id, "found id of the group in the form, it's #$group_id";
+
+ $m->submit_form(
+ form_name => 'ModifyGroup',
+ fields => {
+ Name => '',
+ },
+ );
+ $m->text_contains('The group name you supplied is invalid');
}
commit 6da6bebc9227aa015cc8731d734e1c8cb8ee8a77
Author: Jason May <jasonmay at bestpractical.com>
Date: Tue Sep 27 14:05:03 2011 -0400
Use wantarray so we don't get false positives on column updates
diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index 9d083c3..623af1b 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -540,23 +540,23 @@ sub ValidateName {
#If name is '' or '0', complain
if (!$name) {
- return (undef, "Queue name is required.");
+ return wantarray ? (undef, "Queue name is required.") : 0;
}
my $tempqueue = RT::Queue->new(RT->SystemUser);
$tempqueue->Load($name);
#If this queue exists, return undef
- if ( $tempqueue->Name() && $tempqueue->id != $self->id) {
- return (undef, "Queue already exists.");
+ if ( $tempqueue->Name() && $tempqueue->id != $self->id) {
+ return wantarray ? (undef, "Queue already exists.") : 0;
}
#If the queue doesn't exist, return 1
elsif (my $q = $self->SUPER::ValidateName($name)) {
- return ($q);
+ return wantarray ? ($q) : $q;
}
else {
- return (undef, "That's not a valid name.");
+ return wantarray ? (undef, "That's not a valid name.") : 0;
}
}
commit 8b41d7f80b931c36372c735b9b121fd1e418e5e2
Author: Jason May <jasonmay at bestpractical.com>
Date: Tue Sep 27 18:52:27 2011 -0400
Do not validate the group name in the component
This already gets validated when the record gets updated.
diff --git a/share/html/Admin/Groups/Modify.html b/share/html/Admin/Groups/Modify.html
index 86ed7e4..01777e8 100755
--- a/share/html/Admin/Groups/Modify.html
+++ b/share/html/Admin/Groups/Modify.html
@@ -128,26 +128,18 @@ if ($Create) {
if ($Group->Id) {
my @fields = qw(Description Name );
- my ($val_ok, $val_msg) = $Group->ValidateName($Name, for_update_only => 1);
- if ($val_ok) {
- my @fieldresults = UpdateRecordObject ( AttributesRef => \@fields,
- Object => $Group,
- ARGSRef => \%ARGS );
- push (@results, at fieldresults);
- push @results, ProcessObjectCustomFieldUpdates( ARGSRef => \%ARGS, Object => $Group );
- # Warn about duplicate groups
- my $dupcheck = RT::Groups->new(RT->SystemUser);
- $dupcheck->LimitToUserDefinedGroups();
- $dupcheck->Limit( FIELD => 'Name', VALUE => $Group->Name );
- 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);
- }
- }
- else {
- # Not sure why we can't pass a message
- push @warnings, $val_msg || 'The group name you supplied is invalid.';
+ my @fieldresults = UpdateRecordObject ( AttributesRef => \@fields,
+ Object => $Group,
+ ARGSRef => \%ARGS );
+ push (@results, at fieldresults);
+ push @results, ProcessObjectCustomFieldUpdates( ARGSRef => \%ARGS, Object => $Group );
+ # Warn about duplicate groups
+ my $dupcheck = RT::Groups->new(RT->SystemUser);
+ $dupcheck->LimitToUserDefinedGroups();
+ $dupcheck->Limit( FIELD => 'Name', VALUE => $Group->Name );
+ 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.
commit cf76e473f879a20cb28fd0b3aab9338d3b7dccbd
Author: Jason May <jasonmay at bestpractical.com>
Date: Tue Sep 27 18:53:17 2011 -0400
If the caller wants an array, also provide the validation message
This allows for back compat and potentially more descriptive errors at
the same time.
diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index b0a7bd9..b232899 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -510,8 +510,9 @@ sub ValidateName {
if ($self->Domain and $self->Domain eq 'UserDefined') {
my ($ok, $msg) = $self->_ValidateUserDefinedName($value, %args);
- # It's really too bad we can't pass along the actual error
- return 0 if not $ok;
+ if (not $ok) {
+ return wantarray ? (0, $msg) : 0;
+ }
}
return 1;
}
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list