From 71927e7ee069c33019780158670df2415ee1ef3b Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Mon, 31 Mar 2014 15:12:20 +0000 Subject: Bug 989650 - backport bug 294021 to bmo/4.2 to allow requestees to set attachment flags even if they don't have editbugs privs r=glob --- Bugzilla/Attachment.pm | 15 ++++++------- Bugzilla/Flag.pm | 2 +- Bugzilla/WebService/Bug.pm | 2 +- attachment.cgi | 30 +++++++++++++++++++++++--- template/en/default/flag/list.html.tmpl | 38 ++++++++++++++++++++------------- 5 files changed, 59 insertions(+), 28 deletions(-) diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index 2b2159346..f4f325908 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -713,28 +713,27 @@ sub get_attachments_by_bug { =pod -=item C +=item C Description: validates if the user is allowed to view and edit the attachment. Only the submitter or someone with editbugs privs can edit it. Only the submitter and users in the insider group can view private attachments. -Params: $attachment - the attachment object being edited. - $product_id - the product ID the attachment belongs to. +Params: none Returns: 1 on success, 0 otherwise. =cut sub validate_can_edit { - my ($attachment, $product_id) = @_; + my $self = shift; my $user = Bugzilla->user; # The submitter can edit their attachments. - return ($attachment->attacher->id == $user->id - || ((!$attachment->isprivate || $user->is_insider) - && $user->in_group('editbugs', $product_id))) ? 1 : 0; + return ($self->attacher->id == $user->id + || ((!$self->isprivate || $user->is_insider) + && $user->in_group('editbugs', $self->bug->product_id))) ? 1 : 0; } =item C @@ -771,7 +770,7 @@ sub validate_obsolete { || ThrowUserError('invalid_attach_id', $vars); # Check that the user can view and edit this attachment. - $attachment->validate_can_edit($bug->product_id) + $attachment->validate_can_edit || ThrowUserError('illegal_attachment_edit', { attach_id => $attachment->id }); if ($attachment->bug_id != $bug->bug_id) { diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index c3f7a2df7..60da3fb44 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -849,7 +849,7 @@ sub extract_flags_from_cgi { # Extract a list of existing flag IDs. my @flag_ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param()); - return () if (!scalar(@flagtype_ids) && !scalar(@flag_ids)); + return ([], []) unless (scalar(@flagtype_ids) || scalar(@flag_ids)); my (@new_flags, @flags); foreach my $flag_id (@flag_ids) { diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index 80a055ae2..ef3b14f63 100644 --- a/Bugzilla/WebService/Bug.pm +++ b/Bugzilla/WebService/Bug.pm @@ -909,7 +909,7 @@ sub update_attachment { || ThrowUserError("invalid_attach_id", { attach_id => $id }); my $bug = $attachment->bug; $attachment->_check_bug; - $attachment->validate_can_edit($bug->product_id) + $attachment->validate_can_edit || ThrowUserError("illegal_attachment_edit", { attach_id => $id }); push @attachments, $attachment; diff --git a/attachment.cgi b/attachment.cgi index 18ac6f57a..3ff16eb44 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -668,7 +668,7 @@ sub update { my $attachment = validateID(); my $bug = $attachment->bug; $attachment->_check_bug; - my $can_edit = $attachment->validate_can_edit($bug->product_id); + my $can_edit = $attachment->validate_can_edit; if ($can_edit) { $attachment->set_description(scalar $cgi->param('description')); @@ -721,11 +721,35 @@ sub update { extra_data => $attachment->id }); } + my ($flags, $new_flags) = + Bugzilla::Flag->extract_flags_from_cgi($bug, $attachment, $vars); + if ($can_edit) { - my ($flags, $new_flags) = - Bugzilla::Flag->extract_flags_from_cgi($bug, $attachment, $vars); $attachment->set_flags($flags, $new_flags); } + # Requestees can set flags targetted to them, even if they cannot + # edit the attachment. Flag setters can edit their own flags too. + elsif (scalar @$flags) { + my @flag_ids = map { $_->{id} } @$flags; + my $flag_objs = Bugzilla::Flag->new_from_list(\@flag_ids); + my %flag_list = map { $_->id => $_ } @$flag_objs; + + my @editable_flags; + foreach my $flag (@$flags) { + my $flag_obj = $flag_list{$flag->{id}}; + if ($flag_obj->setter_id == $user->id + || ($flag_obj->requestee_id && $flag_obj->requestee_id == $user->id)) + { + push(@editable_flags, $flag); + } + } + + if (scalar @editable_flags) { + $attachment->set_flags(\@editable_flags, []); + # Flag changes must be committed. + $can_edit = 1; + } + } # Figure out when the changes were made. my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)'); diff --git a/template/en/default/flag/list.html.tmpl b/template/en/default/flag/list.html.tmpl index ecc919a38..0d84e9bff 100644 --- a/template/en/default/flag/list.html.tmpl +++ b/template/en/default/flag/list.html.tmpl @@ -18,7 +18,7 @@ # Contributor(s): Myk Melez #%] -[% IF user.id && !read_only_flags && (!bug || bug.check_can_change_field('flagtypes.name', 0, 1)) %] +[% IF user.id && (!bug || bug.check_can_change_field('flagtypes.name', 0, 1)) %] [%# We list flags by looping twice over the flag types relevant for the bug. # In the first loop, we display existing flags and then, for active types, @@ -53,7 +53,9 @@ [% FOREACH flag = type.flags %] [% PROCESS flag_row flag = flag type = type %] [% END -%] + [% SET flag = "" %] + [% NEXT IF read_only_flags %] [%-# Step 1b: Display UI for setting flag. %] [% IF (!type.flags || type.flags.size == 0) && type.is_active %] @@ -61,16 +63,18 @@ [% END %] [% END %] - [%# Step 2: Display flag type again (if type is multiplicable). %] - [% FOREACH type = flag_types %] - [% NEXT UNLESS type.flags && type.flags.size > 0 && type.is_multiplicable && type.is_active %] - [% IF !separator_displayed %] - -
- - [% separator_displayed = 1 %] + [% IF !read_only_flags %] + [%# Step 2: Display flag type again (if type is multiplicable). %] + [% FOREACH type = flag_types %] + [% NEXT UNLESS type.flags && type.flags.size > 0 && type.is_multiplicable && type.is_active %] + [% IF !separator_displayed %] + +
+ + [% separator_displayed = 1 %] + [% END %] + [% PROCESS flag_row type = type addl_text = "addl." %] [% END %] - [% PROCESS flag_row type = type addl_text = "addl." %] [% END %] @@ -104,6 +108,7 @@ [% BLOCK flag_row %] [% SET fid = flag ? "flag-$flag.id" : "flag_type-$type.id" %] + [% can_edit_flag = (!read_only_flags || (flag && (flag.setter_id == user.id || (flag.requestee_id && flag.requestee_id == user.id)))) ? 1 : 0 %] @@ -125,12 +130,13 @@ [% END %] title="[% type.description FILTER html %]" onchange="toggleRequesteeField(this);" - class="flag_select flag_type-[% type.id %]"> + class="flag_select flag_type-[% type.id %]" + [% IF !can_edit_flag %] disabled="disabled"[% END %]> [%# Only display statuses the user is allowed to set. %] - [% IF !flag || user.can_request_flag(type) || flag.setter_id == user.id %] + [% IF !flag || (can_edit_flag && user.can_request_flag(type)) || flag.setter_id == user.id %] [% END %] - [% IF type.is_active %] + [% IF type.is_active && can_edit_flag %] [% IF (type.is_requestable && user.can_request_flag(type)) || (flag && flag.status == "?") %] [% END %] @@ -151,12 +157,13 @@ [% SET grant_list = [] %] [% IF Param('usemenuforusers') %] - [% grant_list = type.grant_list %] - [% IF flag && !(type.is_active && type.is_requestable && type.is_requesteeble) %] + [% IF !can_edit_flag || (flag && !(type.is_active && type.is_requestable && type.is_requesteeble)) %] [%# We are here only because there was already a requestee. In this case, the only valid action is to remove the requestee or leave it alone; nothing else. %] [% grant_list = [flag.requestee] %] + [% ELSE %] + [% grant_list = type.grant_list %] [% END %] [% END %] [% SET flag_name = flag ? "requestee-$flag.id" : "requestee_type-$type.id" %] @@ -171,6 +178,7 @@ emptyok => flag_empty_ok classes => ["requestee"] custom_userlist => grant_list + disabled => !can_edit_flag %] [% Hook.process("requestee", "flag/list.html.tmpl") %] -- cgit v1.2.3-24-g4f1b