PrestaShop / prestafony-project

Some resources to help you migrate PrestaShop to Symfony 3
https://github.com/PrestaShop/prestafony-project/projects/1
11 stars 8 forks source link

Make filters accessible in multiple actions #61

Closed sarjon closed 4 years ago

sarjon commented 6 years ago

How it currently works

Let's say we have action that lists products:

// ProductController.php

public function listingAction(ProductsFilters $filters)
{
    // use $filters to list products
}

Currently filters are injected into action by using controller's & action's name (in this case Product and listing) which are used to find them in database. And it works.

Use cases

1. Exporting filtered data

Now I want to use same filters to export products, so I create new action:

// ProductController.php

public function listingAction(ProductsFilters $filters)
{
    // use $filters to list products
}

+public function exportAction(ProductsFilters $filters)
+{
+    // use $filters to export products
+}

And it does not work anymore as filters cannot be found by combination of controller Product and action export, so default values get injected.

2. Multiple listings (grids) in the same page:

Imagine I want to render 2 or more listings in same action, what I would like to do is:

// MyController.php

public function listingAction(
    ProductFilters $productFilters,
    CategoryFilters $categoryFilters
) {
    // use $productFilters and $categoryFilters to list data
}

And this wouldn't work as well, same reason as in 1st use case.

Possible improvement

What if we update abstract class PrestaShop\PrestaShop\Core\Search\Filters with new methods like:

// Filters.php

abstract class Filters extends ParameterBag implements SearchCriteriaInterface
{
    // ...

    /**
     * Get controller for which filters should be used 
     *
     * @return string
     */
    abstract public static function getController();

    /**
     * Get action for which filters should be used 
     *
     * @return string
     */
    abstract public static function getAction();
}

This of course would work as you wont have to parse controller and action name from request, but it would be confusing as it can be used in other controllers and actions than it is configured in filters.

Another solution could be to define some unique key:

// Filters.php

abstract class Filters extends ParameterBag implements SearchCriteriaInterface
{
    // ...

    /**
     * Get filters key
     *
     * @return string
     */
    abstract public static function getKey();
}

And implemented like:

// LogsFilters.php

final class LogsFilters extends Filters
{
    // ...

    /**
     * {@inheritdoc}
     */
    public static function getKey()
    {
        return 'logs_listing';
    }
}

In this case we could define key in controller_action format so it is compatible with current implementation.

mickaelandrieu commented 6 years ago

Hello @sarjon, I share most of the issues you describe here but we have to stick with how search is stored actually.

But, I like your implementation with a specific key, and add a new column called identifier to AdminFilter entity.

Let me think about it :)

mickaelandrieu commented 6 years ago

Ping @Quetzacoalt91, is it ok to add a new column to admin_filter table?

Quetzacoalt91 commented 6 years ago

Of course, just handle it for the upgrades too.