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

[Docs] Rework Product Catalog page #7

Closed mickaelandrieu closed 4 years ago

mickaelandrieu commented 6 years ago

catalogAction explained

This action is in charge of rendering the Product list page, except for the list of products rendered by listAction. This controller has so (too?!) many responsibilities:

  1. Checks the user permissions
  2. Set the user locale in session
  3. Clean the filters coming from the request
  4. Clean again the filters coming from the request
  5. Get products from the request filters
  6. Store the last SQL query for the "Export|SQL manager" actions
  7. If no products are found => render a different template (catalog empty page)
  8. If products are found => create the product categories form
  9. Guess what? one more action on filters coming from the request
  10. Retrieve a permission error if any
  11. Some checks for drag & drop feature...

What I suggest

  1. Task 1 can be done before entering the controller thanks to KernelEvents::CONTROLLER event. More, this can be re-used for almost every action of Symfony pages.

  2. Task 2 is useless as it's done by default by the UserLocaleListener => To be removed

  3. Task 3, 4 and 9 should be done once and for all using a new Immutable object call ProductsFilters that we retrieve directly as catalog Action arguments. This may change the behavior in case of Task 9 but remember, the list of products is retrieved later using list actions...

  4. Task 5 and 6 can stay in catalogAction but improved. Something like:

    list($products, $lastQuery) = $this->get('prestashop.products_provider')->retrieveProductsAndQuery($filters);

    Make so much sense here: more, the service should be immutable and for me, it feels so wrong to store the last executed SQL query inside the service!

  5. Task 7 should be handled by a specific action and can be done before the controller thanks to KernelEvents::CONTROLLER event. This listener is - of course - specific to the ProductController.

  6. Task 8 should of course stay in catalogAction but we don't need the "if" statement anymore :)

  7. Task 10 is useless, the session can be accessed from twig so only a minor update of the related template.

  8. For Task 11, I don't know... because I don't know why it's needed: if it's only to display/hide something in front, this is something that can be managed in Twig.

Cool, but why do we want to do it?

Many reasons:

mickaelandrieu commented 6 years ago

More food for the brain... how can we use the Grid component here while keeping the compatibility on templates and hooks...?

Gains => Every Grid works the exact same way Pros => I'm not sure we can keep retro compatibility on templating ... WDYT @eternoendless ?