[RFC|Merged] Enhanced Team Page

[ToonArmy]

You could have a veeeery long list of forums for large boards, so I think it would still break the layout. But you could solve this issue with a jQuery tooltip box.

It's unlikely that the layout would break as the text will simply wrap. It's the amount of unnecessary space that it takes up. Case in point (not including private forums, which would make the column even bigger):

You could rowspan all the common ones, but it'll still get pretty messy quite quickly though.

[EXreaction]

How about this from a functional point of view:

a) In the ACP groups can be marked as "Show on team page".

b) In the ACP we provide a backend to manage the team page. The page lists all the groups that have the "Show on team page" flag enabled. It is possible to move groups up and down in the hierarchy and it is possible to add spacers (to separate the administrators from moderators in the template for example).


From the development perspective this can probably be archived by using a single integer column in the forums table.


We have to think about what happens if a user is in multiple groups. It should probably be selectable if the users should show in all groups or only in the highest in the hierarchy.

I think that might be a bit overkill. Moving up/down on the display order of groups I could see (could be used for the legend as well), but letting them control exactly where each user will appear is too much I think.


I don't think we should list the forums without using a selection box. It's simply a huge unreadable block of text.

[Highway of Life]

Indeed, unless we change it in other places, the forumlist such as the jumpbox is also displayed in a drop-down... that might be the most efficient method without jQuery.

[bantu]

I think that might be a bit overkill. Moving up/down on the display order of groups I could see (could be used for the legend as well), but letting them control exactly where each user will appear is too much I think.

Errm. I was not talking about every single user; just the groups. So you can adjust the group order to what you want.

Example A
Administrators
Moderators
< Spacer >
Contributors

Example B
Administrators
< Spacer >
Contributors
Moderators

I also made a typo, it should work with an integer column in the groups table, not forums table.

[nickvergessen]

I'd like bantu's suggestion. For the "Display user in multiple groups" I'd add a simple bool-config.
I think that would be not to hard to write and have lot more functionality than the current thing.

[imkingdavid]

I agree with bantu's idea of implementation as well. @EXreaction: I think that the more options for customization, the better. "Overkill" is a matter of opinion, and things can be added into an "advanced" options box that contains generally used defaults automatically set if the admin doesn't want to change them. Yes, it may mean a little more work on the development side, but it allows admins to have things show exactly how they want it to.

[nickvergessen]

I made some code changes, to view the users by groups.

I think this makes a lot more sense than the current solution and should als peform a lot better. We've had problems with query performance on the team page quite a few times. If you could create a ticket on the tracker and submit an actual patch (for adding the acp option etc.) either on github or in text form that'd be great!

There's a little problem here, we either can not display the forums (than we need to display some other information), the user has m_ permissions, or we still need the expensive query, we just have a $user_ids array to help this time, but I guess that is still kind of expensive?

[nickvergessen]

I introduced two configs now.

1. Allowing the mulitple/single appearance of users, as per bantu's post:
   

   We have to think about what happens if a user is in multiple groups. It should probably be selectable if the users should show in all groups or only in the highest in the hierarchy.

2. Allowing to view the forums, in which the user is a moderator. So Big-Boards can easily disable this and save the 4 heavy queries.

The Output should be ready now:
http://github.com/nickvergessen/phpbb3/ ... icket;9549

Integration into the ACP is going to come soon.

[nickvergessen]

Should be ready for testing now.

[bantu]

I had a look at the code and have some comments.

• The huge switch block should probably be devided into separate functions (lesson learned from 3.0.x).
  Most of those functions/methods will only take the group_id as a parameter anyway.
• functions/methods should use the function/method keywords (public, private, protected, static).
• functions/methods should have proper documentation about the arguments they take and return
• Queries using the primary key do not need sql_query_limit(). There is only one row for what you're looking for anyway.
• Some queries can probably be joined together. For 'add' it could probably be

  Code: Select all

  UPDATE phpbb_groups SET group_teampage = MAX(group_teampage) + 1 WHERE group_id = $var GROUP BY group_id

  or something like that. This way we should be protected from any race conditions there.
• This code block appears four times.

  Code: Select all

  $sql = 'SELECT group_teampage
    FROM ' . GROUPS_TABLE . '
    WHERE group_id = ' . $group_id;
$result = $db->sql_query_limit($sql, 1);
$current_teampage = (int) $db->sql_fetchfield('group_teampage');
$db->sql_freeresult($result);  

• If 0 in "SET group_teampage = 0" is something special it should probably be a (class) constant.
• There should be some documentation about how the integer field is organised.

Maybe everything should be put into a separate class.

Other than that, it looks good. At least code wise. Haven't looked at functionality yet.