0

I have been asked to assess the coding standards for a customised Magento installation and have come across four folders that have been moved into the local/Mage folder in order override the core/Mage.

The folders that have been moved are:

Mage/Customer
Mage/Page
Mage/Review
Mage/Wishlist

The main concern is Page (for obvious reasons) as it controls the template that is being used. I'm not sure how I could begin to seperate the site in order to not override the core in this way so would like some advice please?

What would be the best approach here??

local/Mage/Page/config.xml

<config>
    <modules>
        <Mage_Page>
            <version>1.6.0.0</version>
        </Mage_Page>
    </modules>
    <global>
        <models>
            <page>
                <class>Mage_Page_Model</class>
            </page>
        </models>
        <blocks>
            <page>
                <class>Mage_Page_Block</class>
            </page>
        </blocks>
        <page>
            <layouts>
                <empty module="page" translate="label">
                    <label>Empty</label>
                    <template>page/empty.phtml</template>
                    <layout_handle>page_empty</layout_handle>
                </empty>
                <one_column module="page" translate="label">
                    <label>1 column</label>
                    <template>page/1column.phtml</template>
                    <layout_handle>page_one_column</layout_handle>
                    <is_default>1</is_default>
                </one_column>
                <two_columns_left module="page" translate="label">
                    <label>2 columns with left bar</label>
                    <template>page/2columns-left.phtml</template>
                    <layout_handle>page_two_columns_left</layout_handle>
                </two_columns_left>
                <two_columns_right module="page" translate="label">
                    <label>2 columns with right bar</label>
                    <template>page/2columns-right.phtml</template>
                    <layout_handle>page_two_columns_right</layout_handle>
                </two_columns_right>
                <three_columns module="page" translate="label">
                    <label>3 columns</label>
                    <template>page/3columns.phtml</template>
                    <layout_handle>page_three_columns</layout_handle>
                </three_columns>
                <home module="page" translate="label">
            <label>Home</label>
            <template>page/1column_home.phtml</template>
            <layout_handle>page_home</layout_handle>
        </home>

            </layouts>
        </page>
    </global>
    <frontend>
        <translate>
            <modules>
                <Mage_Page>
                    <files>
                        <default>Mage_Page.csv</default>
                    </files>
                </Mage_Page>
            </modules>
        </translate>
        <layout>
            <updates>
                <page>
                    <file>page.xml</file>
                </page>
            </updates>
        </layout>
    </frontend>
    <adminhtml>
        <translate>
            <modules>
                <Mage_Page>
                    <files>
                        <default>Mage_Page.csv</default>
                    </files>
                </Mage_Page>
            </modules>
        </translate>
    </adminhtml>
    <install>
        <translate>
            <modules>
                <Mage_Page>
                    <files>
                        <default>Mage_Page.csv</default>
                    </files>
                </Mage_Page>
            </modules>
        </translate>
    </install>
    <default>
        <design>
            <head translate="default_description" module="page">
                <default_title>Magento Commerce</default_title>
                <default_description>Default Description</default_description>
                <default_keywords>Magento, Varien, E-commerce</default_keywords>
                <default_robots>*</default_robots>
                <default_media_type>text/html</default_media_type>
                <default_charset>utf-8</default_charset>
            </head>
            <header translate="welcome" module="page">
                <logo_src>images/logo.gif</logo_src>
                <logo_alt>Magento Commerce</logo_alt>
                <welcome>Default welcome msg!</welcome>
            </header>
            <footer translate="copyright" module="page">
                <copyright>&amp;copy; 2012 Magento Demo Store. All Rights Reserved.</copyright>
            </footer>
        </design>
        <system>
            <media_storage_configuration>
                <allowed_resources>
                    <site_favicons>favicon</site_favicons>
                </allowed_resources>
            </media_storage_configuration>
        </system>
    </default>
</config>
6

I'm going to respectfully disagree with Fabian if only just to take the contrarian point of view (his assessment of a better way is spot-on):

It's never okay.

Copying files to the local code pool should be a method of last resort. These last resorts include the following reasons:

  • Every method in the class has to change substantially (could still use a rewrite, but ok.)
  • You're backporting a class file from a newer or older version of Magento to support a plugin or Connect module you're working with. This is a rare edge case; copying the class and falling back to local code pool allows you to do this gracefully without actually overwriting the core itself.
  • You're patching Magento until a core patch is released for a known or suspected issue (and you've probably reported it as a legitimate bug with a diff to patch)
  • You're making any changes at all to Zend Framework or Varien lib files

Everything else can be done with rewrites, observers. I take the view that copying to app/code/local/Mage is the easy way out and should never be used short of debugging.

Using app/code/local/Mage makes it difficult to debug issues in stores, especially when changing developers or people unfamiliar are having to sort through issues.

It also makes upgrading more troublesome. Rather than upgrading your core, you have to now manually diff all app/code/local/Mage class files against their now-upgraded canonicals. This is nightmarish; Fabian touched on this.

|improve this answer|||||
  • Thank for the contrarian view. So basically its a mess!!?! I was attempting to create a local module in order to load the template BUT although this method could work it actually doesn't since some elements are not loading. It also isn't a very elegant way to do it. So my question is this: Should I rebuild the template from scratch??? – user1704524 Jul 24 '13 at 21:34
  • If I understand correctly, yes, you should consider moving any method calls or modified methods from the default class files into their own modules. Because this is theme related I (ignorantly) assume that the only real concern would be in blocks. You'll need to make sure that all block method calls in templates exist in the modified classes. It's not impossible - but you'll have to be very thorough. – philwinkle Jul 24 '13 at 21:43
  • its gonna be a loooooonnng! – user1704524 Jul 24 '13 at 21:45
5

Yes it is, but...

Is it okay to override core/Mage/Page by moving it to the local codePool?

First: yes it is, this is the intention of the include_path. But if you think a little bit deeper (is this correct english?) then No. It is a very bad idea.

When you overwrite the class this way, you have big problems after upgrades, because the old (changed) class is loaded an none of the changes is used of the newer version.

Better way

The better way is to use <rewrite>s. The big advantage is, that you extend the original mage class and therefore you are able to upgrade your magento core. Afterwards only the changes you made explicitly by overwriting the method are still existing. The other updates are used.

|improve this answer|||||
  • Thanks for your answer and you have confirmed what I was thinking. The code that I need to refactor is overriding Mage/Page in order to load the actual template itself... I found this to be a strange way to go about it but would you say I should (a). Create a module using rewrites to still load the template in the intended fashion OR (b). Rebuild the template from scratch using the correct coding standard? Your opinion would be appreciated... – user1704524 Jul 24 '13 at 19:52
  • Extending the core models is definitely the better way. – Ryre Jul 24 '13 at 19:55
  • I'm sorry I can't follow you. user2157. But maybe I can answer anyway. If Magento's Page Class loads ANY template hardcoded, it is wrong. Magento has a great layout-xml mechanism to load templates. this should be used. If you show me the Page-class I maybe can answer better. – Fabian Blechschmidt Jul 24 '13 at 20:01
  • 1
    Sorry, I'm out. templateing is not my part. "My" frontend-dev used the local.xml in the template afaik to add the templates, but I'm not sure. What you can definitly do is to put all that stuff in your own config.xml, because all the XMLs are merged together... – Fabian Blechschmidt Jul 24 '13 at 20:11
  • 1
    rewrites are alway bad, because you can only have one. If you have two extension which rewrites the same class you are in trouble. Therefore if you can solve a problem without rewriting (with observers, templates, layout-updates, ...) use this way ;-) – Fabian Blechschmidt Jul 25 '13 at 15:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.