[RFC|Merged] Enhanced Team Page

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
User avatar
ToonArmy
Registered User
Posts: 335
Joined: Fri Mar 26, 2004 7:31 pm
Location: Bristol, UK
Contact:

Re: The team page

Post by ToonArmy » Fri Apr 16, 2010 11:03 pm

protoTech wrote:
Highway of Life wrote: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.
Chris SmithBlogXMOOhlohArea51WikiNo support via PM/IM
Image

User avatar
EXreaction
Registered User
Posts: 1555
Joined: Sat Sep 10, 2005 2:15 am

Re: The team page

Post by EXreaction » Sat Apr 17, 2010 12:58 am

bantu wrote: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.

User avatar
Highway of Life
Registered User
Posts: 1399
Joined: Tue Feb 08, 2005 10:18 pm
Location: I'd love to change the World, but they won't give me the Source Code
Contact:

Re: The team page

Post by Highway of Life » Sat Apr 17, 2010 2:30 am

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.
Image

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: The team page

Post by bantu » Sat Apr 17, 2010 9:34 am

EXreaction wrote: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.

User avatar
nickvergessen
Former Team Member
Posts: 733
Joined: Sun Oct 07, 2007 11:54 am
Location: Stuttgart, Germany
Contact:

Re: The team page

Post by nickvergessen » Sat Apr 17, 2010 10:46 am

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.
Member of the Development-TeamNo Support via PM

User avatar
imkingdavid
Registered User
Posts: 1050
Joined: Thu Jul 30, 2009 12:06 pm

Re: The team page

Post by imkingdavid » Mon Apr 19, 2010 2:08 am

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.
I do custom MODs. PM for a quote!
View My: MODs | Portfolio
Please do NOT contact for support via PM or email.
Remember, the enemy's gate is down.

User avatar
nickvergessen
Former Team Member
Posts: 733
Joined: Sun Oct 07, 2007 11:54 am
Location: Stuttgart, Germany
Contact:

Re: The team page

Post by nickvergessen » Mon Apr 19, 2010 12:29 pm

I made some code changes, to view the users by groups.
teampage.png
(19.44 KiB) Downloaded 3423 times
naderman wrote: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?
Member of the Development-TeamNo Support via PM

User avatar
nickvergessen
Former Team Member
Posts: 733
Joined: Sun Oct 07, 2007 11:54 am
Location: Stuttgart, Germany
Contact:

Re: The team page

Post by nickvergessen » Mon Apr 19, 2010 2:10 pm

I introduced two configs now.
  1. Allowing the mulitple/single appearance of users, as per bantu's post:
    bantu wrote: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.
Member of the Development-TeamNo Support via PM

User avatar
nickvergessen
Former Team Member
Posts: 733
Joined: Sun Oct 07, 2007 11:54 am
Location: Stuttgart, Germany
Contact:

Re: The team page

Post by nickvergessen » Fri Apr 23, 2010 12:22 pm

Should be ready for testing now.
Member of the Development-TeamNo Support via PM

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: The team page

Post by bantu » Fri Apr 23, 2010 3:54 pm

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.

Post Reply