Bug 279693: Move UserCanBlessGroup() into a Bugzilla::User function

Patch By Max Kanat-Alexander <mkanat@kerio.com> r=LpSolit, a=myk

diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 8809d80dd7fd050b315e830f1cdc517bad0e905a..4d4787525da9c841d4d373e2857317faf1f9b4b4 100644 (file)
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -247,6 +247,38 @@ sub groups {
     return $self->{groups};
 }
 
+sub bless_groups {
+    my $self = shift;
+
+    return $self->{bless_groups} if defined $self->{bless_groups};
+    return {} unless $self->id;
+
+    my $dbh = Bugzilla->dbh;
+    # Get all groups for the user where:
+    #    + They have direct bless privileges
+    #    + They are a member of a group that inherits bless privs.
+    # Because of the second requirement, derive_groups must be up-to-date
+    # for this to function properly in all circumstances.
+    my $bless_groups = $dbh->selectcol_arrayref(
+        q{SELECT DISTINCT groups.name, groups.id
+            FROM groups, user_group_map, group_group_map AS ggm
+           WHERE user_group_map.user_id = ?
+             AND ((user_group_map.isbless = 1
+                   AND groups.id=user_group_map.group_id)
+                  OR (groups.id = ggm.grantor_id
+                      AND ggm.grant_type = } . GROUP_BLESS .
+                   q{ AND user_group_map.group_id = ggm.member_id
+                      AND user_group_map.isbless = 0))},
+         { Columns=>[1,2] }, $self->{id});
+
+    # The above gives us an arrayref [name, id, name, id, ...]
+    # Convert that into a hashref
+    my %bless_groups_hashref = @$bless_groups;
+    $self->{bless_groups} = \%bless_groups_hashref;
+
+    return $self->{bless_groups};
+}
+
 sub in_group {
     my ($self, $group) = @_;
 
@@ -467,32 +499,15 @@ sub derive_groups {
 sub can_bless {
     my $self = shift;
 
-    return $self->{can_bless} if defined $self->{can_bless};
-    return 0 unless $self->id;
-
-    my $dbh = Bugzilla->dbh;
-    # First check if the user can explicitly bless a group
-    my $res = $dbh->selectrow_arrayref(q{SELECT 1
-                                           FROM user_group_map
-                                          WHERE user_id=?
-                                            AND isbless=1},
-                                       undef,
-                                       $self->{id});
-    if (!$res) {
-        # Now check if user is a member of a group that can bless a group
-        $res = $dbh->selectrow_arrayref(q{SELECT 1
-                                            FROM user_group_map, group_group_map
-                                           WHERE user_group_map.user_id=?
-                                             AND user_group_map.group_id=member_id
-                                             AND group_group_map.grant_type=} .
-                                                 GROUP_BLESS,
-                                        undef,
-                                        $self->{id});
+    if (!scalar(@_)) {
+        # If we're called without an argument, just return 
+        # whether or not we can bless at all.
+        return scalar(keys %{$self->bless_groups}) ? 1 : 0;
     }
 
-    $self->{can_bless} = $res ? 1 : 0;
-
-    return $self->{can_bless};
+    # Otherwise, we're checking a specific group
+    my $group_name = shift;
+    return exists($self->bless_groups->{$group_name});
 }
 
 sub flatten_group_membership {
@@ -1136,6 +1151,13 @@ intended for cases where we are not looking at the currently logged in user,
 and only need to make a quick check for the group, where calling C<groups>
 and getting all of the groups would be overkill.
 
+=item C<bless_groups>
+
+Returns a hashref of group names for groups that the user can bless. The keys
+are the names of the groups, whilst the values are the respective group ids.
+(This is so that a set of all groupids for groups the user can bless can be
+obtained by C<values(%{$user->bless_groups})>.)
+
 =item C<can_see_bug(bug_id)>
 
 Determines if the user can see the specified bug.
@@ -1198,7 +1220,12 @@ all MySQL supported, this will go away.
 
 =item C<can_bless>
 
-Returns C<1> if the user can bless at least one group. Otherwise returns C<0>.
+When called with no arguments:
+Returns C<1> if the user can bless at least one group, returns C<0> otherwise.
+
+When called with one argument:
+Returns C<1> if the user can bless the group with that name, returns
+C<0> otherwise.
 
 =item C<set_flags>
 =item C<get_flag>

diff --git a/editusers.cgi b/editusers.cgi
index c6b342efd429472cbd6834162635e520f02e4a6b..4c2a8a19357d2b1841ea5c70a4c99da0a4d9ad9d 100755 (executable)
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -153,7 +153,7 @@ sub EmitFormElements ($$$$)
             print "<TD COLSPAN=2 ALIGN=LEFT><B>User is a member of these groups</B></TD>\n";
             while (MoreSQLData()) {
                 my ($groupid, $name, $description, $checked, $isderived, $isregexp) = FetchSQLData();
-                next unless ($editall || UserCanBlessGroup($name));
+                next unless ($editall || Bugzilla->user->can_bless($name));
                 PushGlobalSQLState();
                 SendSQL("SELECT user_id " .
                         "FROM user_group_map " .
@@ -726,7 +726,7 @@ if ($action eq 'update') {
     my $chggrp = 0;
     SendSQL("SELECT id, name FROM groups");
     while (my ($groupid, $name) = FetchSQLData()) {
-        next unless ($editall || UserCanBlessGroup($name));
+        next unless ($editall || Bugzilla->user->can_bless($name));
         if ($::FORM{"oldgroup_$groupid"} != ($::FORM{"group_$groupid"} ? 1 : 0)) {
             # group membership changed
             PushGlobalSQLState();

diff --git a/globals.pl b/globals.pl
index 2d01cb5f1b0cf5741526a23e5c03df4af9d1b5ff..1bfbe2306957f14cef01deb0463f4f7bbcf470b7 100644 (file)
--- a/globals.pl
+++ b/globals.pl
@@ -1033,36 +1033,6 @@ sub UserInGroup {
     return defined Bugzilla->user->groups->{$_[0]};
 }
 
-sub UserCanBlessGroup {
-    my ($groupname) = (@_);
-    PushGlobalSQLState();
-    # check if user explicitly can bless group
-    SendSQL("SELECT groups.id FROM groups, user_group_map 
-        WHERE groups.id = user_group_map.group_id 
-        AND user_group_map.user_id = $::userid
-        AND isbless = 1
-        AND groups.name = " . SqlQuote($groupname));
-    my $result = FetchOneColumn();
-    PopGlobalSQLState();
-    if ($result) {
-        return 1;
-    }
-    PushGlobalSQLState();
-    # check if user is a member of a group that can bless this group
-    # this group does not count
-    SendSQL("SELECT groups.id FROM groups, user_group_map, 
-        group_group_map 
-        WHERE groups.id = grantor_id 
-        AND user_group_map.user_id = $::userid
-        AND user_group_map.isbless = 0
-        AND group_group_map.grant_type = " . GROUP_BLESS . "
-        AND user_group_map.group_id = member_id
-        AND groups.name = " . SqlQuote($groupname));
-    $result = FetchOneColumn();
-    PopGlobalSQLState();
-    return $result; 
-}
-
 sub BugInGroupId {
     my ($bugid, $groupid) = (@_);
     PushGlobalSQLState();

diff --git a/userprefs.cgi b/userprefs.cgi
index 974791d5232010fd121aeb35d4e2555e113b6044..342df065ba759532151049663713f6f168822684 100755 (executable)
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -282,7 +282,7 @@ sub DoPermissions {
             "ORDER BY name");
     while (MoreSQLData()) {
         my ($nam, $desc) = FetchSQLData();
-        if (UserCanBlessGroup($nam)) {
+        if (Bugzilla->user->can_bless($nam)) {
             push(@set_bits, {"desc" => $desc, "name" => $nam});
         }
     }