Skip to content
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

Ajax failing in decorating controller. #1008

Open
wants to merge 4 commits into
base: 1.7.x
Choose a base branch
from

Conversation

Rho-bur
Copy link
Contributor

@Rho-bur Rho-bur commented May 21, 2021

Questions Answers
Description? Ajax failing in decorating controller; mention regarding the need to implement all the Actions methods of the decorated controller.
Fixed ticket? Fixes #{issue number here} if there is a related issue

Rho-bur added 4 commits May 20, 2021 13:48
… the need of implementing the decorated controller action methods
Ajax calls not working with decorated controllers; mention to clarify the need of implementing the decorated controller action methods
Update override-decorate-controller.md
…plement the decorated controller action methods

On some edge cases we might need to perform an Ajax call from our Javascript file (or the Twig template) to our decorating DemoController.
Curently this will not work though, what we need to do in such a case is to create a regular admincontroller e.g. mymodule/src/Controller/Admin/DemoAjaxController.php with a method, create a route for that method in our mymodule/src/config/routes.yml, generate the URL as described here, [symfony docs](https://devdocs.prestashop.com/1.7/modules/concepts/controllers/admin-controllers/route-generation/) and perform our ajax call using the resulting Symfony route.
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello I think this was not wanted 😉 ?

```

On some edge cases we might need to perform an Ajax call from our Javascript file (or the Twig template) to our decorating DemoController.
Curently this will not work though, what we need to do in such a case is to create a regular admincontroller e.g. mymodule/src/Controller/Admin/DemoAjaxController.php with a method, create a route for that method in our mymodule/src/config/routes.yml, generate the URL as described here, [symfony docs](https://devdocs.prestashop.com/1.7/modules/concepts/controllers/admin-controllers/route-generation/) and perform our ajax call using the resulting Symfony route.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, this sounds like a bug 😄 actually. Can you tell me more? Would you have a code sample I could analyze?

Copy link
Contributor Author

@Rho-bur Rho-bur May 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
I'll attach the files here as the repository is private.
Ajax does not work in the decorating RmvProductController but works on AdminActionsController.
Routes in main class:
Media::addJsDef([ 'admin_products_url' => $this->context->link->getAdminLink('AdminRmvActions'), 'symfonyUrl' => $this->context->link->getAdminLink('RmvProduct', true, array('route' => 'rmvpreciousmetals_rmv_unit')), 'adminCommandUrl' => $this->context->link->getAdminLink('AdminActions', true, array('route' => 'rmvpreciousmetals_run_command')), ]);

routes.yml:

rmvpreciousmetals_rmv_unit:
  path: rmvpreciousmetals/update-product
  methods:  [POST]
  defaults:
    _controller: 'PrestaShop\Module\RmvPreciousMetals\Controller\Admin\AdminActionsController::setProductPricesAction'
    _legacy_controller: AdminActions

rmvpreciousmetals_run_command:
  path: rmvpreciousmetals/run-product
  methods:  [GET|POST]
  defaults:
    _controller: 'PrestaShop\Module\RmvPreciousMetals\Controller\Admin\AdminActionsController::sendApiRequest'
    _legacy_controller: AdminActions

When using the RmvProductController action (a new action not from the decoratedController) in the rmvpreciousmetals_rmv_unit path the ajax does not work, switching it to a regular controller not decorating anything (AdminActionsController) works.

Here is the slack thread I have opened for this, slack

code.zip

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you think seeing the full code will help I could grant you access to my private repo, no problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for late answer, week has been very busy I did not have time to explore the topic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, no worries, I'll wait!

@matks matks changed the base branch from master to 1.7.x August 5, 2021 16:32
Comment on lines +166 to +169
public function anAction($id, Request $request)
{
return $this->decoratedController->anAction($id, $request);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function anAction($id, Request $request)
{
return $this->decoratedController->anAction($id, $request);
}
public function anAction($id, Request $request)
{
return $this->decoratedController->anAction($id, $request);
}

Copy link
Contributor

@matthieu-rolland matthieu-rolland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the PR needs to be rebased, there are some conflicts (meanwhile you have some feedbacks already ;) )

@kpodemski
Copy link
Contributor

@thomasnares let's try cherry pick this PR for v8 and finish it 👍🏻

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

Successfully merging this pull request may close these issues.

7 participants