[Pushed] forumteam display
#1
There has already been another discussion about this subject, and after some better observations I am sure now that the showteam.php module, which displays the members of forumteam groups, is buggy.

As it is now the showteam.php module will only work correctly when persons are member of one forumteam only, but it goes wrong when one person is member of two or more forumteams.

As an example I consider the testforum which I am using to develop a forum for a large (main) committee with several subcommittees. The testforum is limited in the number of members of the committees, in realty the committees will be far larger. The structure can be visualized as below:

[Image: Committee%20structure.png]

All members of the main committee have this committee as their primary & display usergroup. The main committee members who are also member of a subcommittee have this membership as an additional user group. When the forumteam is displayed, the composition of the main committe is displayed right with all 9 members with names displayed in green:

[Image: Main_Committee_new_showteam.png]

But that of the subcommittees not. As shown below for the 1st subcommittee the subcommittee members who are not member of the main committee (names in blue) are listed, but the (two) members who are also member of the main committee (names in green) are not listed:

[Image: SubCommittee_1_old_showteam.png]

Don't look at the text, this is Dutch, but you see that two of the six members are missing. Why this is becomes clear when you look at the source of showteam.php. Below the code snippet where users are assigned to a group.


// Are they also in another group which is being shown on the list?
if($user['displaygroup'] != 0)
{
	$group = $user['displaygroup'];
}
else
{
	$group = $user['usergroup'];
}

You see that a user is assigned to either his displaygroup or, when this is not set, to his usergroup. The additional groups are not considered at all!
This is completely in contradiction with how, for example, user permissions are determined in function forum_permissions, see the code snippet below where the user's usergroups are gathered:

$gid = $mybb->user['usergroup'];
if(isset($mybb->user['additionalgroups']))
{
	$gid .= ",".$mybb->user['additionalgroups'];
}

So here only the combination of usergroup and additional groups is considered. The display group is omitted, because this should be one of the additional groups!

So I decided to make an improved version of showteam.php. To start with, the query for selecting the users that are member of a forumteam group must be chaged, apart from the obvious errors that are in the original query at line 72. Originally this was:

$query = $db->simple_select("users", "uid, username, displaygroup, usergroup, ignorelist, hideemail, receivepms, lastactive, lastvisit, invisible", "displaygroup IN ($groups_in) OR (displaygroup='0' AND usergroup IN ($groups_in)) OR uid IN ($users_in)", array('order_by' => 'username'));

And this was changed into:

$where = "uid IN ($users_in) OR usergroup IN ($groups_in)";
if ($groups_in != 0)
{
	$groups = explode(',',$groups_in);
	foreach ($groups as $group)
	{
		$where .= " OR {$group} IN (additionalgroups)";
	}
}
$query = $db->simple_select("users", "uid, username, displaygroup, usergroup, additionalgroups, ignorelist, hideemail, receivepms, lastactive, lastvisit, invisible", $where, array('order_by' => 'username'));

In lines 73 - 110 (original code) the users found by this query are assigned to an array with forumteam users. This contains some errors and does not take the additional groups of users into account. I only give the replacement part for these lines below:

while($user = $db->fetch_array($query))
{
	// If this user is a moderator
	if(isset($moderators[$user['uid']]))
	{
		foreach($moderators[$user['uid']] as $forum)
		{
			if($forum_permissions[$forum['fid']]['canview'] == 1)
			{
				$forum_url = get_forum_link($forum['fid']);
				eval("\$forumlist .= \"".$templates->get("showteam_moderators_forum")."\";");
			}
			else
			{
				$forumlist .= $lang->forum_hidden."<br /";
			}
		}
		$user['forumlist'] = $forumlist; 
		$forumlist = '';
		
		$usergroups[6]['user_list'][$user['uid']] = $user;
	}
	// Is this user also in other group(s) which is being shown on the list?
	$groups = array();
	if ($user['usergroup'] !=6)
	{
		$groups[] = $user['usergroup'];
	}
	if ($user['additionalgroups'])
	{
		$groups = array_merge($groups,explode(',',$user['additionalgroups']));
	}
	foreach ($groups as $group)
	{
		if($group !=6 && $usergroups[$group])
		{
			$usergroups[$group]['user_list'][$user['uid']] = $user;
		}
	}
}

Last but not least there is one line that must be added to the language file global.lang.php:

$l['forum_hidden'] = "(Hidden)";

This addition is needed because in the original version it went wrong when a user displayed the forum team and there was a moderator of a forum that is hidden for him. This is also corrected in the changes above.

When the forumteam is displayed with the new version of showteam.php, the output for the 1-st subcommitte looks like:

[Image: SubCommittee_1_new_showteam.png]

Which is the correct composition of this subcommittee, so with the two members of the main committee in green included.


I have solved it for my forum with a plugin, which in essence is the corrected version of showteam.php. But because there are several bugs in the present code I decided to present it here for implementing in the core of a future version of MyBB. I understant this will not be an urgent matter. Smile
Regards, Ad Bakker (NL)
Reply
#2
Ad, do you use github at all? Perhaps add this as a pull request.
Random Fish and Sims Maniac
MY PLUGINS
Help MyBBSupport help you - remember to mark your threads as solved


Reply
#3
To be honest: no. Smile 

I just rolled into this because I am developing the forum I mentioned in my post. I leave the overall theme identical to the original, but try to make it as simple as possible because most users will be unexperienced with forums. That also means that I test everything out, and also try to remove things which are redundant and things which will not be used by the members. As a relax period in between I look around here. Shy

So I do not have any experience with github, except that I heard about it and sometimes follow a link to it. 
Regards, Ad Bakker (NL)
Reply
#4
Yea, I do that with my themes too; I removed a lot of fluff from mine.

Anyhoo - github. It's actually fun and a great place to share and save your code. If you leave it open source then it is basically free to use Smile
Random Fish and Sims Maniac
MY PLUGINS
Help MyBBSupport help you - remember to mark your threads as solved


Reply
#5
Bump - I'm not sure whether and what exactly needs to be fixed (tired atm, didn't read the whole thread). While I agree that it shouldn't be a problem to display a user in different teams I'm not sure about the rest, eg the display style. Would be nice if someone else could take a look.
Support PMs will be ignored!
Reply
#6
I definitely agree that additional groups should be considered and the displaygroup should influcence the color names, just like in other places on the forum. I also think that group leaderships should be shown, if there are any.

As for moderators in a forum they can't see - this should never be the case, why would you add someone as a moderator to a forum that can't be moderated by them? Instead of showing "(Hidden)" we should skip these moderators.

But the suggested fixes won't work, at least this part of the 1st suggested query is incorrect:
" OR {$group} IN (additionalgroups)";
You can't use a column in an IN clause in all DB engines we support, correct me if I'm wrong. CONCAT etc. or FIND_IN_SET will need to be used instead.
Reply
#7
(2015-06-28, 02:53 PM)Destroy666 Wrote: You can't use a column in an IN clause in all DB engines we support

The concept works with me with mysqli, but I'm not sure whether it works with other engines.
As an alternative:

" OR FIND_IN_SET({$group},additionalgroups)"; 

I have tested it, and also this works. This should then work with all DB engines?
Regards, Ad Bakker (NL)
Reply
#8
There are cases where we have that query and it's different for PgSQL/SQLite Wink

So:
- Consider additional groups
- Show group leaders
- Use correct colors
- Check whether forum permissions are considered

Anything I've missed?
Support PMs will be ignored!
Reply
#9
I think if you look in the code it has the group id for global moderators hardcoded at 6. That should be changed.
Random Fish and Sims Maniac
MY PLUGINS
Help MyBBSupport help you - remember to mark your threads as solved


Reply
#10
Nope, the group IDs are always the same. 4 is always admin, 2 always registered etc. There was some discussions already about that and there's no way to remove that. Otherwise the whole Merge System and quite a lot of functions would be long broken.
Support PMs will be ignored!
Reply


Forum Jump:


Users browsing this thread: 1 Guest(s)