InvoicePlane / InvoicePlane

A self-hosted open source application for managing your invoices, clients and payments.
https://www.invoiceplane.com
Other
2.55k stars 799 forks source link

System settings fail silently if timezone is not properly declared #32

Closed dekomote closed 9 years ago

dekomote commented 10 years ago

System settings fail silently if timezone is not properly declared.

The code in question is https://github.com/InvoicePlane/InvoicePlane/blob/master/application/modules/settings/controllers/settings.php#L140

This can be frustrating for users installing the software on some hosting providers.

Kovah commented 10 years ago

Took a look at this issue. As every webserver should have set a timezone by default this is a very unlikely issue. Could you provide information about the error that is thrown (in the logs)?

Kovah commented 10 years ago

@dekomote Any news about this?

lommes commented 9 years ago

I came across this issue on several systems (some time ago) where, depending on OS and PHP-version, date.timezone in php.ini had no value and systems timezone was not available to PHP

There should be a warning in E-STRICT (see http://php.net/manual/de/function.date-default-timezone-get.php#74632). It seems that in PHP prior 5.4 a value was guessed if no value was present.

roundcube for example checks during setup if the value is present.

Jadaw1n commented 9 years ago

Do I assume right, that this code should work better? new DateTime(null, new DateTimeZone(date_default_timezone_get())); As per the documentation, if date_default_timezone_get() can't get any useful value it just returns "UTC". This is a valid string for DateTimeZone. And if this is valid, then new DateTime should also work. And if date.timezone is not valid there are enough other errors thrown.

lommes commented 9 years ago

Yes, that seems to fix it and it still triggers a Warning.

Same issue should appear when creating invoices and quotes because DateTime is also used in: application\helpers\date_helper.php application\modules\invoices\models\mdl_invoices.php application\modules\quotes\models\mdl_quotes.php

I was able to reproduce the error on Win 7 and xampp. When you use development as ENVIRONMENT in index.php many warnings occur because of the missing timezone.

lommes commented 9 years ago

An alternative would be to modify setup to check timezone. If that's your prefered approach I can provide a PR for that.

lommes commented 9 years ago

In my opinion it would be best to include a check in setup with a warning and setting the timezone to date_default_timezone_get() at an early point of each request (maybe in construct of Admin_Controller or MX_Controller) so https://github.com/InvoicePlane/InvoicePlane/pull/154 and the modification of CI core files would be not necessary

Jadaw1n commented 9 years ago

Yeah it's probably better if we check this in the setup. If something new uses DateTime the same problem will happen again. So if you can submit your PR... :)