Commit 6fac08b9 authored by Frédéric Buclin's avatar Frédéric Buclin

Bug 529974: Let users with local editcomponents privs manage flags for products they can administer

a=LpSolit (module owner)
parent 4c2a23f4
......@@ -112,6 +112,9 @@ use constant UPDATE_VALIDATORS => {
sub create {
my $class = shift;
my $dbh = Bugzilla->dbh;
$dbh->bz_start_transaction();
$class->check_required_create_fields(@_);
my $params = $class->run_create_validators(@_);
......@@ -126,6 +129,9 @@ sub create {
$flagtype->set_clusions({ inclusions => $inclusions,
exclusions => $exclusions });
$flagtype->update();
$dbh->bz_commit_transaction();
return $flagtype;
}
......@@ -161,7 +167,7 @@ sub update {
FROM flags
INNER JOIN bugs
ON flags.bug_id = bugs.bug_id
LEFT OUTER JOIN flaginclusions AS i
LEFT JOIN flaginclusions AS i
ON (flags.type_id = i.type_id
AND (bugs.product_id = i.product_id
OR i.product_id IS NULL)
......@@ -351,7 +357,6 @@ sub set_request_group { $_[0]->set('request_group_id', $_[1]); }
sub set_clusions {
my ($self, $list) = @_;
my $dbh = Bugzilla->dbh;
my %products;
foreach my $category (keys %$list) {
......@@ -360,22 +365,33 @@ sub set_clusions {
foreach my $prod_comp (@{$list->{$category} || []}) {
my ($prod_id, $comp_id) = split(':', $prod_comp);
my $component;
my $prod_name = '__Any__';
my $comp_name = '__Any__';
# Does the product exist?
if ($prod_id && detaint_natural($prod_id)) {
$products{$prod_id} ||= new Bugzilla::Product($prod_id);
next unless defined $products{$prod_id};
if ($prod_id) {
$products{$prod_id} ||= Bugzilla::Product->check({ id => $prod_id });
detaint_natural($prod_id);
$prod_name = $products{$prod_id}->name;
# Does the component belong to this product?
if ($comp_id && detaint_natural($comp_id)) {
($component) = grep { $_->id == $comp_id } @{$products{$prod_id}->components};
next unless $component;
if ($comp_id) {
detaint_natural($comp_id)
|| ThrowCodeError('param_must_be_numeric',
{ function => 'Bugzilla::FlagType::set_clusions' });
my ($component) = grep { $_->id == $comp_id } @{$products{$prod_id}->components}
or ThrowUserError('product_unknown_component',
{ product => $prod_name, comp_id => $comp_id });
$comp_name = $component->name;
}
else {
$comp_id = 0;
}
}
else {
$prod_id = 0;
$comp_id = 0;
}
$prod_id ||= 0;
$comp_id ||= 0;
my $prod_name = $prod_id ? $products{$prod_id}->name : '__Any__';
my $comp_name = $comp_id ? $component->name : '__Any__';
$clusions{"$prod_name:$comp_name"} = "$prod_id:$comp_id";
$clusions_as_hash{$prod_id}->{$comp_id} = 1;
}
......@@ -520,15 +536,16 @@ sub get_clusions {
my $dbh = Bugzilla->dbh;
my $list =
$dbh->selectall_arrayref("SELECT products.id, products.name, " .
" components.id, components.name " .
"FROM flagtypes, flag${type}clusions " .
"LEFT OUTER JOIN products " .
" ON flag${type}clusions.product_id = products.id " .
"LEFT OUTER JOIN components " .
" ON flag${type}clusions.component_id = components.id " .
"WHERE flagtypes.id = ? " .
" AND flag${type}clusions.type_id = flagtypes.id",
$dbh->selectall_arrayref("SELECT products.id, products.name,
components.id, components.name
FROM flagtypes
INNER JOIN flag${type}clusions
ON flag${type}clusions.type_id = flagtypes.id
LEFT JOIN products
ON flag${type}clusions.product_id = products.id
LEFT JOIN components
ON flag${type}clusions.component_id = components.id
WHERE flagtypes.id = ?",
undef, $id);
my (%clusions, %clusions_as_hash);
foreach my $data (@$list) {
......@@ -667,7 +684,7 @@ sub sqlify_criteria {
$join_clause .= "AND (e.component_id = $component_id OR e.component_id IS NULL) ";
}
else {
$addl_join_clause = "AND e.component_id IS NULL OR (i.component_id != e.component_id) ";
$addl_join_clause = "AND e.component_id IS NULL OR (i.component_id = e.component_id) ";
}
$join_clause .= "AND ((e.product_id = $product_id $addl_join_clause) OR e.product_id IS NULL)";
......
......@@ -1006,6 +1006,49 @@ sub check_can_admin_product {
return $product;
}
sub check_can_admin_flagtype {
my ($self, $flagtype_id) = @_;
my $flagtype = Bugzilla::FlagType->check({ id => $flagtype_id });
my $can_fully_edit = 1;
if (!$self->in_group('editcomponents')) {
my $products = $self->get_products_by_permission('editcomponents');
# You need editcomponents privs for at least one product to have
# a chance to edit the flagtype.
scalar(@$products)
|| ThrowUserError('auth_failure', {group => 'editcomponents',
action => 'edit',
object => 'flagtypes'});
my $can_admin = 0;
my $i = $flagtype->inclusions_as_hash;
my $e = $flagtype->exclusions_as_hash;
# If there is at least one product for which the user doesn't have
# editcomponents privs, then don't allow him to do everything with
# this flagtype, independently of whether this product is in the
# exclusion list or not.
my %product_ids;
map { $product_ids{$_->id} = 1 } @$products;
$can_fully_edit = 0 if grep { !$product_ids{$_} } keys %$i;
unless ($e->{0}->{0}) {
foreach my $product (@$products) {
my $id = $product->id;
next if $e->{$id}->{0};
# If we are here, the product has not been explicitly excluded.
# Check whether it's explicitly included, or at least one of
# its components.
$can_admin = ($i->{0}->{0} || $i->{$id}->{0}
|| scalar(grep { !$e->{$id}->{$_} } keys %{$i->{$id}}));
last if $can_admin;
}
}
$can_admin || ThrowUserError('flag_type_not_editable', { flagtype => $flagtype });
}
return wantarray ? ($flagtype, $can_fully_edit) : $flagtype;
}
sub can_request_flag {
my ($self, $flag_type) = @_;
......@@ -2261,6 +2304,21 @@ not be aware of the existence of the product.
Returns: On success, a product object. On failure, an error is thrown.
=item C<check_can_admin_flagtype($flagtype_id)>
Description: Checks whether the user is allowed to edit properties of the flag type.
If the flag type is also used by some products for which the user
hasn't editcomponents privs, then the user is only allowed to edit
the inclusion and exclusion lists for products he can administrate.
Params: $flagtype_id - a flag type ID.
Returns: On success, a flag type object. On failure, an error is thrown.
In list context, a boolean indicating whether the user can edit
all properties of the flag type is also returned. The boolean
is false if the user can only edit the inclusion and exclusions
lists.
=item C<can_request_flag($flag_type)>
Description: Checks whether the user can request flags of the given type.
......
......@@ -99,10 +99,6 @@ tbody.file pre:empty {
width: 3em;
}
.warning {
color: red
}
table.attachment_info th {
text-align: right;
vertical-align: top;
......
......@@ -385,6 +385,10 @@ input.requestee {
font-size: x-large;
}
.warning {
color: red;
}
.throw_error {
background-color: #ff0000;
color: black;
......
......@@ -76,7 +76,8 @@
<a href="editcomponents.cgi">components</a>, <a href="editversions.cgi">versions</a>
and <a href="editmilestones.cgi">milestones</a> directly.</dd>
[% class = user.in_group('editcomponents') ? "" : "forbidden" %]
[% class = (user.in_group('editcomponents')
|| user.get_products_by_permission('editcomponents').size) ? "" : "forbidden" %]
<dt id="flags" class="[% class %]"><a href="editflagtypes.cgi">Flags</a></dt>
<dd class="[% class %]">A flag is a custom 4-states attribute of [% terms.bugs %]
and/or attachments. These states are: granted, denied, requested and undefined.
......
......@@ -17,6 +17,7 @@
#
# Contributor(s): Myk Melez <myk@mozilla.org>
# Mark Bickford <markhb@maine.rr.com>
# Frédéric Buclin <LpSolit@gmail.com>
#%]
[% PROCESS global/variables.none.tmpl %]
......@@ -42,22 +43,24 @@
table#form th { text-align: right; vertical-align: baseline; white-space: nowrap; }
table#form td { text-align: left; vertical-align: baseline; }
"
onload="var f = document.forms[0]; selectProduct(f.product, f.component, null, null, '__Any__');"
onload="var f = document.forms['flagtype_properties'];
selectProduct(f.product, f.component, null, null, '__Any__');"
javascript_urls=["js/productform.js"]
doc_section = doc_section
%]
<form method="post" action="editflagtypes.cgi">
<form id="flagtype_properties" method="post" action="editflagtypes.cgi">
<input type="hidden" name="action" value="[% action FILTER html %]">
<input type="hidden" name="can_fully_edit" value="[% can_fully_edit FILTER html %]">
<input type="hidden" name="id" value="[% type.id %]">
<input type="hidden" name="token" value="[% token FILTER html %]">
<input type="hidden" name="target_type" value="[% type.target_type FILTER html %]">
<input type="hidden" name="check_clusions" value="[% check_clusions FILTER none %]">
[% FOREACH category = type.inclusions %]
<input type="hidden" name="inclusions" value="[% category.value FILTER html %]">
[% FOREACH category = inclusions.values %]
<input type="hidden" name="inclusions" value="[% category FILTER html %]">
[% END %]
[% FOREACH category = type.exclusions %]
<input type="hidden" name="exclusions" value="[% category.value FILTER html %]">
[% FOREACH category = exclusions.values %]
<input type="hidden" name="exclusions" value="[% category FILTER html %]">
[% END %]
[%# Add a hidden button at the top of the form so that the user pressing "return"
......@@ -69,8 +72,8 @@
<th>Name:</th>
<td>
a short name identifying this type.<br>
<input type="text" name="name" value="[% type.name FILTER html %]"
size="50" maxlength="50">
<input type="text" name="name" value="[% type.name FILTER html %]" size="50"
maxlength="50" [%- ' disabled="disabled"' UNLESS can_fully_edit %]>
</td>
</tr>
......@@ -83,6 +86,7 @@
minrows = 4
cols = 80
defaultcontent = type.description
disabled = !can_fully_edit
%]
</td>
</tr>
......@@ -94,6 +98,12 @@
the products/components to which [% type.target_type == "bug" ? terms.bugs : "attachments" %]
must (inclusions) or must not (exclusions) belong in order for users
to be able to set flags of this type for them.
[% UNLESS can_fully_edit %]
<p class="warning">This flagtype also applies to some products you are not allowed
to edit (and so which are not displayed in the lists below). Your limited privileges
means you are only allowed to add and remove this flagtype to/from products you can
edit, but not to edit other properties of the flagtype.</p>
[% END %]
<table>
<tr>
<td style="vertical-align: top;">
......@@ -101,17 +111,13 @@
<select name="product" onchange="selectProduct(this, this.form.component, null, null, '__Any__');">
<option value="">__Any__</option>
[% FOREACH prod = products %]
<option value="[% prod.name FILTER html %]"
[% "selected" IF type.product.name == prod.name %]>
[% prod.name FILTER html %]</option>
<option value="[% prod.name FILTER html %]">[% prod.name FILTER html %]</option>
[% END %]
</select><br>
<select name="component">
<option value="">__Any__</option>
[% FOREACH comp = components %]
<option value="[% comp FILTER html %]"
[% "selected" IF type.component.name == comp %]>
[% comp FILTER html %]</option>
<option value="[% comp FILTER html %]">[% comp FILTER html %]</option>
[% END %]
</select><br>
<input type="submit" name="categoryAction-include" value="Include">
......@@ -119,12 +125,12 @@
</td>
<td style="vertical-align: top;">
<b>Inclusions:</b><br>
[% PROCESS "global/select-menu.html.tmpl" name="inclusion_to_remove" multiple="1" size="7" options=type.inclusions %]<br>
[% PROCESS category_select name="inclusion_to_remove" categories = inclusions %]<br>
<input type="submit" name="categoryAction-removeInclusion" value="Remove Inclusion">
</td>
<td style="vertical-align: top;">
<b>Exclusions:</b><br>
[% PROCESS "global/select-menu.html.tmpl" name="exclusion_to_remove" multiple="1" size="7" options=type.exclusions %]<br>
[% PROCESS category_select name="exclusion_to_remove" categories = exclusions %]<br>
<input type="submit" name="categoryAction-removeExclusion" value="Remove Exclusion">
</td>
</tr>
......@@ -139,7 +145,8 @@
this type will be sorted when displayed to users in a list; ignore if you
don't care what order the types appear in or if you want them to appear
in alphabetical order.<br>
<input type="text" name="sortkey" value="[% type.sortkey || 1 %]" size="5" maxlength="5">
<input type="text" name="sortkey" value="[% type.sortkey || 1 %]" size="5" maxlength="5"
[%- ' disabled="disabled"' UNLESS can_fully_edit %]>
</td>
</tr>
......@@ -147,6 +154,7 @@
<th>&nbsp;</th>
<td>
<input type="checkbox" id="is_active" name="is_active"
[%- ' disabled="disabled"' UNLESS can_fully_edit %]
[% " checked" IF type.is_active || !type.is_active.defined %]>
<label for="is_active">active (flags of this type appear in the UI and can be set)</label>
</td>
......@@ -156,6 +164,7 @@
<th>&nbsp;</th>
<td>
<input type="checkbox" id="is_requestable" name="is_requestable"
[%- ' disabled="disabled"' UNLESS can_fully_edit %]
[% " checked" IF type.is_requestable || !type.is_requestable.defined %]>
<label for="is_requestable">requestable (users can ask for flags of this type to be set)</label>
</td>
......@@ -172,7 +181,8 @@
<kbd>[% Param('emailsuffix') %]</kbd> will <em>not</em> be appended
to these addresses, so you should add it explicitly if so desired.
[% END %]<br>
<input type="text" name="cc_list" value="[% type.cc_list FILTER html %]" size="80" maxlength="200">
<input type="text" name="cc_list" value="[% type.cc_list FILTER html %]" size="80"
maxlength="200" [%- ' disabled="disabled"' UNLESS can_fully_edit %]>
</td>
</tr>
......@@ -180,6 +190,7 @@
<th>&nbsp;</th>
<td>
<input type="checkbox" id="is_requesteeble" name="is_requesteeble"
[%- ' disabled="disabled"' UNLESS can_fully_edit %]
[% " checked" IF type.is_requesteeble || !type.is_requesteeble.defined %]>
<label for="is_requesteeble">specifically requestable (users can ask specific other users
to set flags of this type as opposed to just asking the wind)</label>
......@@ -190,6 +201,7 @@
<th>&nbsp;</th>
<td>
<input type="checkbox" id="is_multiplicable" name="is_multiplicable"
[%- ' disabled="disabled"' UNLESS can_fully_edit %]
[% " checked" IF type.is_multiplicable || !type.is_multiplicable.defined %]>
<label for="is_multiplicable">multiplicable (multiple flags of this type can be set on
the same [% type.target_type == "bug" ? terms.bug : "attachment" %])</label>
......@@ -201,7 +213,7 @@
<td>
the group allowed to grant/deny flags of this type
(to allow all users to grant/deny these flags, select no group).<br>
[% PROCESS select selname = "grant_group" %]
[% PROCESS group_select selname = "grant_group" %]
</td>
</tr>
......@@ -211,7 +223,7 @@
if flags of this type are requestable, the group allowed to request them
(to allow all users to request these flags, select no group).<br>
Note that the request group alone has no effect if the grant group is not defined!<br>
[% PROCESS select selname = "request_group" %]
[% PROCESS group_select selname = "request_group" %]
</td>
</tr>
......@@ -233,8 +245,8 @@
[%# Block for SELECT fields #%]
[%############################################################################%]
[% BLOCK select %]
<select name="[% selname %]" id="[% selname %]">
[% BLOCK group_select %]
<select name="[% selname %]" id="[% selname %]" [%- ' disabled="disabled"' UNLESS can_fully_edit %]>
<option value="">(no group)</option>
[% FOREACH group = groups %]
<option value="[% group.name FILTER html %]"
......@@ -244,3 +256,13 @@
[% END %]
</select>
[% END %]
[% BLOCK category_select %]
<select name="[% name FILTER html %]" multiple="multiple" size="7">
[% FOREACH option = categories.keys.sort %]
<option value="[% categories.$option FILTER html %]">
[% option FILTER html %]
</option>
[% END %]
</select>
[% END %]
\ No newline at end of file
......@@ -641,6 +641,16 @@
[% END %]
is invalid.
[% ELSIF error == "flag_type_cannot_deactivate" %]
[% title = "Cannot Deactivate Flag Type" %]
Sorry, but the flag type '[% flagtype.name FILTER html %]' also applies to some
products you cannot see, and so you are not allowed to deactivate it.
[% ELSIF error == "flag_type_cannot_delete" %]
[% title = "Flag Type Deletion Not Allowed" %]
Sorry, but the flag type '[% flagtype.name FILTER html %]' also applies to some
products you cannot see, and so you are not allowed to delete it.
[% ELSIF error == "flag_type_cc_list_invalid" %]
[% title = "Flag Type CC List Invalid" %]
[% admindocslinks = {'flags-overview.html#flags-admin' => 'Administering Flags'} %]
......@@ -661,6 +671,11 @@
The name <em>[% name FILTER html %]</em> must be 1-50 characters long
and must not contain any spaces or commas.
[% ELSIF error == "flag_type_not_editable" %]
[% title = "Flag Type Not Editable" %]
You are not allowed to edit properties of the '[% flagtype.name FILTER html %]'
flag type, because this flag type is not available for the products you can administer.
[% ELSIF error == "flag_type_not_multiplicable" %]
[% docslinks = {'flags-overview.html' => 'An overview on Flags',
'flags.html' => 'Using Flags'} %]
......@@ -1311,6 +1326,7 @@
[%+ constants.USER_PASSWORD_MIN_LENGTH FILTER html %] characters long.
[% ELSIF error == "product_access_denied" %]
[% title = "Product Access Denied" %]
Either the product
[%+ IF id.defined %]
with the id [% id FILTER html %]
......@@ -1388,6 +1404,15 @@
'versions.html' => 'Administering versions'} %]
You must enter a valid version to create a new product.
[% ELSIF error == "product_unknown_component" %]
[% title = "Unknown Component" %]
Product '[% product FILTER html %]' has no component
[% IF comp_id %]
with ID [% comp_id FILTER html %].
[% ELSE %]
named '[% comp FILTER html %]'.
[% END %]
[% ELSIF error == "query_name_exists" %]
[% title = "Search Name Already In Use" %]
The name <em>[% name FILTER html %]</em> is already used by another
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment