summaryrefslogtreecommitdiffstats
path: root/Bugzilla
diff options
context:
space:
mode:
authormkanat%bugzilla.org <>2008-08-08 08:26:33 +0200
committermkanat%bugzilla.org <>2008-08-08 08:26:33 +0200
commit50dbcc4e7a38642856cbeeef88d2b3a4a884b5e1 (patch)
tree424d04a868fb5fdffbfa2dd65cc8903ece84895f /Bugzilla
parentaca14df0a0daca1f2eb637d400285e3f14add35e (diff)
downloadbugzilla-50dbcc4e7a38642856cbeeef88d2b3a4a884b5e1.tar.gz
bugzilla-50dbcc4e7a38642856cbeeef88d2b3a4a884b5e1.tar.xz
Bug 442031: Make Bugzilla::User::groups return an arrayref of Bugzilla::Group objects (instead of a hashref of group ids and names).
Diffstat (limited to 'Bugzilla')
-rw-r--r--Bugzilla/BugMail.pm24
-rw-r--r--Bugzilla/Chart.pm3
-rw-r--r--Bugzilla/Search.pm5
-rw-r--r--Bugzilla/User.pm97
4 files changed, 60 insertions, 69 deletions
diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm
index dc48e83de..20bb7e254 100644
--- a/Bugzilla/BugMail.pm
+++ b/Bugzilla/BugMail.pm
@@ -489,9 +489,7 @@ sub Send {
# If we are using insiders, and the comment is private, only send
# to insiders
my $insider_ok = 1;
- $insider_ok = 0 if (Bugzilla->params->{"insidergroup"} &&
- ($anyprivate != 0) &&
- (!$user->groups->{Bugzilla->params->{"insidergroup"}}));
+ $insider_ok = 0 if $anyprivate && !$user->is_insider;
# We shouldn't send mail if this is a dependency mail (i.e. there
# is something in @depbugs), and any of the depending bugs are not
@@ -562,9 +560,9 @@ sub sendMail {
next;
}
# Only send estimated_time if it is enabled and the user is in the group
- if (($f ne 'estimated_time' && $f ne 'deadline') ||
- $user->groups->{Bugzilla->params->{'timetrackinggroup'}}) {
-
+ if (($f ne 'estimated_time' && $f ne 'deadline')
+ || $user->is_timetracker)
+ {
my $desc = $fielddescription{$f};
$head .= multiline_sprintf(FORMAT_DOUBLE, ["$desc:", $value],
FORMAT_2_SIZE);
@@ -584,14 +582,12 @@ sub sendMail {
($diff->{'fieldname'} eq 'estimated_time' ||
$diff->{'fieldname'} eq 'remaining_time' ||
$diff->{'fieldname'} eq 'work_time' ||
- $diff->{'fieldname'} eq 'deadline')){
- if ($user->groups->{Bugzilla->params->{"timetrackinggroup"}}) {
- $add_diff = 1;
- }
- } elsif (($diff->{'isprivate'})
- && Bugzilla->params->{'insidergroup'}
- && !($user->groups->{Bugzilla->params->{'insidergroup'}})
- ) {
+ $diff->{'fieldname'} eq 'deadline'))
+ {
+ $add_diff = 1 if $user->is_timetracker;
+ } elsif ($diff->{'isprivate'}
+ && !$user->is_insider)
+ {
$add_diff = 0;
} else {
$add_diff = 1;
diff --git a/Bugzilla/Chart.pm b/Bugzilla/Chart.pm
index a119e4b7c..1f232f310 100644
--- a/Bugzilla/Chart.pm
+++ b/Bugzilla/Chart.pm
@@ -382,8 +382,7 @@ sub getSeriesIDs {
sub getVisibleSeries {
my %cats;
- # List of groups the user is in; use -1 to make sure it's not empty.
- my $grouplist = join(", ", (-1, values(%{Bugzilla->user->groups})));
+ my $grouplist = Bugzilla->user->groups_as_string;
# Get all visible series
my $dbh = Bugzilla->dbh;
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 2f8d86c8e..77000ce31 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -775,8 +775,9 @@ sub init {
" ON bug_group_map.bug_id = bugs.bug_id ";
if ($user->id) {
- if (%{$user->groups}) {
- $query .= " AND bug_group_map.group_id NOT IN (" . join(',', values(%{$user->groups})) . ") ";
+ if (scalar @{ $user->groups }) {
+ $query .= " AND bug_group_map.group_id NOT IN ("
+ . $user->groups_as_string . ") ";
}
$query .= " LEFT JOIN cc ON cc.bug_id = bugs.bug_id AND cc.who = " . $user->id;
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 26febfcd9..9bd5b9e9f 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -359,75 +359,62 @@ sub groups {
my $self = shift;
return $self->{groups} if defined $self->{groups};
- return {} unless $self->id;
+ return [] unless $self->id;
my $dbh = Bugzilla->dbh;
- my $groups = $dbh->selectcol_arrayref(q{SELECT DISTINCT groups.name, group_id
- FROM groups, user_group_map
- WHERE groups.id=user_group_map.group_id
- AND user_id=?
- AND isbless=0},
- { Columns=>[1,2] },
- $self->id);
-
- # The above gives us an arrayref [name, id, name, id, ...]
- # Convert that into a hashref
- my %groups = @$groups;
- my @groupidstocheck = values(%groups);
- my %groupidschecked = ();
+ my $groups_to_check = $dbh->selectcol_arrayref(
+ q{SELECT DISTINCT group_id
+ FROM user_group_map
+ WHERE user_id = ? AND isbless = 0}, undef, $self->id);
+
my $rows = $dbh->selectall_arrayref(
- "SELECT DISTINCT groups.name, groups.id, member_id
- FROM group_group_map
- INNER JOIN groups
- ON groups.id = grantor_id
- WHERE grant_type = " . GROUP_MEMBERSHIP);
- my %group_names = ();
- my %group_membership = ();
+ "SELECT DISTINCT grantor_id, member_id
+ FROM group_group_map
+ WHERE grant_type = " . GROUP_MEMBERSHIP);
+
+ my %group_membership;
foreach my $row (@$rows) {
- my ($member_name, $grantor_id, $member_id) = @$row;
- # Just save the group names
- $group_names{$grantor_id} = $member_name;
-
- # And group membership
- push (@{$group_membership{$member_id}}, $grantor_id);
+ my ($grantor_id, $member_id) = @$row;
+ push (@{ $group_membership{$member_id} }, $grantor_id);
}
# Let's walk the groups hierarchy tree (using FIFO)
# On the first iteration it's pre-filled with direct groups
# membership. Later on, each group can add its own members into the
# FIFO. Circular dependencies are eliminated by checking
- # $groupidschecked{$member_id} hash values.
+ # $checked_groups{$member_id} hash values.
# As a result, %groups will have all the groups we are the member of.
- while ($#groupidstocheck >= 0) {
+ my %checked_groups;
+ my %groups;
+ while (scalar(@$groups_to_check) > 0) {
# Pop the head group from FIFO
- my $member_id = shift @groupidstocheck;
+ my $member_id = shift @$groups_to_check;
# Skip the group if we have already checked it
- if (!$groupidschecked{$member_id}) {
+ if (!$checked_groups{$member_id}) {
# Mark group as checked
- $groupidschecked{$member_id} = 1;
+ $checked_groups{$member_id} = 1;
# Add all its members to the FIFO check list
# %group_membership contains arrays of group members
# for all groups. Accessible by group number.
- foreach my $newgroupid (@{$group_membership{$member_id}}) {
- push @groupidstocheck, $newgroupid
- if (!$groupidschecked{$newgroupid});
- }
- # Note on if clause: we could have group in %groups from 1st
- # query and do not have it in second one
- $groups{$group_names{$member_id}} = $member_id
- if $group_names{$member_id} && $member_id;
+ my $members = $group_membership{$member_id};
+ my @new_to_check = grep(!$checked_groups{$_}, @$members);
+ push(@$groups_to_check, @new_to_check);
+
+ $groups{$member_id} = 1;
}
}
- $self->{groups} = \%groups;
+
+ $self->{groups} = Bugzilla::Group->new_from_list([keys %groups]);
return $self->{groups};
}
sub groups_as_string {
my $self = shift;
- return (join(',',values(%{$self->groups})) || '-1');
+ my @ids = map { $_->id } @{ $self->groups };
+ return scalar(@ids) ? join(',', @ids) : '-1';
}
sub bless_groups {
@@ -483,7 +470,7 @@ sub bless_groups {
sub in_group {
my ($self, $group, $product_id) = @_;
- if (exists $self->groups->{$group}) {
+ if (scalar grep($_->name eq $group, @{ $self->groups })) {
return 1;
}
elsif ($product_id && detaint_natural($product_id)) {
@@ -512,8 +499,7 @@ sub in_group {
sub in_group_id {
my ($self, $id) = @_;
- my %j = reverse(%{$self->groups});
- return exists $j{$id} ? 1 : 0;
+ return grep($_->id == $id, @{ $self->groups }) ? 1 : 0;
}
sub get_products_by_permission {
@@ -828,7 +814,7 @@ sub visible_groups_direct {
my $sth;
if (Bugzilla->params->{'usevisibilitygroups'}) {
- my $glist = join(',',(-1,values(%{$self->groups})));
+ my $glist = $self->groups_as_string;
$sth = $dbh->prepare("SELECT DISTINCT grantor_id
FROM group_group_map
WHERE member_id IN($glist)
@@ -873,7 +859,7 @@ sub queryshare_groups {
}
}
else {
- @queryshare_groups = values(%{$self->groups});
+ @queryshare_groups = map { $_->id } @{ $self->groups };
}
}
@@ -1531,6 +1517,17 @@ sub is_global_watcher {
return $self->{'is_global_watcher'};
}
+sub is_timetracker {
+ my $self = shift;
+
+ if (!defined $self->{'is_timetracker'}) {
+ my $tt_group = Bugzilla->params->{'timetrackinggroup'};
+ $self->{'is_timetracker'} =
+ ($tt_group && $self->in_group($tt_group)) ? 1 : 0;
+ }
+ return $self->{'is_timetracker'};
+}
+
sub get_userlist {
my $self = shift;
@@ -1871,10 +1868,8 @@ is_default - a boolean to indicate whether the user has chosen to make
=item C<groups>
-Returns a hashref of group names for groups the user is a member of. 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 is in can be
-obtained by C<values(%{$user-E<gt>groups})>.)
+Returns an arrayref of L<Bugzilla::Group> objects representing
+groups that this user is a member of.
=item C<groups_as_string>