Last Comment Bug 537834 - Buglist results using atom ctype do not display users with empty real names
: Buglist results using atom ctype do not display users with empty real names
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Query/Bug List (show other bugs)
: 3.4.4
: All All
: -- normal (vote)
: Bugzilla 3.4
Assigned To: Reed Loden [:reed] (use needinfo?)
: default-qa
Mentors:
https://bugzilla.mozilla.org/buglist....
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-01-04 15:55 PST by Reed Loden [:reed] (use needinfo?)
Modified: 2010-02-28 12:15 PST (History)
2 users (show)
LpSolit: approval+
LpSolit: approval3.6+
LpSolit: approval3.4+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch - v1 (2.57 KB, patch)
2010-01-04 16:43 PST, Reed Loden [:reed] (use needinfo?)
no flags Details | Diff | Review
patch - v2 (3.52 KB, patch)
2010-01-04 17:35 PST, Reed Loden [:reed] (use needinfo?)
LpSolit: review+
Details | Diff | Review

Description Reed Loden [:reed] (use needinfo?) 2010-01-04 15:55:58 PST
It seems that if the reporter or assignee does not have a real name set, nothing is displayed in Atom results. If I go to the buglist link in the URL field, I see Atom results like the following:

    <author>
      <name></name>
    </author>
...
      &lt;/tr&gt;&lt;tr class=&quot;bz_feed_reporter&quot;&gt;
        &lt;td&gt;Reporter&lt;/td&gt;
        &lt;td&gt;&lt;/td&gt;

The reporter for bug 537663 does not have a real name set.

Here's some code from buglist.cgi...

    my @email_fields = qw(assigned_to reporter qa_contact);
...
    foreach my $col (@email_fields) {
        my $sql = "map_${col}.login_name";
        if (!Bugzilla->user->id) {
             $sql = $dbh->sql_string_until($sql, $dbh->quote('@'));
        }
        $special_sql{$col} = $sql;
        $columns{"${col}_realname"}->{name} = "map_${col}.realname";
    }

It's only looking at .realname, which can be empty.
Comment 1 Frédéric Buclin 2010-01-04 16:03:59 PST
That's not what I call a major bug. The realname is not displayed, that's all. If the whole bug was missing from the buglist, this would be another story.
Comment 2 Reed Loden [:reed] (use needinfo?) 2010-01-04 16:09:51 PST
(In reply to comment #1)
> That's not what I call a major bug. The realname is not displayed, that's all.
> If the whole bug was missing from the buglist, this would be another story.

Except that if there isn't a realname, _nothing_ is displayed, which can cause problems to people using that format for things.
Comment 3 Frédéric Buclin 2010-01-04 16:19:52 PST
(In reply to comment #2)
> Except that if there isn't a realname, _nothing_ is displayed, which can cause
> problems to people using that format for things.

Nothing as in the guy's identity isn't displayed, or as in the bug is totally missing from the list?
Comment 4 Reed Loden [:reed] (use needinfo?) 2010-01-04 16:26:49 PST
(In reply to comment #3)
> Nothing as in the guy's identity isn't displayed, or as in the bug is totally
> missing from the list?

The guy's identity isn't displayed.
Comment 5 Reed Loden [:reed] (use needinfo?) 2010-01-04 16:43:55 PST
Created attachment 419996 [details] [diff] [review]
patch - v1

Something like this...
Comment 6 Frédéric Buclin 2010-01-04 17:01:59 PST
For the record, this bug exists since Bugzilla 2.20, see bug 82878. From what I can read in buglist.cgi line 730, we append required fields for Atom *after* the fallback added at line 705, so bug.assignee and bug.reporter could as well be undefined in the Atom template. Please check this point.
Comment 7 Reed Loden [:reed] (use needinfo?) 2010-01-04 17:35:32 PST
Created attachment 420008 [details] [diff] [review]
patch - v2

Indeed, added reporter and assigned_to to that list and tested it on landfill. Works.
Comment 8 Max Kanat-Alexander 2010-01-04 18:41:56 PST
Comment on attachment 420008 [details] [diff] [review]
patch - v2

Those need to be email-filtered, no?
Comment 9 Reed Loden [:reed] (use needinfo?) 2010-01-04 19:09:57 PST
(In reply to comment #8)
> (From update of attachment 420008 [details] [diff] [review])
> Those need to be email-filtered, no?

I don't think it needs to be, no, as Util::email_filter() just does the same thing that the buglist.cgi does earlier:
        if (!Bugzilla->user->id) {
             $sql = $dbh->sql_string_until($sql, $dbh->quote('@'));
        }

Is there some other use case of which I should be aware that would apply here?
Comment 10 Reed Loden [:reed] (use needinfo?) 2010-01-04 19:10:25 PST
(In reply to comment #9)
> I don't think it needs to be, no, as Util::email_filter() just does the same
> thing that the buglist.cgi does earlier:

s/the/this/
Comment 11 Max Kanat-Alexander 2010-01-04 19:49:32 PST
(In reply to comment #9)
> Is there some other use case of which I should be aware that would apply here?

  Ah, no, that handles it. That should be fine. :-)
Comment 12 Frédéric Buclin 2010-02-28 11:16:08 PST
Comment on attachment 420008 [details] [diff] [review]
patch - v2

r=Lpsolit
Comment 13 Frédéric Buclin 2010-02-28 11:22:03 PST
You will have to fix the bitrot for 3.4. You use so many context lines that you catch unrelated changes.
Comment 14 Reed Loden [:reed] (use needinfo?) 2010-02-28 12:15:56 PST
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/                       
modified buglist.cgi
modified template/en/default/list/list.atom.tmpl
Committed revision 7033.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.6/                         
modified buglist.cgi
modified template/en/default/list/list.atom.tmpl
Committed revision 7005.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.4/                         
modified buglist.cgi
modified template/en/default/list/list.atom.tmpl
Committed revision 6732.

Note You need to log in before you can comment on or make changes to this bug.