Not a bug Some Users Unclickable

[Ozzy47]

Affected version

2.1.8

Since the update some users in the Who Has Visited widget are unclickable. Happens in the default style and with all addons disabled.

 

[Mike]

That's not a standard widget, so it's fairly hard for us to guess what might've happened.

Presumably it's using the <xf:username> tag and I don't believe anything has changed in that.

We'd ideally need to be able to see a reproduction case in a standard widget/location.

 

[Ozzy47]

I'll check it out tomorrow and follow up.

 

[Snog]

I'm finding a lot of odd behavior on my sites.

This particular one is because another add-on is assigning a defaultWith...

public static function getStructure(Structure $structure)
{
    $structure = parent::getStructure($structure);

    $structure->relations['MemberList'] = [
        'entity' => 'Snog\Members:Member',
        'type' => entity::TO_ONE,
        'conditions' => 'user_id',
        'primary' => true
    ];

    $structure->defaultWith[] = 'MemberList';

    return $structure;
}

Which in this case where the username is unclickable, the user_id has been set to NULL when the defaultWith = null (in bold below).

11 => array:56 [▼
"user_id" => null
"username" => "Mandy"
"email" => "XXX@xxx.com"
"custom_title" => "......"
"language_id" => 1
"style_id" => 13
"timezone" => "America/New_York"
"visible" => 1
"activity_visible" => 1
"user_group_id" => 2
"secondary_group_ids" => ""
"display_style_group_id" => 2
"permission_combination_id" => 2
"message_count" => 11831
"conversations_unread" => 1
"register_date" => 1467608891
"last_activity" => 1584587977
"trophy_points" => 140
"alerts_unread" => 2
"avatar_date" => 1583109846
"avatar_width" => 300
"avatar_height" => 373
"avatar_highdpi" => 0
"gravatar" => ""
"user_state" => "valid"
"is_moderator" => 0
"is_admin" => 0
"is_banned" => 0
"reaction_score" => 3995
"warning_points" => 0
"is_staff" => 0
"snog_flag" => "US"
"secret_key" => "xxxxxxx"
"privacy_policy_accepted" => 1526417384
"terms_accepted" => 1554241378
"allow_view_profile" => "members"
"allow_post_profile" => "members"
"allow_send_personal_conversation" => "members""user_id" => null
"allow_view_identities" => "members"
"allow_receive_news_feed" => "members"
"snog_flag_view" => "everyone"
"snog_osb_view" => "everyone"
"snog_groups_view" => "everyone"
"member" => null
"moderator" => null
"owner" => null
"pending" => null
"invites" => null
"parent_id" => 0
"parent_list" => "13,0"
"title" => "Light"
"description" => ""
"properties" => "{"avatarBg":"rgb(255, 255, 255)","badge":{"color":"rgb(255, 255, 255)","background-color":"rgb(28, 28, 28)","border-radius":"2px"},"bbCodeBlock":{"background-co ▶"
"last_modified_date" => 1584624914
"user_selectable" => 1
"designer_mode" => null
]

Removing the defaultWith from the other add-on makes the usernames clickable again.

 

[Mike]

So the first things I notice from your dump is that:

1. This appears to be an array, not an entity.
2. There are clearly multiple tables worth of data here. I can see a mix of user columns and style columns pretty clearly.

So it looks to me like you're running some sort of direct query across multiple tables and combining the results into one flat array based solely on column name. In that case, you appear to have two tables with shared column names. In your example, it would appear to be user_id in the XF user table and presumably your member table. But since the record doesn't exist in your table, it's coming back as null. Given all that, it looks like MySQL is acting as I would expect.

The entity system avoids this by determining what data comes from what table/entity and handling that for you. If you're doing a direct query across tables, you'd normally need to manually handle aliasing to avoid conflicts.

 

[Snog]

The question is, why would this rear it's ugly head now after literally years of working?

 

[Ozzy47]

This was fixed in 2.1.8 PL2 update

 

[Snog]

This was fixed in 2.1.8 PL2 update

Now ain't that interesting.

 

[Chris D]

It is interesting and it may be related to a similar issue that affected template modifications related to the execution order.

Unfortunately that didn’t become clear until late in the day yesterday.

Mike’s comments about what appears to be your approach to using arrays rather than entities in some cases is still very relevant and is very likely something you should address.

Anyway, glad it’s working now.

 

[Snog]

Mike’s comments about what appears to be your approach to using arrays rather than entities in some cases is still very relevant and is very likely something you should address.

The data is cached by simple cache as an array directly from the object. But, that's beside the point. It is working now.

... SNIP
$finder = \XF::finder('XF:User');

$finder->isValidUser()
    ->where('last_activity', '>=', time() - ($period * 3600))
    ->order('last_activity', 'DESC')
    ->limit($limit);

$users = $finder->fetchRaw();

$cacheArray = [
    'users' => $users,
    'last_update' => time(),
    'period' => $period,
    'grandtotal' => $countFinder->total(),
    'update' => $options['update']
];

$app->simpleCache()->getSet('MDATA')->setValue('values', $cacheArray);
$values = $cacheArray;

 

[Mike]

There may be something else at play, but what I explained above still applies. Because you're using fetchRaw, column names across tables that collide will give you the value of the column in the last table. So if you include your MemberList relation by default and a record doesn't exist in that table for a user, the user_id would come out as null.

You'd either want to explicitly bypass the defaultWith options (which is possible via XF::em()->getFinder()) or you'd want to just fetch() from your finder and turn the resulting user entities into an array with the toArray() method entities provide.