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

convert timesheets to UTC with support for user timezone #372

Merged
merged 33 commits into from
Feb 8, 2019
Merged

Conversation

kevinpapst
Copy link
Member

@kevinpapst kevinpapst commented Oct 23, 2018

Fixes #336 , Fixes #507 , Fixes #511 , Fixes #531

Before this PR is/was merged Kimai stored datetimes in the local timezone. But this is a huge problem for various reasons (for technical backgrounds, please read this and this), so this PR changes the way how times are stored.

  • Now times are stored in UTC and will be converted for the frontend into the users timezone
  • The timezone in which the record was created, will be stored together with the times in UTC
  • There is a new command that converts all existing timesheet record - this needs to be executed before you start using Kimai after the update!
  • Introduce a new boolean setting kimai.timesheet.rules.allow_future_times to allow future records (default = true, regression bug from 0.7).

Before you start two advices:

  • create a backup (at least of your timesheet table)
  • do not run that command more than once on the same entries, it does change the times on every run

This command can be safely executed if you did NOT import data from Kimai v1 before. This will convert ALL records in your database:

bin/console kimai:convert-timezone

As Kimai 1 saved the times in another format, they were imported in UTC already, so they don't need conversion => they would be wrong if you convert them. So if you imported data from Kimai v1, you need to know the IDs that need to be converted. The command can take a start and an end ID.

Lets say you started with an import of 1000 records from Kimai 1, then the first ID that needs to be converted is 1001 which you append to the command like this:

bin/console kimai:convert-timezone -f 1001

If you want to convert the first 100 records, then skip the next 1000 and convert the rest, you need to execute two commands:

bin/console kimai:convert-timezone -f 1 -l 100
bin/console kimai:convert-timezone -f 1101

If you have question about the conversion, post a comment (even if the PR is closed already).

Sorry for the inconvience to everyone, this is why Kimai 2 is not yet released in a major version

@kevinpapst kevinpapst added this to the 0.6 - Permissions milestone Oct 23, 2018
@kevinpapst kevinpapst added the help wanted needs a developer who want to add this feature label Nov 9, 2018
@kevinpapst kevinpapst removed this from the 0.6 - Global activities milestone Nov 9, 2018
# Conflicts:
#	src/EventSubscriber/UserPreferenceSubscriber.php
#	tests/Controller/ProfileControllerTest.php
# Conflicts:
#	src/Controller/Admin/AboutController.php
#	src/EventSubscriber/UserPreferenceSubscriber.php
@kevinpapst kevinpapst removed the help wanted needs a developer who want to add this feature label Jan 31, 2019
Copy link

@blochwitz blochwitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

really happy that you've found a solution.

your changes solves my issue #507 and all previous imported data from kimai1 have correct time now.

converting entries i've created in kimai2 works fine too...

Copy link

@blochwitz blochwitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Kevin,

i'm sorry but after checking the frontend i found a bug in calendar view: in list view all entries displayed with correct time but in calendar 1. the initial view won't display any entry (after switching it will) and 2. all entries displayed one hour earlier.

bildschirmfoto 2019-02-01 um 10 56 39
bildschirmfoto 2019-02-01 um 10 57 09

@kevinpapst
Copy link
Member Author

Yes, there was one important part missing, which was added just now:
the timezone in which a record was created is now saved as well and should be used in all places

@kevinpapst kevinpapst added this to the 0.8 - Export, Timezones, UI milestone Feb 1, 2019
@jaapcrezee
Copy link

jaapcrezee commented Feb 2, 2019

If I checked out this branch and run bin/console kimai:convert-timezone I get the following error:

  Command "kimai:convert-timezone" is not defined.  

  Did you mean one of these?                        
      kimai:create-user                             
      kimai:import-v1                               
      kimai:reset-dev                               
      kimai:version                                 

Do I have to restart something first (how?)?
BTW I am running kimai from docker kimai/kimai2:prod.

@kevinpapst kevinpapst changed the title added dynamic user timezone convert timesheets to UTC with support for user timezone Feb 4, 2019
# Conflicts:
#	public/build/manifest.json
#	src/Entity/Timesheet.php
#	src/Twig/DateExtensions.php
#	templates/admin/timesheet.html.twig
#	templates/timesheet/index.html.twig
#	tests/Twig/DateExtensionsTest.php
#	var/data/kimai_test.sqlite
@maaikez
Copy link

maaikez commented Feb 10, 2019

I'm sorry it took some time to answer.

I did not convert anything, I started with an empty kimai. Checked out the timezone branch and tested. But I can see you did some work on this merged with master. So if the docker container is working again (after updating we encoutered lots of errors :(), we will test and come back to you. I'm sorry if that takes some time.

@maaikez
Copy link

maaikez commented Feb 10, 2019

So the docker container works again.

We updated to the latest version. We did several things:

  • Set the time in the container to UTC
  • Set the time in the container to localtime
    Combined with:
  • Set the timezone (in user settings) in the application to UTC
  • Set the timezone (in user settings) in the application to localtime (in my case: Amsterdam).

Whatever we do, when creating a new entry, start time is one hour early (UTC). Nothing changes when doing one of the changes above.

When going to the calender view of this week, there is a red stripe on the current day / time. That stripe is really the current local time for me. The red stripe doesn't change place when changing to Amsterdam or UTC.

So it looks like nothing is happening actually when changing the timezone. And then I am only looking at what happens when creating a new entry.

However, it is great we can now start activities in the future, so it is not a problem if you change the time to +1 hour. Not how it should be ;).

I created #554 for this.

@kevinpapst
Copy link
Member Author

kevinpapst commented Feb 10, 2019

The calendar is not really interesting, that a javascript component using your computers local time (for the red stripe at least).

The timesheet table has the begin and timezone columns. Are the times in there correct?
I'd first like to understand if your problem is frontend or backend.

Kimai uses the system time and converts it to UTC before saving. When reading from the DB it will convert it back to the timezone, which was used while recording (this can either be the PHP timezone or the users timezone - if you configured one).

Kimai does rely on the correct system time and correctly configured timezones. Sounds as if the docker container might be a source of your problem. Do you have a chance to test with the built-in PHP web server on your local machine (see dev installation docu)?

@kevinpapst
Copy link
Member Author

Ok, lets discuss your topic in #554

blueowl04 added a commit to blueowl04/kimai2_ynh that referenced this pull request Mar 11, 2019
blueowl04 added a commit to blueowl04/kimai2_ynh that referenced this pull request Mar 11, 2019
blueowl04 added a commit to blueowl04/kimai2_ynh that referenced this pull request Mar 11, 2019
blueowl04 added a commit to blueowl04/kimai2_ynh that referenced this pull request Mar 15, 2019
blueowl04 added a commit to blueowl04/kimai2_ynh that referenced this pull request Mar 26, 2019
@wouterVE
Copy link

wouterVE commented Apr 3, 2019

Upon executing the command bin/console kimai:convert-timezone I got the following error:

10:07:59 ERROR     [console] Error thrown while running command "kimai:convert-timezone". Message: "An exception occurred in driver: could not find driver" ["exception" => Doctrine\DBAL\Exception\DriverException { …},"command" => "kimai:convert-timezone","message" => "An exception occurred in driver: could not find driver"]

In AbstractMySQLDriver.php line 106:

  An exception occurred in driver: could not find driver

In PDOConnection.php line 31:

  could not find driver

In PDOConnection.php line 27:

  could not find driver

I checked and the PDO extension is installed for PHP:

 $ php -m | grep PDO
 PDO

What else could be wrong?

kr
wouter

@kevinpapst
Copy link
Member Author

If Kimi works in the web, your PHP-Cli is missing the proper driver - as the exception says.

root@myserver:/var/www# php -m|grep pdo
pdo-mysql

Having PDO alone is not enough, you need to activate the specific DB driver.

@wouterVE
Copy link

wouterVE commented Apr 3, 2019

I've got ubuntu 16.04 so pdo-mysql is included in php-gd I've learned.

I've added the following 2 lines in /php/7.2/cli/php.ini

extension=pdo.so
extension=pdo_mysql.so

but still pdo-mysql is not enabled.

@kevinpapst
Copy link
Member Author

Afaik in Ubuntu you enable a PHP extension by linking its ini to /etc/php/7.2/cli/conf.d/.
Please don't get me wrong, but that is not a Kimai problem...

@wouterVE
Copy link

wouterVE commented Apr 4, 2019

You're right, this is purely a PHP problem.
It turned out I was missing php-mysql (only php7.2-mysql was installed)
Using the following command did solve my problem:

$sudo apt-get install php-mysql

Thanks for your help and keep up the good work!

wouter

@lock
Copy link

lock bot commented Jun 3, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. If you use Kimai on a daily basis, please consider donating to support further development of Kimai.

@lock lock bot locked and limited conversation to collaborators Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants