Update DeductionTimeCalculator.php

[omiT-84]

The get value returns a string or null if it does not exist. Change if statement to cope with that.

[kevinpapst]

Did you try that with the not-yet-released 2.0?

[omiT-84]

Yes I did with 2.0.5

[omiT-84]

I just upgraded my current version to 2.0 2 weeks ago to have a sneak preview.

[kevinpapst]

Awesome, beta testing is highly appreciated! Still the behavior is weird, as this is a CheckboxType, which should always be (bool) or null: https://github.com/Keleo/DeductionTimeBundle/blob/main/EventSubscriber/ActivityMetaDefinitionSubscriber.php#L37 https://github.com/kimai/kimai/blob/main/src/Entity/MetaTableTypeTrait.php#L88

And !is_bool() covers null, so I don't understand what's going on. At which point did the failure occur? When saving a timesheet?

[omiT-84]

I had a hard time finding this too, because I had no debugging and just added some error_log() lines. I did a gettype($meta) and received a string. But I get what you mean. It should be a bool if the getvalue is doing its thing....

[kevinpapst]

But still: what exactly did you do. Youo say "I get a string when it does not exist". But in that case null should always be returned. Or did you configure it and then deleted the activity?

I just want to understand what I shall test / debug to see what is going on.

[omiT-84]

I'm sorry, I should have written my test more clearly.

I created an activity called "break time" with the checkbox deduction hours on and used it on a project. When entering hours in the weekly view, it did not go to negative, so I wanted to see where this was stuck. I noticed it never went beyond that if statement and was wondering what that meta value was and added an error_log line there. Existing activities with no checkbox, gave a null as expected, because there was no id in the activities meta table. Activities with the checkbox on gave a string "1" and activities with the checkbox off, give a string "0" back.

[kevinpapst]

Thanks for clarifying. I'll do a test myself and report back.

[omiT-84]

By the way, kimai2 still calculates the totals wrong, because it does not take negative duration yet, but that discussion belongs in the other repo. To think of it. This might also belong in the other repo, since this smells more like the getvalue or setvalue is going wrong.

[omiT-84]

One final remark. The database I used was upgraded multiple times all the way up to 2.0.5 The field type of the kimai2_activities_meta.value field is {text NULL}

[kevinpapst]

Why "still calculates wrong"?

This plugin is used and sponsored by a huge customer, who uses it for bookkeeping with dozens of users, so I doubt that it fails. But that is with 1.30.x.

It was never really tested with 2.0, so you maybe right here. In which case I might need help, as I do not need the plugin yet...

[omiT-84]

Yes I am talking about 2.x The 1.30 is still working.

[kevinpapst]

Please stop using fill works like "still", especially if that word changes the meaning of your sentence. This is really confusing my developer brain, which is always looking for a deeper meaning behind it 😁

Thanks for the screenshot, that helps!

[omiT-84]

hahaha, you are absolutely right about that one. I will try not to do that, but I am still human. 😁 Just tested invoice and exports and they seem to calculate fine. The database is also showing negative and positive duration times at the correct records. Than it might be a matter of opinion wether that overview should have the total duration as an absolute number yes or no. Technically it is saying totally registered hours and that also includes break time, since it is registered. I will leave that one up to your professional opinion 😉.

[kevinpapst]

I pushed a change that fixes the string vs bool problem: https://github.com/Keleo/DeductionTimeBundle/releases/tag/2.0.1

The calculation issue in "weekly times" is a core problem. Closing here.

Thank you for finding the problem and posting a possible fix 👍

[omiT-84]

No problem, happy I could help. I like your fix a lot better by the way. Great job.

[kevinpapst]

Next minor release contains a fix to detect negative durations, in case you want to test that: https://github.com/kimai/kimai/pull/3914