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

Proposed fix for https://github.com/zendesk/magento_extension/issues/85 #111

Merged
merged 1 commit into from
Apr 25, 2018

Conversation

dave-swift
Copy link
Contributor

@dave-swift dave-swift commented Mar 7, 2016

Fix for #85, which addresses the assumption Mage::app()->getFrontController() is always set. It's best to see the comments in the ticket.

The short of it is that it removes Mage::app()->getFrontController() from the observer model since observers are not always called from a controller (eg, API, cron, command line, etc).

@joseconsador
Copy link
Contributor

@dave-swift Thanks for this. Can you add a description in the PR though. We're reviewing it now.

Jose

@RoussKS
Copy link

RoussKS commented Dec 5, 2017

Hi,

Is this PR going to be accepted or will there be a fix for this issue?
We have similar issues using Magento's API to create orders.
Magento version is 1.9.2.4 fully patched until the latest SUPEE-10415

Many Thanks

@schmengler
Copy link

I can confirm that it is impossible to create orders via API with the extension, because getFrontController() returns null during API requests, but blocks are rendered for the order confirmation email.

This is a serious problem and the PR solves it, so please include it in the next release. If you hesitate because it changes too much, a simple fix would be to include the following lines at the beginnning of the observer methods setHook and insertBlock

    if (! Mage::app()->getFrontController()->getAction()) {
        return;
    }

@yifeiwu
Copy link

yifeiwu commented Apr 11, 2018

@joseconsador

@joseconsador joseconsador merged commit 26262df into agnostack:master Apr 25, 2018
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

Successfully merging this pull request may close these issues.

6 participants