Skip to content

Commit 71927e7

Browse files
dklawrendklawren
committed
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
1 parent 8e4cf05 commit 71927e7

File tree

5 files changed

+59
-28
lines changed

5 files changed

+59
-28
lines changed

‎Bugzilla/Attachment.pm‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -713,28 +713,27 @@ sub get_attachments_by_bug {
713713

714714
=pod
715715
716-
=item C<validate_can_edit($attachment, $product_id)>
716+
=item C<validate_can_edit>
717717
718718
Description: validates if the user is allowed to view and edit the attachment.
719719
Only the submitter or someone with editbugs privs can edit it.
720720
Only the submitter and users in the insider group can view
721721
private attachments.
722722
723-
Params: $attachment - the attachment object being edited.
724-
$product_id - the product ID the attachment belongs to.
723+
Params: none
725724
726725
Returns: 1 on success, 0 otherwise.
727726
728727
=cut
729728

730729
sub validate_can_edit {
731-
my ($attachment, $product_id) = @_;
730+
my $self = shift;
732731
my $user = Bugzilla->user;
733732

734733
# The submitter can edit their attachments.
735-
return ($attachment->attacher->id == $user->id
736-
|| ((!$attachment->isprivate || $user->is_insider)
737-
&& $user->in_group('editbugs', $product_id))) ? 1 : 0;
734+
return ($self->attacher->id == $user->id
735+
|| ((!$self->isprivate || $user->is_insider)
736+
&& $user->in_group('editbugs', $self->bug->product_id))) ? 1 : 0;
738737
}
739738

740739
=item C<validate_obsolete($bug, $attach_ids)>
@@ -771,7 +770,7 @@ sub validate_obsolete {
771770
|| ThrowUserError('invalid_attach_id', $vars);
772771

773772
# Check that the user can view and edit this attachment.
774-
$attachment->validate_can_edit($bug->product_id)
773+
$attachment->validate_can_edit
775774
|| ThrowUserError('illegal_attachment_edit', { attach_id => $attachment->id });
776775

777776
if ($attachment->bug_id != $bug->bug_id) {

‎Bugzilla/Flag.pm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -849,7 +849,7 @@ sub extract_flags_from_cgi {
849849
# Extract a list of existing flag IDs.
850850
my @flag_ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param());
851851

852-
return () if (!scalar(@flagtype_ids) && !scalar(@flag_ids));
852+
return ([], []) unless (scalar(@flagtype_ids) || scalar(@flag_ids));
853853

854854
my (@new_flags, @flags);
855855
foreach my $flag_id (@flag_ids) {

‎Bugzilla/WebService/Bug.pm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -909,7 +909,7 @@ sub update_attachment {
909909
|| ThrowUserError("invalid_attach_id", { attach_id => $id });
910910
my $bug = $attachment->bug;
911911
$attachment->_check_bug;
912-
$attachment->validate_can_edit($bug->product_id)
912+
$attachment->validate_can_edit
913913
|| ThrowUserError("illegal_attachment_edit", { attach_id => $id });
914914

915915
push @attachments, $attachment;

‎attachment.cgi‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,7 @@ sub update {
668668
my $attachment = validateID();
669669
my $bug = $attachment->bug;
670670
$attachment->_check_bug;
671-
my $can_edit = $attachment->validate_can_edit($bug->product_id);
671+
my $can_edit = $attachment->validate_can_edit;
672672

673673
if ($can_edit) {
674674
$attachment->set_description(scalar $cgi->param('description'));
@@ -721,11 +721,35 @@ sub update {
721721
extra_data => $attachment->id });
722722
}
723723

724+
my ($flags, $new_flags) =
725+
Bugzilla::Flag->extract_flags_from_cgi($bug, $attachment, $vars);
726+
724727
if ($can_edit) {
725-
my ($flags, $new_flags) =
726-
Bugzilla::Flag->extract_flags_from_cgi($bug, $attachment, $vars);
727728
$attachment->set_flags($flags, $new_flags);
728729
}
730+
# Requestees can set flags targetted to them, even if they cannot
731+
# edit the attachment. Flag setters can edit their own flags too.
732+
elsif (scalar @$flags) {
733+
my @flag_ids = map { $_->{id} } @$flags;
734+
my $flag_objs = Bugzilla::Flag->new_from_list(\@flag_ids);
735+
my %flag_list = map { $_->id => $_ } @$flag_objs;
736+
737+
my @editable_flags;
738+
foreach my $flag (@$flags) {
739+
my $flag_obj = $flag_list{$flag->{id}};
740+
if ($flag_obj->setter_id == $user->id
741+
|| ($flag_obj->requestee_id && $flag_obj->requestee_id == $user->id))
742+
{
743+
push(@editable_flags, $flag);
744+
}
745+
}
746+
747+
if (scalar @editable_flags) {
748+
$attachment->set_flags(\@editable_flags, []);
749+
# Flag changes must be committed.
750+
$can_edit = 1;
751+
}
752+
}
729753

730754
# Figure out when the changes were made.
731755
my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');

‎template/en/default/flag/list.html.tmpl‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
# Contributor(s): Myk Melez <myk@mozilla.org>
1919
#%]
2020

21-
[% IF user.id && !read_only_flags && (!bug || bug.check_can_change_field('flagtypes.name', 0, 1)) %]
21+
[% IF user.id && (!bug || bug.check_can_change_field('flagtypes.name', 0, 1)) %]
2222

2323
[%# We list flags by looping twice over the flag types relevant for the bug.
2424
# In the first loop, we display existing flags and then, for active types,
@@ -53,24 +53,28 @@
5353
[% FOREACH flag = type.flags %]
5454
[% PROCESS flag_row flag = flag type = type %]
5555
[% END -%]
56+
5657
[% SET flag = "" %]
58+
[% NEXT IF read_only_flags %]
5759

5860
[%-# Step 1b: Display UI for setting flag. %]
5961
[% IF (!type.flags || type.flags.size == 0) && type.is_active %]
6062
[% PROCESS flag_row type = type %]
6163
[% END %]
6264
[% END %]
6365

64-
[%# Step 2: Display flag type again (if type is multiplicable). %]
65-
[% FOREACH type = flag_types %]
66-
[% NEXT UNLESS type.flags && type.flags.size > 0 && type.is_multiplicable && type.is_active %]
67-
[% IF !separator_displayed %]
68-
<tbody class="bz_flag_type">
69-
<tr><td colspan="3"><hr></td></tr>
70-
</tbody>
71-
[% separator_displayed = 1 %]
66+
[% IF !read_only_flags %]
67+
[%# Step 2: Display flag type again (if type is multiplicable). %]
68+
[% FOREACH type = flag_types %]
69+
[% NEXT UNLESS type.flags && type.flags.size > 0 && type.is_multiplicable && type.is_active %]
70+
[% IF !separator_displayed %]
71+
<tbody class="bz_flag_type">
72+
<tr><td colspan="3"><hr></td></tr>
73+
</tbody>
74+
[% separator_displayed = 1 %]
75+
[% END %]
76+
[% PROCESS flag_row type = type addl_text = "addl." %]
7277
[% END %]
73-
[% PROCESS flag_row type = type addl_text = "addl." %]
7478
[% END %]
7579
</table>
7680

@@ -104,6 +108,7 @@
104108

105109
[% BLOCK flag_row %]
106110
[% SET fid = flag ? "flag-$flag.id" : "flag_type-$type.id" %]
111+
[% can_edit_flag = (!read_only_flags || (flag && (flag.setter_id == user.id || (flag.requestee_id && flag.requestee_id == user.id)))) ? 1 : 0 %]
107112
<tbody[% ' class="bz_flag_type"' IF !flag %]>
108113
<tr>
109114
<td>
@@ -125,12 +130,13 @@
125130
[% END %]
126131
title="[% type.description FILTER html %]"
127132
onchange="toggleRequesteeField(this);"
128-
class="flag_select flag_type-[% type.id %]">
133+
class="flag_select flag_type-[% type.id %]"
134+
[% IF !can_edit_flag %] disabled="disabled"[% END %]>
129135
[%# Only display statuses the user is allowed to set. %]
130-
[% IF !flag || user.can_request_flag(type) || flag.setter_id == user.id %]
136+
[% IF !flag || (can_edit_flag && user.can_request_flag(type)) || flag.setter_id == user.id %]
131137
<option value="X"></option>
132138
[% END %]
133-
[% IF type.is_active %]
139+
[% IF type.is_active && can_edit_flag %]
134140
[% IF (type.is_requestable && user.can_request_flag(type)) || (flag && flag.status == "?") %]
135141
<option value="?" [% "selected" IF flag && flag.status == "?" %]>?</option>
136142
[% END %]
@@ -151,12 +157,13 @@
151157
<span style="white-space: nowrap;">
152158
[% SET grant_list = [] %]
153159
[% IF Param('usemenuforusers') %]
154-
[% grant_list = type.grant_list %]
155-
[% IF flag && !(type.is_active && type.is_requestable && type.is_requesteeble) %]
160+
[% IF !can_edit_flag || (flag && !(type.is_active && type.is_requestable && type.is_requesteeble)) %]
156161
[%# We are here only because there was already a requestee. In this case,
157162
the only valid action is to remove the requestee or leave it alone;
158163
nothing else. %]
159164
[% grant_list = [flag.requestee] %]
165+
[% ELSE %]
166+
[% grant_list = type.grant_list %]
160167
[% END %]
161168
[% END %]
162169
[% SET flag_name = flag ? "requestee-$flag.id" : "requestee_type-$type.id" %]
@@ -171,6 +178,7 @@
171178
emptyok => flag_empty_ok
172179
classes => ["requestee"]
173180
custom_userlist => grant_list
181+
disabled => !can_edit_flag
174182
%]
175183
[% Hook.process("requestee", "flag/list.html.tmpl") %]
176184
</span>

0 commit comments

Comments
 (0)