timetrace icon indicating copy to clipboard operation
timetrace copied to clipboard

Check record collisions when editing a record

Open dominikbraun opened this issue 3 years ago • 5 comments

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:

  • 3PM - 4PM
  • 5PM - 8PM

And you want to insert a new one:

  • 7PM - 9PM

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

dominikbraun avatar Dec 03 '21 12:12 dominikbraun

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.

dsa0x avatar Jan 20 '22 21:01 dsa0x

@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.

dominikbraun avatar Jan 20 '22 21:01 dominikbraun

@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)

dsa0x avatar Jan 20 '22 21:01 dsa0x

@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...

dominikbraun avatar Jan 20 '22 22:01 dominikbraun

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

dsa0x avatar Jan 20 '22 22:01 dsa0x