segment-boneyard / analytics-magento

[DEPRECATED] The hassle-free way to integrate analytics into any Magento store.
15 stars 19 forks source link

Instantiate action controllers through Mage::getModel to make them rewritable #70

Closed 7ochem closed 9 years ago

7ochem commented 9 years ago

Instantiate action controllers through Mage::getModel to make them rewritable

For the project I'm using this module for, we have an alternative category page with a full action name other than 'catalog_category_view'. So I want the Segment_Analytics_Model_Controller_Page::_getCategoryPageHandles() method to also return this alternative page full action name. I cannot rewrite the Segment_Analytics_Model_Controller_Page class through the core Magento rewrite system because in Segment_Analytics_Model_Front_Controller the classes are instantiated directly. Why was this done that way?

Typical rewrite I want to be able to use:

<config>
    [...]
    <global>
        <models>
            <my_segmentanalyticsextended>
                <class>My_SegmentAnalyticsExtended_Model</class>
            </my_segmentanalyticsextended>
            <segment_analytics>
                <rewrite>
                    <controller_page>My_SegmentAnalyticsExtended_Model_Controller_Page</controller_page>
                </rewrite>
            </segment_analytics>
        </models>
    </global>
    [...]
</config>

I'll issue a PR changing:

    $class = 'Segment_Analytics_Model_Controller_' . ucwords($action->action);
    $controller = new $class;

Into:

    $model = 'segment_analytics/controller_' . $action->action;
    $controller = Mage::getModel($model);

CC: @ianstormtaylor / @sperand-io

7ochem commented 9 years ago

The pull request (#71) I issued successfully solved my problem as described in this issue. Here are some test results:

furniture - google chrome_042

And the call to Segment appearing in the debugger:

selection_043

sperand-io commented 9 years ago

I'm afraid I don't have the depth of magento knowledge to effectively triage this — can't confirm it won't have any negative impacts. Definitely appears to be helpful but I'm cautious about merging changes I don't fully understand as we're currently in the "lights on" stage of maintenance here.

We'll be auditing this plugin in the future which may include a rewrite and in any case will involve a 3rd party magento expert — when we do, we'll give this some thought.

Thanks so much for the report and the PR!