Hi,

I'm currently integrating OpenID support to the forum module and so I looked at the code.. I don't understand the SQL statement in CurrentlyOnline():

return DataObject::get("Member",
	"LastVisited > NOW() - INTERVAL 15 MINUTE",
	"FirstName, Surname",
	"INNER JOIN Group_Members ON Group_Members.GroupID IN (1, 2, 3) " .
	    "AND Group_Members.MemberID = Member.ID");

Why has the GroupID to be 1, 2, or 3? Are that special groups? I think that's a bug and should be removed, but I'm not sure so I ask first here :-)

Yes, this is a bug. This code was written before the Permission::check() system existed.

Instead, you're going to want to restrict the people listed to those with a particular permission - probably VIEW_FORUM?

Makes it really sense to restrict the people listed to those with a particular permission?
Maybe we should simple filter out members without "Forum ranking" and add in the backend the possibility to leave it blank!?

Would it not make more sense to move that method to the Member class so that it can be reused also in other places?

The question is really "who is a user of the forum?". You only want to list people in the currently online view who can use the forum.

It seems like users of the forum would be people who have the rights to access it. Many other pieces of functionality are controlled via the permission system, and so, it makes sense that this is too. For example - we are using the permission system as a way of identifying different types of members in other projects - it's more robust than checking for membership in a particular group since you can apply the permission code to the group of your choice.

If you use the forum ranking value, what happens when there's a forum that doesn't make use of forum values? The CurrentlyOnline() breaks. I don't think that this is appropriate.

You don't have the same problem with the permission check, because it's impossible to set up a usable forum without giving people the VIEW_FORUM permission.

> If you use the forum ranking value, what happens when there's a forum that doesn't
>make use of forum values? The CurrentlyOnline() breaks. I don't think that this is
> appropriate.
>
> You don't have the same problem with the permission check, because it's impossible to
> set up a usable forum without giving people the VIEW_FORUM permission.

Good point! So I think it would be the best to create Member::currently_only() to get all only members and in Forum's CurrentlyOnline() check the permission of each online member and output only those who have the VIEW_FORUM permission.

What do you think about this approach?