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

Error when we save an optimizer with from date and/or to date values #917

Closed
PierreLeMaguer opened this issue May 17, 2018 · 11 comments
Closed
Assignees
Labels

Comments

@PierreLeMaguer
Copy link
Contributor

Preconditions

The problem occurs when we try to create/update an optimizer with non null fromdate AND todate. The back office is configure to use French date format.

Magento Version : 2.1.12

ElasticSuite Version : 2.3.11

Environment : Dev and Prod

Steps to reproduce

  1. Go to the Back office
  2. Create an optimizer with from date and to date.

Expected result

  1. Optimizer Saved with right values on the fields from_date and to_date

Actual result

Optimizer not saved and error : DateTime::__construct(): Failed to parse time string (23/05/2018) at position 0 (2): Unexpected character
Exception #0 (Exception): DateTime::__construct(): Failed to parse time string (23/05/2018) at position 0 (2): Unexpected character
#0 /var/www/algam/vendor/smile/elasticsuite/src/module-elasticsuite-catalog-optimizer/Model/Optimizer.php(289): DateTime->__construct('23/05/2018')
#1 /var/www/algam/vendor/smile/elasticsuite/src/module-elasticsuite-catalog-optimizer/Controller/Adminhtml/Optimizer/Save.php(55): Smile\ElasticsuiteCatalogOptimizer\Model\Optimizer->validateData(Object(Magento\Framework\DataObject))
#2 /var/www/algam/var/generation/Smile/ElasticsuiteCatalogOptimizer/Controller/Adminhtml/Optimizer/Save/Interceptor.php(24): Smile\ElasticsuiteCatalogOptimizer\Controller\Adminhtml\Optimizer\Save->execute()
#3 /var/www/algam/vendor/magento/framework/App/Action/Action.php(102): Smile\ElasticsuiteCatalogOptimizer\Controller\Adminhtml\Optimizer\Save\Interceptor->execute()
#4 /var/www/algam/vendor/magento/module-backend/App/AbstractAction.php(226): Magento\Framework\App\Action\Action->dispatch(Object(Magento\Framework\App\Request\Http))
#5 /var/www/algam/vendor/magento/framework/Interception/Interceptor.php(74): Magento\Backend\App\AbstractAction->dispatch(Object(Magento\Framework\App\Request\Http))
#6 /var/www/algam/vendor/magento/framework/Interception/Chain/Chain.php(70): Smile\ElasticsuiteCatalogOptimizer\Controller\Adminhtml\Optimizer\Save\Interceptor->___callParent('dispatch', Array)
#7 /var/www/algam/vendor/magento/framework/Interception/Chain/Chain.php(63): Magento\Framework\Interception\Chain\Chain->invokeNext('Smile\Elasticsu...', 'dispatch', Object(Smile\ElasticsuiteCatalogOptimizer\Controller\Adminhtml\Optimizer\Save\Interceptor), Array, 'adminAuthentica...')

  1. [Screenshot, logs]

One solution I found: /home/pilem/Travail/SmileProject/algam/vendor/smile/elasticsuite/src/module-elasticsuite-catalog-optimizer/Model/Optimizer.php:l288-289 -> replace new \DateTime() by $this->dateFilter->filter()

@romainruaud
Copy link
Collaborator

Yup, I am able to reproduce this one with my back-office set to french.

Feel free to propose us a PR if you have a working fix. But I'm not sure replacing new \DateTime() by $this->dateFilter->filter() will produce a comparable object.

Are you sure that if ($fromDate > $toDate) comparaison still works with your fix ?

However, since you are using an old and now unmaintained version of the module, we will probably not backport this fix into 2.3.x version.

But since the issue is occuring on a public method, you should be able to fix it on your project with an "around" plugin.

Thank you for reporting it.

Best regards

@romainruaud romainruaud added this to To do in Release 2.5.11 via automation May 17, 2018
@afoucret
Copy link
Contributor

But since the issue is occuring on a public method, you should be able to fix it on your project with an "around" plugin.

Would prefer a PR that fix the bug in the project !!!

@Amadeco
Copy link

Amadeco commented May 17, 2018

Good afternoon,

This is seems related to an old bug already mentionned in previous versions of Magento.
For reference :
magento/magento2#13139
magento/magento2#12399

This is will be fixed in magento 2.3.
magento/magento2#10686

The solution for now, if you do not want to extend smile elastic extension / magento core system, is to set your back-office in English language.

I am able to reproduce this error on our staging website, magento 2.2.4.

Regards,

Ilan PARMENTIER

@afoucret
Copy link
Contributor

@romainruaud : Can you have a look at this one ?

@romainruaud
Copy link
Collaborator

I don't understand, it's fixed by PR #918

@romainruaud romainruaud assigned afoucret and unassigned romainruaud Jun 11, 2018
@afoucret afoucret assigned romainruaud and unassigned afoucret Jun 11, 2018
@afoucret
Copy link
Contributor

I want you to test the PR carefully.

@djenvert
Copy link

Or write acceptance test 😀 #troll

@afoucret
Copy link
Contributor

That would be fine !!!

@romainruaud
Copy link
Collaborator

PR is working as intended.

Tested on an english Back-Office, and also with a french one. No issue, date is properly saved and displayed with both locales.

@romainruaud
Copy link
Collaborator

Just one thing, the commit could be cherry picked to 2.5.x instead of master.

@afoucret
Copy link
Contributor

PR merged

Release 2.5.12 automation moved this from To do to Done Jun 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

No branches or pull requests

5 participants