projecthamster / hamster

GNOME time tracker
http://projecthamster.org
GNU General Public License v3.0
1.08k stars 249 forks source link

Tasks in the morning are storen on the wrong day #419

Closed ego2dot0 closed 5 years ago

ego2dot0 commented 5 years ago

If start a task early in the morning (<5am), the task is added the day before, instead of the current day. Correcting this task is a problem too. I have to add I it tomorrow in the edit view, to insert it today.

This behaviour overrides all the early day tracking of the day before.

GeraldJansen commented 5 years ago

This is a feature for people that work late at night and into the early hours of the next day.

You can change the behaviour in the settings (Time Tracker Preferences -> New day starts at: 05:00).

ego2dot0 commented 5 years ago

That feature has an bug. If I add a task from 04:00 - 06:00 at 16th (15th day before) of June. It will be added at 14th of June. Same if i start a new task in the night. It will be placed 24h hours before and overrides existing times.

add-task result-task

[Edit]: Attached screenshots

ederag commented 5 years ago

Thanks for the screenshots. On current master (0ddce419f4e367f27b76dff6eb6494945bbd7fe0), the activity overlapping end-of-day time appears at the "correct" day, i.e. the hamster day of the start time. (that would be the 15th in your example, not the 14th as in the screenshot). So previous data would not be overwritten.

What is your version ?

Nevertheless, in master there is a remaining bug because the duration is negative. I'll look into that. Thanks again for the report !

pwhipp commented 5 years ago

The negative duration will disappear if you restart the app

ederag commented 5 years ago

The negative duration will disappear if you restart the app

@pwhipp Can't reproduce (negative is persistent here). Which version are you using ?

pwhipp commented 5 years ago

@ederag Hmm... no version in 'help about' :| It was master.zip downloaded on July 8th.

I did also set the day start time in the tracking settings appropriately before restarting (e.g. 02:30 in my case because I often have to start work at 04:30).

ederag commented 5 years ago

@pwhipp So we should have the same results. Thanks for giving the day start time. What were the start and end times of the activity ?

pwhipp commented 5 years ago

@ederag I can't remember but it would have started with the default day start time and I regularly start work between 04:30 and 05:30 so I'm guessing the offending times were in that range.

ederag commented 5 years ago

I did also set the day start time in the tracking settings appropriately before restarting (e.g. 02:30 in my case)

@pwhipp Got it; after restart the activity was not overlapping the hamster day start time any longer. Yet a change in the hamster day start time was taken into account only after hamster restart. Actually just changing the date (left/right arrows) back and forth seems to be enough to update the display. That would be a different bug. It will have to wait until the forthcoming release, it seems too late to touch sensitive parts of the code.

GeraldJansen commented 5 years ago

I can confirm that there is a problem assigning the start_time from the edit_activity window (add or update).
My day_start is 05:00 and it is currently late afternoon on 2018-08-25. Starting tracking a new ongoing task at 04:00 with "Add Earlier Activity" or from the command line results in:

activity|start time|end time|duration minutes|category|description|tags
Test from command line|2019-08-25 04:00:00||856.0|Test||
Test from edit_activity window|2019-08-26 04:00:00||-584.0|Test||

Stopping tracking results in the silent deletion of the second activity.
Inserting a completed activity "04:00 06:00 Test@Test" from edit_activity results in start date later than end date: Test|2019-08-26 04:00:00|2019-08-25 06:00:00|-1320.0|Test||

I guess the real date should always be stored in the DB whereas the hamster_day should only be used to display the activities and statistics. I can try to resolve this.

ederag commented 5 years ago

@GeraldJansen Thanks for the test case, will be handy. i also fully agree with

I guess the real date should always be stored in the DB whereas the hamster_day should only be used to display the activities and statistics.

But that should already be the case: in the db UTC datetimes are stored.

But hold on, I might just have found the culprit by chance, while working on a partial rework of Fact (was necessary for #423, and since we missed the ubuntu-19.10 train, it is very tempting to embrace the v3.0 major version number change meaning)

GeraldJansen commented 5 years ago

It is now 2019-08-25 20:19 and if I insert "03:00-04:00 newtest" in "Add Earlier Activity" I get: [~]$ sqlite3 .local/share/hamster-applet/hamster.db 'select * from facts where start_time >= "2019-08-25";' 7976|452|2019-08-26 03:00:00|2019-08-26 04:00:00| I would expect those dates to be the 25th, not the 26th, i.e. adding an earlier activity, not a future activity. (btw, it seems local time is stored rather than UTC, but that is another issue...)

ederag commented 5 years ago

OK, the "negative" duration is indeed fixed in my experimental branch.

I would expect those dates to be the 25th, not the 26th, i.e. adding an earlier activity, not a future activity.

There was a request for adding facts in the future (#255), and the current behavior has a clear rule: the hamster-day is the day given in the date bar at the top. The title should be changed from "add earlier activity" to "add activity" in this respect.

After thinking about it, changing to "always in the past" might also be dangerous, that could lead to truncating a previous fact, for someone expecting the fact to be added in the future, in current hamster-day. Yet I understand that it can be puzzling at first... That is tricky, need to be careful.

That would be solved with a hint showing where the fact would appear in the time-line. An undo/redo would help too, but that's dreaming.

(btw, it seems local time is stored rather than UTC, but that is another issue...)

Sorry, you are right. I got confused with the dbus analyzed this afternoon, that passes absolute values (seconds from epoch), which is good. New issue opened for that (#434).

GeraldJansen commented 5 years ago

Okay, I guess that's coherent behavior (now I understand why the timeline extends to the next day). Documentation may need improvement concerning future activities. Agree to changing "Add Earlier Activity" to "Add activity" (same style as "Update activity"). Is it reasonable to expect "hamster track newtest 03:00 04:00" to give the same result? Again, the --help option should clarify the behaviour.

ederag commented 5 years ago

Hopefully fixed in PR #437, please provide feedback there.

GeraldJansen commented 5 years ago

I have tested the PR and the problems of wrong start and/or end date seem to be solved.

However, personally I find the warning for times spanning start of day rather strange. Once I know that specifying an activity start time between 00:00 and say 05:00 means tomorrow morning rather than earlier this morning, it is only natural that the end time refer to tomorrow and not today. No warning is needed in my opinion.

ederag commented 5 years ago

Thanks for the feeback. Agreed, once understood, it is rather straightforward. The warning is there to catch mistakes such as entering 14.00 13.00 in a preceding day, inserting a 23h long activity, with a high probability of truncating an existing activity. And we do not have undo yet...

GeraldJansen commented 5 years ago

Hadn't thought of previous days. Okay.

GeraldJansen commented 5 years ago

There is still a problem if start of day is different than 05:00. For example, if start of day is set to 04:00, any activities created with start-time between 04:00 and 05:00 (inclusive) are displayed on the previous day instead of today. Activities starting exactly at 05:00 are shown both on the previous and on the current day (and their times are included in both statistics). Somewhere a fixed value of 05:00 is being used instead for the conf value.

ederag commented 5 years ago

Thanks again for the reports and the feedback. Should be solved. More work about dates is discussed in #438.