Skip to main content

This site requires you to update your browser. Your browsing experience maybe affected by not having the most up to date version.

Archive /

Our old forums are still available as a read-only archive.

Moderators: martimiz, Sean, biapar, Willr, Ingo, simon_w

Forum module: Question about CurrentlyOnline()


Reply


5 Posts   1463 Views

Avatar
Markus

Google Summer of Code Hacker, 152 Posts

8 August 2007 at 1:22am

Edited: 08/08/2007 1:25am

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 :-)

Avatar
Sam

Administrator, 685 Posts

8 August 2007 at 12:51pm

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?

Avatar
Markus

Google Summer of Code Hacker, 152 Posts

9 August 2007 at 2:34am

Edited: 09/08/2007 2:56am

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?

Avatar
Sam

Administrator, 685 Posts

9 August 2007 at 3:41pm

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.

Avatar
Markus

Google Summer of Code Hacker, 152 Posts

10 August 2007 at 3:28am

> 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?