Error date time to save #11306

raumatbel · 2017-10-09T12:10:27Z

Description

Save date time correctly in different timezone and locale

Problem

The function$formatter->parse($date)always return false if the date doesn't contains time and the function new \DateTime($date) return the error "Invalid input datetime format of value".

Solution

Before of the function$formatter->parse($date) it checks if the string contains time or not. If it doesn't contains, it concat the value 00:00 to the string "date".

Fixed Issues (if relevant)

Set product as new "from" and "to" not being interpreted correctly #10580: Set product as new "from" and "to" not being interpreted correctly
Error: Invalid input datetime format of value '25/07/+00201717' #10485: Error: Invalid input datetime format of value '25/07/+00201717

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

orlangur · 2017-10-10T21:20:50Z

lib/internal/Magento/Framework/Stdlib/DateTime/Timezone.php

-        \IntlDateFormatter::SHORT,
-    ];
+    protected $_allowedFormats
+        = [


Please change this formatting back to normal & consistent with other code.

orlangur · 2017-10-10T21:22:01Z

lib/internal/Magento/Framework/Stdlib/DateTime/Timezone.php

+                if (\Zend_Locale::isLocale($locale)) {
+                    $localeDate = $locale;
+                }
+                $date = $formatter->parse($date) ?: (new \Zend_Date(


Zend_Locale and Zend_Date must not be used, there was a separate refactoring effort to get rid of them and some static tests are checking they are not used.

There should be some intl or PHP library equivalents.

@orlangur I have used this because the format date is in Zend

@orlangur What can you recommend me to continue with this?

Only native intl classes, like \DateTime, \Locale etc.

orlangur · 2017-10-10T21:23:23Z

lib/internal/Magento/Framework/Stdlib/DateTime/Timezone.php

@@ -3,38 +3,47 @@
 * Copyright © Magento, Inc. All rights reserved.
 * See COPYING.txt for license details.
 */
+


Looks like some new lines are automatically added by your IDE, please avoid this.

orlangur · 2017-10-10T21:23:31Z

lib/internal/Magento/Framework/Stdlib/DateTime/Timezone.php

@@ -218,6 +238,7 @@ public function scopeTimeStamp($scope = null)
        @date_default_timezone_set($timezone);
        $date = date('Y-m-d H:i:s');
        @date_default_timezone_set($currentTimezone);
+


Looks like some new lines are automatically added by your IDE, please avoid this.

okorshenko · 2017-11-07T22:23:32Z

Hi @raumatbel
Could you please fix broken tests so that we can proceed with PR acceptance?
Thank you

raumatbel · 2017-11-08T02:54:13Z

Hi @okorshenko, I have been very busy but in this week I'm going to try solve the problem with the tests

gwharton · 2017-11-08T11:10:31Z

Hi, Once approved, can these changes be merged into 2.1 and 2.2 aswell?

raumatbel · 2017-11-08T11:19:12Z

That is the idea. Firstly I want solve this problem in the main branch and after I do the backports.

gwharton · 2017-11-08T11:21:40Z

Hi there @raumatbel

I'm new to github so don't know the process. Thanks for letting me know. I've just seen so many issues reported under 2.1/2.2, fixed in 2.3-develop and then get forgotten. I didnt know if stuff gets automatically backported, or whether a new PR has to be created etc... Anyway, I see you have it in hand.

Thanks

raumatbel · 2017-11-08T11:27:39Z

Welcome @gwharton.
Normally if the issue is reported in the 3 branches, you have to do 3 PR, one for every branch for that the magento team can merged directly.

orlangur · 2017-11-17T11:09:04Z

lib/internal/Magento/Framework/Stdlib/DateTime/Timezone.php

-                $date = $formatter->parse($date) ?: (new \DateTime($date))->getTimestamp();
+
+                $localeDate = !empty($locale) ? $locale : null;
+                $date = $formatter->parse($date) ?: (new \Zend_Date(


Still not solved: #11306 (comment)

I'm trying to solve this problem, but it's not easy and it will take me a while fix it.

orlangur · 2017-11-17T11:09:39Z

lib/internal/Magento/Framework/Stdlib/DateTime/Timezone.php

@@ -150,6 +158,7 @@ public function getDateTimeFormat($type)
    }

    /**
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)


Refactor bad smelling code instead of suppressing warning whenever possible.

This annotation needs to be removed and corresponding code refactored (unlike the PHPMD.CouplingBetweenObjects it should be pretty easy).

gwharton · 2017-11-17T11:54:50Z

I don't know if it is related, but there is another timezone/time/formatting bug in #10663

orlangur

Thanks for improving implementation using only native PHP classes!

After changes are applied, please force push as a single commit.

orlangur · 2017-11-27T21:16:45Z

lib/internal/Magento/Framework/Stdlib/DateTime/Timezone.php

@@ -150,6 +158,7 @@ public function getDateTimeFormat($type)
    }

    /**
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)


This annotation needs to be removed and corresponding code refactored (unlike the PHPMD.CouplingBetweenObjects it should be pretty easy).

orlangur · 2017-11-27T21:23:22Z

lib/internal/Magento/Framework/Stdlib/DateTime/Timezone.php

@@ -174,7 +183,13 @@ public function date($date = null, $locale = null, $useTimezone = true, $include
                    $timeType,
                    new \DateTimeZone($timezone)
                );
+
+                //Know if the date contains time or not
+                if (!preg_match('/(.*).(\d{2}):(\d{2})/', $date)) {


Looking at logic of this method I believe it should be rewritten into

if ($includeTime && !preg_match('\d{2}:\d{2}', $date))

(regexp note: no need in brackets, first part seems to be useless)
(comment could be removed with such condition)

orlangur · 2017-11-27T21:33:09Z

@raumatbel would be nice to rebase against latest 2.3-develop and check if #10754 (comment) is true (we can do it on our side during testing as well).

xmav · 2017-11-29T00:03:57Z

lib/internal/Magento/Framework/Stdlib/DateTime/Timezone.php

+
+                //Know if the date contains time or not
+                if (!preg_match('/(.*).(\d{2}):(\d{2})/', $date)) {
+                    $date .= " 00:00:00";


With $timeType set to \IntlDateFormatter::SHORT
"00:00:00" should be changed to "0:00am" to be parsed

http://php.net/manual/en/class.intldateformatter.php

orlangur · 2017-12-14T23:39:01Z

Hi @raumatbel, according to QA report #10485 is still reproduced after these changes. Could you please check if it works for you?

raumatbel · 2017-12-23T11:54:20Z

Hi @orlangur I have checked the issue again. Without my changes the product I can not save it due to the problem of the dates, but with my changes it works fine. Do you can check again? If it doesn't work, can you indicate the steps to reproduce?

magento-engcom-team · 2018-01-09T08:40:12Z

@raumatbel

Steps to reproduce:
Login to Magento admin
Change admin locale to the United Kingdom.
Try to create new product
Put "Set product as new from" on creating product page.
-Try to save the product.

Expected result:
Product is saved successfully as no changes are made from the previous state of the product

Actual result
An error Invalid input DateTime format of value '25/07/+00201717' appears and the product is not saved

okorshenko · 2018-01-26T21:01:15Z

Hi @raumatbel
we are closing this PR due to inactivity. If you would like to proceed with this PR, feel free reopen it. Thank you

raumatbel · 2018-01-29T09:11:34Z

Hi @okorshenko , I apologize for the delay in answering but I'm very busy this weeks. This weekend I'm going to review again. I reopen PR

raumatbel · 2018-02-03T11:20:22Z

@okorshenko @orlangur I have been reviewing and I have checked that with this PR this error is fixed. I attach the screenshots in the branch 2.3-develop and my branch with my solution.

Settings account

Product in branch 2.3-develop

Product with PR

magento-engcom-team · 2018-02-08T02:00:37Z

@raumatbel thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

pablobae · 2018-02-08T08:37:40Z

This fix works! Good job, @raumatbel

raumatbel · 2018-02-08T08:52:04Z

@magento-engcom-team I had already permissions in this repository as "member".

SamB-GB · 2018-03-30T16:43:59Z

This problem is still present in 2.2.3

rmakara · 2018-04-18T08:39:15Z

This problem appears in 2.1.9. Could somebody backport it? :) I would really appreciate!

okorshenko added the SQUASHTOBERFEST label Oct 9, 2017

okorshenko self-assigned this Oct 9, 2017

okorshenko added this to the October 2017 milestone Oct 9, 2017

okorshenko added the develop label Oct 9, 2017

magento-team added the Progress: needs update label Oct 9, 2017

vkublytskyi added the partner contribution label Oct 10, 2017

orlangur requested changes Oct 10, 2017

View reviewed changes

okorshenko assigned orlangur Oct 13, 2017

magento-engcom-team changed the base branch from develop to 2.3-develop October 20, 2017 15:32

okorshenko modified the milestones: October 2017, November 2017 Nov 1, 2017

magento-engcom-team added bugfix Fixed in 2.3.x The issue has been fixed in 2.3 release line labels Nov 7, 2017

magento-team added Progress: needs update and removed Progress: needs update labels Nov 10, 2017

orlangur requested changes Nov 17, 2017

View reviewed changes

raumatbel force-pushed the FR#ErrorDateTimeDevelop branch from 924cbe5 to 0d3b92e Compare November 25, 2017 17:59

magento-team removed the Progress: needs update label Nov 27, 2017

orlangur requested changes Nov 27, 2017

View reviewed changes

magento-team added the Progress: needs update label Nov 27, 2017

xmav reviewed Nov 29, 2017

View reviewed changes

raumatbel force-pushed the FR#ErrorDateTimeDevelop branch from 0d3b92e to c77a9de Compare November 29, 2017 19:53

magento-engcom-team added the Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release label Dec 12, 2017

okorshenko removed the 2.3.x label Dec 14, 2017

okorshenko modified the milestones: December 2017, January 2018 Jan 8, 2018

orlangur removed the Progress: accept label Jan 9, 2018

okorshenko closed this Jan 26, 2018

raumatbel reopened this Jan 29, 2018

raumatbel mentioned this pull request Feb 3, 2018

Set product as new "from" and "to" not being interpreted correctly #10580

Closed

okorshenko modified the milestones: January 2018, February 2018 Feb 7, 2018

This was referenced Feb 8, 2018

Error: Invalid input datetime format of value '25/07/+00201717' #10485

Closed

2.1.8 : Cart Rules date is invalid format in French #10686

Closed

Magento 2.1.8 : Cart Rules + New Customer (Please enter a valid date) Locale AU #10754

Closed

magento-engcom-team merged commit b237558 into magento:2.3-develop Feb 8, 2018

magento-engcom-team pushed a commit that referenced this pull request Feb 8, 2018

MAGETWO-81339: Error date time to save #11306

4aaa109

gwharton mentioned this pull request Feb 8, 2018

[Backport-2.2] of PR-#11306 Save date time correctly in different timezone and local #13560

Closed

4 tasks

Cristiano81 mentioned this pull request Mar 12, 2018

Exception Error in Catalog Price Rule while Backend language is not English #12399

Closed

magento-engcom-team added the Award: complex label Mar 22, 2018

orlangur mentioned this pull request Jun 5, 2018

Fixed Datetime Error on Newsletter Template #15783

Closed

4 tasks

Error date time to save #11306

Error date time to save #11306

Uh oh!

Conversation

raumatbel commented Oct 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

Solution

Fixed Issues (if relevant)

Contribution checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raumatbel Oct 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

okorshenko commented Nov 7, 2017

Uh oh!

raumatbel commented Nov 8, 2017

Uh oh!

gwharton commented Nov 8, 2017

Uh oh!

raumatbel commented Nov 8, 2017

Uh oh!

gwharton commented Nov 8, 2017

Uh oh!

raumatbel commented Nov 8, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gwharton commented Nov 17, 2017

Uh oh!

orlangur left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

orlangur Nov 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

orlangur commented Nov 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

orlangur commented Dec 14, 2017

Uh oh!

raumatbel commented Dec 23, 2017

Uh oh!

magento-engcom-team commented Jan 9, 2018

Uh oh!

okorshenko commented Jan 26, 2018

Uh oh!

raumatbel commented Jan 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

raumatbel commented Feb 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

magento-engcom-team commented Feb 8, 2018

Uh oh!

pablobae commented Feb 8, 2018

raumatbel commented Oct 9, 2017 •

edited

Loading

raumatbel Oct 13, 2017 •

edited

Loading

orlangur Nov 27, 2017 •

edited

Loading

raumatbel commented Jan 29, 2018 •

edited

Loading

raumatbel commented Feb 3, 2018 •

edited

Loading