Control panel module system

General discussion of development ideas and the approaches taken in the 3.x branch of phpBB. The next feature release of phpBB 3 will be 3.3/Proteus.
Forum rules
Please do not post support questions regarding installing, updating, or upgrading phpBB 3.2.x. If you need support for phpBB 3.2.x please visit the 3.2.x Support Forum on phpbb.com.

If you have questions regarding writing extensions please post in Extension Writers Discussion to receive proper guidance from our staff and community.
User avatar
mrgoldy
Registered User
Posts: 38
Joined: Fri Dec 18, 2015 9:41 pm
Location: The Netherlands
Contact:

Re: Control panel module system

Post by mrgoldy »

I know, that's where we are clear on.
But how everything is defined. Cause we have three things:
- Routes (eg. /admin/index) that have to be defined
- Controllers (eg. phpbb.admin.controller) that have to be defined
- Lists for the *CP Menu (eg. General, Index, Settings, Board settings) that has to be created

And a valid point of cross reference is brought up. In simple terms, we want to get rid of the _info.php files to generate the list items.
But that is all fine and dandy, but we still need the associated language string, routes, structure (category, subcategory and children), authorization and by the looks of things icons. Not to mention we have to take extensions into consideration.

Nicofuma also mentioned wanting to create all 3 (routes, controller, list) from one declaration. I've tried various versions but non are suitable. Hence I am requesting an example of how the set up should look like.
phpBB Studio Proud member of the Studio!

CHItA
Development Team
Development Team
Posts: 162
Joined: Thu Mar 12, 2015 1:43 pm
Location: Budapest, Hungary

Re: Control panel module system

Post by CHItA »

I guess you could just start working on moving everything into controllers as it should be fairly straightforward and a big help (also a big task). That way you don't need to wait around for our feedback as we all seem to be uncertain how to properly handle the menu related code this time.

User avatar
mrgoldy
Registered User
Posts: 38
Joined: Fri Dec 18, 2015 9:41 pm
Location: The Netherlands
Contact:

Re: Control panel module system

Post by mrgoldy »

Okay, so I saw a new light :bulb:

This way the amount of 'declarations' is minimized.
We will have to create all '*cp routes' ourselves and not through a routing.yml file.
Which should be better anyways, so we define the prefix (eg, '/admin', '/moderator', '/user').

We will have service declaration files per control panel.
services_menu_acp.yml or services_acp_menu.yml

Code: Select all

services:
    acp.menu:
        class: phpbb\di\service_collection
        arguments: ['@service_container']
        tags: [{ name: service_collection, tag: acp }]

    acp_general:
        tags: { name: acp }
        class: \phpbb\cp\item
        shared: false
        arguments:
            -
                auth: acl_a
                icon: cogs
                route: acp_index
                parent: ''
                before: ''

    acp_index:
        tags: { name: acp }
        class: \phpbb\cp\item
        shared: false
        arguments:
            -
                auth: acl_a_board
                icon: home
                route:
                    path: /index
                    defaults:
                        _controller: acp.main:main
                        page: 1
                parent: acp_general
                before: ''
                pagination: 'page'
or

Code: Select all

services:
    acp.menu:
        class: phpbb\di\service_collection
        arguments: ['@service_container']
        tags: [{ name: service_collection, tag: acp }]

    acp_general:
        tags: { name: acp }
        class: \phpbb\cp\item
        shared: false
        arguments:
            - acl_a
            - cogs
            - acp_index
            - ''
            - ''

    acp_index:
        tags: { name: acp }
        class: \phpbb\cp\item
        shared: false
        arguments:
            - acl_a_board
            - home
            -
                path: /index
                defaults:
                    _controller: acp.main:main
                    page: 1
            - acp_general
            - ''
            - 'page'
The first declarations, has a bit of a different arguments set up. But doing it like so, means there is only argument. An associated array. The benefit of this, is that the order does not matter and we have literal keys and only arguments have to be specified that are needed for that specific declaration.
The second declarations, use the 'regular' way of defining arguments. Downside of this is that if you want to specify the 8th argument, you are obliged to specify the first 7 aswell.

Now when the container is build, we can grab all the menu's. Iterate over the items and grab their routes. If the route is defined (in my above example, it is an array if a route should be created), we create a route with the item's name, eg acp_index, and the options defined in the array. That should be exactly the same someone would normally do in a routing.yml file.

Then if the pagination argument is set, it will also create a "pagination" route, eg acp_index_pagination. The value of the pagination argument, "page", is added to the path, path: /index/{page} and removed from the defaults array.

Now in the (adm_)page_header() function we run a new function: build_cp_menu(). The "building" as the menu's need to be globally available anyway - as per request of Hanakin. The adm_page_header will only create the acp menu, while page_header will create the ucp and mcp. The title can easily be created from strtoupper(acp_index).

Then in a/the Symfony Routing Loader, we check which route is being used. If it's one created by the *cp menu's, we do two things: 1. check authentication, to see if the user is authorised to access these controllers. 2. Set the menu item and its parents to "active" for the template.

---

This means that there are two service declarations: The controller and the Menu item.
There is only one file handling the 'menu items', not all in a separate file.
The 'core' handles the language strings (titles) and routes.
The 'core' handles the authentication of the initial controllers and their menu's parents.
phpBB Studio Proud member of the Studio!

User avatar
mrgoldy
Registered User
Posts: 38
Joined: Fri Dec 18, 2015 9:41 pm
Location: The Netherlands
Contact:

Re: Control panel module system

Post by mrgoldy »

I quickly spoke with nicofuma, and he says it looks nice.
He also brought up a good point about the arguments, as symfony allows "named arguments".
This means prefixing them with $.

Code: Select all

    acp_general:
        tags: { name: acp }
        class: \phpbb\cp\item
        shared: false
        arguments:
            $auth: acl_a
            $icon: cogs
            $route: acp_index
            $parent: ''
            $before: ''
Then we can use those exact variables in the constructor, regardless of order(!).
public function __construct($auth = '', $before = '', $icon = '', $parent = '')
phpBB Studio Proud member of the Studio!

User avatar
3Di
Registered User
Posts: 773
Joined: Tue Nov 01, 2005 9:50 pm
Location: Milano (I) Frankfurt (D)
Contact:

Re: Control panel module system

Post by 3Di »

posey wrote:
Tue May 14, 2019 10:47 am
Then we can use those exact variables in the constructor, regardless of order(!).
public function __construct($auth = '', $before = '', $icon = '', $parent = '')
That's interesting as well, indeed.
Please PM me only to request paid works. Thx.
Want to compensate me for my interest? Donate
My development's activity º PhpStorm's proud user
Extensions, Scripts, MOD porting, Update/Upgrades
👨‍🏫 | Take a tour to | The Studio | 👨‍🏫

User avatar
rxu
Registered User
Posts: 132
Joined: Tue Apr 04, 2006 4:28 pm
Contact:

Re: Control panel module system

Post by rxu »

Just for the note: afaik this is the Symfony 3.3+ feature so can be suitable for phpBB 3.3+ only :)
Image

User avatar
mrgoldy
Registered User
Posts: 38
Joined: Fri Dec 18, 2015 9:41 pm
Location: The Netherlands
Contact:

Re: Control panel module system

Post by mrgoldy »

Well, seeing this is targeted for 4.0 that should be fine.
I am currently coding against master branch and it's working as intended.
One thing to note though, is that the order in which arguments are defined in .yml does not matter, however if you want to define the Xth (say 4) parameter in the class' constructor, you have to define the first 3 aswell. Which is fine, just a note.
phpBB Studio Proud member of the Studio!

User avatar
mrgoldy
Registered User
Posts: 38
Joined: Fri Dec 18, 2015 9:41 pm
Location: The Netherlands
Contact:

Re: Control panel module system

Post by mrgoldy »

Okay, so the initial conversion is complete.
With a stack of 5 dependent PR's, everything is moved and functioning.
  1. ACP Modules to controllers
  2. MCP Modules to controllers
  3. UCP Modules to controllers
  4. CP Services and items
  5. CP Common code and tests
The Travis and AppVeyor tests are also passing.

However, a clean install with this conversion does not yet work.
This is cause all 'references' to "modules" have not been removed yet.
So when installing and you get to 54%, the installer wants to install modules, include the (removed) includes/*cp/info/*.php files.
See phpbb\install\module\install_data\task\add_modules.php.

However, I am not too sure what I can delete, have to retain or adjust.
  • Module functions: to be removed
    includes/functions_module.php
  • Module table: to be removed (with a migration)
    includes/constants.php
    tables.yml
  • Module installation: ???
    phpbb\install\module\install_data\task\add_modules.php
  • Module migration data: ???
If someone could give me some guidance / point me in the right direction, that would be helpful and greatly appreciated.
phpBB Studio Proud member of the Studio!

User avatar
rxu
Registered User
Posts: 132
Joined: Tue Apr 04, 2006 4:28 pm
Contact:

Re: Control panel module system

Post by rxu »

To get the right direction :P what is the final CP concept now? Especially, how are parent/child modules being handled? :)
Image

User avatar
mrgoldy
Registered User
Posts: 38
Joined: Fri Dec 18, 2015 9:41 pm
Location: The Netherlands
Contact:

Re: Control panel module system

Post by mrgoldy »

Control panel 'items' are now declared as service parameters.

Code: Select all

    mcp.mcp_cat_main:
        route: 'mcp_index'

    mcp.mcp_index:
        parent: 'mcp_cat_main'
        route:
            path: /index
            defaults:
                _controller: mcp.front:main
                
    mcp.mcp_view_forum:
        auth: '(forum_id || topic_id || post_id) && acl_m_,forum_id'
        parent: 'mcp_cat_main'
        route:
            path: /forum
            defaults:
                _controller: mcp.main:main
                mode: 'forum'
                page: 1
The parameter uses acp., mcp. or ucp. prefix to indicate to which control panel they belong.
Followed by the item name, which is used as language key for displaying the item, eg. $this->language->lang('MCP_VIEW_FORUM');.
It can still declare an auth: string, which is validated to see if the user is authorised.
It should declare a parent: string, listing the item name of the parent.
It could declare a route: string or array. If it's a string, it is considered a category. If it's an array, it's considered an actual item. It will automatically create a route for this item, where the route name is the item's name: $this->helper->route('mcp_view_forum');. If the route array has the page: set, it will automatically create a pagination route: $this->helper->route('mcp_view_forum_pagination');.

A new Kernel Request Subscriber has been added, which checks which route is being used for the current request. If the route belongs to the ACP, MCP, or UCP, it starts constructing that control panel. It first does some common checks, such as checking the authentication for the current 'active' item and it's parents. Then it is send of to the specific constructor. The control panel constructors are basically the old adm/index.php, mcp.php and ucp.php respectively.

The MCP and UCP menu's are now build in the page header, as requested by Hanakin, so they're globally available.
The ACP menu is still only available in the ACP.

This now means that everything has paths (/admin/settings/board, /mod/forum, /user/manage/bookmarks).
Everything is now properly done with controllers, with proper service dependencies (no more global).
And exceptions can be thrown in the control panels.
phpBB Studio Proud member of the Studio!

Post Reply