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

RESOLVED FIXED

Status

()

bugzilla.mozilla.org
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Florian Bender, Assigned: glob)

Tracking

Production

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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).
(Reporter)

Comment 1

5 years ago
The needinfo? flag from the summary area above works as expected for me, only the ni? field below the comment field is ignored.
(Assignee)

Updated

5 years ago
Assignee: nobody → glob
(Assignee)

Comment 2

5 years ago
confirming.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 3

5 years ago
Created attachment 820362 [details] [diff] [review]
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.
Attachment #820362 - Flags: review?(dkl)
(Assignee)

Updated

5 years ago
Summary: needinfo below comment field is ignored → users without canconfirm cannot set needinfo, and can clear needinfo requests which aren't targeted at them
(Assignee)

Updated

5 years ago
Duplicate of this bug: 923603
(Assignee)

Comment 5

5 years ago
Created attachment 824001 [details] [diff] [review]
927778_2.patch
Attachment #820362 - Attachment is obsolete: true
Attachment #820362 - Flags: review?(dkl)
Attachment #824001 - Flags: review?(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.
Attachment #824001 - Flags: review?(dkl) → review+
(Assignee)

Comment 7

5 years ago
(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://bjones%40mozilla.com@bzr.mozilla.org/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.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.