\I18n\Time setTimezone does not work as expected #1807

nowackipawel · 2019-03-11T01:07:10Z

I looked into \Time code and.... it does not look good at all.

Additionally setTimezone does not work as excepted .


			$t0 = new \CodeIgniter\I18n\Time($transaction->tra_created_at);
			$t1 = $t0->setTimezone('America/Los_Angeles');
			d($t0, $t1);

https://codeigniter4.github.io/CodeIgniter4/libraries/time.html?highlight=settimezone

gives me:

https://screenshots.firefox.com/QrGBqc9Is0QPTGKo/dev.zdamyto.com
my $appTimezone === 'UTC'

I tried to fix it, but this code looks completely messy.

The text was updated successfully, but these errors were encountered:

lonnieezell · 2019-03-11T02:26:48Z

Any particular concerns, or just a general thumbing your nose at the code?

lonnieezell · 2019-03-11T02:33:36Z

What exactly were you trying to fix? All times are converted internally to UTC to handle any conversions, and, IIRC, might have been how the DateTime instance that it's extending handles things internally. It's been a while since I've looked at it.

If you print the time out does it give the correct timezone? The tests seem to indicate that it's working as expected.

lonnieezell · 2019-03-11T03:05:24Z

With the current version of the code, here's what I get:

$t0 = new Time('now', 'UTC');
$t1 = $t0->setTimezone(new \DateTimeZone('America/Chicago'));
d($t0, $t1);

// Gives: 
$t0 CodeIgniter\I18n\Time (6) 2019-03-11 03:00:40.329165+00:00 UTC
$t1 CodeIgniter\I18n\Time (6) 2019-03-11 03:00:40-05:00 CDT

and setting $appTimezone to UTC I get:

$t0 = new Time('2016-02-23 12:14:23');
$t1 = $t0->setTimezone(new \DateTimeZone('America/Chicago'));

$t0 CodeIgniter\I18n\Time (6) 2016-02-23 12:14:23+00:00 UTC
$t1 CodeIgniter\I18n\Time (6) 2016-02-23 12:14:23-06:00 CST

Both of which act as expected.

Is your code up to date? Though we haven't made changes to that in quite a while, I don't think. If it is reasonably up to date then what environment is that running on? OS? Server? PHP version? Can you verify what the system thinks date_default_timezone_get on your environment when you're trying to run that?

nowackipawel · 2019-03-11T10:34:33Z

Any particular concerns, or just a general thumbing your nose at the code?

Mainly thumbing nose, but I cannot understand why in almost all the cases we are returning new instance instead of working on the one which was passed / using to make operation on it. Not sure about performance.

What exactly were you trying to fix? All times are converted internally to UTC to handle any conversions, and, IIRC, might have been how the DateTime instance that it's extending handles things internally. It's been a while since I've looked at it.

If you print the time out does it give the correct timezone? The tests seem to indicate that it's working as expected.

I didn't get right time when I am printing properties like created_at / updated_at of Entities or when I am using format() method. I think that dates should be casted to appTimezone and imho they didn't.

According to:
your test I am working on:

newest repo
PHP 7.3.2-3+ubuntu18.10.1+deb.sury.org+1 (cli) (built: Feb 8 2019 15:44:30) ( NTS ),
PHP set: timelib version | 2018.01RC3, PHP set: Default timezone | Europe/Berlin
mariadb, xubuntu 18.10, newest nginx
and I am getting results as I've linked before.

The original issue was because I've passed string instead of \DateTimeZone instance - that's is because I looked many times in \I18n\Time code and internally it is using strings or \DateTimeZone interchangeably - explanation bellow.

///DO NOT READ THIS PART - BEGIN
Dates in my code do not change if I modify: App->appTimezone and America/Los_Angeles in the code bellow:

			$t0 = new \CodeIgniter\I18n\Time($transaction->tra_created_at);
			$t1 = $t0->setTimezone('America/Los_Angeles');
			d($t0, $t1);

/// DO NOT READ THIS PART - END

Hovewer I am experiencing another issue which is related to preious one and mentioned it above. Using code like this:

$t = new \DateTime('2019-03-10 00:00:00+01:00 CET'); // OR $t = new \DateTime('2019-03-10 00:00:00'); 
$t->setTimezone(new \DateTimeZone('AEDT'));
var_dump($t, $t->format('Y-m-d H:i:s'));

I get natural and expected results - time is converted to AEDT:

object(DateTime)#1 (3) {
  ["date"]=>
  string(26) "2019-03-10 18:00:00.000000"
  ["timezone_type"]=>
  int(2)
  ["timezone"]=>
  string(4) "AEDT"
}
string(19) "2019-03-10 18:00:00" // TIME IS CONVERTED TO AEDT

When I am trying to do the same with CI and \I18n\Time:

$t = new \CodeIgniter\I18n\Time('2019-03-10 00:00:00+01:00 CET'); // OR '2019-03-10 00:00:00' ; OR '2019-03-10 00:00:00', 'UTC'
$t->setTimezone(new \DateTimeZone('AEDT'));
var_dump($t, $t->format('Y-m-d H:i:s'));

I get:

object(CodeIgniter\I18n\Time)#76 (6) {
  ["timezone":protected]=>
  object(DateTimeZone)#83 (2) {
    ["timezone_type"]=>
    int(3)
    ["timezone"]=>
    string(13) "Europe/Berlin"
  }
  ["locale":protected]=>
  string(11) "en_US_POSIX"
  ["toStringFormat":protected]=>
  string(19) "yyyy-MM-dd HH:mm:ss"
  ["date"]=>
  string(26) "2019-03-10 00:00:00.000000"
  ["timezone_type"]=>
  int(1)
  ["timezone"]=>
  string(6) "+01:00"
}
string(19) "2019-03-10 00:00:00" // TIME IS NOT CONVERTED TO AEDT

lonnieezell · 2019-03-11T13:18:14Z

Mainly thumbing nose, but I cannot understand why in almost all the cases we are returning new instance instead of working on the one which was passed / using to make operation on it. Not sure about performance.

That's not a mess, it's a conscious design choice :) There's a large swath of PHP devs that feel that all objects should be immutable. While I'm not one of those, somewhere during the development of this library it started making sense for it to be an immutable class. Don't remember the specifics. That was awhile ago. I just verified and the first line of the docs does state it is immutable.

Because it's immutable and doesn't make changes on the original class, your final example won't work because you're not capturing the returned new Time instance. It would need to be like:

$t = new \CodeIgniter\I18n\Time('2019-03-10 00:00:00+01:00 CET'); // OR '2019-03-10 00:00:00' ; OR '2019-03-10 00:00:00', 'UTC'
$t = $t->setTimezone(new \DateTimeZone('AEDT'));

The original issue was because I've passed string instead of \DateTimeZone instance - that's is because I looked many times in \I18n\Time code and internally it is using strings or \DateTimeZone interchangeably - explanation bellow.

According to the docs passing a string should work just fine. If it's not then that's a bug. I'll check into that.

nowackipawel · 2019-03-11T15:04:37Z

I didn't notice it is immutable in docs, but in fact I didn't check for it and only for specific methods explanations. However I think I write my tests in a way as you pointed it out. I will pay more attention to docs.

Thank you for wide explanation.

According to your hints about how to use it. I've checked it and:

option1:
			$t0 = new \CodeIgniter\I18n\Time('2019-03-10 02:11:11', 'UTC');
			$t1 = $t0->setTimezone(new \DateTimeZone('America/Los_Angeles'));
			var_dump($t0, $t1, $t1->format('Y-m-d H:i:s'));
option2:
			$t0 = new \CodeIgniter\I18n\Time('2019-03-10 02:11:11+01:00 CET');
			$t1 = $t0->setTimezone(new \DateTimeZone('America/Los_Angeles'));
			var_dump($t0, $t1, $t1->format('Y-m-d H:i:s'));
option3:
			$t0 = new \CodeIgniter\I18n\Time('2019-03-10 02:11:11+01:00 CET');
			$t1 = $t0->setTimezone('America/Los_Angeles'); // no \DateTimeZone
			var_dump($t0, $t1, $t1->format('Y-m-d H:i:s'));
option4:
			$t0 = new \CodeIgniter\I18n\Time('2019-03-10 02:11:11+01:00 CET');
			$t1 = $t0->setTimezone(new \DateTimeZone('Asia/Bangkok')); // or without \DateTimeZone
			var_dump($t0, $t1, $t1->format('Y-m-d H:i:s'));

gives me:

object(CodeIgniter\I18n\Time)#77 (6) {
  ["timezone":protected]=>
  object(DateTimeZone)#84 (2) {
    ["timezone_type"]=>
    int(3)
    ["timezone"]=>
    string(3) "UTC"
  }
  ["locale":protected]=>
  string(11) "en_US_POSIX"
  ["toStringFormat":protected]=>
  string(19) "yyyy-MM-dd HH:mm:ss"
  ["date"]=>
  string(26) "2019-03-10 02:11:11.000000"
  ["timezone_type"]=>
option1:  int(3) / option2&3: int(1) / 
  ["timezone"]=>
option1: "UTC" / option2&3: "+01:00" /
}
object(CodeIgniter\I18n\Time)#88 (6) {
  ["timezone":protected]=>
  object(DateTimeZone)#87 (2) {
    ["timezone_type"]=>
    int(3)
    ["timezone"]=>
    string(19) "America/Los_Angeles"
  }
  ["locale":protected]=>
  string(11) "en_US_POSIX"
  ["toStringFormat":protected]=>
  string(19) "yyyy-MM-dd HH:mm:ss"
  ["date"]=>
  string(26) "2019-03-10 03:11:11.000000"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(19) "America/Los_Angeles"
}
string(19) "2019-03-10 03:11:11"


for option4 there is more changes:


object(CodeIgniter\I18n\Time)#77 (6) {
  ["timezone":protected]=>
  object(DateTimeZone)#84 (2) {
    ["timezone_type"]=>
    int(3)
    ["timezone"]=>
    string(3) "UTC"
  }
  ["locale":protected]=>
  string(11) "en_US_POSIX"
  ["toStringFormat":protected]=>
  string(19) "yyyy-MM-dd HH:mm:ss"
  ["date"]=>
  string(26) "2019-03-10 02:11:11.000000"
  ["timezone_type"]=>
  int(1)
  ["timezone"]=>
  string(6) "+01:00"
}
object(CodeIgniter\I18n\Time)#88 (6) {
  ["timezone":protected]=>
  object(DateTimeZone)#87 (2) {
    ["timezone_type"]=>
    int(3)
    ["timezone"]=>
    string(12) "Asia/Bangkok"
  }
  ["locale":protected]=>
  string(11) "en_US_POSIX"
  ["toStringFormat":protected]=>
  string(19) "yyyy-MM-dd HH:mm:ss"
  ["date"]=>
  string(26) "2019-03-10 02:11:11.000000"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(12) "Asia/Bangkok"
}
string(19) "2019-03-10 02:11:11"

so date looks like it is converted between UTC and my "PHP set: Default timezone | Europe/Berlin" option 1,2,3 and there is no change in date for option4.

Adds possibility to set mutateDate to return \DateTime instead of \CodeIgniter\I18n\Time instances. To make it work simpy use this code in your Entity child constructor: ``` public function __construct(?array $data = null) { parent::__construct($data, ['dateMutateAs' => 'DateTime']); } ``` or ``` public function __construct(?array $data = null) { parent::__construct($data); $this->setMutateDateAs('DateTime'); // will return boolean - value changed === true. } ``` I think it is required to add some code to make timezone changes work for \CodeIgniter\I18n\Time instance. For make this work in predictable way I think we should make some changes in \CodeIgniter\I18n\Time (codeigniter4#1807)

jim-parry · 2019-03-25T03:36:13Z

@nowackipawel This is a long & convoluted thread. Has the issue been resolved? Can the issue be closed?

atishhamte · 2019-04-08T06:25:50Z

@nowackipawel, any update on this?

nowackipawel · 2019-04-08T07:10:47Z

@jim-parry , @atishamte
I made PR which was good in my opinion, it allows user to select type of class (DateTime or \I18\Time) for storing datetime properties (#1815) , but I have to agree with @lonnieezell doubts. The most important was that Entity class has to store datetime as \I18\Time.
There are to solutions:

according to CI architecture and deep \I18\Time integration the easiest solution to make it work as excepted is to extend Entity class - i.e. https://pastebin.com/EFXSpKkR
replace entire \I18\Time what is big job.

lonnieezell · 2019-04-09T03:15:06Z

I think the question, though, is with the 4 examples, are they all working as expected within how the framework is designed (nothing with extending, etc)...

MGatner · 2020-08-25T15:51:03Z

Most of this thread is opinions about immutable classes, which won't change. I confirmed that "Option 4" above now works as expected (changes the time). Since all other tests are passing this is officially resolved.

nowackipawel changed the title ~~\I18n\Time looks buggy~~ \I18n\Time looks buggy [+setTimezone] Mar 11, 2019

nowackipawel changed the title ~~\I18n\Time looks buggy [+setTimezone]~~ \I18n\Time setTimezone does not work as expected Mar 11, 2019

nowackipawel mentioned this issue Mar 12, 2019

Avibility to change mutateDate behaviour #1815

Closed

jim-parry added the can of worms label Sep 17, 2019

devorama mentioned this issue Jul 12, 2020

Bug: CodeIgniter\I18n -> Class 'Locale' not found #3294

Closed

MGatner closed this as completed Aug 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

\I18n\Time setTimezone does not work as expected #1807

\I18n\Time setTimezone does not work as expected #1807

nowackipawel commented Mar 11, 2019 •

edited

Loading

lonnieezell commented Mar 11, 2019

lonnieezell commented Mar 11, 2019

lonnieezell commented Mar 11, 2019

nowackipawel commented Mar 11, 2019

lonnieezell commented Mar 11, 2019

nowackipawel commented Mar 11, 2019

jim-parry commented Mar 25, 2019

atishhamte commented Apr 8, 2019

nowackipawel commented Apr 8, 2019

lonnieezell commented Apr 9, 2019

MGatner commented Aug 25, 2020

\I18n\Time setTimezone does not work as expected #1807

\I18n\Time setTimezone does not work as expected #1807

Comments

nowackipawel commented Mar 11, 2019 • edited Loading

lonnieezell commented Mar 11, 2019

lonnieezell commented Mar 11, 2019

lonnieezell commented Mar 11, 2019

nowackipawel commented Mar 11, 2019

lonnieezell commented Mar 11, 2019

nowackipawel commented Mar 11, 2019

jim-parry commented Mar 25, 2019

atishhamte commented Apr 8, 2019

nowackipawel commented Apr 8, 2019

lonnieezell commented Apr 9, 2019

MGatner commented Aug 25, 2020

nowackipawel commented Mar 11, 2019 •

edited

Loading