Who's online block doesn't work with swappable session backends and lazy session creation

The proposed solution sounds fine to me. I also could see removing this block from core and letting session backend projects ship with a more capable block if they choose.

#335411: Switch to Symfony2-based session handling is not able to support how this currently works either, limiting to logged in users sounds good to me as well.

Yep, sounds like a solution. Either way to fix it, we need this to unblock #2 issue.

Love this idea. I remember loving the who's online block when I created my first Drupal site some 6-7 years ago. Over time I came to realize that I couldn't trust the information that Drupal was sending me because those users weren't really online, they were online at some point (who knows when).

We need to decouple our session handling in order to move forward so let's do this. Should we do it here or the Symfony 2 session issue?

I propose that we do that here. Session suffers from some remaining bugs and one critical feature to restore before being ready for commit, so let's rush this patch here and now.

Ok, I'm a bit confused. I can't find any reference to anonymous user-related features in that block. In fact, it explicitly excludes them everywhere where it can.

Let's see if I missed something but this looks easy enough. Verified that the tests here now also pass with #335411: Switch to Symfony2-based session handling.

You could replace the query by a SELECT u.uid FROM users u WHERE u.timestamp >= :timestamp AND u.uid <> 0 DESC LIMIT 10 instead, then do a foreach(user_load_multiple(db_query($sql, $args)->fetchCol()) as $account) { $build[] = theme('username', array('account' => $account)); }

It will do 2 SQL queries instead of 1, that's not really good, but if the username is not supposed to be displayed differently, this actually will allow modules and theme to override it, using an incomplete $account structure can seriously disturb themes that overrides the theme('username') function.

This is a quick and ugly fix, but we can then introspect later the new Field and Entity APIs improvements and see if we can change that to a single EFQ2 query instead, maybe, for performances, if needed.

Hm, the user block cmi patch landed in the meantime, here's a re-roll.

@pounard: Yeah, that needs to happen, I know, but I first wanted to check that this is passing and then do further refactorings.

Ok, no worries.

Yeah, we broke the anonymous user count with D7 already, and I still didn't manage to update admin_menu for that:
#1733384: anonymous users count is always 0

Why does admin_menu contain those numbers? Simple, and two-fold: For one, my original reason was to see approximately how many visitors/users are currently on the site, which gives a helpful hint at whether it may be safe to perform configuration changes or take the site offline. Second, further real world usage turned out that it's sometimes interesting to see/learn whether you're the only authenticated user on the site, which allows to draw individual conclusions, especially in small site admin/team environments.

Now, the "Who's online" block is a very typical feature of forum sites. (I'd say, >80% actually.)

If you'd ask me, I'd even like to see that on drupal.org — and in fact, the D6 Profile module sorts user profiles by last access time; e.g.: these people are ordered by their last access time on d.o: http://drupal.org/profile/profile_my_mentors/sun

So, if this functionality gets removed from User module, then I'd prefer to move it into a separate core module.

Users are entities, therefore you should be using Entity Field Query TNG to query for them, not raw SQL. If we want quick and dirty for now, we should at least drop a @todo to convert later so that we don't forget.

Catch ideas actually makes a lot of sense.

I'd say we first need to make this patch quick and ugly so it doesn't rely on the {sessions} table, and leave a huge @todo inside: it would unblock the SF session patch. Then, we leave someone (can be one of us) to clean up this feature and re-implement it on a sane basis.

@Berdir @catch @sun @Crell What do you think?

Yes, discussed that with @catch in IRC. I'll do a re-roll (or someone else, will probably not have time today) with a user_load_multiple() and leave it like that otherwise and then we can make improve it further in a follow-up.

Thanks!

What about this?

Used user_load_multiple() but did not change the query to use EntityFieldQuery. There are like a bazillion direct queries in core that access entity base tables and they will need to be updated but that's really not the point of this issue IMHO.

Also added a @todo but I'm not sure where exactly it should be placed and what exactly it should say.

+1 for me, this patch is OK for as a temporary fallback for unblocking the session patch. In all cases, this isn't worst that HEAD is so it won't hurt anyone.

Eh, let's do. SCOTCH will be rewriting this block as a plugin anyway.