-
Notifications
You must be signed in to change notification settings - Fork 529
Module AdminControllers for back office documentation has to be adjus… #2058
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
base: 9.x
Are you sure you want to change the base?
Conversation
| autoconfigure: true | ||
| tags: | ||
| - { name: controller.service_arguments } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these ones are doublons, you can either use the tag alon Or the autoconfigure alone but both are doing almost the same thing
So I would favor relying on autoconfigure: true and removing the tag part to keep the definition as simple as possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, regarding the autowiring, maybe we should add an example when you want to inject a service manually But we're entering more in Symfony many possibilities for DI, so instead of redocumenting Symfony we should probably add a notice about the pre-requisite knowledge from the framework itself
The same links that were added in the 9.0 changes pages:
- https://symfony.com/doc/6.4/controller/service.html
- https://symfony.com/doc/6.4/service_container/service_subscribers_locators.html
- https://symfony.com/doc/6.4/service_container.html
and maybe https://symfony.com/doc/6.4/controller.html for a more generic view on what a Symfony controller can do
| } | ||
| ``` | ||
| One of the two service configuration options above is **essential and required** for your controller to work properly. Without this configuration: | ||
| - **The controller will not function** and will throw errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - **The controller will not function** and will throw errors | |
| - **The controller will not work** and will throw errors |
I'm not sure which one is more correct in english, maybe I'm biased with my french point of view 😅
| **Understanding the configuration:** | ||
| - `autowire: true` - Automatically injects services in constructors and method parameters | ||
| - `autoconfigure: true` - Automatically configures the controller as a service and enables all controller features | ||
| - `controller.service_arguments` tag - Required if your controller doesn't extend `AbstractController` to enable method parameter injection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's true! But should we mention this? And by that I mean should we encourage people having controller that do not extend AbstractController, so not PrestaShopAdminController in the end
Or should we simply document: Use this class PrestaShopAdminController and that's enough documentation because any other type of controllers means you know what you're doing with the Symfony framework, so you are on your own because it's your specific need and your expertise
| - `$this->getConfiguration()` - Access configuration service | ||
| - `$this->getTranslator()` - Access translator service | ||
| - `$this->getRouter()` - Access router service | ||
| - `$this->getFlashBag()` - Access flash messages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this a doublon with the paragraph above that lists even more thelper methods?
| See the [full list of helper methods](https://github.com/PrestaShop/PrestaShop/blob/9.0.0/src/PrestaShopBundle/Controller/Admin/PrestaShopAdminController.php) in the class. | ||
|
|
||
| {{% notice info %}} | ||
| **Learn more about Symfony controllers:** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that's what I mentioned in another comment, perfect!
Uh oh!
There was an error while loading. Please reload this page.