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

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

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.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: implements behavior

Here's the patch that makes the fix.  mkanat, wanna take a look at it?

Comment 2

[Frédéric Buclin]

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 ?.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: addresses lpsolit's review comments

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v3: corrects code comment

Comment 5

[Frédéric Buclin]

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

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v4: updates error message

Comment 7

[Frédéric Buclin]

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

Comment 8

[Frédéric Buclin]

(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...

Comment 9

[Myk Melez [:myk] [@mykmelez]]

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

Comment 10

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v5: the version that actually got checked in to the repository