Skip to content

Allow dynamic CRUD controllers linkage to Dashboard controllers #6790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
speller opened this issue Feb 5, 2025 · 5 comments
Open

Allow dynamic CRUD controllers linkage to Dashboard controllers #6790

speller opened this issue Feb 5, 2025 · 5 comments

Comments

@speller
Copy link

speller commented Feb 5, 2025

With pretty URLs, it is now possible to easily have multiple dashboard controllers. But the issue is that under the hood, EA tries to apply ALL CRUD controllers to ALL Dashboards when generating URLs. For example, if we have AdminDashboard and CustomerDashboard controllers in different namespaces, EA will link ALL available CRUDs to both dashboards making customer CRUDs available for admins and admin CRUDs available customers!

final class AdminRouteGenerator implements AdminRouteGeneratorInterface
{
    ...
    private function generateAdminRoutes(): array
    {
        ...
        foreach ($this->dashboardControllers as $dashboardController) {
            ...
            foreach ($this->crudControllers as $crudController) {
                ...
                    $adminRoutes[$adminRouteName] = $adminRoute;
                ...
            }
            ...
        }
    }
}

The suggestion is to implement a basic dynamic CRUD controllers filtering. I'm using the following patch:

--- a/src/Router/AdminRouteGenerator.php
+++ b/src/Router/AdminRouteGenerator.php
@@ -131,6 +131,9 @@
             // then, create the routes of the CRUD controllers associated with the dashboard
             foreach ($this->crudControllers as $crudController) {
                 $crudControllerFqcn = $crudController::class;
+                if (method_exists($dashboardController, 'isCrudControllerAllowed') && !$dashboardController::isCrudControllerAllowed($crudControllerFqcn)) {
+                    continue;
+                }

                 if (null !== $allowedCrudControllers && !\in_array($crudControllerFqcn, $allowedCrudControllers, true)) {
                     continue;

It adds the support of the isCrudControllerAllowed method to dashboard controllers so I can do the following filtering:

namespace App\Controller\Admin; // All Admin controllers are in this namespace

class DashboardController extends AbstractDashboardController
{
    /**
     * Allow only CRUD controllers from the current namespace.
     */
    public static function isCrudControllerAllowed(string $controllerFqcn): bool
    {
        return str_starts_with($controllerFqcn, __NAMESPACE__ . '\\');
    }
}

Which easily and effectively removes all useless CRUD controllers from the dashboard routes.

The current method of removing CRUDs by adding them to the AdminDashboard attribute is inconvenient in this case because we have many CRUD controllers and manual list management is hard and ineffective - someone may forget to add an exclusion without any errors/warnings/hints potentially exposing unneeded functionality to users that should not have access to.

The above is an example of the idea implementation and it can be implemented in many ways. It's up to the authors to choose one.

I could imagine another option like this:

namespace App\Controller\Admin;

#[AdminDashboard('/admin/dashboard', 'admin_dashboard', crudFilterTag: 'admin')]
class DashboardController extends AbstractDashboardController

And then in the Symfony config:

  App\Controller\Admin\:
    resource: '../src/Controller/Admin/'
    tags: 
      - 'controller.service_arguments'
      - name: 'ea.crud_filter', value: 'admin'

  App\Controller\Customer\:
    resource: '../src/Controller/Customer/'
    tags: 
      - 'controller.service_arguments'
      - name: 'ea.crud_filter', value: 'customer'

This way, all controllers in these two namespaces will have the corresponding ea.crud_filter tag value so AdminRouteGenerator will match it to the AdminDashboard::$crudFilterTag value and decide whether to add the CRUD to the dashboard or not.

@ben29
Copy link

ben29 commented Feb 5, 2025

Maybe you can create pr?

@speller
Copy link
Author

speller commented Apr 4, 2025

@ben29 I added a PR #6892

@ben29
Copy link

ben29 commented Apr 7, 2025

I think it’s better to filter by Namespace

@speller
Copy link
Author

speller commented Apr 9, 2025

@ben29 It'll be a breaking change, so I would prefer to let maintainers decide.

@ben29
Copy link

ben29 commented Apr 9, 2025

@ben29 It'll be a breaking change, so I would prefer to let maintainers decide.

That’s the main issue with the new urls, with multi dashboards,
The namespace, should be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants