698697 - User autocomplete on b.m.o. returns disabled users

Status: RESOLVED FIXED

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

Description

[Gervase Markham [:gerv]]

User autocomplete on b.m.o. returns disabled users. (It works on landfill's 4.0 install.) This bug has caused someone to CC the "wrong me" on a bug.

Failure example:

* Visit https://bugzilla.mozilla.org/show_bug.cgi?id=687165
* Open CC edit box
* Type "gerv"
* Note presence of 4 accounts marked "Not Gerv's Account", all of which are disabled, 
  plus one account belonging to another person

Working example:

* Visit https://landfill.bugzilla.org/bugzilla-4.0-branch/show_bug.cgi?id=1
* Open CC edit box
* Type "gervase"
* Note presence of 2 accounts, but not the disabled account 
  "[e-mail]"

js/field.js instantiates the JSON-RPC request for this around line 680, and sets include_disabled to 1, so I'm not immediately sure what's wrong.

Gerv

Comment 1

[:glob ✱]

(In reply to Gervase Markham [:gerv] from comment #0)
> User autocomplete on b.m.o. returns disabled users. (It works on landfill's
> 4.0 install.) This bug has caused someone to CC the "wrong me" on a bug.
>
> js/field.js instantiates the JSON-RPC request for this around line 680, and
> sets include_disabled to 1, so I'm not immediately sure what's wrong.

excluding disabled users resulted in an unacceptable performance hit.
see bug 670251 and bug 658929

looks like we should backport the 4.2 fix to bmo, which involves a schema change.

Comment 2

[Gervase Markham [:gerv]]

:glob: Well, the arguments against a backport advanced in those bugs are still reasonable. Perhaps we should instead focus on either helping Max push 4.2 out of the door, or even upgrading to 4.2 before release in order to dogfood it?

Gerv

Comment 3

[:glob ✱]

(In reply to Gervase Markham [:gerv] from comment #2)
> :glob: Well, the arguments against a backport advanced in those bugs are
> still reasonable. Perhaps we should instead focus on either helping Max push
> 4.2 out of the door, or even upgrading to 4.2 before release in order to
> dogfood it?

i'm helping max and lpsolit out where i can (as is dkl).

i'm firmly against running a pre-release of bugzilla in production; i don't think the risks are worth the rewards.

Comment 4

[David Lawrence [:dkl]]

Attachment: Patch to add is_enabled to profiles table for bmo (v1)

Here is a patch specific for BMO that backports the fix for 4.2 and above. We should think about doing this since we are not going to run an unreleased version of Bugzilla and it is hard to say for sure when 4.2 will be officially released.

./checksetup.pl on my local copy:
real	1m31.794s
user	0m6.993s
sys	0m0.446s

I don't feel like this will be a major outage for BMO as we are not changing any schema related to larger tables such as bugs, longdescs or bugs_activity.

Upon passing review, we could get this pushed out on thursday or monday during an off-peak time if IT can get to it.

dkl

Comment 5

[:glob ✱]

Comment on attachment 571153 [details] [diff] [review]
Patch to add is_enabled to profiles table for bmo (v1)

r=glob

please hold off committing this until we've scheduled the outage :)

Comment 6

[:glob ✱]

committed to obtain timings from staging:

Committing to: bzr+ssh://[e-mail]/bmo/4.0/
modified editusers.cgi
modified token.cgi
modified Bugzilla/Auth.pm
modified Bugzilla/User.pm
modified Bugzilla/DB/Schema.pm
modified Bugzilla/Install/DB.pm
modified Bugzilla/WebService/User.pm
modified js/field.js
modified template/en/default/admin/users/list.html.tmpl
modified template/en/default/global/messages.html.tmpl
Committed revision 7975.

Comment 7

[:glob ✱]

this is now live :)