Closed Bug 850986 Opened 8 years ago Closed 8 years ago

don't allow setting a flag's requestee to a disabled account

Categories

(Bugzilla :: Attachments & Requests, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 5.0

People

(Reporter: glob, Assigned: glob)

Details

Attachments

(1 file)

currently you can set a flag's requestee to a disabled account.

i don't think this should be allowed -- that account will have no ability to action the request.
This is trivial to implement (the check is done in Bugzilla::Flag::_check_requestee), but I don't see why Bugzilla should behave differently with requestees compared the other user fields. It's legal to set a disabled account as assignee, QA contact or CC member. I could imagine that some installations use some generic reviewers@foo.com user account as requestee, which is disabled, and which points to a mailing-list that all reviewers are members of, so that they all get notifications. I don't know if some installations work this way, but who knows?

Also, the fact that the auto-completion feature excludes disabled accounts makes this RFE less necessary. You would have to know and to type the full email address in the requestee field.
Assignee: general → attach-and-request
Component: Bugzilla-General → Attachments & Requests
Attached patch patch v1Splinter Review
Attachment #725271 - Flags: review?(dkl)
Comment on attachment 725271 [details] [diff] [review]
patch v1

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

Looks good and works in my testing. r=dkl
Attachment #725271 - Flags: review?(dkl) → review+
Flags: approval?
Flags: approval4.4?
FWIW, I vote for a- based on comment #1.
what i think makes flags different from other user fields is it's an explicitly targeted request requiring action.  the setter's expectation is that the requestee has been notified, and in many cases, the bug will stall at this point.

> You would have to know and to type the full email address in the requestee field.

why should a full username work, while a partial (but unique) username fail?
I won't approve it for 4.4. I prefer a full development cycle for such a change and see the feedback we get. But even for 5.0, I will let justdave decide. I'm not a fan of such a restriction despite I understand the goal of it.
Assignee: attach-and-request → glob
Status: NEW → ASSIGNED
Flags: approval4.4? → approval4.4-
Target Milestone: --- → Bugzilla 5.0
Flags: needinfo?(justdave)
I think this makes sense, but it counts as a feature change, thus ineligible for 4.4 because it's already rc.  I'm fine with this going on trunk though.
Flags: needinfo?(justdave)
per comment 7, guess I should have gone ahead and set the flag.

Looking at the diff, it looks like deleting 2 lines of code is all you have to do to revert this is someone was actually using it with the disabled user representing a list method.  Although I would think a mailing list being intentionally used that way would be left enabled so it would show up in autocomplete.
Flags: approval? → approval+
(In reply to Dave Miller from comment #8)
> Although I would think a mailing list being
> intentionally used that way would be left enabled

No, it wouldn't, else everybody would be able to log in using this account as the "forgot password" email would be sent to this address.
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Flag.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 8652.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.