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

Method Mage_GiftMessage_Model_Observer::checkoutEventCreateOrder() does not exist #1154

Closed
midlan opened this issue Aug 20, 2020 · 12 comments
Closed
Labels

Comments

@midlan
Copy link
Contributor

midlan commented Aug 20, 2020

I have created test to test all observers and it screams:

Exception: Method Mage_GiftMessage_Model_Observer::checkoutEventCreateOrder() does not exist (observer: giftmessage; event: adminhtml_sales_order_create_create_order)

Event definition:
app/code/core/Mage/GiftMessage/etc/config.xml

            <adminhtml_sales_order_create_create_order>
                <observers>
                    <giftmessage>
                        <type>model</type>
                        <class>giftmessage/observer</class>
                        <method>checkoutEventCreateOrder</method>
                    </giftmessage>
                </observers>
            </adminhtml_sales_order_create_create_order>

And it is right, there is no such method there:
https://github.com/OpenMage/magento-lts/blob/1.9.4.x/app/code/core/Mage/GiftMessage/Model/Observer.php

@midlan midlan added the bug label Aug 20, 2020
@colinmollenhour
Copy link
Member

Nice find! Just delete the XML I suppose..

@midlan
Copy link
Contributor Author

midlan commented Aug 21, 2020

Seems right.

Looks like the event name is built with fullActionName but createAction never existed in Mage_Adminhtml_Sales_Order_CreateController according my research.

@tmotyl
Copy link
Contributor

tmotyl commented Aug 21, 2020

@midlan btw can you share your test? It would be nice to get it integrated in openmage.

@sreichel
Copy link
Contributor

@tmotyl I already work on a PR for N98-magerun ...

@sreichel sreichel added Cleanup and removed bug labels Aug 22, 2020
@midlan
Copy link
Contributor Author

midlan commented Aug 24, 2020

@tmotyl my test looks is this:

<?php

use Tester\Assert;

/**
 * @testCase
 */
class ObserversTest extends Tester\TestCase {

    public function testObservers() {

        $eventAreas = array(
            Mage_Core_Model_App_Area::AREA_GLOBAL,
            Mage_Core_Model_App_Area::AREA_FRONTEND,
            Mage_Core_Model_App_Area::AREA_ADMINHTML,
        );

        foreach ($eventAreas as $eventArea) {
            $eventConfig = Mage::app()->getConfig()->getNode(sprintf('%s/events', $eventArea));
            if ($eventConfig instanceof Mage_Core_Model_Config_Element) {
                foreach ($eventConfig->children() as $eventName => $event) {
                    foreach ($event->observers->children() as $observerName => $observer) {

                        /** create observer instance @see Mage_Core_Model_App::dispatchEvent */
                        $getInstanceMethod = 'getSingleton'; //singleton is default

                        switch($observer->type ?? '') {
                            case 'disabled':
                                continue 2; //do not check disabled observers

                            case 'object':
                            case 'model':
                            $getInstanceMethod = 'getModel';
                                break;
                        }

                        //check object exists
                        $modelClass = (string) $observer->class;

                        $observerInstance = Mage::{$getInstanceMethod}($modelClass);

                        Assert::type(
                            'object',
                            $observerInstance,
                            "Model '{$modelClass}' was not found. (observer: {$observerName}; event: {$eventName})"
                        );

                        //check method is public
                        $method = (string) $observer->method;

                        try {
                            $reflection = new ReflectionMethod($observerInstance, $method);

                        }
                        catch (ReflectionException $e) {
                            throw new Exception($e->getMessage() . " (observer: {$observerName}; event: {$eventName})");
                        }

                        Assert::true(
                            $reflection->isPublic(),
                            "Method '{$modelClass}::{$method}' is not public. (observer: {$observerName}; event: {$eventName})"
                        );

                    }
                }
            }
        }
    }

}

It is not phpunit but nette/tester; but I think migrations should be pretty easy

@midlan
Copy link
Contributor Author

midlan commented Mar 4, 2021

#1156
@colinmollenhour review pls?

@Flyingmana
Copy link
Contributor

@midlan we could also add a testrun to our github action with nette/tester if this is more comfortable.

@midlan
Copy link
Contributor Author

midlan commented Apr 6, 2021

Phpunit is more stable (more users, more development). We are using nette-tester because it runs in multiple threads so the tests are finished in less time.

@joshua-bn
Copy link
Contributor

@midlan do you have more tests? This is getting off-topic, but the community would benefit a lot from more tests.

@midlan
Copy link
Contributor Author

midlan commented Apr 6, 2021

I have lots of tests for our modules, but I have some for core:

tests/unit/app/code/core/Mage/Core/Model/App/ObserversTest.php
tests/unit/app/code/core/Mage/Core/Model/AbstractTest.php
tests/unit/app/code/core/Mage/Core/Model/ConfigTest.php
tests/unit/app/code/core/Mage/Catalog/controllers/CategoryControllerTest.php
tests/unit/app/code/core/Mage/Adminhtml/CatalogCategoryControllerTest.php
tests/unit/app/code/core/Mage/Adminhtml/Block/Widget/GridTest.php
tests/unit/app/code/core/Mage/Adminhtml/TinyMceFindAndReplaceTest.php
tests/unit/app/code/core/Mage/Adminhtml/Model/System/Config/Backend/FilenameTest.php
tests/unit/app/code/core/Mage/Adminhtml/SystemXmlTest.php
tests/unit/app/code/core/Mage/Cms/controllers/IndexControllerTest.php
tests/unit/app/code/core/Mage/CatalogSearch/controllers/ResultControllerTest.php
tests/unit/app/code/core/Mage/Admin/UserTest.php

Maybe we can discus it in discussion?

@addison74
Copy link
Contributor

Can we close this issue?

@midlan
Copy link
Contributor Author

midlan commented Jun 8, 2022

yeah, it is fixed:
#1156

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

No branches or pull requests

7 participants