Moodle

Just a question because I'm not all that great at SQL but on the statement
WHERE ul.userid = u.id

Does that only then return users who have their id matching in the mdl_user_lastaccess table? If this is so, shouldn't the mdl_user_lastaccess table also be written to for the site course? If a user logs into moodle, they will not be recognized as online until they have entered a course for the first time. I might be completely off on this though.

Thanks, Matthew!

Yu, I'm not sure why the GROUP BY would fix this but if it does, great!

What I think happens is that the call $pusers = get_records_sql($SQL, 0, 50) gets the first 50 records, but of those records some of the user id's are the same. So out of 50 records you might only have 9 unique users. So it wouldn't be returning the 50 people online, but the 50 last logged accesses. Grouping the id's makes sure that the duplicates are not counted towards the 50.

Hi, thanks for the report and fix, it is in cvs. Cheers.

I was think about this problem over the weekend and I'm glad I could help. But I was wondering if it might be time to deprecate the lastaccess field in the mdl_user table, and start writing a mdl_lastaccess entry for site access?

adding

add_to_log(SITEID, 'course', 'view', "view.php?id=".SITEID, SITEID);

makes users who log on immediately show up in the online users block.

This seems to have crept in with the fix for this bug, so I'm reopening it to make sure some-one notices!

I now see the following error message:

ERROR: column "u.username" must appear in the GROUP BY clause or be used in an aggregate function

SELECT u.id, u.username, u.firstname, u.lastname, u.picture, u.lastaccess, ul.timeaccess FROM mdl_user_lastaccess ul, mdl_user u WHERE ul.userid = u.id AND (ul.timeaccess > 1181144300 OR u.lastaccess > 1181144300) GROUP BY u.id ORDER BY ul.timeaccess DESC

• line 779 of lib/dmllib.php: call to debugging()
• line 1020 of lib/dmllib.php: call to get_recordset_sql()
• line 90 of blocks/online_users/block_online_users.php: call to get_records_sql()
• line 202 of blocks/moodleblock.class.php: call to block_online_users->get_content()
• line 226 of blocks/moodleblock.class.php: call to block_base->is_empty()
• line 341 of lib/blocklib.php: call to block_base->_print_block()
• line 76 of admin/stickyblocks.php: call to blocks_print_group()

If you need the GROUP BY statement you appear to have to put all the SELECT statement columns in it. Could just be a postgres thing perhaps? I don't have access to any other DB systems to try it on them.

The other solution is to remove the GROUP BY entirely - but then you're left with your original problem. Not sure which is the best way to try and address this

$SQL = "SELECT u.id, u.username, u.firstname, u.lastname, u.picture, u.lastaccess, ul.timeaccess
FROM {$CFG->prefix}user_lastaccess ul,
{$CFG->prefix}user u
$groupmembers
WHERE
ul.userid = u.id AND ul.timeaccess = (SELECT max(timeaccess) FROM {$CFG->prefix}user_lastaccess WHERE userid=u.id $courseselect $timeselect $groupselect)
ORDER BY ul.timeaccess DESC";

This is the way to do it without a groupby clause.

Hi, added everything in SELECT statement into the GROUP BY clause.

Cheers

adding all the statements into the GROUP BY causes the same results that we had before. Duplicates that cause the online users block to be incorrect.

Instead of retrieving the last 50 users who have used the site it is returning the last 50 hits to the site minus duplicate users.

$SQL = "SELECT u.id, u.username, u.firstname, u.lastname, u.picture, max(u.lastaccess) as lastaccess, max(ul.timeaccess) as timeaccess
FROM {$CFG->prefix}user_lastaccess ul,
{$CFG->prefix}user u
$groupmembers
WHERE
ul.userid = u.id
$courseselect
$timeselect
$groupselect
GROUP BY u.id, u.username, u.firstname, u.lastname, u.picture
ORDER BY lastaccess DESC";

try this SQL...I know it works in MYSQL, but I can't test it in Postgres. I think it might fix the aggregate functions problem...and it returns the correct results.

Matthew, looks like it's working OK on PostgreSQL. At least with my version (8.1.9 on CentOS).

Wow, thanks for this code Matthew. Can someone with the Moodle team take a look at this fix?

On our site the online users block (from 1.8.2+) consistently said 40 users were online yet the website performed very slowly as if there were actually many more users online.

I added Matthew's new code, and our 40 users online changed to 108 users online! I've been checking over the past few days and the user count seems very accurate based on the performance of the Moodle site.

Hmmm on My Moodle (Authorization: External Database) this fix does NOT work.

If in query is "user_lastaccess.userid = user.id" that user must enter to course that user will be show in Online Users Block.

If user never enter to course, he hasn't adequate record in user_lastaccess.
In table user_lastaccess are fields "id | userid | courseid | timeaccess"

Help me understand what you mean by (does NOT work). The way the online users block works right now is that it will tell you the last 50 people that have accessed your course in the last (60 minutes default). It will NOT tell you the last 50 users to access Moodle in the last 60 minutes who are enrolled in your course. This is nothing that the fix has done, the group logic and course logic are unchanged. Am I correct that this is your issue?

Ok. I write about "Online Users Block" on main site Moodle.

I explain you on example:

1. Create new account for example "test"
2. Log as "test"
3. Look "Online Users Block" on main site and You see that account "test" isn't on this block
4. When you enter to any course, account "test" will be in "Online Users Block" on main site.
5. Logout "test"
6. Wait 6 minutes
7. Log as "test"
8. Look "Online Users Block" on main site and You see that account "test" is on this block.

Isn't this more of an issue of Moodle not updating the mdl_user_last_access table on login than the online users block not finding the user record?

Hi,

Order by lastaccess seems wrong though, because it's attached to the user, it is not the timestamp of the last course visit. It makes sense only if the block is placed on the front page. Sebastian I think we might need to log lastaccess of the front page course access to fix that problem, or use an outter join and allow null condition in the SQL.

Cheers,

Yu

I have split this sql to 2 parts, depending on if it's displayed on a course or the site page. I used Mathew's patch for this. Please help test.

Cheers,

Yu