Bug 354627: Improve the UI for adding/removing inheritance in editgroups.cgi

Patch By Max Kanat-Alexander <mkanat@bugzilla.org r=LpSolit, a=LpSolit


git-svn-id: svn://10.0.0.236/trunk@221368 18797224-902f-48f8-a5cc-f745e15eee43
This commit is contained in:
mkanat%bugzilla.org
2007-03-06 04:54:07 +00:00
parent e71fb4c48e
commit d118f53d94
7 changed files with 601 additions and 528 deletions

View File

@@ -57,34 +57,6 @@ $user->in_group('creategroups')
my $action = trim($cgi->param('action') || '');
my $token = $cgi->param('token');
# Add missing entries in bug_group_map for bugs created while
# a mandatory group was disabled and which is now enabled again.
sub fix_bug_permissions {
my $gid = shift;
my $dbh = Bugzilla->dbh;
detaint_natural($gid);
return unless $gid;
my $bug_ids =
$dbh->selectcol_arrayref('SELECT bugs.bug_id
FROM bugs
INNER JOIN group_control_map
ON group_control_map.product_id = bugs.product_id
LEFT JOIN bug_group_map
ON bug_group_map.bug_id = bugs.bug_id
AND bug_group_map.group_id = group_control_map.group_id
WHERE group_control_map.group_id = ?
AND group_control_map.membercontrol = ?
AND bug_group_map.group_id IS NULL',
undef, ($gid, CONTROLMAPMANDATORY));
my $sth = $dbh->prepare('INSERT INTO bug_group_map (bug_id, group_id) VALUES (?, ?)');
foreach my $bug_id (@$bug_ids) {
$sth->execute($bug_id, $gid);
}
}
# CheckGroupID checks that a positive integer is given and is
# actually a valid group ID. If all tests are successful, the
# trimmed group ID is returned.
@@ -148,6 +120,66 @@ sub CheckGroupRegexp {
return $regexp;
}
# A helper for displaying the edit.html.tmpl template.
sub get_current_and_available {
my ($group, $vars) = @_;
my @all_groups = Bugzilla::Group->get_all;
my @members_current = @{$group->grant_direct(GROUP_MEMBERSHIP)};
my @member_of_current = @{$group->granted_by_direct(GROUP_MEMBERSHIP)};
my @bless_from_current = @{$group->grant_direct(GROUP_BLESS)};
my @bless_to_current = @{$group->granted_by_direct(GROUP_BLESS)};
my (@visible_from_current, @visible_to_me_current);
if (Bugzilla->params->{'usevisibilitygroups'}) {
@visible_from_current = @{$group->grant_direct(GROUP_VISIBLE)};
@visible_to_me_current = @{$group->granted_by_direct(GROUP_VISIBLE)};
}
# Figure out what groups are not currently a member of this group,
# and what groups this group is not currently a member of.
my (@members_available, @member_of_available,
@bless_from_available, @bless_to_available,
@visible_from_available, @visible_to_me_available);
foreach my $group_option (@all_groups) {
if (Bugzilla->params->{'usevisibilitygroups'}) {
push(@visible_from_available, $group_option)
if !grep($_->id == $group_option->id, @visible_from_current);
push(@visible_to_me_available, $group_option)
if !grep($_->id == $group_option->id, @visible_to_me_current);
}
# The group itself should never show up in the bless or
# membership lists.
next if $group_option->id == $group->id;
push(@members_available, $group_option)
if !grep($_->id == $group_option->id, @members_current);
push(@member_of_available, $group_option)
if !grep($_->id == $group_option->id, @member_of_current);
push(@bless_from_available, $group_option)
if !grep($_->id == $group_option->id, @bless_from_current);
push(@bless_to_available, $group_option)
if !grep($_->id == $group_option->id, @bless_to_current);
}
$vars->{'members_current'} = \@members_current;
$vars->{'members_available'} = \@members_available;
$vars->{'member_of_current'} = \@member_of_current;
$vars->{'member_of_available'} = \@member_of_available;
$vars->{'bless_from_current'} = \@bless_from_current;
$vars->{'bless_from_available'} = \@bless_from_available;
$vars->{'bless_to_current'} = \@bless_to_current;
$vars->{'bless_to_available'} = \@bless_to_available;
if (Bugzilla->params->{'usevisibilitygroups'}) {
$vars->{'visible_from_current'} = \@visible_from_current;
$vars->{'visible_from_available'} = \@visible_from_available;
$vars->{'visible_to_me_current'} = \@visible_to_me_current;
$vars->{'visible_to_me_available'} = \@visible_to_me_available;
}
}
# If no action is specified, get a list of all groups available.
unless ($action) {
@@ -169,62 +201,10 @@ unless ($action) {
if ($action eq 'changeform') {
# Check that an existing group ID is given
my $group_id = CheckGroupID($cgi->param('group'));
my ($name, $description, $regexp, $isactive, $isbuggroup) =
$dbh->selectrow_array("SELECT name, description, userregexp, " .
"isactive, isbuggroup " .
"FROM groups WHERE id = ?", undef, $group_id);
my $group = new Bugzilla::Group($group_id);
# For each group, we use left joins to establish the existence of
# a record making that group a member of this group
# and the existence of a record permitting that group to bless
# this one
my @groups;
my $group_list =
$dbh->selectall_arrayref('SELECT groups.id, groups.name, groups.description,
CASE WHEN group_group_map.member_id IS NOT NULL
THEN 1 ELSE 0 END,
CASE WHEN B.member_id IS NOT NULL
THEN 1 ELSE 0 END,
CASE WHEN C.member_id IS NOT NULL
THEN 1 ELSE 0 END
FROM groups
LEFT JOIN group_group_map
ON group_group_map.member_id = groups.id
AND group_group_map.grantor_id = ?
AND group_group_map.grant_type = ?
LEFT JOIN group_group_map as B
ON B.member_id = groups.id
AND B.grantor_id = ?
AND B.grant_type = ?
LEFT JOIN group_group_map as C
ON C.member_id = groups.id
AND C.grantor_id = ?
AND C.grant_type = ?
ORDER by name',
undef, ($group_id, GROUP_MEMBERSHIP,
$group_id, GROUP_BLESS,
$group_id, GROUP_VISIBLE));
foreach (@$group_list) {
my ($grpid, $grpnam, $grpdesc, $grpmember, $blessmember, $membercansee) = @$_;
my $group = {};
$group->{'grpid'} = $grpid;
$group->{'grpnam'} = $grpnam;
$group->{'grpdesc'} = $grpdesc;
$group->{'grpmember'} = $grpmember;
$group->{'blessmember'} = $blessmember;
$group->{'membercansee'}= $membercansee;
push(@groups, $group);
}
$vars->{'group_id'} = $group_id;
$vars->{'name'} = $name;
$vars->{'description'} = $description;
$vars->{'regexp'} = $regexp;
$vars->{'isactive'} = $isactive;
$vars->{'isbuggroup'} = $isbuggroup;
$vars->{'groups'} = \@groups;
get_current_and_available($group, $vars);
$vars->{'group'} = $group;
$vars->{'token'} = issue_session_token('edit_group');
print $cgi->header();
@@ -481,82 +461,61 @@ if ($action eq 'delete') {
if ($action eq 'postchanges') {
check_token_data($token, 'edit_group');
# ZLL: Bug 181589: we need to have something to remove explicitly listed users from
# groups in order for the conversion to 2.18 groups to work
my $action;
if ($cgi->param('remove_explicit_members')) {
$action = 1;
} elsif ($cgi->param('remove_explicit_members_regexp')) {
$action = 2;
} else {
$action = 3;
}
my ($gid, $chgs, $name, $regexp) = doGroupChanges();
$vars->{'action'} = $action;
$vars->{'changes'} = $chgs;
$vars->{'gid'} = $gid;
$vars->{'name'} = $name;
if ($action == 2) {
$vars->{'regexp'} = $regexp;
}
my $changes = doGroupChanges();
delete_token($token);
my $group = new Bugzilla::Group($cgi->param('group_id'));
get_current_and_available($group, $vars);
$vars->{'message'} = 'group_updated';
$vars->{'group'} = $group;
$vars->{'changes'} = $changes;
$vars->{'token'} = issue_session_token('edit_group');
print $cgi->header();
$template->process("admin/groups/change.html.tmpl", $vars)
$template->process("admin/groups/edit.html.tmpl", $vars)
|| ThrowTemplateError($template->error());
exit;
}
if (($action eq 'remove_all_regexp') || ($action eq 'remove_all')) {
if ($action eq 'confirm_remove') {
my $group = new Bugzilla::Group(CheckGroupID($cgi->param('group_id')));
$vars->{'group'} = $group;
$vars->{'regexp'} = CheckGroupRegexp($cgi->param('regexp'));
$vars->{'token'} = issue_session_token('remove_group_members');
$template->process('admin/groups/confirm-remove.html.tmpl', $vars)
|| ThrowTemplateError($template->error());
exit;
}
if ($action eq 'remove_regexp') {
check_token_data($token, 'remove_group_members');
# remove all explicit users from the group with
# gid = $cgi->param('group') that match the regular expression
# stored in the DB for that group or all of them period
my $gid = CheckGroupID($cgi->param('group'));
my ($name, $regexp) =
$dbh->selectrow_array('SELECT name, userregexp FROM groups
WHERE id = ?', undef, $gid);
my $group = new Bugzilla::Group(CheckGroupID($cgi->param('group_id')));
my $regexp = CheckGroupRegexp($cgi->param('regexp'));
$dbh->bz_lock_tables('groups WRITE', 'profiles READ',
'user_group_map WRITE');
my $sth = $dbh->prepare("SELECT user_group_map.user_id, profiles.login_name
FROM user_group_map
INNER JOIN profiles
ON user_group_map.user_id = profiles.userid
WHERE user_group_map.group_id = ?
AND grant_type = ?
AND isbless = 0");
$sth->execute($gid, GRANT_DIRECT);
my $users = $group->members_direct();
my $sth_delete = $dbh->prepare(
"DELETE FROM user_group_map
WHERE user_id = ? AND isbless = 0 AND group_id = ?");
my @users;
my $sth2 = $dbh->prepare("DELETE FROM user_group_map
WHERE user_id = ?
AND isbless = 0
AND group_id = ?");
while ( my ($userid, $userlogin) = $sth->fetchrow_array() ) {
if ((($regexp =~ /\S/) && ($userlogin =~ m/$regexp/i))
|| ($action eq 'remove_all'))
{
$sth2->execute($userid, $gid);
my $user = {};
$user->{'login'} = $userlogin;
push(@users, $user);
my @deleted;
foreach my $member (@$users) {
if ($regexp eq '' || $member->login =~ m/$regexp/i) {
$sth_delete->execute($member->id, $group->id);
push(@deleted, $member);
}
}
$dbh->bz_unlock_tables();
$vars->{'users'} = \@users;
$vars->{'name'} = $name;
$vars->{'regexp'} = $regexp;
$vars->{'remove_all'} = ($action eq 'remove_all');
$vars->{'gid'} = $gid;
$vars->{'users'} = \@deleted;
$vars->{'regexp'} = $regexp;
delete_token($token);
print $cgi->header();
$template->process("admin/groups/remove.html.tmpl", $vars)
@@ -586,116 +545,100 @@ sub doGroupChanges {
'priority READ', 'bug_severity READ', 'rep_platform READ',
'op_sys READ');
# Check that the given group ID and regular expression are valid.
# If tests are successful, trimmed values are returned by CheckGroup*.
my $gid = CheckGroupID($cgi->param('group'));
my $regexp = CheckGroupRegexp($cgi->param('regexp'));
# Check that the given group ID is valid and make a Group.
my $group = new Bugzilla::Group(CheckGroupID($cgi->param('group_id')));
# The name and the description of system groups cannot be edited.
# We then need to know if the group being edited is a system group.
my $isbuggroup = $dbh->selectrow_array('SELECT isbuggroup FROM groups
WHERE id = ?', undef, $gid);
my $name;
my $desc;
my $isactive;
my $chgs = 0;
# We trust old values given by the template. If they are hacked
# in a way that some of the tests below become negative, the
# corresponding attributes are not updated in the DB, which does
# not hurt.
if ($isbuggroup) {
# Check that the group name and its description are valid
# and return trimmed values if tests are successful.
$name = CheckGroupName($cgi->param('name'), $gid);
$desc = CheckGroupDesc($cgi->param('desc'));
$isactive = $cgi->param('isactive') ? 1 : 0;
if ($name ne $cgi->param('oldname')) {
$chgs = 1;
$dbh->do('UPDATE groups SET name = ? WHERE id = ?',
undef, ($name, $gid));
# If the group is used by some parameters, we have to update
# these parameters too.
my $update_params = 0;
foreach my $group (SPECIAL_GROUPS) {
if ($cgi->param('oldname') eq Bugzilla->params->{$group}) {
SetParam($group, $name);
$update_params = 1;
}
}
write_params() if $update_params;
}
if ($desc ne $cgi->param('olddesc')) {
$chgs = 1;
$dbh->do('UPDATE groups SET description = ? WHERE id = ?',
undef, ($desc, $gid));
}
if ($isactive ne $cgi->param('oldisactive')) {
$chgs = 1;
$dbh->do('UPDATE groups SET isactive = ? WHERE id = ?',
undef, ($isactive, $gid));
# If the group was mandatory for some products before
# we deactivated it and we now activate this group again,
# we have to add all bugs created while this group was
# disabled in bug_group_map to correctly protect them.
if ($isactive) { fix_bug_permissions($gid); }
}
}
if ($regexp ne $cgi->param('oldregexp')) {
$chgs = 1;
$dbh->do('UPDATE groups SET userregexp = ? WHERE id = ?',
undef, ($regexp, $gid));
Bugzilla::Group::RederiveRegexp($regexp, $gid);
if (defined $cgi->param('regexp')) {
$group->set_user_regexp($cgi->param('regexp'));
}
my $sthInsert = $dbh->prepare('INSERT INTO group_group_map
(member_id, grantor_id, grant_type)
VALUES (?, ?, ?)');
my $sthDelete = $dbh->prepare('DELETE FROM group_group_map
WHERE member_id = ?
AND grantor_id = ?
AND grant_type = ?');
foreach my $b (grep {/^oldgrp-\d*$/} $cgi->param()) {
if (defined($cgi->param($b))) {
$b =~ /^oldgrp-(\d+)$/;
my $v = $1;
my $grp = $cgi->param("grp-$v") || 0;
if (($v != $gid) && ($cgi->param("oldgrp-$v") != $grp)) {
$chgs = 1;
if ($grp != 0) {
$sthInsert->execute($v, $gid, GROUP_MEMBERSHIP);
} else {
$sthDelete->execute($v, $gid, GROUP_MEMBERSHIP);
}
}
my $bless = $cgi->param("bless-$v") || 0;
my $oldbless = $cgi->param("oldbless-$v");
if ((defined $oldbless) and ($oldbless != $bless)) {
$chgs = 1;
if ($bless != 0) {
$sthInsert->execute($v, $gid, GROUP_BLESS);
} else {
$sthDelete->execute($v, $gid, GROUP_BLESS);
}
}
my $cansee = $cgi->param("cansee-$v") || 0;
if (Bugzilla->params->{"usevisibilitygroups"}
&& ($cgi->param("oldcansee-$v") != $cansee)) {
$chgs = 1;
if ($cansee != 0) {
$sthInsert->execute($v, $gid, GROUP_VISIBLE);
} else {
$sthDelete->execute($v, $gid, GROUP_VISIBLE);
}
}
if ($group->is_bug_group) {
if (defined $cgi->param('name')) {
$group->set_name($cgi->param('name'));
}
if (defined $cgi->param('desc')) {
$group->set_description($cgi->param('desc'));
}
# Only set isactive if we came from the right form.
if (defined $cgi->param('regexp')) {
$group->set_is_active($cgi->param('isactive'));
}
}
my $changes = $group->update();
my $sth_insert = $dbh->prepare('INSERT INTO group_group_map
(member_id, grantor_id, grant_type)
VALUES (?, ?, ?)');
my $sth_delete = $dbh->prepare('DELETE FROM group_group_map
WHERE member_id = ?
AND grantor_id = ?
AND grant_type = ?');
# First item is the type, second is whether or not it's "reverse"
# (granted_by) (see _do_add for more explanation).
my %fields = (
members => [GROUP_MEMBERSHIP, 0],
bless_from => [GROUP_BLESS, 0],
visible_from => [GROUP_VISIBLE, 0],
member_of => [GROUP_MEMBERSHIP, 1],
bless_to => [GROUP_BLESS, 1],
visible_to_me => [GROUP_VISIBLE, 1]
);
while (my ($field, $data) = each %fields) {
_do_add($group, $changes, $sth_insert, "${field}_add",
$data->[0], $data->[1]);
_do_remove($group, $changes, $sth_delete, "${field}_remove",
$data->[0], $data->[1]);
}
$dbh->bz_unlock_tables();
return $gid, $chgs, $name, $regexp;
return $changes;
}
sub _do_add {
my ($group, $changes, $sth_insert, $field, $type, $reverse) = @_;
my $current;
# $reverse means we're doing a granted_by--that is, somebody else
# is granting us something.
if ($reverse) {
$current = $group->granted_by_direct($type);
}
else {
$current = $group->grant_direct($type);
}
my $add_items = Bugzilla::Group->new_from_list([$cgi->param($field)]);
foreach my $add (@$add_items) {
next if grep($_->id == $add->id, @$current);
$changes->{$field} ||= [];
push(@{$changes->{$field}}, $add->name);
# They go this direction for a normal "This group is granting
# $add something."
my @ids = ($add->id, $group->id);
# But they get reversed for "This group is being granted something
# by $add."
@ids = reverse @ids if $reverse;
$sth_insert->execute(@ids, $type);
}
}
sub _do_remove {
my ($group, $changes, $sth_delete, $field, $type, $reverse) = @_;
my $remove_items = Bugzilla::Group->new_from_list([$cgi->param($field)]);
foreach my $remove (@$remove_items) {
my @ids = ($remove->id, $group->id);
# See _do_add for an explanation of $reverse
@ids = reverse @ids if $reverse;
# Deletions always succeed and are harmless if they fail, so we
# don't need to do any checks.
$sth_delete->execute(@ids, $type);
$changes->{$field} ||= [];
push(@{$changes->{$field}}, $remove->name);
}
}