[Rt-commit] rt branch, 4.0.2-releng, created. rt-4.0.1-263-g88fe4cd

Alex Vandiver alexmv at bestpractical.com
Thu Aug 4 21:04:15 EDT 2011


The branch, 4.0.2-releng has been created
        at  88fe4cda74fef7a79333ec0f843d9e781fe7bac4 (commit)

- Log -----------------------------------------------------------------
commit 7c85c01001ba272fb8dc7cbe7a8f971b412dafbe
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Jul 14 14:15:49 2011 -0400

    Test that group dashboards are correctly listed

diff --git a/t/web/dashboards-groups.t b/t/web/dashboards-groups.t
index f424d68..ba64cc6 100644
--- a/t/web/dashboards-groups.t
+++ b/t/web/dashboards-groups.t
@@ -1,7 +1,7 @@
 #!/usr/bin/perl -w
 use strict;
 
-use RT::Test nodata => 1, tests => 35;
+use RT::Test nodata => 1, tests => 39;
 my ($baseurl, $m) = RT::Test->started_ok;
 
 my $url = $m->rt_base_url;
@@ -95,3 +95,22 @@ $m->content_contains("inner dashboard", "we now have SeeGroupDashboard right");
 $m->content_lacks("Permission denied");
 $m->content_contains('Subscription', "Subscription link not hidden because we have SubscribeDashboard");
 
+
+$m->get_ok("/Dashboards/index.html");
+
+TODO: {
+    local $TODO = "We currently entirely fail to show group dashboards";
+    $m->content_contains("inner dashboard", "We can see the inner dashboard from the UI");
+}
+
+my ($group) = grep {$_->isa("RT::Group") and $_->Id == $inner_group->Id}
+    RT::Dashboard->new($currentuser)->_PrivacyObjects;
+ok($group, "Found the group in  the privacy objects list");
+
+
+TODO: {
+    local $TODO = "We currently entirely fail to show group dashboards";
+    ($group) = grep {$_->isa("RT::Group") and $_->Id == $inner_group->Id}
+        RT::Dashboard->new($currentuser)->ObjectsForLoading;
+    ok($group, "Found the group in the objects for loading");
+}

commit 47b1c48129a77fe9ef6e6ad69e7860ea27a3e200
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Jul 14 14:36:00 2011 -0400

    Split the group rights checking into its own file

diff --git a/t/api/group-rights.t b/t/api/group-rights.t
new file mode 100644
index 0000000..9e4031a
--- /dev/null
+++ b/t/api/group-rights.t
@@ -0,0 +1,78 @@
+use strict;
+use warnings;
+use RT::Test nodata => 1, tests => 17;
+
+RT::Group->AddRights(
+    'RTxGroupRight' => 'Just a right for testing rights',
+);
+
+# this company is split into two halves, the hacks and the non-hacks
+# herbert is a hacker but eric is not.
+my $herbert = RT::User->new(RT->SystemUser);
+my ($ok, $msg) = $herbert->Create(Name => 'herbert');
+ok($ok, $msg);
+
+my $eric = RT::User->new(RT->SystemUser);
+($ok, $msg) = $eric->Create(Name => 'eric');
+ok($ok, $msg);
+
+my $hacks = RT::Group->new(RT->SystemUser);
+($ok, $msg) = $hacks->CreateUserDefinedGroup(Name => 'Hackers');
+ok($ok, $msg);
+
+my $employees = RT::Group->new(RT->SystemUser);
+($ok, $msg) = $employees->CreateUserDefinedGroup(Name => 'Employees');
+ok($ok, $msg);
+
+($ok, $msg) = $employees->AddMember($hacks->PrincipalId);
+ok($ok, $msg);
+
+($ok, $msg) = $hacks->AddMember($herbert->PrincipalId);
+ok($ok, $msg);
+
+($ok, $msg) = $employees->AddMember($eric->PrincipalId);
+ok($ok, $msg);
+
+ok($employees->HasMemberRecursively($hacks->PrincipalId), 'employees has member hacks');
+ok($employees->HasMemberRecursively($herbert->PrincipalId), 'employees has member herbert');
+ok($employees->HasMemberRecursively($eric->PrincipalId), 'employees has member eric');
+
+ok($hacks->HasMemberRecursively($herbert->PrincipalId), 'hacks has member herbert');
+ok(!$hacks->HasMemberRecursively($eric->PrincipalId), 'hacks does not have member eric');
+
+{ # Eric doesn't have the right yet
+    my $groups = RT::Groups->new(RT::CurrentUser->new($eric));
+    $groups->LimitToUserDefinedGroups;
+    $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
+
+    is_deeply([sort map { $_->Name } @{ $groups->ItemsArrayRef }], [], 'no joined groups have RTxGroupRight yet');
+}
+
+{ # Neither does Herbert
+    my $groups = RT::Groups->new(RT::CurrentUser->new($herbert));
+    $groups->LimitToUserDefinedGroups;
+    $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
+    is_deeply([sort map { $_->Name } @{ $groups->ItemsArrayRef }], [], 'no joined groups have RTxGroupRight yet');
+}
+
+# Grant it to employees, on employees
+($ok, $msg) = $employees->PrincipalObj->GrantRight(Right => 'RTxGroupRight', Object => $employees);
+ok($ok, $msg);
+
+{ # Eric is an Employee directly, so has it on Employees
+    my $groups = RT::Groups->new(RT::CurrentUser->new($eric));
+    $groups->LimitToUserDefinedGroups;
+    $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
+    is_deeply([sort map { $_->Name } @{ $groups->ItemsArrayRef }], ['Employees']);
+}
+
+{ # Herbert is an Employee by way of Hackers, so should have it on both
+    my $groups = RT::Groups->new(RT::CurrentUser->new($herbert));
+    $groups->LimitToUserDefinedGroups;
+    $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
+
+    TODO: {
+        local $TODO = "not recursing across groups within groups yet";
+        is_deeply([sort map { $_->Name } @{ $groups->ItemsArrayRef }], ['Employees', 'Hackers']);
+    }
+}
diff --git a/t/api/groups.t b/t/api/groups.t
index 33f0671..e368283 100644
--- a/t/api/groups.t
+++ b/t/api/groups.t
@@ -1,53 +1,45 @@
 use strict;
 use warnings;
-use RT::Test nodata => 1, tests => 44;
+use RT::Test nodata => 1, tests => 27;
 
 RT::Group->AddRights(
     'RTxGroupRight' => 'Just a right for testing rights',
 );
 
 {
-
-# next had bugs
-# Groups->Limit( FIELD => 'id', OPERATOR => '!=', VALUE => xx );
-my $g = RT::Group->new(RT->SystemUser);
-my ($id, $msg) = $g->CreateUserDefinedGroup(Name => 'GroupsNotEqualTest');
-ok ($id, "created group #". $g->id) or diag("error: $msg");
-
-my $groups = RT::Groups->new(RT->SystemUser);
-$groups->Limit( FIELD => 'id', OPERATOR => '!=', VALUE => $g->id );
-$groups->LimitToUserDefinedGroups();
-my $bug = grep $_->id == $g->id, @{$groups->ItemsArrayRef};
-ok (!$bug, "didn't find group");
-
-
+    my $g = RT::Group->new(RT->SystemUser);
+    my ($id, $msg) = $g->CreateUserDefinedGroup(Name => 'GroupsNotEqualTest');
+    ok ($id, "created group #". $g->id) or diag("error: $msg");
+
+    my $groups = RT::Groups->new(RT->SystemUser);
+    $groups->Limit( FIELD => 'id', OPERATOR => '!=', VALUE => $g->id );
+    $groups->LimitToUserDefinedGroups();
+    my $bug = grep $_->id == $g->id, @{$groups->ItemsArrayRef};
+    ok (!$bug, "didn't find group");
 }
 
-{
-
-my $u = RT::User->new(RT->SystemUser);
-my ($id, $msg) = $u->Create( Name => 'Membertests'. $$ );
-ok ($id, 'created user') or diag "error: $msg";
-
-my $g = RT::Group->new(RT->SystemUser);
-($id, $msg) = $g->CreateUserDefinedGroup(Name => 'Membertests');
-ok ($id, $msg);
-
-my ($aid, $amsg) =$g->AddMember($u->id);
-ok ($aid, $amsg);
-ok($g->HasMember($u->PrincipalObj),"G has member u");
-
-my $groups = RT::Groups->new(RT->SystemUser);
-$groups->LimitToUserDefinedGroups();
-$groups->WithMember(PrincipalId => $u->id);
-is ($groups->Count , 1,"found the 1 group - " . $groups->Count);
-is ($groups->First->Id , $g->Id, "it's the right one");
-
 
+{
+    my $u = RT::User->new(RT->SystemUser);
+    my ($id, $msg) = $u->Create( Name => 'Membertests'. $$ );
+    ok ($id, 'created user') or diag "error: $msg";
+
+    my $g = RT::Group->new(RT->SystemUser);
+    ($id, $msg) = $g->CreateUserDefinedGroup(Name => 'Membertests');
+    ok ($id, $msg);
+
+    my ($aid, $amsg) =$g->AddMember($u->id);
+    ok ($aid, $amsg);
+    ok($g->HasMember($u->PrincipalObj),"G has member u");
+
+    my $groups = RT::Groups->new(RT->SystemUser);
+    $groups->LimitToUserDefinedGroups();
+    $groups->WithMember(PrincipalId => $u->id);
+    is ($groups->Count , 1,"found the 1 group - " . $groups->Count);
+    is ($groups->First->Id , $g->Id, "it's the right one");
 }
 
-{
-    no warnings qw/redefine once/;
+no warnings qw/redefine/;
 
 my $q = RT::Queue->new(RT->SystemUser);
 my ($id, $msg) =$q->Create( Name => 'GlobalACLTest');
@@ -124,78 +116,3 @@ is($groups->Count, 1, "RTxGroupRight found for RTxObj2");
 $groups = RT::Groups->new(RT->SystemUser);
 $groups->WithRight(Right => 'RTxGroupRight', Object => $RTxObj2, EquivObjects => [ $RTxSysObj ]);
 is($groups->Count, 1, "RTxGroupRight found for RTxObj2");
-
-}
-
-{
-    # this company is split into two halves, the hacks and the non-hacks
-    # herbert is a hacker but eric is not.
-    my $herbert = RT::User->new(RT->SystemUser);
-    my ($ok, $msg) = $herbert->Create(Name => 'herbert');
-    ok($ok, $msg);
-
-    my $eric = RT::User->new(RT->SystemUser);
-    ($ok, $msg) = $eric->Create(Name => 'eric');
-    ok($ok, $msg);
-
-    my $hacks = RT::Group->new(RT->SystemUser);
-    ($ok, $msg) = $hacks->CreateUserDefinedGroup(Name => 'Hackers');
-    ok($ok, $msg);
-
-    my $employees = RT::Group->new(RT->SystemUser);
-    ($ok, $msg) = $employees->CreateUserDefinedGroup(Name => 'Employees');
-    ok($ok, $msg);
-
-    ($ok, $msg) = $employees->AddMember($hacks->PrincipalId);
-    ok($ok, $msg);
-
-    ($ok, $msg) = $hacks->AddMember($herbert->PrincipalId);
-    ok($ok, $msg);
-
-    ($ok, $msg) = $employees->AddMember($eric->PrincipalId);
-    ok($ok, $msg);
-
-    ok($employees->HasMemberRecursively($hacks->PrincipalId), 'employees has member hacks');
-    ok($employees->HasMemberRecursively($herbert->PrincipalId), 'employees has member herbert');
-    ok($employees->HasMemberRecursively($eric->PrincipalId), 'employees has member eric');
-
-    ok($hacks->HasMemberRecursively($herbert->PrincipalId), 'hacks has member herbert');
-    ok(!$hacks->HasMemberRecursively($eric->PrincipalId), 'hacks does not have member eric');
-
-    {
-        my $groups = RT::Groups->new(RT::CurrentUser->new($eric));
-        $groups->LimitToUserDefinedGroups;
-        $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
-
-        is_deeply([sort map { $_->Name } @{ $groups->ItemsArrayRef }], [], 'no joined groups have RTxGroupRight yet');
-    }
-
-    {
-        my $groups = RT::Groups->new(RT::CurrentUser->new($herbert));
-        $groups->LimitToUserDefinedGroups;
-        $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
-        is_deeply([sort map { $_->Name } @{ $groups->ItemsArrayRef }], [], 'no joined groups have RTxGroupRight yet');
-    }
-
-    ($ok, $msg) = $employees->PrincipalObj->GrantRight(Right => 'RTxGroupRight', Object => $employees);
-    ok($ok, $msg);
-
-    {
-        my $groups = RT::Groups->new(RT::CurrentUser->new($eric));
-        $groups->LimitToUserDefinedGroups;
-        $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
-        is_deeply([sort map { $_->Name } @{ $groups->ItemsArrayRef }], ['Employees']);
-    }
-
-    {
-        my $groups = RT::Groups->new(RT::CurrentUser->new($herbert));
-        $groups->LimitToUserDefinedGroups;
-        $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
-
-        TODO: {
-            local $TODO = "not recursing across groups within groups yet";
-            is_deeply([sort map { $_->Name } @{ $groups->ItemsArrayRef }], ['Employees', 'Hackers']);
-        }
-    }
-}
-

commit 4f94d8c95604a2c5d612f3ee147f492b9d15bd94
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Jul 14 14:40:26 2011 -0400

    Clarify the tests as to who should have which right if queried directly

diff --git a/t/api/group-rights.t b/t/api/group-rights.t
index 9e4031a..d6c51d6 100644
--- a/t/api/group-rights.t
+++ b/t/api/group-rights.t
@@ -1,6 +1,6 @@
 use strict;
 use warnings;
-use RT::Test nodata => 1, tests => 17;
+use RT::Test nodata => 1, tests => 21;
 
 RT::Group->AddRights(
     'RTxGroupRight' => 'Just a right for testing rights',
@@ -60,6 +60,14 @@ ok(!$hacks->HasMemberRecursively($eric->PrincipalId), 'hacks does not have membe
 ok($ok, $msg);
 
 { # Eric is an Employee directly, so has it on Employees
+    my $e = RT::Group->new(RT::CurrentUser->new($eric));
+    $e->Load($employees->Id);
+    ok($e->CurrentUserHasRight("RTxGroupRight"), "Eric has the right on employees");
+
+    my $h = RT::Group->new(RT::CurrentUser->new($eric));
+    $h->Load($hacks->Id);
+    ok(!$h->CurrentUserHasRight("RTxGroupRight"), "Doesn't have it on hackers");
+
     my $groups = RT::Groups->new(RT::CurrentUser->new($eric));
     $groups->LimitToUserDefinedGroups;
     $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
@@ -67,6 +75,14 @@ ok($ok, $msg);
 }
 
 { # Herbert is an Employee by way of Hackers, so should have it on both
+    my $e = RT::Group->new(RT::CurrentUser->new($herbert));
+    $e->Load($employees->Id);
+    ok($e->CurrentUserHasRight("RTxGroupRight"), "Herbert has the right on employees");
+
+    my $h = RT::Group->new(RT::CurrentUser->new($herbert));
+    $h->Load($hacks->Id);
+    ok(!$h->CurrentUserHasRight("RTxGroupRight"), "Also has it on hackers");
+
     my $groups = RT::Groups->new(RT::CurrentUser->new($herbert));
     $groups->LimitToUserDefinedGroups;
     $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');

commit 00746c072bfdd4615593fcd1867085ffea741f1a
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Jul 14 14:59:39 2011 -0400

    Test with a group with no rights, but which the user is a member of
    
    This is the _actual_ case that d76fd32 should have added a test for, as
    the previous behavior was similar to "in the group directly, and/or has
    the right."  Both Eric and Herbert had the right previously by way of
    the System shenanigans earier in the file.

diff --git a/t/api/group-rights.t b/t/api/group-rights.t
index d6c51d6..3a2ae98 100644
--- a/t/api/group-rights.t
+++ b/t/api/group-rights.t
@@ -1,6 +1,6 @@
 use strict;
 use warnings;
-use RT::Test nodata => 1, tests => 21;
+use RT::Test nodata => 1, no_plan => 1;
 
 RT::Group->AddRights(
     'RTxGroupRight' => 'Just a right for testing rights',
@@ -40,6 +40,16 @@ ok($employees->HasMemberRecursively($eric->PrincipalId), 'employees has member e
 ok($hacks->HasMemberRecursively($herbert->PrincipalId), 'hacks has member herbert');
 ok(!$hacks->HasMemberRecursively($eric->PrincipalId), 'hacks does not have member eric');
 
+# There's also a separate group, "Other", which both are a member of.
+my $other = RT::Group->new(RT->SystemUser);
+($ok, $msg) = $other->CreateUserDefinedGroup(Name => 'Other');
+ok($ok, $msg);
+($ok, $msg) = $other->AddMember($eric->PrincipalId);
+ok($ok, $msg);
+($ok, $msg) = $other->AddMember($herbert->PrincipalId);
+ok($ok, $msg);
+
+
 { # Eric doesn't have the right yet
     my $groups = RT::Groups->new(RT::CurrentUser->new($eric));
     $groups->LimitToUserDefinedGroups;
@@ -59,36 +69,44 @@ ok(!$hacks->HasMemberRecursively($eric->PrincipalId), 'hacks does not have membe
 ($ok, $msg) = $employees->PrincipalObj->GrantRight(Right => 'RTxGroupRight', Object => $employees);
 ok($ok, $msg);
 
-{ # Eric is an Employee directly, so has it on Employees
-    my $e = RT::Group->new(RT::CurrentUser->new($eric));
-    $e->Load($employees->Id);
-    ok($e->CurrentUserHasRight("RTxGroupRight"), "Eric has the right on employees");
-
-    my $h = RT::Group->new(RT::CurrentUser->new($eric));
-    $h->Load($hacks->Id);
-    ok(!$h->CurrentUserHasRight("RTxGroupRight"), "Doesn't have it on hackers");
+{ # Eric is an Employee directly, so has it on Employees, but not on Hack or Other
+    my $g = RT::Group->new(RT::CurrentUser->new($eric));
+    $g->Load($employees->Id);
+    ok($g->CurrentUserHasRight("RTxGroupRight"), "Eric has the right on employees");
+    $g->Load($hacks->Id);
+    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Doesn't have it on hackers");
+    $g->Load($other->Id);
+    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Nor on other");
 
     my $groups = RT::Groups->new(RT::CurrentUser->new($eric));
     $groups->LimitToUserDefinedGroups;
     $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
-    is_deeply([sort map { $_->Name } @{ $groups->ItemsArrayRef }], ['Employees']);
+    my %has_right = map { ($_->Name => 1) } @{ $groups->ItemsArrayRef };
+    ok(delete $has_right{Employees}, "Has the right on a group it's in");
+    ok(not(delete $has_right{Hackers}), "Doesn't have the right on a group it's not in");
+    ok(not(delete $has_right{Other}), "Doesn't have the right on a group it's in");
+    ok(not(keys %has_right), "Has no other groups")
 }
 
 { # Herbert is an Employee by way of Hackers, so should have it on both
-    my $e = RT::Group->new(RT::CurrentUser->new($herbert));
-    $e->Load($employees->Id);
-    ok($e->CurrentUserHasRight("RTxGroupRight"), "Herbert has the right on employees");
-
-    my $h = RT::Group->new(RT::CurrentUser->new($herbert));
-    $h->Load($hacks->Id);
-    ok(!$h->CurrentUserHasRight("RTxGroupRight"), "Also has it on hackers");
+    my $g = RT::Group->new(RT::CurrentUser->new($herbert));
+    $g->Load($employees->Id);
+    ok($g->CurrentUserHasRight("RTxGroupRight"), "Herbert has the right on employees");
+    $g->Load($hacks->Id);
+    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Also has it on hackers");
+    $g->Load($other->Id);
+    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "But not on other");
 
     my $groups = RT::Groups->new(RT::CurrentUser->new($herbert));
     $groups->LimitToUserDefinedGroups;
     $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
 
+    my %has_right = map { ($_->Name => 1) } @{ $groups->ItemsArrayRef };
+    ok(delete $has_right{Employees}, "Has the right on a group it's in");
     TODO: {
-        local $TODO = "not recursing across groups within groups yet";
-        is_deeply([sort map { $_->Name } @{ $groups->ItemsArrayRef }], ['Employees', 'Hackers']);
+        local $TODO = "Doesn't recurse properly";
+        ok(delete $has_right{Hackers}, "Grants the right recursively");
     }
+    ok(not(delete $has_right{Other}), "Doesn't have the right on a group it's in");
+    ok(not(keys %has_right), "Has no other groups")
 }

commit a3438d7e00940b0e05839b8e5ec8e74a5e994b2a
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Jul 14 15:09:16 2011 -0400

    Add tests to show that the users don't have the right prior to the grant

diff --git a/t/api/group-rights.t b/t/api/group-rights.t
index 3a2ae98..1219677 100644
--- a/t/api/group-rights.t
+++ b/t/api/group-rights.t
@@ -51,6 +51,14 @@ ok($ok, $msg);
 
 
 { # Eric doesn't have the right yet
+    my $g = RT::Group->new(RT::CurrentUser->new($eric));
+    $g->Load($employees->Id);
+    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Eric doesn't yet have the right on employees");
+    $g->Load($hacks->Id);
+    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Nor on hackers");
+    $g->Load($other->Id);
+    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Nor on other");
+
     my $groups = RT::Groups->new(RT::CurrentUser->new($eric));
     $groups->LimitToUserDefinedGroups;
     $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
@@ -59,6 +67,14 @@ ok($ok, $msg);
 }
 
 { # Neither does Herbert
+    my $g = RT::Group->new(RT::CurrentUser->new($herbert));
+    $g->Load($employees->Id);
+    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Herbert doesn't yet have the right on employees");
+    $g->Load($hacks->Id);
+    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Nor on hackers");
+    $g->Load($other->Id);
+    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Nor on other");
+
     my $groups = RT::Groups->new(RT::CurrentUser->new($herbert));
     $groups->LimitToUserDefinedGroups;
     $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');

commit 86a78900a79961dc4bc0e67a30f70e2016356599
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Jul 14 15:44:07 2011 -0400

    Fix the join direction on ForWhichCurrentUserHasRight
    
    Previously, you only saw the group if your _direct_ membership in it was
    what granted the right.  However, what we actually care about is if:
     (a) You are a member of the group, by any means.
    AND
     (b) The group grants you the right; that is, if it is a member of any
         group which has that right granted to it.

diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index ddd427b..5a74360 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -348,13 +348,22 @@ sub ForWhichCurrentUserHasRight {
         @_,
     );
 
-    my $member = $self->WithMember(
+    # Groups which the current user is a member of
+    $self->WithMember(
         PrincipalId => $self->CurrentUser->Id,
         Recursively => 1,
     );
 
-    my $acl = $self->_JoinACL( %args );
+    # Which are themselves a member of a any group...
+    my $member = $self->Join(
+        ALIAS1 => 'main',
+        FIELD1 => 'id',
+        TABLE2 => 'CachedGroupMembers',
+        FIELD2 => 'MemberId'
+    );
 
+    # ...with the right in question
+    my $acl = $self->_JoinACL( %args );
     $self->Join(
         ALIAS1 => $member,
         FIELD1 => 'GroupId',
diff --git a/t/api/group-rights.t b/t/api/group-rights.t
index 1219677..172ff0f 100644
--- a/t/api/group-rights.t
+++ b/t/api/group-rights.t
@@ -119,10 +119,7 @@ ok($ok, $msg);
 
     my %has_right = map { ($_->Name => 1) } @{ $groups->ItemsArrayRef };
     ok(delete $has_right{Employees}, "Has the right on a group it's in");
-    TODO: {
-        local $TODO = "Doesn't recurse properly";
-        ok(delete $has_right{Hackers}, "Grants the right recursively");
-    }
+    ok(delete $has_right{Hackers}, "Grants the right recursively");
     ok(not(delete $has_right{Other}), "Doesn't have the right on a group it's in");
     ok(not(keys %has_right), "Has no other groups")
 }

commit 9b4f9541312680e57355060e56198f7db16fd27d
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Jul 14 19:38:51 2011 -0400

    Rewrite ForWhichCurrentUserHasRight, and add more tests
    
    The previous implementation didn't depend in any way on what object the
    right was granted on, merely the membership of the group, and what
    groups that group was in.  The correct implementation examines the
    ObjectType and ObjectId parameters of the ACL table.  Specifically, it
    returns groups which are the target of ACLs, which were granted by a
    group, which the current user is a member of.  That is, groups which the
    current user has a particular right on, as the name of the method
    implies.
    
    t/api/group-rights.t has been refactored to be more concise, and cover
    all possibilities of where the ACL can be granted, and to what.
    ForWhichCurrentUserHasRight now agrees with CurrentUserHasRight for all
    cases.

diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index 5a74360..b5df742 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -348,27 +348,26 @@ sub ForWhichCurrentUserHasRight {
         @_,
     );
 
-    # Groups which the current user is a member of
-    $self->WithMember(
-        PrincipalId => $self->CurrentUser->Id,
-        Recursively => 1,
-    );
+    # Groups which are the target object of an ACL with that right, or
+    # where the target is the system object (a global right)
+    my $acl = $self->_JoinACL( %args );
+    $self->_AddSubClause(
+        ACLObjects => "( (main.id = $acl.ObjectId AND $acl.ObjectType = 'RT::Group')"
+                   . " OR $acl.ObjectType = 'RT::System')");
 
-    # Which are themselves a member of a any group...
+    # ...and where that right is granted to any group..
     my $member = $self->Join(
-        ALIAS1 => 'main',
-        FIELD1 => 'id',
+        ALIAS1 => $acl,
+        FIELD1 => 'PrincipalId',
         TABLE2 => 'CachedGroupMembers',
-        FIELD2 => 'MemberId'
+        FIELD2 => 'GroupId',
     );
 
-    # ...with the right in question
-    my $acl = $self->_JoinACL( %args );
-    $self->Join(
-        ALIAS1 => $member,
-        FIELD1 => 'GroupId',
-        ALIAS2 => $acl,
-        FIELD2 => 'PrincipalId',
+    # ...with the current user in it
+    $self->Limit(
+        ALIAS => $member,
+        FIELD => 'MemberId',
+        VALUE => $self->CurrentUser->Id,
     );
 
     return;
diff --git a/t/api/group-rights.t b/t/api/group-rights.t
index 172ff0f..ce5aad7 100644
--- a/t/api/group-rights.t
+++ b/t/api/group-rights.t
@@ -6,7 +6,7 @@ RT::Group->AddRights(
     'RTxGroupRight' => 'Just a right for testing rights',
 );
 
-# this company is split into two halves, the hacks and the non-hacks
+# this company is split into two halves, the hackers and the non-hackers
 # herbert is a hacker but eric is not.
 my $herbert = RT::User->new(RT->SystemUser);
 my ($ok, $msg) = $herbert->Create(Name => 'herbert');
@@ -16,29 +16,29 @@ my $eric = RT::User->new(RT->SystemUser);
 ($ok, $msg) = $eric->Create(Name => 'eric');
 ok($ok, $msg);
 
-my $hacks = RT::Group->new(RT->SystemUser);
-($ok, $msg) = $hacks->CreateUserDefinedGroup(Name => 'Hackers');
+my $hackers = RT::Group->new(RT->SystemUser);
+($ok, $msg) = $hackers->CreateUserDefinedGroup(Name => 'Hackers');
 ok($ok, $msg);
 
 my $employees = RT::Group->new(RT->SystemUser);
 ($ok, $msg) = $employees->CreateUserDefinedGroup(Name => 'Employees');
 ok($ok, $msg);
 
-($ok, $msg) = $employees->AddMember($hacks->PrincipalId);
+($ok, $msg) = $employees->AddMember($hackers->PrincipalId);
 ok($ok, $msg);
 
-($ok, $msg) = $hacks->AddMember($herbert->PrincipalId);
+($ok, $msg) = $hackers->AddMember($herbert->PrincipalId);
 ok($ok, $msg);
 
 ($ok, $msg) = $employees->AddMember($eric->PrincipalId);
 ok($ok, $msg);
 
-ok($employees->HasMemberRecursively($hacks->PrincipalId), 'employees has member hacks');
+ok($employees->HasMemberRecursively($hackers->PrincipalId), 'employees has member hackers');
 ok($employees->HasMemberRecursively($herbert->PrincipalId), 'employees has member herbert');
 ok($employees->HasMemberRecursively($eric->PrincipalId), 'employees has member eric');
 
-ok($hacks->HasMemberRecursively($herbert->PrincipalId), 'hacks has member herbert');
-ok(!$hacks->HasMemberRecursively($eric->PrincipalId), 'hacks does not have member eric');
+ok($hackers->HasMemberRecursively($herbert->PrincipalId), 'hackers has member herbert');
+ok(!$hackers->HasMemberRecursively($eric->PrincipalId), 'hackers does not have member eric');
 
 # There's also a separate group, "Other", which both are a member of.
 my $other = RT::Group->new(RT->SystemUser);
@@ -50,76 +50,81 @@ ok($ok, $msg);
 ok($ok, $msg);
 
 
-{ # Eric doesn't have the right yet
-    my $g = RT::Group->new(RT::CurrentUser->new($eric));
-    $g->Load($employees->Id);
-    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Eric doesn't yet have the right on employees");
-    $g->Load($hacks->Id);
-    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Nor on hackers");
-    $g->Load($other->Id);
-    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Nor on other");
+# Everyone can SeeGroup on all three groups
+my $everyone = RT::Group->new( RT->SystemUser );
+($ok, $msg) = $everyone->LoadSystemInternalGroup( 'Everyone' );
+ok($ok, $msg);
+$everyone->PrincipalObj->GrantRight(Right => 'SeeGroup', Object => $employees);
+$everyone->PrincipalObj->GrantRight(Right => 'SeeGroup', Object => $hackers);
+$everyone->PrincipalObj->GrantRight(Right => 'SeeGroup', Object => $other);
+
+sub CheckRights {
+    my $cu = shift;
+    my %groups = (Employees => 0, Hackers => 0, Other => 0, @_);
+    my $name = $cu->Name;
 
-    my $groups = RT::Groups->new(RT::CurrentUser->new($eric));
+    my $groups = RT::Groups->new(RT::CurrentUser->new($cu));
     $groups->LimitToUserDefinedGroups;
     $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
+    my %has_right = map { ($_->Name => 1) } @{ $groups->ItemsArrayRef };
 
-    is_deeply([sort map { $_->Name } @{ $groups->ItemsArrayRef }], [], 'no joined groups have RTxGroupRight yet');
+    local $Test::Builder::Level = $Test::Builder::Level + 1;
+    for my $groupname (sort keys %groups) {
+        my $g = RT::Group->new(RT::CurrentUser->new($cu));
+        $g->LoadUserDefinedGroup($groupname);
+        if ($groups{$groupname}) {
+            ok( $g->CurrentUserHasRight("RTxGroupRight"), "$name has right on $groupname (direct query)" );
+            ok( delete $has_right{$groupname},            "..and also in ForWhichCurrentUserHasRight");
+        } else {
+            ok( !$g->CurrentUserHasRight("RTxGroupRight"), "$name doesn't have right on $groupname (direct query)" );
+            ok( !delete $has_right{$groupname},            "..and also not in ForWhichCurrentUserHasRight");
+        }
+    }
+    ok(not(keys %has_right), "ForWhichCurrentUserHasRight has no extra groups");
 }
 
-{ # Neither does Herbert
-    my $g = RT::Group->new(RT::CurrentUser->new($herbert));
-    $g->Load($employees->Id);
-    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Herbert doesn't yet have the right on employees");
-    $g->Load($hacks->Id);
-    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Nor on hackers");
-    $g->Load($other->Id);
-    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Nor on other");
+# Neither should have it on any group yet
+CheckRights($eric);
+CheckRights($herbert);
 
-    my $groups = RT::Groups->new(RT::CurrentUser->new($herbert));
-    $groups->LimitToUserDefinedGroups;
-    $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
-    is_deeply([sort map { $_->Name } @{ $groups->ItemsArrayRef }], [], 'no joined groups have RTxGroupRight yet');
-}
 
-# Grant it to employees, on employees
-($ok, $msg) = $employees->PrincipalObj->GrantRight(Right => 'RTxGroupRight', Object => $employees);
-ok($ok, $msg);
+# Grant it to employees, on employees.  Both Herbert and Eric will have
+# it on employees, though Herbert gets it by way of hackers.  Neither
+# will have it on hackers, because the target does not recurse.
+$employees->PrincipalObj->GrantRight( Right => 'RTxGroupRight', Object => $employees);
+CheckRights($eric,    Employees => 1);
+CheckRights($herbert, Employees => 1);
 
-{ # Eric is an Employee directly, so has it on Employees, but not on Hack or Other
-    my $g = RT::Group->new(RT::CurrentUser->new($eric));
-    $g->Load($employees->Id);
-    ok($g->CurrentUserHasRight("RTxGroupRight"), "Eric has the right on employees");
-    $g->Load($hacks->Id);
-    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Doesn't have it on hackers");
-    $g->Load($other->Id);
-    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Nor on other");
 
-    my $groups = RT::Groups->new(RT::CurrentUser->new($eric));
-    $groups->LimitToUserDefinedGroups;
-    $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
-    my %has_right = map { ($_->Name => 1) } @{ $groups->ItemsArrayRef };
-    ok(delete $has_right{Employees}, "Has the right on a group it's in");
-    ok(not(delete $has_right{Hackers}), "Doesn't have the right on a group it's not in");
-    ok(not(delete $has_right{Other}), "Doesn't have the right on a group it's in");
-    ok(not(keys %has_right), "Has no other groups")
-}
+# Grant it to employees, on hackers.  This means both Eric and Herbert
+# will have the right on hackers, but not on employees.
+$employees->PrincipalObj->RevokeRight(Right => 'RTxGroupRight', Object => $employees);
+$employees->PrincipalObj->GrantRight( Right => 'RTxGroupRight', Object => $hackers);
+CheckRights($eric,    Hackers => 1);
+CheckRights($herbert, Hackers => 1);
 
-{ # Herbert is an Employee by way of Hackers, so should have it on both
-    my $g = RT::Group->new(RT::CurrentUser->new($herbert));
-    $g->Load($employees->Id);
-    ok($g->CurrentUserHasRight("RTxGroupRight"), "Herbert has the right on employees");
-    $g->Load($hacks->Id);
-    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "Also has it on hackers");
-    $g->Load($other->Id);
-    ok(!$g->CurrentUserHasRight("RTxGroupRight"), "But not on other");
 
-    my $groups = RT::Groups->new(RT::CurrentUser->new($herbert));
-    $groups->LimitToUserDefinedGroups;
-    $groups->ForWhichCurrentUserHasRight(Right => 'RTxGroupRight');
+# Grant it to hackers, on employees.  Eric will have it nowhere, and
+# Herbert will have it on employees.  Note that the target of the right
+# itself does _not_ recurse down, so Herbert will not have it on
+# hackers.
+$employees->PrincipalObj->RevokeRight(Right => 'RTxGroupRight', Object => $hackers);
+$hackers->PrincipalObj->GrantRight(   Right => 'RTxGroupRight', Object => $employees);
+CheckRights($eric);
+CheckRights($herbert, Employees => 1);
 
-    my %has_right = map { ($_->Name => 1) } @{ $groups->ItemsArrayRef };
-    ok(delete $has_right{Employees}, "Has the right on a group it's in");
-    ok(delete $has_right{Hackers}, "Grants the right recursively");
-    ok(not(delete $has_right{Other}), "Doesn't have the right on a group it's in");
-    ok(not(keys %has_right), "Has no other groups")
-}
+
+# Grant it globally to hackers; herbert will see the right on all
+# employees, hackers, and other.
+$hackers->PrincipalObj->RevokeRight(  Right => 'RTxGroupRight', Object => $employees);
+$hackers->PrincipalObj->GrantRight(   Right => 'RTxGroupRight', Object => RT->System);
+CheckRights($eric);
+CheckRights($herbert, Employees => 1, Hackers => 1, Other => 1 );
+
+
+# Grant it globally to employees; both eric and herbert will see the
+# right on all employees, hackers, and other.
+$hackers->PrincipalObj->RevokeRight(  Right => 'RTxGroupRight', Object => RT->System);
+$employees->PrincipalObj->GrantRight( Right => 'RTxGroupRight', Object => RT->System);
+CheckRights($eric,    Employees => 1, Hackers => 1, Other => 1 );
+CheckRights($herbert, Employees => 1, Hackers => 1, Other => 1 );
diff --git a/t/web/dashboards-groups.t b/t/web/dashboards-groups.t
index ba64cc6..382dd12 100644
--- a/t/web/dashboards-groups.t
+++ b/t/web/dashboards-groups.t
@@ -98,19 +98,13 @@ $m->content_contains('Subscription', "Subscription link not hidden because we ha
 
 $m->get_ok("/Dashboards/index.html");
 
-TODO: {
-    local $TODO = "We currently entirely fail to show group dashboards";
-    $m->content_contains("inner dashboard", "We can see the inner dashboard from the UI");
-}
+$m->content_contains("inner dashboard", "We can see the inner dashboard from the UI");
 
 my ($group) = grep {$_->isa("RT::Group") and $_->Id == $inner_group->Id}
     RT::Dashboard->new($currentuser)->_PrivacyObjects;
 ok($group, "Found the group in  the privacy objects list");
 
 
-TODO: {
-    local $TODO = "We currently entirely fail to show group dashboards";
-    ($group) = grep {$_->isa("RT::Group") and $_->Id == $inner_group->Id}
-        RT::Dashboard->new($currentuser)->ObjectsForLoading;
-    ok($group, "Found the group in the objects for loading");
-}
+($group) = grep {$_->isa("RT::Group") and $_->Id == $inner_group->Id}
+    RT::Dashboard->new($currentuser)->ObjectsForLoading;
+ok($group, "Found the group in the objects for loading");

commit 0707c1866ed8b1556ab29a5abd9b2668302d0788
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Jul 14 19:44:12 2011 -0400

    Add back prevention for "used only once" warnings, removed in the split in 47b1c48

diff --git a/t/api/groups.t b/t/api/groups.t
index e368283..d2dc126 100644
--- a/t/api/groups.t
+++ b/t/api/groups.t
@@ -39,7 +39,7 @@ RT::Group->AddRights(
     is ($groups->First->Id , $g->Id, "it's the right one");
 }
 
-no warnings qw/redefine/;
+no warnings qw/redefine once/;
 
 my $q = RT::Queue->new(RT->SystemUser);
 my ($id, $msg) =$q->Create( Name => 'GlobalACLTest');

commit db7b47776c8e8747bd8869af24ba21fa0581784a
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jul 18 15:52:46 2011 -0400

    Prevent disabled groups from allowing rights in ForWhichCurrentUserHasRight

diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index b5df742..e4b916f 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -362,6 +362,11 @@ sub ForWhichCurrentUserHasRight {
         TABLE2 => 'CachedGroupMembers',
         FIELD2 => 'GroupId',
     );
+    $self->Limit(
+        ALIAS => $member,
+        FIELD => 'Disabled',
+        VALUE => '0',
+    );
 
     # ...with the current user in it
     $self->Limit(
diff --git a/t/api/group-rights.t b/t/api/group-rights.t
index ce5aad7..ead94ef 100644
--- a/t/api/group-rights.t
+++ b/t/api/group-rights.t
@@ -128,3 +128,10 @@ $hackers->PrincipalObj->RevokeRight(  Right => 'RTxGroupRight', Object => RT->Sy
 $employees->PrincipalObj->GrantRight( Right => 'RTxGroupRight', Object => RT->System);
 CheckRights($eric,    Employees => 1, Hackers => 1, Other => 1 );
 CheckRights($herbert, Employees => 1, Hackers => 1, Other => 1 );
+
+
+# Disable the employees group.  Neither eric nor herbert will see the
+# right anywhere.
+$employees->SetDisabled(1);
+CheckRights($eric);
+CheckRights($herbert);

commit 66262242cc17df86f6e5ab69b9518de564fac96e
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jul 18 15:53:38 2011 -0400

    Add a plan back to the testfile

diff --git a/t/api/group-rights.t b/t/api/group-rights.t
index ead94ef..0494c28 100644
--- a/t/api/group-rights.t
+++ b/t/api/group-rights.t
@@ -1,6 +1,6 @@
 use strict;
 use warnings;
-use RT::Test nodata => 1, no_plan => 1;
+use RT::Test nodata => 1, tests => 114;
 
 RT::Group->AddRights(
     'RTxGroupRight' => 'Just a right for testing rights',

commit 9cdbe85361145fe3b058157862ad30f1e5966876
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jul 18 17:02:23 2011 -0400

    Rather than create two identically-named "inner dashboards", name them differently

diff --git a/t/web/dashboards-groups.t b/t/web/dashboards-groups.t
index 382dd12..17ec369 100644
--- a/t/web/dashboards-groups.t
+++ b/t/web/dashboards-groups.t
@@ -1,7 +1,7 @@
 #!/usr/bin/perl -w
 use strict;
 
-use RT::Test nodata => 1, tests => 39;
+use RT::Test nodata => 1, tests => 40;
 my ($baseurl, $m) = RT::Test->started_ok;
 
 my $url = $m->rt_base_url;
@@ -64,18 +64,21 @@ $user_obj->PrincipalObj->GrantRight(Right => 'CreateGroupDashboard', Object => $
 $m->follow_link_ok({ id => 'home-dashboard_create'});
 $m->form_name('ModifyDashboard');
 is_deeply([$m->current_form->find_input('Privacy')->possible_values], ["RT::User-" . $user_obj->Id, "RT::Group-" . $inner_group->Id], "the only selectable privacies are user and inner group (not outer group)");
-$m->field("Name" => 'inner dashboard');
+$m->field("Name" => 'broken dashboard');
 $m->field("Privacy" => "RT::Group-" . $inner_group->Id);
 $m->content_lacks('Delete', "Delete button hidden because we are creating");
-
 $m->click_button(value => 'Create');
-
 $m->content_contains("saved", "we lack SeeGroupDashboard, so we end up back at the index.");
+
 $user_obj->PrincipalObj->GrantRight(
     Right  => 'SeeGroupDashboard',
     Object => $inner_group,
 );
-$m->reload;
+$m->follow_link_ok({ id => 'home-dashboard_create'});
+$m->form_name('ModifyDashboard');
+$m->field("Name" => 'inner dashboard');
+$m->field("Privacy" => "RT::Group-" . $inner_group->Id);
+$m->click_button(value => 'Create');
 $m->content_lacks("Permission denied", "we now have SeeGroupDashboard");
 $m->content_contains("Saved dashboard inner dashboard");
 $m->content_lacks('Delete', "Delete button hidden because we lack DeleteDashboard");

commit f6cb772b25445c69f3024ede7a020d3657134d1d
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jul 18 17:03:05 2011 -0400

    Make dashboards which are only visible from superuser rights not appear in the menu

diff --git a/lib/RT/Dashboard.pm b/lib/RT/Dashboard.pm
index 0756098..1aa9bf8 100644
--- a/lib/RT/Dashboard.pm
+++ b/lib/RT/Dashboard.pm
@@ -379,6 +379,10 @@ sub Subscription {
 
 sub ObjectsForLoading {
     my $self = shift;
+    my %args = (
+        IncludeSuperusers => 1,
+        @_
+    );
     my @objects;
 
     my $CurrentUser = $self->CurrentUser;
@@ -390,7 +394,7 @@ sub ObjectsForLoading {
     $groups->LimitToUserDefinedGroups;
     $groups->ForWhichCurrentUserHasRight(
         Right             => 'SeeGroupDashboard',
-        IncludeSuperusers => 1,
+        %args,
     );
     my $attrs = $groups->Join(
         ALIAS1 => 'main',
diff --git a/share/html/Dashboards/Elements/ListOfDashboards b/share/html/Dashboards/Elements/ListOfDashboards
index a871f79..3927e7a 100644
--- a/share/html/Dashboards/Elements/ListOfDashboards
+++ b/share/html/Dashboards/Elements/ListOfDashboards
@@ -49,7 +49,9 @@
 # put the list of dashboards into the navigation
 use RT::Dashboard;
 
-my @objs = RT::Dashboard->new($session{CurrentUser})->ObjectsForLoading;
+my @objs = RT::Dashboard->new($session{CurrentUser})->ObjectsForLoading(
+               IncludeSuperusers => $IncludeSuperusers
+           );
 
 my %dashboard_map;
 
@@ -75,3 +77,6 @@ $m->callback(%ARGS, dashboards => \@dashboards, CallbackName => 'ModifyDashboard
 
 return @dashboards;
 </%init>
+<%args>
+$IncludeSuperusers => 1
+</%args>
diff --git a/share/html/Elements/Tabs b/share/html/Elements/Tabs
index 865aace..ee02c32 100755
--- a/share/html/Elements/Tabs
+++ b/share/html/Elements/Tabs
@@ -61,7 +61,7 @@ my $query_string = sub {
 my $build_main_nav = sub {
 
     my $home = Menu->child( home => title => loc('Homepage'), path => '/' );
-    my @dashboards = $m->comp("/Dashboards/Elements/ListOfDashboards");
+    my @dashboards = $m->comp("/Dashboards/Elements/ListOfDashboards", IncludeSuperusers => 0);
     my $limit      = 7;
 
     my $more = 0;
diff --git a/t/web/dashboards-groups.t b/t/web/dashboards-groups.t
index 17ec369..0c16798 100644
--- a/t/web/dashboards-groups.t
+++ b/t/web/dashboards-groups.t
@@ -1,7 +1,7 @@
 #!/usr/bin/perl -w
 use strict;
 
-use RT::Test nodata => 1, tests => 40;
+use RT::Test nodata => 1, tests => 48;
 my ($baseurl, $m) = RT::Test->started_ok;
 
 my $url = $m->rt_base_url;
@@ -100,9 +100,11 @@ $m->content_contains('Subscription', "Subscription link not hidden because we ha
 
 
 $m->get_ok("/Dashboards/index.html");
-
 $m->content_contains("inner dashboard", "We can see the inner dashboard from the UI");
 
+$m->get_ok("/index.html");
+$m->content_contains("inner dashboard", "We can see the inner dashboard from the menu drop-down");
+
 my ($group) = grep {$_->isa("RT::Group") and $_->Id == $inner_group->Id}
     RT::Dashboard->new($currentuser)->_PrivacyObjects;
 ok($group, "Found the group in  the privacy objects list");
@@ -111,3 +113,22 @@ ok($group, "Found the group in  the privacy objects list");
 ($group) = grep {$_->isa("RT::Group") and $_->Id == $inner_group->Id}
     RT::Dashboard->new($currentuser)->ObjectsForLoading;
 ok($group, "Found the group in the objects for loading");
+
+
+# With superuser, the dashboards of groups we're not in should not show
+# up in the menu, but should in the dashboard list.
+$user_obj->PrincipalObj->RevokeRight(
+    Right  => 'SeeGroupDashboard',
+    Object => $inner_group,
+);
+$user_obj->PrincipalObj->GrantRight(
+    Right  => 'SuperUser',
+    Object => RT->System,
+);
+$inner_group->DeleteMember($user_obj->PrincipalObj->Id);
+ok(!$outer_group->HasMemberRecursively($user_obj->PrincipalId), "outer no longer has user recursively");
+ok(!$inner_group->HasMemberRecursively($user_obj->PrincipalId), "inner no longer has user recursively");
+$m->get_ok("/Dashboards/index.html");
+$m->content_contains("inner dashboard", "Superuser can see dashboards in their own groups");
+$m->get_ok("/index.html");
+$m->content_lacks("inner dashboard", "Also in the menu");

commit 2e85388e291dffa14b9817d0bb1305d1b4be5b0f
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jul 18 17:21:24 2011 -0400

    Explain why we're skipping superusers in the menu

diff --git a/share/html/Elements/Tabs b/share/html/Elements/Tabs
index ee02c32..5db9bc3 100755
--- a/share/html/Elements/Tabs
+++ b/share/html/Elements/Tabs
@@ -61,6 +61,10 @@ my $query_string = sub {
 my $build_main_nav = sub {
 
     my $home = Menu->child( home => title => loc('Homepage'), path => '/' );
+    # We explicitly exclude superusers; otherwise the dashboards for
+    # groups you're not in (but can see the dashboards of by dint of
+    # being a superuser) would push the useful ones from the groups
+    # you're actually in off of the stack.
     my @dashboards = $m->comp("/Dashboards/Elements/ListOfDashboards", IncludeSuperusers => 0);
     my $limit      = 7;
 

commit 415d6c197467f1e21d81bea2f2486b00b356d17c
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jul 20 20:27:10 2011 -0400

    Having SeeGroupDashboard on a group is insufficient; you must also be a member
    
    This restriction is currently enforced in RT::SharedSetting->_GetObject,
    and means that granting SeeGroupDashboard globally (on RT::System) means
    "you can see group dashboards for all groups you are in", as opposed to
    the previous implementation which caused it to mean "you can see all
    group dashboards."
    
    We may move eventually to SeeGroupDashboard globally having the latter
    meaning, but that would be a semantic change, and would require other UI
    changes.

diff --git a/lib/RT/Dashboard.pm b/lib/RT/Dashboard.pm
index 1aa9bf8..0822b0b 100644
--- a/lib/RT/Dashboard.pm
+++ b/lib/RT/Dashboard.pm
@@ -385,17 +385,26 @@ sub ObjectsForLoading {
     );
     my @objects;
 
+    # If you've been granted the SeeOwnDashboard global right (which you
+    # could have by way of global user right or global group right), you
+    # get to see your own dashboards
     my $CurrentUser = $self->CurrentUser;
     push @objects, $CurrentUser->UserObj
         if $CurrentUser->HasRight(Object => $RT::System, Right => 'SeeOwnDashboard');
 
-
+    # Find groups for which: (a) you are a member of the group, and (b)
+    # you have been granted SeeGroupDashboard on (by any means), and (c)
+    # have at least one dashboard
     my $groups = RT::Groups->new($CurrentUser);
     $groups->LimitToUserDefinedGroups;
     $groups->ForWhichCurrentUserHasRight(
         Right             => 'SeeGroupDashboard',
         %args,
     );
+    $groups->WithMember(
+        Recursively => 1,
+        PrincipalId => $CurrentUser->UserObj->PrincipalId
+    );
     my $attrs = $groups->Join(
         ALIAS1 => 'main',
         FIELD1 => 'id',
@@ -412,10 +421,11 @@ sub ObjectsForLoading {
         FIELD => 'Name',
         VALUE => 'Dashboard',
     );
-
     push @objects, @{ $groups->ItemsArrayRef };
 
-
+    # Finally, if you have been granted the SeeDashboard right (which
+    # you could have by way of global user right or global group right),
+    # you can see system dashboards.
     push @objects, RT::System->new($CurrentUser)
         if $CurrentUser->HasRight(Object => $RT::System, Right => 'SeeDashboard');
 

commit d13be25f579c898303e5e027498bce07597ce79d
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jul 20 20:31:14 2011 -0400

    Ensure that we do not return disabled groups

diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index e4b916f..6a5b510 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -348,7 +348,10 @@ sub ForWhichCurrentUserHasRight {
         @_,
     );
 
-    # Groups which are the target object of an ACL with that right, or
+    # Non-disabled groups...
+    $self->LimitToEnabled;
+
+    # ...which are the target object of an ACL with that right, or
     # where the target is the system object (a global right)
     my $acl = $self->_JoinACL( %args );
     $self->_AddSubClause(

commit 469aff87d02d1fa6115c55cf93f5350274e3be96
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jul 20 20:33:41 2011 -0400

    Tests for superuser rights, and group membership interactions

diff --git a/t/web/dashboards-groups.t b/t/web/dashboards-groups.t
index 0c16798..6726d17 100644
--- a/t/web/dashboards-groups.t
+++ b/t/web/dashboards-groups.t
@@ -1,7 +1,7 @@
 #!/usr/bin/perl -w
 use strict;
 
-use RT::Test nodata => 1, tests => 48;
+use RT::Test nodata => 1, tests => 64;
 my ($baseurl, $m) = RT::Test->started_ok;
 
 my $url = $m->rt_base_url;
@@ -109,26 +109,87 @@ my ($group) = grep {$_->isa("RT::Group") and $_->Id == $inner_group->Id}
     RT::Dashboard->new($currentuser)->_PrivacyObjects;
 ok($group, "Found the group in  the privacy objects list");
 
+my @loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id, "RT::Group-".$inner_group->Id],
+    "We can load from ourselves (SeeOwnDashboard) and a group we are with SeeGroupDashboard"
+);
 
-($group) = grep {$_->isa("RT::Group") and $_->Id == $inner_group->Id}
-    RT::Dashboard->new($currentuser)->ObjectsForLoading;
-ok($group, "Found the group in the objects for loading");
-
-
-# With superuser, the dashboards of groups we're not in should not show
-# up in the menu, but should in the dashboard list.
+# If you are granted SeeGroupDashboard globally, you can only see
+# dashboards in groups you are in.
 $user_obj->PrincipalObj->RevokeRight(
     Right  => 'SeeGroupDashboard',
     Object => $inner_group,
 );
 $user_obj->PrincipalObj->GrantRight(
-    Right  => 'SuperUser',
+    Right  => 'SeeGroupDashboard',
     Object => RT->System,
 );
+$m->get_ok("/Dashboards/index.html");
+$m->content_contains("inner dashboard", "Having SeeGroupDashboard gobally is fine for groups you are in");
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id, "RT::Group-".$inner_group->Id],
+    "SeeGroupDashboard globally still works for groups you are in"
+);
+
 $inner_group->DeleteMember($user_obj->PrincipalObj->Id);
 ok(!$outer_group->HasMemberRecursively($user_obj->PrincipalId), "outer no longer has user recursively");
 ok(!$inner_group->HasMemberRecursively($user_obj->PrincipalId), "inner no longer has user recursively");
 $m->get_ok("/Dashboards/index.html");
-$m->content_contains("inner dashboard", "Superuser can see dashboards in their own groups");
+$m->content_lacks("inner dashboard", "But global SeeGroupDashboard isn't enough for other groups");
+$m->no_warnings_ok;
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id],
+    "We only have our SeeOwnDashboard right, as we are no longer in inner"
+);
+
+# Similarly, if you're a SuperUser, you still only see dashboards for
+# groups you belong to
+$user_obj->PrincipalObj->RevokeRight(
+    Right  => 'SeeGroupDashboard',
+    Object => RT->System,
+);
+$user_obj->PrincipalObj->GrantRight(
+    Right  => 'SuperUser',
+    Object => RT->System,
+);
+$m->get_ok("/Dashboards/index.html");
+$m->content_lacks("inner dashboard", "Superuser can't see dashboards in groups they're not in");
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id, "RT::System-1"],
+    "We pick up the system-level SeeDashboard right from superuser"
+);
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading(IncludeSuperusers => 0);
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id, "RT::System-1"],
+    "IncludeSuperusers only cuts out _group_ dashboard objects for loading, not user and system ones"
+);
+
+$inner_group->AddMember($user_obj->PrincipalId);
+$m->get_ok("/Dashboards/index.html");
+$m->content_contains("inner dashboard", "Superuser can see dashboards in groups they are in");
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id, "RT::Group-".$inner_group->Id, "RT::System-1"],
+    "Becoming a member of the group makes it a possibility"
+);
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading(IncludeSuperusers => 0);
+is_deeply(
+    \@loading,
+    ["RT::User-".$user_obj->Id, "RT::System-1"],
+    "But only via superuser"
+);
+
+$m->get_ok("/Dashboards/index.html");
+$m->content_contains("inner dashboard", "The dashboards list includes superuser rights");
 $m->get_ok("/index.html");
-$m->content_lacks("inner dashboard", "Also in the menu");
+$m->content_lacks("inner dashboard", "But the menu skips them");

commit c5a3dd2406ae4cab3e300dd4af2ee104a176662d
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jul 20 21:05:18 2011 -0400

    Rename IncludeSuperusers in ObjectsForLoading to reflect that it only acts on group rights

diff --git a/lib/RT/Dashboard.pm b/lib/RT/Dashboard.pm
index 0822b0b..135babb 100644
--- a/lib/RT/Dashboard.pm
+++ b/lib/RT/Dashboard.pm
@@ -380,7 +380,7 @@ sub Subscription {
 sub ObjectsForLoading {
     my $self = shift;
     my %args = (
-        IncludeSuperusers => 1,
+        IncludeSuperuserGroups => 1,
         @_
     );
     my @objects;
@@ -399,7 +399,7 @@ sub ObjectsForLoading {
     $groups->LimitToUserDefinedGroups;
     $groups->ForWhichCurrentUserHasRight(
         Right             => 'SeeGroupDashboard',
-        %args,
+        IncludeSuperusers => $args{IncludeSuperuserGroups},
     );
     $groups->WithMember(
         Recursively => 1,
diff --git a/share/html/Dashboards/Elements/ListOfDashboards b/share/html/Dashboards/Elements/ListOfDashboards
index 3927e7a..49a48ad 100644
--- a/share/html/Dashboards/Elements/ListOfDashboards
+++ b/share/html/Dashboards/Elements/ListOfDashboards
@@ -50,7 +50,7 @@
 use RT::Dashboard;
 
 my @objs = RT::Dashboard->new($session{CurrentUser})->ObjectsForLoading(
-               IncludeSuperusers => $IncludeSuperusers
+               IncludeSuperuserGroups => $IncludeSuperuserGroups
            );
 
 my %dashboard_map;
@@ -78,5 +78,5 @@ $m->callback(%ARGS, dashboards => \@dashboards, CallbackName => 'ModifyDashboard
 return @dashboards;
 </%init>
 <%args>
-$IncludeSuperusers => 1
+$IncludeSuperuserGroups => 1
 </%args>
diff --git a/share/html/Elements/Tabs b/share/html/Elements/Tabs
index 5db9bc3..2ad66bd 100755
--- a/share/html/Elements/Tabs
+++ b/share/html/Elements/Tabs
@@ -65,7 +65,7 @@ my $build_main_nav = sub {
     # groups you're not in (but can see the dashboards of by dint of
     # being a superuser) would push the useful ones from the groups
     # you're actually in off of the stack.
-    my @dashboards = $m->comp("/Dashboards/Elements/ListOfDashboards", IncludeSuperusers => 0);
+    my @dashboards = $m->comp("/Dashboards/Elements/ListOfDashboards", IncludeSuperuserGroups => 0);
     my $limit      = 7;
 
     my $more = 0;
diff --git a/t/web/dashboards-groups.t b/t/web/dashboards-groups.t
index 6726d17..ac2a5ac 100644
--- a/t/web/dashboards-groups.t
+++ b/t/web/dashboards-groups.t
@@ -166,7 +166,7 @@ is_deeply(
     ["RT::User-".$user_obj->Id, "RT::System-1"],
     "We pick up the system-level SeeDashboard right from superuser"
 );
- at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading(IncludeSuperusers => 0);
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading(IncludeSuperuserGroups => 0);
 is_deeply(
     \@loading,
     ["RT::User-".$user_obj->Id, "RT::System-1"],
@@ -182,7 +182,7 @@ is_deeply(
     ["RT::User-".$user_obj->Id, "RT::Group-".$inner_group->Id, "RT::System-1"],
     "Becoming a member of the group makes it a possibility"
 );
- at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading(IncludeSuperusers => 0);
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading(IncludeSuperuserGroups => 0);
 is_deeply(
     \@loading,
     ["RT::User-".$user_obj->Id, "RT::System-1"],

commit d258f535e789a62590858640df387af331da54bd
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jul 20 21:06:03 2011 -0400

    Make the dashboard portlet not include groups visible because of superuser privs, like the menu

diff --git a/share/html/Dashboards/Elements/ShowDashboards b/share/html/Dashboards/Elements/ShowDashboards
index ec32426..e0b8626 100644
--- a/share/html/Dashboards/Elements/ShowDashboards
+++ b/share/html/Dashboards/Elements/ShowDashboards
@@ -64,5 +64,8 @@
 <%init>
 use RT::Dashboards;
 
-my @Objects = RT::Dashboard->new($session{CurrentUser})->ObjectsForLoading;
+my @Objects = RT::Dashboard->new($session{CurrentUser})->ObjectsForLoading(IncludeSuperuserGroups => $IncludeSuperuserGroups);
 </%init>
+<%args>
+$IncludeSuperuserGroups => 1
+</%args>
diff --git a/share/html/Elements/Dashboards b/share/html/Elements/Dashboards
index cccae74..77eccde 100644
--- a/share/html/Elements/Dashboards
+++ b/share/html/Elements/Dashboards
@@ -52,6 +52,6 @@
     titleright_href => RT->Config->Get('WebPath').'/Dashboards/index.html',
 &>
 
-<& /Dashboards/Elements/ShowDashboards &>
+<& /Dashboards/Elements/ShowDashboards, IncludeSuperuserGroups => 0 &>
 
 </&>

commit f75bbeab6f3ab55e544edc5abda078d421b58fc0
Merge: aa660db d258f53
Author: Kevin Falcone <falcone at bestpractical.com>
Date:   Thu Aug 4 12:36:16 2011 -0400

    Merge branch '4.0/group-dashboards' into 4.0-trunk


commit 88fe4cda74fef7a79333ec0f843d9e781fe7bac4
Merge: f75bbea 1fa7623
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Aug 4 19:43:23 2011 -0400

    Merge branch '3.8-trunk' into 4.0-trunk
    
    Conflicts:
    	lib/RT/I18N.pm
    	lib/RT/Ticket_Overlay.pm
    	lib/RT/Transaction_Overlay.pm
    	share/html/Search/Bulk.html
    	share/html/Ticket/Elements/PreviewScrips
    	t/mail/sendmail.t
    	t/mail/wrong_mime_charset.t

diff --cc lib/RT/I18N.pm
index a1f2af5,4c70922..b9c1c54
mode 100644,100755..100644
--- a/lib/RT/I18N.pm
+++ b/lib/RT/I18N.pm
@@@ -241,42 -243,17 +236,16 @@@ sub SetMIMEEntityToEncoding 
  
      if ( $enc ne $charset && $body ) {
          my $string = $body->as_string or return;
+ 
 -        # {{{ Convert the body
 -        $RT::Logger->debug( "Converting '$charset' to '$enc' for " . $head->mime_type . " - " . ( $head->get('subject') || 'Subjectless message' ) );
++        $RT::Logger->debug( "Converting '$charset' to '$enc' for "
++              . $head->mime_type . " - "
++              . ( $head->get('subject') || 'Subjectless message' ) );
+ 
          # NOTE:: see the comments at the end of the sub.
 -        Encode::_utf8_off( $string);
 +        Encode::_utf8_off($string);
-         my $orig_string = $string;
- 
-         # Convert the body
-         eval {
-             $RT::Logger->debug( "Converting '$charset' to '$enc' for "
-                   . $head->mime_type . " - "
-                   . ( $head->get('subject') || 'Subjectless message' ) );
-             Encode::from_to( $string, $charset => $enc, Encode::FB_CROAK );
-         };
- 
-         if ($@) {
-             $RT::Logger->error( "Encoding error: " 
-                   . $@
-                   . " falling back to iso-8859-1 => $enc" );
-             $string = $orig_string;
-             eval {
-                 Encode::from_to(
-                     $string,
-                     'iso-8859-1' => $enc,
-                     Encode::FB_CROAK
-                 );
-             };
-             if ($@) {
-                 $RT::Logger->error( "Encoding error: " 
-                       . $@
-                       . " forcing conversion to $charset => $enc" );
-                 $string = $orig_string;
-                 Encode::from_to( $string, $charset => $enc );
-             }
-         }
- 
-         # }}}
+         Encode::from_to( $string, $charset => $enc );
  
 -        # }}}
 -
 -        my $new_body = MIME::Body::InCore->new( $string);
 +        my $new_body = MIME::Body::InCore->new($string);
  
          # set up the new entity
          $head->mime_attr( "content-type" => 'text/plain' )
@@@ -384,37 -357,6 +347,27 @@@ sub DecodeMIMEWordsToEncoding 
  
  	$str .= $prefix . $enc_str . $trailing;
      }
 +    }
 +
 +# handle filename*=ISO-8859-1''%74%E9%73%74%2E%74%78%74, see also rfc 2231
 +    @list = $str =~ m/(.*?\*=)([^']*?)'([^']*?)'(\S+)(.*?)(?=(?:\*=|$))/gcs;
 +    if (@list) {
 +        $str = '';
 +        while (@list) {
 +            my ( $prefix, $charset, $language, $enc_str, $trailing ) =
 +              splice @list, 0, 5;
 +            $prefix =~ s/\*=$/=/; # remove the *
 +            $enc_str =~ s/%(\w{2})/chr hex $1/eg;
 +            unless ( $charset eq $to_charset ) {
-                 my $orig_str = $enc_str;
-                 local $@;
-                 eval {
-                     Encode::from_to( $enc_str, $charset, $to_charset,
-                         Encode::FB_CROAK );
-                 };
-                 if ($@) {
-                     $enc_str = $orig_str;
-                     $charset = _GuessCharset($enc_str);
-                     Encode::from_to( $enc_str, $charset, $to_charset );
-                 }
++                Encode::from_to( $enc_str, $charset, $to_charset );
 +            }
 +            $enc_str = qq{"$enc_str"}
 +              if $enc_str =~ /[,;]/
 +              and $enc_str !~ /^".*"$/
 +              and (!$field || $field =~ /^(?:To$|From$|B?Cc$|Content-)/i);
 +            $str .= $prefix . $enc_str . $trailing;
 +        }
 +     }
  
      # We might have \n without trailing whitespace, which will result in
      # invalid headers.
@@@ -582,32 -478,10 +535,9 @@@ sub SetMIMEHeadToEncoding 
          my @values = $head->get_all($tag);
          $head->delete($tag);
          foreach my $value (@values) {
-             Encode::_utf8_off($value);
-             my $orig_value = $value;
              if ( $charset ne $enc ) {
-                 eval {
-                     Encode::from_to( $value, $charset => $enc, Encode::FB_CROAK );
-                 };
-                 if ($@) {
-                     $RT::Logger->error( "Encoding error: " 
-                           . $@
-                           . " falling back to iso-8859-1 => $enc" );
-                     $value = $orig_value;
-                     eval {
-                         Encode::from_to(
-                             $value,
-                             'iso-8859-1' => $enc,
-                             Encode::FB_CROAK
-                         );
-                     };
-                     if ($@) {
-                         $RT::Logger->error( "Encoding error: " 
-                               . $@
-                               . " forcing conversion to $charset => $enc" );
-                         $value = $orig_value;
-                         Encode::from_to( $value, $charset => $enc );
-                     }
-                 }
 -
+                 Encode::_utf8_off($value);
+                 Encode::from_to( $value, $charset => $enc );
              }
              $value = DecodeMIMEWordsToEncoding( $value, $enc, $tag )
                  unless $preserve_words;
diff --cc t/mail/sendmail.t
index 05a59bd,1f97bbb..bb5d2db
--- a/t/mail/sendmail.t
+++ b/t/mail/sendmail.t
@@@ -3,8 -3,7 +3,7 @@@
  use strict;
  use File::Spec ();
  
- use RT::Test tests => 139;
- use Test::Warn;
 -use RT::Test tests => 137;
++use RT::Test tests => 141;
  
  use RT::EmailParser;
  use RT::Tickets;
@@@ -315,18 -323,8 +314,33 @@@ $parser->ParseMIMEEntityFromScalar($con
  &text_html_redef_sendmessage;
  
   %args =        (message => $content, queue => 1, action => 'correspond');
 - RT::Interface::Email::Gateway(\%args);
 - $tickets = RT::Tickets->new($RT::SystemUser);
 +
- warnings_like {
-  RT::Interface::Email::Gateway(\%args);
- }
- [
-     qr/Encoding error: "\\x\{041f\}" does not map to iso-8859-1 .*/,
-     qr/Encoding error: "\\x\{041f\}" does not map to iso-8859-1 .*/
-     ],
- "The badly formed Russian spam we have isn't actually well-formed UTF8, which makes Encode (correctly) warn";
++{
++
++my @warnings;
++local $SIG{__WARN__} = sub {
++    push @warnings, "@_";
++};
++
++RT::Interface::Email::Gateway(\%args);
 +
++TODO: {
++        local $TODO =
++'need a better approach of encoding converter, should be fixed in 4.2';
++ok( @warnings == 1 || @warnings == 2, "1 or 2 warnings are ok" );
++ok( @warnings == 1 || ( @warnings == 2 && $warnings[1] eq $warnings[0] ),
++    'if there are 2 warnings, they should be same' );
++
++like(
++    $warnings[0],
++    qr/\QEncoding error: "\x{041f}" does not map to iso-8859-1/,
++"The badly formed Russian spam we have isn't actually well-formed UTF8, which makes Encode (correctly) warn",
++);
++
++}
++}
 +
 + $tickets = RT::Tickets->new(RT->SystemUser);
  $tickets->OrderBy(FIELD => 'id', ORDER => 'DESC');
  $tickets->Limit(FIELD => 'id' ,OPERATOR => '>', VALUE => '0');
   $tick = $tickets->First();
diff --cc t/mail/wrong_mime_charset.t
index 7636de4,71a574f..511a7e6
--- a/t/mail/wrong_mime_charset.t
+++ b/t/mail/wrong_mime_charset.t
@@@ -16,29 -16,12 +16,31 @@@ my $mime           = MIME::Entity->buil
  # set the wrong charset mime in purpose
  $mime->head->mime_attr( "Content-Type.charset" => 'utf8' );
  
 +my @warnings;
 +local $SIG{__WARN__} = sub {
 +    push @warnings, "@_";
 +};
 +
  RT::I18N::SetMIMEEntityToEncoding( $mime, 'iso-8859-1' );
  
+ TODO: {
+         local $TODO =
+ 'need a better approach of encoding converter, should be fixed in 4.2';
+ 
 +# this is a weird behavior for different perl versions, 5.12 warns twice,
 +# which is correct since we do the encoding thing twice, for Subject
 +# and Data respectively.
 +# but 5.8 and 5.10 warns only once.
 +ok( @warnings == 1 || @warnings == 2, "1 or 2 warnings are ok" );
- ok(
-     @warnings == 1 || $warnings[1] eq $warnings[0],
-     'if there are 2 warnings, they should be same'
- );
++ok( @warnings == 1 || ( @warnings == 2 && $warnings[1] eq $warnings[0] ),
++    'if there are 2 warnings, they should be same' );
 +
 +like(
 +    $warnings[0],
 +    qr/\QEncoding error: "\x{fffd}" does not map to iso-8859-1/,
 +"We can't encode something into the wrong encoding without Encode complaining"
 +);
 +
  my $subject = decode( 'iso-8859-1', $mime->head->get('Subject') );
  chomp $subject;
  is( $subject, $test_string, 'subject is set to iso-8859-1' );

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


More information about the Rt-commit mailing list