465589 - Already selected user accounts are no longer displayed when usemenuforusers is enabled and the account has been disabled

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect, P1)

Description

[Eiren Smith]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_5_5; en-us) AppleWebKit/525.26.2 (KHTML, like Gecko) Version/3.2 Safari/525.26.12
Build Identifier: 

Bugzilla 3.2RC2 from CVS. No patches.

Only happens if the user is disabled.

If I enable that user (by deleting all "Disable text:" in editusers.cgi) then I see the correct Assignee (him) in edit_bugs.cgi.

Also, if I turn the usemenuforusers Parameter OFF, I see the correct Assignee (him) in edit_bugs.cgi.

I did not notice this behaviour in 3.0.3 nor in 3.0.5.

Reproducible: Always

Steps to Reproduce:
1. Turn usemenuforusers Parameter ON.
2. Find a bug assigned to a disabled user.
3. View that bug in show_bug.cgi.
Actual Results:  
N.B. Because usemenuforusers is ON, there's a list of users to choose from next to "Assigned To:".

Observed (incorrect) behavior: The wrong user is shown as the assignee in that list of users. And if you click on that control to see the whole list of assignable users, you'll see that there is even a check mark next to that wrong user. That wrong user is whoever happens to be at the top of this list of users.

Expected Results:  
The assigned (disabled) user should show as the assignee in the list of users next to "Assigned To:" in show_bugs.cgi when usermenuforusers Parameter is ON.

The assigned (disabled) user is not visible in that list of assignable users at all.

The behaviour I'm reporting here is especially surprising since bug 97662 is marked WONTFIX.

We really like usemenuforusers but this bug makes it more trouble than help for us so I'll likely need to turn it off until this is remedied.

Comment 1

[Max Kanat-Alexander]

Um, that is surprising. If this is true, it's a blocker for 3.2.

Comment 2

[Frédéric Buclin]

OK, this is a regression due to bug 339437. show_bug.cgi doesn't define custom_userlist and so global/userselect.html.tmpl falls back to $user->get_userlist to populate the user menu. But this user method excludes disabled accounts, explaining why this user account is never displayed in the menu.

bug/edit.html.tmpl correctly passes the current assignee to userselect.html.tmpl, but the current value is only used to decide which option in the drop-down menu should have the 'selected' attribute set. It would probably make more sense to append the default value to the list if it hasn't been found in the given list; this way, we are sure the current value is always available (and it's not invasive).

Comment 3

[Frédéric Buclin]

(In reply to comment #2)
> OK, this is a regression due to bug 339437.

I take that back. This bug is in no way responsible for this issue. This problem has always been here, since the 'usemenuforusers' feature has been implemented in Bugzilla 2.20. I still think we should fix it for 3.2.

Comment 4

[Max Kanat-Alexander]

If it's not a regression, though, it's not a blocker.

Comment 5

[Mark Ruedy]

Attachment: Patch to include bugs current but disabled assigned_to/qacontact in user menus

Because this is a blocker for me to deploy 3.2, I devised this patch.  It's not gone through a whole lot of testing but it seems to work in my setup.

Let me know if you need anything else from me on this, otherwise... Enjoy!

Comment 6

[Frédéric Buclin]

Comment on attachment 352167 [details] [diff] [review]
Patch to include bugs current but disabled assigned_to/qacontact in user menus

This is only a partial fix on a single page. I have a patch which fixes all user-select boxes at once.

Comment 7

[Frédéric Buclin]

Attachment: patch, v1

This fix no longer needs the do_not_change hack, nor does it need useless empty values (either an empty value is legal and we set emptyok => 1, or we don't pass value => "" at all, as "" is not legal).

If an originally selected user account has not been listed, the userselect template now appends it to the end of the list and selects it. This works well with the do_not_change value as this one is not a valid user account and so hasn't been listed yet; it will be automatically appended and selected, as desired.

Comment 8

[Teemu Mannermaa (:wicked)]

Comment on attachment 353065 [details] [diff] [review]
patch, v1

1) This does not fix one case of the reported problem. That happens when a component has a default assign to or QA contact set to a disabled user. In this case enter_bug.cgi uses JS to populate the assigned_to/qa_contact fields based on the selected component. Unfortunately JS code fails to change the selection as there is no corresponding value in the list. To make things worse, this could lead to bug filed with incorrect default values because values from previously selected component (that doesn't have disabled users selected) remain in the select fields.

2) I don't think I like the way this moves the --do_not_change-- special value (as well as the missing values) to the bottom of the user list instead of top as it used to be.

3) This does not provide real name information for the missing users but I guess we can live without that for these special cases.

I did test your patch and it does fix most cases of the reported problem but I'll r- this for now in case you want to fix at least point 1.

Comment 9

[Frédéric Buclin]

(In reply to comment #8)
> I'll r- this for now in case you want to fix at least point 1.

Yeah, as I said on IRC yesterday, points 2 and 3 are intentional, so I don't plan to address them. I will investigate point 1, though.

Comment 10

[Frédéric Buclin]

Attachment: patch, v2

Fixed point 1 reported by wicked. I had to implement a clone() VMethod to not alter $user->get_userlist.

Comment 11

[Frédéric Buclin]

Attachment: patch, v2.1

Avoid duplicated code to populate assignees and QA contacts lists.

Comment 12

[Frédéric Buclin]

Attachment: patch, v2.2

Bah, I must write RETURN instead of NEXT in a BLOCK.

Comment 13

[Frédéric Buclin]

I hope this new bug summary is clear and general enough.

tip:

Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.94; previous revision: 1.93
done
Checking in t/008filter.t;
/cvsroot/mozilla/webtools/bugzilla/t/008filter.t,v  <--  008filter.t
new revision: 1.29; previous revision: 1.28
done
Checking in template/en/default/admin/components/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/components/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.16; previous revision: 1.15
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.144; previous revision: 1.143
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v  <-- create.html.tmpl
new revision: 1.91; previous revision: 1.90
done
Checking in template/en/default/global/userselect.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/userselect.html.tmpl,v  <-- userselect.html.tmpl
new revision: 1.10; previous revision: 1.9
done
Checking in template/en/default/list/edit-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/edit-multiple.html.tmpl,v  <--  edit-multiple.html.tmpl
new revision: 1.53; previous revision: 1.52
done


3.2:

Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.89.2.1; previous revision: 1.89
done
Checking in t/008filter.t;
/cvsroot/mozilla/webtools/bugzilla/t/008filter.t,v  <--  008filter.t
new revision: 1.28.2.1; previous revision: 1.28
done
Checking in template/en/default/admin/components/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/components/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.14.2.2; previous revision: 1.14.2.1
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.125.2.16; previous revision: 1.125.2.15
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v  <-- create.html.tmpl
new revision: 1.83.2.6; previous revision: 1.83.2.5
done
Checking in template/en/default/global/userselect.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/userselect.html.tmpl,v  <-- userselect.html.tmpl
new revision: 1.9.2.1; previous revision: 1.9
done
Checking in template/en/default/list/edit-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/edit-multiple.html.tmpl,v  <--  edit-multiple.html.tmpl
new revision: 1.49.2.3; previous revision: 1.49.2.2
done