[Rt-commit] rt branch, unique-group-names, created. rt-3.9.4-530-g84b68b3
Thomas Sibley
trs at bestpractical.com
Thu Nov 18 20:48:25 EST 2010
The branch, unique-group-names has been created
at 84b68b3784bddb2d30fe342e86308d6e570eed0d (commit)
- Log -----------------------------------------------------------------
commit 90da0c0bff03450eedb1f55edfac8dfa6872a16f
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Nov 18 19:54:26 2010 -0500
Enforce unique group names for user defined groups
Existing non-unique groups will continue to function, but it is now not
possible through the UI or API to create another non-unique group or
rename an existing group to an already taken name.
The Pg and SQLite schemas have required this for 7-8 years, but the
MySQL and Oracle schemas never have. A tool to report non-unique group
names will appear in the near future.
diff --git a/lib/RT/Group_Overlay.pm b/lib/RT/Group_Overlay.pm
index 686f455..3250b63 100755
--- a/lib/RT/Group_Overlay.pm
+++ b/lib/RT/Group_Overlay.pm
@@ -413,6 +413,12 @@ sub _Create {
@_
);
+ # Enforce uniqueness on user defined group names
+ if ($args{'Domain'} and $args{'Domain'} eq 'UserDefined') {
+ my ($ok, $msg) = $self->_ValidateUserDefinedName($args{'Name'});
+ return ($ok, $msg) if not $ok;
+ }
+
$RT::Handle->BeginTransaction() unless ($args{'InsideTransaction'});
# Groups deal with principal ids, rather than user ids.
# When creating this group, set up a principal Id for it.
@@ -423,7 +429,6 @@ sub _Create {
);
$principal->__Set(Field => 'ObjectId', Value => $principal_id);
-
$self->SUPER::Create(
id => $principal_id,
Name => $args{'Name'},
@@ -487,7 +492,37 @@ sub CreateUserDefinedGroup {
return($self->_Create( Domain => 'UserDefined', Type => '', Instance => '', @_));
}
+=head2 ValidateName VALUE
+
+Enforces unique user defined group names when updating
+=cut
+
+sub ValidateName {
+ my ($self, $value) = @_;
+
+ if ($self->Domain and $self->Domain eq 'UserDefined') {
+ my ($ok, $msg) = $self->_ValidateUserDefinedName($value);
+ # It's really too bad we can't pass along the actual error
+ return 0 if not $ok;
+ }
+ return 1;
+}
+
+=head2 _ValidateUserDefinedName VALUE
+
+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);
+ return (0, $self->loc("Group name '[_1]' is already in use", $value))
+ if $dupcheck->id;
+ return 1;
+}
=head2 _CreateACLEquivalenceGroup { Principal }
diff --git a/t/web/admin_groups.t b/t/web/admin_groups.t
new file mode 100644
index 0000000..45c1b9f
--- /dev/null
+++ b/t/web/admin_groups.t
@@ -0,0 +1,60 @@
+#!/usr/bin/perl
+use strict;
+use warnings;
+
+use RT::Test tests => 25;
+
+my ( $url, $m ) = RT::Test->started_ok;
+ok( $m->login(), 'logged in' );
+
+{
+ diag "test creating a group" if $ENV{TEST_VERBOSE};
+ $m->get_ok( $url . '/Admin/Groups/Modify.html?Create=1' );
+ $m->content_contains('Create a new group', 'found title');
+ $m->submit_form_ok({
+ form_number => 3,
+ fields => { Name => 'test group' },
+ });
+ $m->content_contains('Group created', 'found results');
+ $m->content_contains('Modify the group test group', 'found title');
+}
+
+{
+ diag "test creating another group" if $ENV{TEST_VERBOSE};
+ $m->get_ok( $url . '/Admin/Groups/Modify.html?Create=1' );
+ $m->content_contains('Create a new group', 'found title');
+ $m->submit_form_ok({
+ form_number => 3,
+ fields => { Name => 'test group2' },
+ });
+ $m->content_contains('Group created', 'found results');
+ $m->content_contains('Modify the group test group2', 'found title');
+}
+
+{
+ diag "test creating an overlapping group" if $ENV{TEST_VERBOSE};
+ $m->get_ok( $url . '/Admin/Groups/Modify.html?Create=1' );
+ $m->content_contains('Create a new group', 'found title');
+ $m->submit_form_ok({
+ 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');
+}
+
+{
+ diag "test updating a group name to overlap" if $ENV{TEST_VERBOSE};
+ $m->get_ok( $url . '/Admin/Groups/' );
+ $m->follow_link_ok({text => 'test group2'}, 'found title');
+ $m->content_contains('Modify the group test group2');
+ $m->submit_form_ok({
+ form_number => 3,
+ fields => { Name => 'test group' },
+ });
+ $m->content_lacks('Name changed', "name not changed");
+ $m->content_contains('Illegal value for Name', 'found error message');
+ $m->content_contains('test group', 'did not find new name');
+}
+
commit 84b68b3784bddb2d30fe342e86308d6e570eed0d
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Nov 18 20:27:19 2010 -0500
Warn about existing duplicate group names
diff --git a/share/html/Admin/Groups/Modify.html b/share/html/Admin/Groups/Modify.html
index bc5c585..697483f 100755
--- a/share/html/Admin/Groups/Modify.html
+++ b/share/html/Admin/Groups/Modify.html
@@ -96,7 +96,7 @@
</form>
<%INIT>
-my ($title, @results, $Disabled, $EnabledChecked);
+my ($title, @results, @warnings, $Disabled, $EnabledChecked);
my $Group = RT::Group->new($session{'CurrentUser'});
@@ -132,6 +132,14 @@ if ($id) {
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 the UI, and it's recommended you rename the groups.", $Group->Name);
+ }
}
#we're asking about enabled on the web page but really care about disabled.
@@ -154,6 +162,8 @@ $m->comp(
ARGSRef => \%ARGS
);
+push @results, @warnings;
+
unless ($Group->Disabled()) {
$EnabledChecked ='checked="checked"';
}
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list