MageTest / BehatMage

Behat for Magento
MIT License
85 stars 31 forks source link

Added created_at to product default attributes #47

Closed robjstanley closed 10 years ago

robjstanley commented 10 years ago

Adding this prevents "No date part in '' found" Zend_Locale exception when creating product fixtures. Tested on Mage EE v1.14.

jamescowie commented 10 years ago

Hello @robjstanley can you attach the fixture you are using. I have not tested the extension in 1.14 yet but it feels strange that you would have to set created_at on a product object. This should be the product that sets when it was created and as like other versions dealt with at the database level.

jamescowie commented 10 years ago

Testing on 1.9 I get the same behaviour:

 No date part in '' found.

I think if this is going to be required in the extension we should change strtotime to be

new \DateTime();

Yet I would like to see why magento is allowing products to be created with a required field of "created_id" this feels to me like it should be set on object persistence. E.g. if you set a created date to the future what will the modified date within magento be if you edit it on the same day.

jamescowie commented 10 years ago

After some more investigation I managed to get some resolution. Can you run the following command and paste the output:

php -i | grep 'locale'

Initially I had no locale set. After modifying my locale settings in php.ini the error went away. Yet within Magento there was no error so I do not fully understand the issue.

robjstanley commented 10 years ago

You're right.

intl.default_locale => no value => no value

As you said, weird how Magento doesn't throw an exception but CLI does.

jamescowie commented 10 years ago

Hello again @robjstanley I am still testing the issues you reported and provided a patch for. Creating a new Vagrant installation using Magento 1.9 I can replicate the issues ( Centos 6 ) Yet using PHP settings I can not resolve the issue. Creating a new product via adminhtml works without error, As well as creating a basic php script that calls

Mage::getModel('catalog/product').......->save()

An alternative implementation that I have found from looking at the Product Model and product duplicate is to add this to the fixture within the create method:

$this->model
            ->setWebsiteIds(array_map(function($website) {
                return $website->getId();
            }, \Mage::app()->getWebsites()))
            ->setData($this->mergeAttributes($attributes))
            ->setCreatedAt(null)
            ->save();

Where I use the setCreatedAt(null)

However before I merge either implementation I would like to test both on more versions of Magento to ensure we are not breaking any functionality within the extension.

jamescowie commented 10 years ago

I have created a branch with my fix ( https://github.com/jamescowie/BehatMage/blob/feature/Fix-No-Date-Locale/src/MageTest/MagentoExtension/Fixture/Product.php#L81 ) I am open to suggestions for if we call the magic method or set as part of the attributes array. 1.7 ignores the value set so backwards compatibility is not effected via either method.

jamescowie commented 10 years ago

Closed but based on the discovery from @robjstanley I have added setCreatedAt() so we are adopting Magento terminology.