dominikbraun / timetrace

A simple CLI for tracking your working time.
Apache License 2.0
665 stars 75 forks source link

Check record collisions when editing a record #203

Open dominikbraun opened 2 years ago

dominikbraun commented 2 years ago

At the moment, record collision checks are only performed when creating a belated record using timetrace create record but not when editing an existing record.

This needs to be changed by calling t.RecordCollides when running timetrace edit record.


What is record collision?

Say you have 2 records:

And you want to insert a new one:

This record collides with the second (5PM - 8PM) record, because it lies within that record. timetrace doesn't allow this.

dsa0x commented 2 years ago

Hi. I am trying to work on this. Is there anything that uniquely identifies a record? I made it work for the EditRecord method, but for the EditRecordManual, it's a bit trickier, since the file editing is done in vim/an external editor.

dominikbraun commented 2 years ago

@dsa0x The unique identifier for a record is its timestamp (start time). Quite frankly, I think the only way to handle record collisions when editing a record manually is to wait until the user has saved the file (i.e. after the t.EditRecordManual(recordTime) call in edit.go), check for collisions, print an error if there is a collision and revert the file to the previous state so that the record remains valid. This can be done using a t.RevertRecord(recordTime) call if I'm not wrong.

dsa0x commented 2 years ago

@dominikbraun I think so too. That's kinda what I did. I simply called t.loadRecordafter cmd.Run, to fetch the file again. But if one edits the start time, then this timestamp changes, and it becomes impossible to track the record. Also, your suggestion might work if we assume that the startTime will not be changed. Is that safe to assume? (meaning if the user changes it, unexpected behaviors will occur)

dominikbraun commented 2 years ago

@dsa0x Right, that's an issue and there is PR #89 to fix this. Speaking of this PR, it probably would make sense to merge it before you implement the collision check...

dsa0x commented 2 years ago

Okay. Cool. I'll wait then. In the meantime, you can assign me to this. Thanks