189366 - real name columns should fall back to e-mail name

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[Brant Gurganus]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030115
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030115

When real names are displayed in columns and the real name is blank, it needs to
fall back to the e-mail name.

Reproducible: Always

Steps to Reproduce:
1. Display a variant of the real names (QA, for example) column in the buglist.
Actual Results:  
There will be blanks.

Expected Results:  
The blanks should be replaced by the e-mail ID.

dupes searched for using "real name column"

Comment 1

[Dave Miller [:justdave]]

*** Bug 258200 has been marked as a duplicate of this bug. ***

Comment 2

[Marc Schumann [:Wurblzap]]

Attachment: Fall back on e-mail addresses

Comment 3

[Frédéric Buclin]

Comment on attachment 158048 [details] [diff] [review]
Fall back on e-mail addresses

>+# Select e-mail addresses to be able to fall back (bug 189366)
>+if (lsearch(\@displaycolumns, "assigned_to_realname") >= 0) {
>+    push (@selectcolumns, "assigned_to");
>+}
>+if (lsearch(\@displaycolumns, "reporter_realname") >= 0) {
>+    push (@selectcolumns, "reporter");
>+}
>+if (lsearch(\@displaycolumns, "qa_contact_realname") >= 0) {
>+    push (@selectcolumns, "qa_contact");
>+}


Another possibility would be to alter the request in order to have something
like:
SELECT CASE WHEN realname = "" THEN login_name ELSE realname END FROM profiles;

But I'm not sure this is faster and lighter (well, you have less informations
in memory and you don't need to alter the template, that could be a good
point). So I'm fine with your solution.

r=LpSolit

Comment 4

[Marc Schumann [:Wurblzap]]

Comment on attachment 158048 [details] [diff] [review]
Fall back on e-mail addresses

(In reply to comment #3)
> SELECT CASE WHEN realname = "" THEN login_name ELSE realname END FROM profiles;

I like that. It turns out it's even cross-DB compatible -- see bug 182136 and
bug 253360. Let's do it. I'll attach a patch.

Comment 5

[Marc Schumann [:Wurblzap]]

Comment on attachment 158048 [details] [diff] [review]
Fall back on e-mail addresses

Well, well, that's what I get for talking without thinking. It turns out
implementing this would require hacking Search.pm, which I deem to risky. So,
let's go with what we have :)

Comment 6

[Frédéric Buclin]

OK, and we can open a new bug later if someone wants to play with it; kind of
improvement of this new feature.

joel, is that really hard and risky to implement?

Comment 7

[Frédéric Buclin]

Humm... Looking at buglist.cgi in greater details, it seems that we could alter
the "Name" column at lines 457-461 for the 3 "map_*.realnames" in order to fall
back to the email address if no real name is set.

What about if we replace "map_*.realname" by "CASE WHEN map_*.realname="" THEN
map_*.login_name ELSE map_*.realname END" ?

Here, * means reporter, qa_contact or assigned_to.

If that works (I haven't tested it), there is no need to alter templates, nor to
make some modifications in Search.pm.

Suggesting this to Marc on IRC, he mentioned the problem if we use the XML or
CSV format where this fall back is not desired. Well, adding a condition to
DefineColumn() like if ($format != "html") {no fall back} should work. :)

Comment 8

[Joel Peshkin]

For display, it doesn't look like a problem.

Comment 9

[Marc Schumann [:Wurblzap]]

Attachment: SQLly solution

Using CASE WHEN.

Comment 10

[Joel Peshkin]

Comment on attachment 171897 [details] [diff] [review]
SQLly solution

r=joel by inspetion, but I'd like to see someone voich for having tested it
pretty thoroughly before it goes in.

Comment 11

[Marc Schumann [:Wurblzap]]

Tested at time of attaching the patch:

- Showing a bug list without any e-mail or real name fields
- Showing a bug list with an e-mail field
- Showing a bug list with a real name field
- Showing a bug list with all e-mail fields
- Showing a bug list with all real name fields

Additionally, a visual check (read: grep) was performed on buglist.cgi and
Bugzilla/Search.pm, looking for map_assigned_to, map_reporter and map_qa_contact
that might conflict with the replacement of lsearch by grep.

Tests and check showed no errors or conflicts, respectively.

Comment 12

[Marc Schumann [:Wurblzap]]

I omitted to confirm that I tested the following combinations:

- Showing a bug list with one e-mail and the corresponding real name field
- Showing a bug list with one e-mail and one non-corresponding real name field
- Showing a bug list with all e-mail and all real name fields

Comment 13

[Frédéric Buclin]

Comment on attachment 171897 [details] [diff] [review]
SQLly solution

I have done some tests too, with and without login name / real name / both /
none using query.cgi (also boolean charts). I also tested request.cgi and
attachment.cgi as they use login and/or real names. I have seen no problem.

Comment 14

[Shane H. W. Travis]

Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.278; previous revision: 1.277
done
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.75; previous revision: 1.74
done