Currently, the user name field handler (views_handler_field-user_name.inc) does not properly use format_username(). That means that anyone implementing hook_username_alter() is going to be ignored. Sadness. :-(

CommentFileSizeAuthor
#2 1252538-views-format-username.patch2.58 KBCrell
#1 1252538-views-format-username.patch2.58 KBCrell

Comments

Crell’s picture

Crell commented
Status: Active » Needs review
StatusFileSize
new 1252538-views-format-username.patch2.58 KB

And patch.

In order to call format_username() we need to have a uid, so this patch now forces them to be loaded. I also opted, at Dave Reid's suggestion, to make it configurable in case someone really wants the unformatted name. The default is to use the formatted name as I think that's the more common use case, and what people would expect coming from Drupal 6. I won't be offended if the maintainers want to change that before committing, however. :-)

This patch was rolled against the RC1 release, but I'm assuming it's small enough that it should still apply to -dev. If not, bah. Let me know and I'll try to reroll.

Crell’s picture

Crell commented
StatusFileSize
new 1252538-views-format-username.patch2.58 KB

And of course that && really ought to be a ||. *sigh*

bojanz’s picture

bojanz commented

The idea makes sense, I'm just worried about:

   function render_link($data, $values) {
+    $account = new stdClass();
+    $account->uid = $this->get_value($values, 'uid');
+    $account->name = $this->get_value($values);

We've had bad experience with passing fake entities to core functions. We might want to ensure we're passing the full user account here.
We're already doing it in some other places, since #1172970: provide a unified way to retrieve result entities landed.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Needs work

Sadly the patch is already commited, forgot to update the issue.

Crell’s picture

Crell commented
Title: Usernames not passed thorugh format_username » Pass full user object to format_username()

Um, OK, so I guess this is now a follow-up / clean up issue? Retitling...

I am wary about loading the full user object, on the grounds that it is still at least one IO hit, even if it's better cached now in D7. That's why I didn't do so originally. If the maintainers want to go that route, though, that's their call.

bojanz’s picture

bojanz commented

We can always ignore it until someone complains (works for a lot of things :P).
The existing code works for core, it's just the module's that implement hook_username_alter() who are going to be surprised.

Crell’s picture

Crell commented

They already are, since core passes nodes and comments to format_username() already. (No joke. It's really pathetic how unstandardized that is.) The only guarantee is the presence of a uid and name property, and since nodes, comments, and users all have that, core gleefully passes all of those at various times.

bojanz’s picture

bojanz commented
Status: Needs work » Fixed

Reality is so fun.

And for anyone stumbling on this through search, here's a clarification:

merlinofchaos: And things implementing format_username() should probably just assume it's a broken object and if they really need more than name/uid they have to user_load
davereid: we do that checking in realname.module for D7.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

JvE’s picture

JvE commented

This caused bug #1609088: Undefined index: uid in views_handler_field->get_value() (line 375 of views_handler_field.inc).