927778 - users without canconfirm cannot set needinfo, and can clear needinfo requests which aren't targeted at them

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: General, defect)

Description

[Florian Bender]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:25.0) Gecko/20100101 Firefox/25.0 (Beta/Release)
Build ID: 20131010180222

Steps to reproduce:

Wrote Bug 704128 Comment 49, entered :doublec under "Need more information from" (this only the most recent example, had this issue since weeks/months). 


Actual results:

When selecting one suggestion, the entry is added to the INPUT and checkbox ticked.

When I clicked "Save changes", the comment was added, but not the ni?. 


Expected results:

ni? should be added alongside the comment.

May be a privilege issue. My account should be pretty standard (regular older account using Persona for login shortly after it became available).

Comment 1

[Florian Bender]

The needinfo? flag from the summary area above works as expected for me, only the ni? field below the comment field is ignored.

Comment 2

[:glob ✱]

confirming.

Comment 3

[:glob ✱]

Attachment: 927778_1.patch

the logic is now:
- anyone can set the needinfo flag
- to clear a needinfo flag, one of the following is required:
  - you set the flag
  - you are the requestee
  - you are in the canconfirm group
  - the flag doesn't have a requestee (ie. "anyone")

note: this also changes the checkbox for needinfo requests of "anyone" to be unchecked by default for users who are not in the needinfo group.

Comment 5

[:glob ✱]

Attachment: 927778_2.patch

Comment 6

[David Lawrence [:dkl]]

Comment on attachment 824001 [details] [diff] [review]
927778_2.patch

Review of attachment 824001 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl

::: extensions/Needinfo/Extension.pm
@@ +105,3 @@
>                  $requestees{$bug->reporter->login} = 1;
>              }
>              # Use qa_contact as requestee

Something I just saw when reviewing is that for consistency, we should wrap the qa_contact part in 

if (Bugzilla->params->{useqacontact} && $bug->qa_contact) { }

We can do as separate bug or work it into this one on commit.

Comment 7

[:glob ✱]

(In reply to David Lawrence [:dkl] from comment #6)
> Something I just saw when reviewing is that for consistency, we should wrap
> the qa_contact part in 

there's no need -- the template won't include the qa_contact option if that param isn't enabled, and there's no gain to checking that param again at that point.

Committing to: bzr+ssh://[e-mail]/bmo/4.2/
modified extensions/Needinfo/Extension.pm
modified extensions/Needinfo/template/en/default/bug/needinfo.html.tmpl
added extensions/Needinfo/template/en/default/hook/global
added extensions/Needinfo/template/en/default/hook/global/user-error-errors.html.tmpl
Committed revision 9111.