diff options
| -rw-r--r-- | Bugzilla/DB.pm | 8 | ||||
| -rw-r--r-- | Bugzilla/DB/Schema.pm | 6 | ||||
| -rw-r--r-- | Bugzilla/Group.pm | 187 | ||||
| -rw-r--r-- | Bugzilla/Install/DB.pm | 14 | ||||
| -rwxr-xr-x | editgroups.cgi | 156 | ||||
| -rw-r--r-- | template/en/default/admin/groups/delete.html.tmpl | 117 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 7 |
7 files changed, 293 insertions, 202 deletions
diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index 7e40c1627..830d2835e 100644 --- a/Bugzilla/DB.pm +++ b/Bugzilla/DB.pm @@ -837,6 +837,14 @@ sub bz_drop_table { } } +sub bz_fk_info { + my ($self, $table, $column) = @_; + my $col_info = $self->bz_column_info($table, $column); + return undef if !$col_info; + my $fk = $col_info->{REFERENCES}; + return $fk; +} + sub bz_rename_column { my ($self, $table, $old_name, $new_name) = @_; diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index a1102dd64..dbea63f79 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -610,10 +610,12 @@ use constant ABSTRACT_SCHEMA => { DEFAULT => '0'}, grant_group_id => {TYPE => 'INT3', REFERENCES => {TABLE => 'groups', - COLUMN => 'id'}}, + COLUMN => 'id', + DELETE => 'SET NULL'}}, request_group_id => {TYPE => 'INT3', REFERENCES => {TABLE => 'groups', - COLUMN => 'id'}}, + COLUMN => 'id', + DELETE => 'SET NULL'}}, ], }, diff --git a/Bugzilla/Group.pm b/Bugzilla/Group.pm index 2e8a975d2..c936593bc 100644 --- a/Bugzilla/Group.pm +++ b/Bugzilla/Group.pm @@ -84,19 +84,47 @@ sub user_regexp { return $_[0]->{'userregexp'}; } sub is_active { return $_[0]->{'isactive'}; } sub icon_url { return $_[0]->{'icon_url'}; } +sub bugs { + my $self = shift; + return $self->{bugs} if exists $self->{bugs}; + my $bug_ids = Bugzilla->dbh->selectcol_arrayref( + 'SELECT bug_id FROM bug_group_map WHERE group_id = ?', + undef, $self->id); + require Bugzilla::Bug; + $self->{bugs} = Bugzilla::Bug->new_from_list($bug_ids); + return $self->{bugs}; +} + sub members_direct { my ($self) = @_; - return $self->{members_direct} if defined $self->{members_direct}; + $self->{members_direct} ||= $self->_get_members(GRANT_DIRECT); + return $self->{members_direct}; +} + +sub members_non_inherited { + my ($self) = @_; + $self->{members_non_inherited} ||= $self->_get_members(); + return $self->{members_non_inherited}; +} + +# A helper for members_direct and members_non_inherited +sub _get_members { + my ($self, $grant_type) = @_; my $dbh = Bugzilla->dbh; + my $grant_clause = $grant_type ? "AND grant_type = $grant_type" : ""; my $user_ids = $dbh->selectcol_arrayref( - "SELECT user_group_map.user_id + "SELECT DISTINCT user_id FROM user_group_map - WHERE user_group_map.group_id = ? - AND grant_type = " . GRANT_DIRECT . " - AND isbless = 0", undef, $self->id); + WHERE isbless = 0 $grant_clause AND group_id = ?", undef, $self->id); require Bugzilla::User; - $self->{members_direct} = Bugzilla::User->new_from_list($user_ids); - return $self->{members_direct}; + return Bugzilla::User->new_from_list($user_ids); +} + +sub flag_types { + my $self = shift; + require Bugzilla::FlagType; + $self->{flag_types} ||= Bugzilla::FlagType::match({ group => $self->id }); + return $self->{flag_types}; } sub grant_direct { @@ -131,6 +159,30 @@ sub granted_by_direct { return $self->{granted_by_direct}->{$type}; } +sub products { + my $self = shift; + return $self->{products} if exists $self->{products}; + my $product_data = Bugzilla->dbh->selectall_arrayref( + 'SELECT product_id, entry, membercontrol, othercontrol, + canedit, editcomponents, editbugs, canconfirm + FROM group_control_map WHERE group_id = ?', {Slice=>{}}, + $self->id); + my @ids = map { $_->{product_id} } @$product_data; + require Bugzilla::Product; + my $products = Bugzilla::Product->new_from_list(\@ids); + my %data_map = map { $_->{product_id} => $_ } @$product_data; + my @retval; + foreach my $product (@$products) { + # Data doesn't need to contain product_id--we already have + # the product object. + delete $data_map{$product->id}->{product_id}; + push(@retval, { controls => $data_map{$product->id}, + product => $product }); + } + $self->{products} = \@retval; + return $self->{products}; +} + ############################### #### Methods #### ############################### @@ -165,6 +217,66 @@ sub update { return $changes; } +sub check_remove { + my ($self, $params) = @_; + + # System groups cannot be deleted! + if (!$self->is_bug_group) { + ThrowUserError("system_group_not_deletable", { name => $self->name }); + } + + # Groups having a special role cannot be deleted. + my @special_groups; + foreach my $special_group (GROUP_PARAMS) { + if ($self->name eq Bugzilla->params->{$special_group}) { + push(@special_groups, $special_group); + } + } + if (scalar(@special_groups)) { + ThrowUserError('group_has_special_role', + { name => $self->name, + groups => \@special_groups }); + } + + return if $params->{'test_only'}; + + my $cantdelete = 0; + + my $users = $self->members_non_inherited; + if (scalar(@$users) && !$params->{'remove_from_users'}) { + $cantdelete = 1; + } + + my $bugs = $self->bugs; + if (scalar(@$bugs) && !$params->{'remove_from_bugs'}) { + $cantdelete = 1; + } + + my $products = $self->products; + if (scalar(@$products) && !$params->{'remove_from_products'}) { + $cantdelete = 1; + } + + my $flag_types = $self->flag_types; + if (scalar(@$flag_types) && !$params->{'remove_from_flags'}) { + $cantdelete = 1; + } + + ThrowUserError('group_cannot_delete', { group => $self }) if $cantdelete; +} + +sub remove_from_db { + my $self = shift; + my $dbh = Bugzilla->dbh; + $self->check_remove(@_); + $dbh->do('DELETE FROM whine_schedules + WHERE mailto_type = ? AND mailto = ?', + undef, MAILTO_GROUP, $self->id); + # All the other tables will be handled by foreign keys when we + # drop the main "groups" row. + $self->SUPER::remove_from_db(@_); +} + # Add missing entries in bug_group_map for bugs created while # a mandatory group was disabled and which is now enabled again. sub _enforce_mandatory { @@ -224,20 +336,6 @@ sub _rederive_regexp { } } -sub members_non_inherited { - my ($self) = @_; - return $self->{members_non_inherited} - if exists $self->{members_non_inherited}; - - my $member_ids = Bugzilla->dbh->selectcol_arrayref( - 'SELECT DISTINCT user_id FROM user_group_map - WHERE isbless = 0 AND group_id = ?', - undef, $self->id) || []; - require Bugzilla::User; - $self->{members_non_inherited} = Bugzilla::User->new_from_list($member_ids); - return $self->{members_non_inherited}; -} - sub flatten_group_membership { my ($self, @groups) = @_; @@ -414,6 +512,53 @@ be a member of this group. =over +=item C<check_remove> + +=over + +=item B<Description> + +Determines whether it's OK to remove this group from the database, and +throws an error if it's not OK. + +=item B<Params> + +=over + +=item C<test_only> + +C<boolean> If you want to only check if the group can be deleted I<at all>, +under any circumstances, specify C<test_only> to just do the most basic tests +(the other parameters will be ignored in this situation, as those tests won't +be run). + +=item C<remove_from_users> + +C<boolean> True if it would be OK to remove all users who are in this group +from this group. + +=item C<remove_from_bugs> + +C<boolean> True if it would be OK to remove all bugs that are in this group +from this group. + +=item C<remove_from_flags> + +C<boolean> True if it would be OK to stop all flagtypes that reference +this group from referencing this group (e.g., as their grantgroup or +requestgroup). + +=item C<remove_from_products> + +C<boolean> True if it would be OK to remove this group from all group controls +on products. + +=back + +=item B<Returns> (nothing) + +=back + =item C<members_non_inherited> Returns an arrayref of L<Bugzilla::User> objects representing people who are diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index adbcb285f..7a9fa72d8 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -594,6 +594,8 @@ sub update_table_definitions { _add_allows_unconfirmed_to_product_table(); + _convert_flagtypes_fks_to_set_null(); + ################################################################ # New --TABLE-- changes should go *** A B O V E *** this point # ################################################################ @@ -3340,6 +3342,18 @@ sub _add_allows_unconfirmed_to_product_table { } } +sub _convert_flagtypes_fks_to_set_null { + my $dbh = Bugzilla->dbh; + foreach my $column (qw(request_group_id grant_group_id)) { + my $fk = $dbh->bz_fk_info('flagtypes', $column); + if ($fk and !defined $fk->{DELETE}) { + # checksetup will re-create the FK with the appropriate definition + # at the end of its table upgrades, so we just drop it here. + $dbh->bz_drop_fk('flagtypes', $column); + } + } +} + 1; __END__ diff --git a/editgroups.cgi b/editgroups.cgi index e8d8cfe24..87a31816d 100755 --- a/editgroups.cgi +++ b/editgroups.cgi @@ -242,64 +242,15 @@ if ($action eq 'new') { if ($action eq 'del') { # Check that an existing group ID is given - my $gid = CheckGroupID($cgi->param('group')); - my ($name, $desc, $isbuggroup) = - $dbh->selectrow_array("SELECT name, description, isbuggroup " . - "FROM groups WHERE id = ?", undef, $gid); - - # System groups cannot be deleted! - if (!$isbuggroup) { - ThrowUserError("system_group_not_deletable", { name => $name }); - } - # Groups having a special role cannot be deleted. - my @special_groups; - foreach my $special_group (SPECIAL_GROUPS) { - if ($name eq Bugzilla->params->{$special_group}) { - push(@special_groups, $special_group); - } - } - if (scalar(@special_groups)) { - ThrowUserError('group_has_special_role', {'name' => $name, - 'groups' => \@special_groups}); - } - - # Group inheritance no longer appears in user_group_map. - my $grouplist = join(',', @{Bugzilla::Group->flatten_group_membership($gid)}); - my $hasusers = - $dbh->selectrow_array("SELECT 1 FROM user_group_map - WHERE group_id IN ($grouplist) AND isbless = 0 " . - $dbh->sql_limit(1)) || 0; - - my ($shared_queries) = + my $group = Bugzilla::Group->check({ id => $cgi->param('group') }); + $group->check_remove({ test_only => 1 }); + $vars->{'shared_queries'} = $dbh->selectrow_array('SELECT COUNT(*) FROM namedquery_group_map - WHERE group_id = ?', - undef, $gid); - - my $bug_ids = $dbh->selectcol_arrayref('SELECT bug_id FROM bug_group_map - WHERE group_id = ?', undef, $gid); - - my $hasbugs = scalar(@$bug_ids) ? 1 : 0; - my $buglist = join(',', @$bug_ids); - - my $hasproduct = Bugzilla::Product->new({'name' => $name}) ? 1 : 0; - - my $hasflags = $dbh->selectrow_array('SELECT 1 FROM flagtypes - WHERE grant_group_id = ? - OR request_group_id = ? ' . - $dbh->sql_limit(1), - undef, ($gid, $gid)) || 0; - - $vars->{'gid'} = $gid; - $vars->{'name'} = $name; - $vars->{'description'} = $desc; - $vars->{'hasusers'} = $hasusers; - $vars->{'hasbugs'} = $hasbugs; - $vars->{'hasproduct'} = $hasproduct; - $vars->{'hasflags'} = $hasflags; - $vars->{'shared_queries'} = $shared_queries; - $vars->{'buglist'} = $buglist; - $vars->{'token'} = issue_session_token('delete_group'); + WHERE group_id = ?', undef, $group->id); + + $vars->{'group'} = $group; + $vars->{'token'} = issue_session_token('delete_group'); print $cgi->header(); $template->process("admin/groups/delete.html.tmpl", $vars) @@ -315,91 +266,14 @@ if ($action eq 'del') { if ($action eq 'delete') { check_token_data($token, 'delete_group'); # Check that an existing group ID is given - my $gid = CheckGroupID($cgi->param('group')); - my ($name, $isbuggroup) = - $dbh->selectrow_array("SELECT name, isbuggroup FROM groups " . - "WHERE id = ?", undef, $gid); - - # System groups cannot be deleted! - if (!$isbuggroup) { - ThrowUserError("system_group_not_deletable", { name => $name }); - } - # Groups having a special role cannot be deleted. - my @special_groups; - foreach my $special_group (SPECIAL_GROUPS) { - if ($name eq Bugzilla->params->{$special_group}) { - push(@special_groups, $special_group); - } - } - if (scalar(@special_groups)) { - ThrowUserError('group_has_special_role', {'name' => $name, - 'groups' => \@special_groups}); - } - - my $cantdelete = 0; - - # Group inheritance no longer appears in user_group_map. - my $grouplist = join(',', @{Bugzilla::Group->flatten_group_membership($gid)}); - my $hasusers = - $dbh->selectrow_array("SELECT 1 FROM user_group_map - WHERE group_id IN ($grouplist) AND isbless = 0 " . - $dbh->sql_limit(1)) || 0; - - if ($hasusers && !defined $cgi->param('removeusers')) { - $cantdelete = 1; - } - - my $hasbugs = $dbh->selectrow_array('SELECT 1 FROM bug_group_map - WHERE group_id = ? ' . - $dbh->sql_limit(1), - undef, $gid) || 0; - if ($hasbugs && !defined $cgi->param('removebugs')) { - $cantdelete = 1; - } - - if (Bugzilla::Product->new({'name' => $name}) - && !defined $cgi->param('unbind')) - { - $cantdelete = 1; - } - - my $hasflags = $dbh->selectrow_array('SELECT 1 FROM flagtypes - WHERE grant_group_id = ? - OR request_group_id = ? ' . - $dbh->sql_limit(1), - undef, ($gid, $gid)) || 0; - if ($hasflags && !defined $cgi->param('removeflags')) { - $cantdelete = 1; - } - - $vars->{'gid'} = $gid; - $vars->{'name'} = $name; - - ThrowUserError('group_cannot_delete', $vars) if $cantdelete; - - $dbh->do('UPDATE flagtypes SET grant_group_id = ? - WHERE grant_group_id = ?', - undef, (undef, $gid)); - $dbh->do('UPDATE flagtypes SET request_group_id = ? - WHERE request_group_id = ?', - undef, (undef, $gid)); - $dbh->do('DELETE FROM namedquery_group_map WHERE group_id = ?', - undef, $gid); - $dbh->do('DELETE FROM user_group_map WHERE group_id = ?', - undef, $gid); - $dbh->do('DELETE FROM group_group_map - WHERE grantor_id = ? OR member_id = ?', - undef, ($gid, $gid)); - $dbh->do('DELETE FROM bug_group_map WHERE group_id = ?', - undef, $gid); - $dbh->do('DELETE FROM group_control_map WHERE group_id = ?', - undef, $gid); - $dbh->do('DELETE FROM whine_schedules - WHERE mailto_type = ? AND mailto = ?', - undef, (MAILTO_GROUP, $gid)); - $dbh->do('DELETE FROM groups WHERE id = ?', - undef, $gid); - + my $group = Bugzilla::Group->check({ id => $cgi->param('group') }); + $vars->{'name'} = $group->name; + $group->remove_from_db({ + remove_from_users => scalar $cgi->param('removeusers'), + remove_from_bugs => scalar $cgi->param('removebugs'), + remove_from_flags => scalar $cgi->param('removeflags'), + remove_from_products => scalar $cgi->param('unbind'), + }); delete_token($token); $vars->{'message'} = 'group_deleted'; diff --git a/template/en/default/admin/groups/delete.html.tmpl b/template/en/default/admin/groups/delete.html.tmpl index d1f1936bd..c1a9403f9 100644 --- a/template/en/default/admin/groups/delete.html.tmpl +++ b/template/en/default/admin/groups/delete.html.tmpl @@ -19,18 +19,14 @@ # Joel Peshkin <bugreport@peshkin.net> # Jacob Steenhagen <jake@bugzilla.org> # Vlad Dascalu <jocuri@softhome.net> + # Max Kanat-Alexander <mkanat@bugzilla.org> #%] [%# INTERFACE: - # gid: number. The group ID. - # name: string. The name of the group. - # description: string. The description of the group. - # hasusers: boolean int. True if the group includes users in it. - # hasbugs: boolean int. True if the group includes bugs in it. - # hasproduct: boolean int. True if the group is binded to a product. - # hasflags: boolean int. True if the group is used by a flag type. - # shared_queries: int. Number of saved searches being shared with this group. - # buglist: string. The list of bugs included in this group. + # group: A Bugzilla::Group object representing the group that is + # about to be deleted. + # shared_queries: int; The number of queries being shared with this + # group. #%] @@ -46,29 +42,34 @@ <th>Description</th> </tr> <tr> - <td>[% gid FILTER html %]</td> - <td>[% name FILTER html %]</td> - <td>[% description FILTER html_light %]</td> + <td>[% group.id FILTER html %]</td> + <td>[% group.name FILTER html %]</td> + <td>[% group.description FILTER html_light %]</td> </tr> </table> <form method="post" action="editgroups.cgi"> - [% IF hasusers %] - <p><b>One or more users belong to this group. You cannot delete - this group while there are users in it.</b> - - <br><a href="editusers.cgi?action=list&groupid=[% gid FILTER html %]&grouprestrict=1">Show - me which users</a> - <input type="checkbox" name="removeusers">Remove - all users from this group for me.</p> + [% IF group.members_non_inherited.size %] + <p><b>[% group.members_non_inherited.size FILTER html %] users belong + directly to this group. You cannot delete this group while there are + users in it.</b> + + <br><a href="editusers.cgi?action=list&groupid= + [%- group.id FILTER url_quote %]&grouprestrict=1">Show + me which users</a> - <label><input type="checkbox" name="removeusers">Remove + all users from this group for me.</label></p> [% END %] - [% IF hasbugs %] - <p><b>One or more [% terms.bug %] reports are visible only to this group. - You cannot delete this group while any [% terms.bugs %] are using it.</b> + [% IF group.bugs.size %] + <p><b>[% group.bugs.size FILTER html %] [%+ terms.bug %] reports are + visible only to this group. You cannot delete this group while any + [%+ terms.bugs %] are using it.</b> - <br><a href="buglist.cgi?bug_id=[% buglist FILTER html %]">Show me - which [% terms.bugs %]</a> - <input type="checkbox" name="removebugs">Remove - all [% terms.bugs %] from this group restriction for me.</p> + <br><a href="buglist.cgi?field0-0-0=bug_group&type0-0-0=equals&value0-0-0= + [%- group.name FILTER url_quote %]">Show me + which [% terms.bugs %]</a> - + <label><input type="checkbox" name="removebugs">Remove + all [% terms.bugs %] from this group restriction for me.</label></p> <p><b>NOTE:</b> It's quite possible to make confidential [% terms.bugs %] public by checking this box. It is <B>strongly</B> suggested @@ -76,21 +77,63 @@ the box.</p> [% END %] - [% IF hasproduct %] - <p><b>This group is tied to the <U>[% name FILTER html %]</U> product. - You cannot delete this group while it is tied to a product.</b> + [% IF group.products.size %] + <p><b>This group is tied to the following products:</b></p> + [% SET any_hidden = 0 %] + <ul> + [% FOREACH data = group.products %] + + [% SET active = [] %] + [% FOREACH control = data.controls.keys.sort %] + [% NEXT IF !data.controls.$control %] + [% IF control == 'othercontrol' OR control == 'membercontrol' %] + [% SWITCH data.controls.$control %] + [% CASE constants.CONTROLMAPMANDATORY %] + [% SET type = "Mandatory" %] + [% CASE constants.CONTROLMAPSHOWN %] + [% SET type = "Shown" %] + [% CASE constants.CONTROLMAPDEFAULT %] + [% SET type = "Default" %] + [% END %] + [% active.push("$control: $type") %] + [% ELSE %] + [% active.push(control) %] + [% END %] + [% END %] + + [% SET hidden = 0 %] + [% IF data.controls.othercontrol == constants.CONTROLMAPMANDATORY + AND data.controls.membercontrol == constants.CONTROLMAPMANDATORY + AND data.controls.entry + %] + [% SET hidden = 1 %] + [% END %] + + <li><a href="editproducts.cgi?action=editgroupcontrols&product= + [%- data.product.name FILTER url_quote %]"> + [%- data.product.name FILTER html %]</a> + ([% active.join(', ') FILTER html %]) + [% IF hidden %] + <strong>WARNING: This product is currently hidden. + Deleting this group will make this product publicly visible. + </strong> + [% END %]</li> + [% END %] + </ul> - <br><input type="checkbox" name="unbind">Delete this group anyway, - and make the product <U>[% name FILTER html %]</U> publicly visible.</p> + <p><label><input type="checkbox" name="unbind">Delete this group anyway, + and remove these controls.</label></p> [% END %] - [% IF hasflags %] + [% IF group.flag_types.size %] <p><b>This group restricts who can make changes to flags of certain types. You cannot delete this group while there are flag types using it.</b> - <br><a href="editflagtypes.cgi?action=list&group=[% gid FILTER html %]">Show - me which types</a> - <input type="checkbox" name="removeflags">Remove all - flag types from this group for me.</p> + <br><a href="editflagtypes.cgi?action=list&group= + [%- group.id FILTER url_quote %]">Show + me which types</a> - + <label><input type="checkbox" name="removeflags">Remove all + flag types from this group for me.</label></p> [% END %] [% IF shared_queries %] @@ -115,7 +158,9 @@ <h2>Confirmation</h2> <p>Do you really want to delete this group?</p> - [% IF (hasusers || hasbugs || hasproduct || hasflags) %] + [% IF group.users.size || group.bugs.size || group.products.size + || group.flags.size + %] <p><b>You must check all of the above boxes or correct the indicated problems first before you can proceed.</b></p> [% END %] @@ -123,7 +168,7 @@ <p> <input type="submit" id="delete" value="Yes, delete"> <input type="hidden" name="action" value="delete"> - <input type="hidden" name="group" value="[% gid FILTER html %]"> + <input type="hidden" name="group" value="[% group.id FILTER html %]"> <input type="hidden" name="token" value="[% token FILTER html %]"> </p> </form> diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index b40ff26b4..80172bb03 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -682,9 +682,10 @@ [% ELSIF error == "group_cannot_delete" %] [% title = "Cannot Delete Group" %] - The <em>[% name FILTER html %]</em> group cannot be deleted because + The <em>[% group.name FILTER html %]</em> group cannot be deleted because there are - <a href="editgroups.cgi?action=del&group=[% gid FILTER url_quote %]">records</a> + <a href="editgroups.cgi?action=del&group= + [%- group.id FILTER url_quote %]">records</a> in the database which refer to it. All references to this group must be removed before you can remove it. @@ -1760,6 +1761,8 @@ flagtype [% ELSIF class == "Bugzilla::Field" %] field + [% ELSIF class == "Bugzilla::Group" %] + group [% ELSIF class == "Bugzilla::Product" %] product [% ELSIF class == "Bugzilla::Search::Saved" %] |