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

Marshal does not detect custom date format when locale parser is enabled #10873

Closed
1 of 3 tasks
micdobro opened this issue Jul 6, 2017 · 1 comment
Closed
1 of 3 tasks
Assignees
Milestone

Comments

@micdobro
Copy link
Contributor

micdobro commented Jul 6, 2017

This is a (multiple allowed):

  • bug

  • enhancement

  • feature-discussion (RFC)

  • CakePHP Version: 3.4.5

  • Platform and Target: NGINX, MariaDB, PHP 7.0.15

What you did

intl.default_locale is set to da_DK.

In bootstrap.php I have the following:

Type::build('time')
    ->useImmutable()
    ->useLocaleParser()
    ->setLocaleFormat('HH.mm'); 

That expects times in the format of: HH.mm - for example 03.20. Separator etc. adheres do Danish locale as described in ICU docs.

What happened

When using this shortened version, i.e. without seconds (03.20), then the marshal() method in class Cake\Database\Type\DateTimeType returns a string instead of FrozenTime object, as one of the tests is raising an exception (and the whole block of code depends on it).

It's precisely this test that raises the exception:

 elseif (is_numeric($value)) {
    $date = new $class('@' . $value);
}

It happens to be executed before this test:

elseif (is_string($value) && $this->_useLocaleParser) {
  return $this->_parseValue($value);
}

Input 03.20.00 is recognized correctly, as it does not evaluate to true on the is_numeric test.

What you expected to happen

My expectation would be: when Cake is instructed to use a locale-aware parser, and I also say to it: expect the input of the form HH.mm (according to ICU constants) - then I want in return a correct time-object instance, that is then correctly serialized into the DB column of type time.
At this point 03.20 is saved into db as 00:00:03.

I have at this point reversed the order of tests, i.e. I put the test that uses $this->_useLocaleParser flag in the else if statement before the is_numeric() test - and then everything works. I cannot see how this could fail in a broader context - hence a bug report to trigger a discussion.

@markstory markstory added this to the 3.4.10 milestone Jul 6, 2017
@markstory markstory self-assigned this Jul 6, 2017
@markstory markstory modified the milestones: 3.4.10, 3.4.11 Jul 10, 2017
markstory added a commit that referenced this issue Jul 11, 2017
Some locale based formats look like numeric values. If locale parsing is
enabled we need to enter that case instead of the numeric one. By
restricting the valid format of timestamps we can do that.

Refs #10873
@markstory
Copy link
Member

Pull request up now.

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

No branches or pull requests

3 participants