Only one running timesheet: automatically stop others

[lduer]

Description

For my usecase, I need the previous Timesheet to stop when the user starts a new one.
The Pull request #308 already mentions this "problem". I tried to improve it.

As suggested in the (comment of 308), I used the active_warning setting to detect if the running entries have to be stopped.

Note: In my opinion, it's not correct to use a parameter from the "theme" stettings to detect the stop-mode. After discussing and reviewing, I'll add the tests and update the documentation.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My code follows the code style
• All files have a license header
• All methods have a doc header with type declarations
• I have updated the documentation accordingly
• I have added tests to cover my changes

[kevinpapst]

@lduer thanks for the contribution!
I agree that a theme setting shouldn't be used as backend flag.
What do you think about this proposal:

Adding these settings to config/packages/kimai.yaml:

kimai:
    timesheet:
        active_records:
            soft_limit: 1
            hard_limit: 1

Soft limit will be converted to the theme setting and hard_limit should be used to detect how many active records a user can have. Default values should be different than the above (probably 3 and 10).

You would need to add the config keys to the Configuration class and map them in the AppExtension.
I'd also like to inject these configs, instead of accessing the container directly from the Controller.
The repository function stopActiveEntries() should receive a limit of how many active records are allowed and stop all others.

[lduer]

For me, the naming and the use of the timesheet settings are ok :) ( I cannot imagine a configuration, where users have to allow 10 simultaneously running timerecords, but that's another part of the story)

In the following week, I'll adapt my changes and add some documentation and tests.

Just for my understanding: The limit in the repository function stopActiveEntries(User $user, int $limit) allows the few latest timesheets and stops the oldest ones, right?

[kevinpapst]

Yes, that's exactly what I had in mind. Keep the newest entries running.
If we set it to 4, there will be for sure an issue that a user cannot start a fifth one ;-)

[lduer]

I did some work but I think some features are still missing:

• Throw an error when inserting an entry and more than X entries are in the time-period (especially for hard_limit: 1: no overlapping timerecords should be possible)
• Do we need some text-output (flash message) to display that "Timerecord XY was stopped" or "x timerecord(s) have been stopped"

[kevinpapst]

Let me think about that.
My first impression: maybe a third setting should be introduced, something like "auto_stop" which defaults to true. In case its false, only an exception is thrown otherwise overlapping entries are stopped. In both cases a message should be shown.

[kevinpapst]

On the one side it is a very handy feature to be able to directly switch a task without first having to stop the last one.
Opposite opinion: Forcing to stop running time entries might feel like a bug, the system shouldn't try to be clever but only tell me that I have to stop an entry before I can start a new one.

I guess we can find users that prefer either way.
For me: best solution: add a third setting that allows to switch the behavior.
But if we want to start with one way, which is easier to implement: automatically stop entries and show a flash message.

What do you say?

[lduer]

I think I cannot follow clearly.

Opposite opinion: Forcing to stop running time entries might feel like a bug, the system shouldn't try to be clever but only tell me that I have to stop an entry before I can start a new one.

Are talking about my implementation of soft_limit and hard_limit?

Regarding my question:

Throw an error when inserting an entry and more than X entries are in the time-period (especially for hard_limit: 1: no overlapping timerecords should be possible)

I've read some of kimai's docs and website and due to "It’s simplicity is its strength" I want to withdraw this suggestion for the moment. I worked on a timetracking implementation (better to say: "time-documentation") few years ago and the strict requirement was not to allow 2 entries at the same time for the same user.

---

For the missing implementation of the flash message: I'm a bit busy at the moment and will submit some code asap.

[kevinpapst]

I was addressing the two possible thoughts one could have about auto-stopping entries:

• On the one side it's a great feature usability wise, saving the user some clicks
• On the other side the system decides for the user and that might go wrong

For the special case with hard_limit=1 auto-stopping is totally reasonable.

I wanted to address that I am simply not sure what the best way would be to implement that feature.
Probably we use auto-stopping for hard_limit=1 and a flash message in every other situation, telling the user to stop one running entry first before the next one can be started. That makes it a bit less complicated for the first run. I'll add some review comments for that.

We should also switch the default hard_limit to 1. You are right, that's probably what most people expect and what is logical. I still want to support multiple running entries, but most of us can just work at one task at a time.

[lduer]

Probably we use auto-stopping for hard_limit=1 and a flash message in every other situation, telling the user to stop one running entry first before the next one can be started.

I'm good with this, implemented your requested changes.

Additional thoughts about your auto_stop setting: the auto_stop: true (default false) will stop the oldest entry automatically. In this line of thinking, auto_stop is not used, when hard_limit: 1

Do you want me to add auto_stop?

[lduer]

And where should I add the translation for the Exception: f813464#diff-9f87c76ff254d43b462a2debdd6e8437R287

Or are these never translated?

[kevinpapst]

I'll have a look this week, sorry for the delay!

[kevinpapst]

Works pretty awesome! I'll push some documentation changes later and then merge it

[kevinpapst]

I pushed a small change, mainly documentation.
But while working on it, it came to my mind that the API Controller has a "start record" action as well:
https://github.com/kevinpapst/kimai2/blob/master/src/API/TimesheetController.php#L156
That needs to be adapted as well.
Its unfortunate that there are multiple actions for the start action, that will be consolidated in the future. But for now, all methods should behave the same... what do you think?

[kevinpapst]

@lduer do you want to have a last look before I merge?

[lduer]

👍 Thanks for your improvements

Too bad that I don't have much free time left for coding at the moment.

[kevinpapst]

Feedback is enough. In that case I will do a last test run and merge it.
Thanks and hope to see you soon!

[immanuelfodor]

Any update on this? I'd also love to see this merged :)

[kevinpapst]

I am so sorry for being so late on this one. Had a lot of private stuff to clarify in the meantime.
Can anyone perform a last test? I canÄt remember the latest state... I guess it was ready to be merged, but I am unsure.
If I don't get feedback, I will have a look soon.

[lduer]

I haven't tested it again. I think it's important to keep in mind, that this merge will break the configuration: kimai.theme.active_warning to kimai.timesheets.active_entries.soft_limit and kimai.timesheets.active_entries.soft_limit:

The only open question from my side: #386 (comment)

[kevinpapst]

Stil having kimai.theme.active_warning in your local.yaml won't break the configuration, I added a fallback - you will only get a deprecation notice.
And I just added logic to translate the exception, so this is solved also. I'll perform a last test and merge it later today.

[lock]

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.