[Rt-commit] rt branch, 4.4/dashboard-group-count-acl-check, created. rt-4.4.4-252-g668fab5d5e

Jim Brandt jbrandt at bestpractical.com
Thu Feb 11 11:51:42 EST 2021


The branch, 4.4/dashboard-group-count-acl-check has been created
        at  668fab5d5e1f5dd409437324e5bba0a8d96ce6f9 (commit)

- Log -----------------------------------------------------------------
commit 49e75811028755f3a077fef491a2e0476a979541
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Thu Feb 11 11:24:58 2021 -0500

    Test create dashboard rights granted via a group
    
    Confirm rights to show the New Dashboard menu option as
    granted via a group. This test is to confirm behavior for
    the changes in the following commit.

diff --git a/t/web/dashboards-permissions.t b/t/web/dashboards-permissions.t
index 433fdd3731..79af39d9ae 100644
--- a/t/web/dashboards-permissions.t
+++ b/t/web/dashboards-permissions.t
@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 
-use RT::Test nodata => 1, tests => 8;
+use RT::Test nodata => 1, tests => undef;
 my ($baseurl, $m) = RT::Test->started_ok;
 
 my $url = $m->rt_base_url;
@@ -27,9 +27,35 @@ $user_obj->PrincipalObj->GrantRight(Right => $_, Object => $RT::System)
 
 ok $m->login(customer => 'customer'), "logged in";
 
-
 $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], "the only selectable privacy is user");
 $m->content_lacks('Delete', "Delete button hidden because we are creating");
 
+diag 'Test group dashboard create rights';
+
+my $user2 = RT::Test->load_or_create_user(
+    Name => 'user2', Password => 'password', Privileged => 1
+);
+ok $user2 && $user2->id, 'loaded or created user';
+
+my $group1 = RT::Test->load_or_create_group(
+    'Group1',
+    Members => [$user2],
+);
+
+ok $m->logout(), "Logged out";
+ok $m->login( 'user2' => 'password' ), "logged in";
+
+$m->content_contains('All Dashboards', 'All Dashboards menu item found' );
+$m->content_lacks('New Dashboard', 'New Dashboard menu correctly not found');
+
+$group1->PrincipalObj->GrantRight(Right => $_, Object => $group1)
+    for qw/SeeGroup CreateGroupDashboard/;
+
+$m->get_ok('/', 'Reloaded home page');
+
+$m->content_contains('All Dashboards', 'All Dashboards menu item found' );
+$m->content_contains('New Dashboard', 'New Dashboard link found via group rights');
+
+done_testing();

commit 668fab5d5e1f5dd409437324e5bba0a8d96ce6f9
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Thu Feb 11 11:30:57 2021 -0500

    Avoid expensive COUNT query on group dashboard rights
    
    The call to ->Count on the groups collection to check if the
    user has the right CreateGroupDashboard generates the query:
    
        SELECT COUNT(DISTINCT main.id) FROM Groups main
            CROSS JOIN ACL ACL_2 JOIN CachedGroupMembers...
    
    This is triggered when dashboard rights are granted at the
    group level and users do not have CreateOwnDashboard. It
    is called when building the menu, so on every page load.
    
    Since we are only checking to see if the user has the right
    via at least one group, just check for one value in the
    results rather than running the COUNT. This should
    generate a less expensive query.

diff --git a/lib/RT/Dashboard.pm b/lib/RT/Dashboard.pm
index 24764b7f27..bc4db6d617 100644
--- a/lib/RT/Dashboard.pm
+++ b/lib/RT/Dashboard.pm
@@ -427,7 +427,9 @@ sub CurrentUserCanCreateAny {
         Right             => 'CreateGroupDashboard',
         IncludeSuperusers => 1,
     );
-    return 1 if $groups->Count;
+
+    # If we got even one group back, that's enough
+    return 1 if $groups->First;
 
     return 1
         if $CurrentUser->HasRight(Object => $RT::System, Right => 'CreateDashboard');

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


More information about the rt-commit mailing list