users without the privilege to set flags should be allowed to rerequest them

RESOLVED FIXED in Bugzilla 2.20

Status

()

Bugzilla
Attachments & Requests
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: myk, Assigned: myk)

Tracking

2.19
Bugzilla 2.20
Bug Flags:
approval +

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

13 years ago
The group restrictions on setting flags not only prevent unprivileged users from
setting flags, they also prevent such users from requesting them.  But users
have a legitimate need to request reconsideration of a decision recorded by the
setting of a flag.  In some cases, the setter has made a mistake.  In others,
new information has come to light or circumstances have changed.

Group restrictions on setting flags should prevent unprivileged users from
setting flags to + or -, but such users should still be allowed to rerequest the
flags.
(Assignee)

Comment 1

13 years ago
Created attachment 181481 [details] [diff] [review]
patch v1: implements behavior

Here's the patch that makes the fix.  mkanat, wanna take a look at it?
Assignee: attach-and-request → myk
Status: NEW → ASSIGNED
Attachment #181481 - Flags: review?(mkanat)

Comment 2

13 years ago
Comment on attachment 181481 [details] [diff] [review]
patch v1: implements behavior

In a two step process, users in the request group could clear any flag. So
alter the next condition to accept both X and ?.
Attachment #181481 - Flags: review?(mkanat) → review-
(Assignee)

Comment 3

13 years ago
Created attachment 181485 [details] [diff] [review]
patch v2: addresses lpsolit's review comments
Attachment #181481 - Attachment is obsolete: true
Attachment #181485 - Flags: review?(LpSolit)
(Assignee)

Comment 4

13 years ago
Created attachment 181486 [details] [diff] [review]
patch v3: corrects code comment
Attachment #181485 - Attachment is obsolete: true
Attachment #181486 - Flags: review?(LpSolit)
(Assignee)

Updated

13 years ago
Attachment #181485 - Flags: review?(LpSolit)

Comment 5

13 years ago
Comment on attachment 181486 [details] [diff] [review]
patch v3: corrects code comment

The code here in Flag.pm is correct. But error messages I get for users who are
neither in the grant group nor in the request group are incorrect, see
user-error.html.tmpl
Attachment #181486 - Flags: review?(LpSolit) → review-
(Assignee)

Comment 6

13 years ago
Created attachment 181488 [details] [diff] [review]
patch v4: updates error message
Attachment #181486 - Attachment is obsolete: true
Attachment #181488 - Flags: review?(LpSolit)

Comment 7

13 years ago
Comment on attachment 181488 [details] [diff] [review]
patch v4: updates error message

With this patch applied, we no longer need to pass 'old_status' to
user-error.html.tmpl. *Nevertheless* I think it would not hurt to replace
<code>[% name FILTER html %]</code>
by
<code>[% name FILTER html %][% IF status == "X" %][% old_status FILTER html
%][% END %]</code>
so that we also have the status of the bug we try to clear (which I personally
like). This way, there is no need to remove the obsolete parameter. ;)

You could fix that on checkin.

r=LpSolit
Attachment #181488 - Flags: review?(LpSolit) → review+

Updated

13 years ago
Flags: approval?
Target Milestone: --- → Bugzilla 2.20

Comment 8

13 years ago
(In reply to comment #7)
> so that we also have the status of the bug we try to clear (which I personally

the status of the *flag* of course...
(Assignee)

Comment 9

13 years ago
Error message changed upon checkin per your review comment.  Thanks Frederic!

Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.38; previous revision: 1.37
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v
 <--  user-error.html.tmpl
new revision: 1.105; previous revision: 1.104
done
Flags: approval? → approval+
(Assignee)

Updated

13 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Assignee)

Comment 10

13 years ago
Created attachment 181789 [details] [diff] [review]
patch v5: the version that actually got checked in to the repository
Attachment #181488 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Whiteboard: [applied to b.m.o]
Whiteboard: [applied to b.m.o]
You need to log in before you can comment on or make changes to this bug.