Closed Bug 465589 Opened 16 years ago Closed 15 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
Bugzilla 3.2

People

(Reporter: eirens, Assigned: LpSolit)

References

Details

Attachments

(1 file, 4 obsolete files)

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.
OS: Mac OS X → Linux
Version: unspecified → 3.2
Um, that is surprising. If this is true, it's a blocker for 3.2.
Flags: blocking3.2?
Keywords: qawanted
OS: Linux → All
Hardware: PC → All
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).
Assignee: ui → create-and-change
Status: UNCONFIRMED → NEW
Component: User Interface → Creating/Changing Bugs
Depends on: 339437
Ever confirmed: true
Flags: blocking3.2? → blocking3.2+
Keywords: qawantedregression
Target Milestone: --- → Bugzilla 3.2
Flags: testcase?
(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.
Depends on: 251669
No longer depends on: 339437
Keywords: regression
If it's not a regression, though, it's not a blocker.
Flags: blocking3.2+ → blocking3.2-
Assignee: create-and-change → LpSolit
Priority: -- → P1
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!
Attachment #352167 - Flags: review?(LpSolit)
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.
Attachment #352167 - Flags: review?(LpSolit) → review-
Attached patch patch, v1 (obsolete) — Splinter Review
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.
Attachment #352167 - Attachment is obsolete: true
Attachment #353065 - Flags: review?(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.
Attachment #353065 - Flags: review?(wicked) → review-
(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.
Status: NEW → ASSIGNED
Attached patch patch, v2 (obsolete) — Splinter Review
Fixed point 1 reported by wicked. I had to implement a clone() VMethod to not alter $user->get_userlist.
Attachment #353065 - Attachment is obsolete: true
Attachment #354565 - Flags: review?(wicked)
Attached patch patch, v2.1 (obsolete) — Splinter Review
Avoid duplicated code to populate assignees and QA contacts lists.
Attachment #354565 - Attachment is obsolete: true
Attachment #354567 - Flags: review?(wicked)
Attachment #354565 - Flags: review?(wicked)
Attached patch patch, v2.2Splinter Review
Bah, I must write RETURN instead of NEXT in a BLOCK.
Attachment #354567 - Attachment is obsolete: true
Attachment #354568 - Flags: review?(wicked)
Attachment #354567 - Flags: review?(wicked)
Attachment #354568 - Flags: review?(wicked) → review+
Flags: approval?
Flags: approval3.2?
Flags: approval?
Flags: approval3.2?
Flags: approval3.2+
Flags: approval+
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
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Summary: Wrong "Assigned To:" user shown in show_bug.cgi if that user is disabled and usemenuforusers = ON. → Already selected user accounts are no longer displayed when usemenuforusers is enabled and the account has been disabled
Blocks: 482365
You need to log in before you can comment on or make changes to this bug.