traggo / server

self-hosted tag-based time tracking
https://traggo.net
GNU General Public License v3.0
1.21k stars 58 forks source link

Forced seconds to 0 when time updated in List view #174

Closed MaximilianSoerenPollak closed 3 months ago

MaximilianSoerenPollak commented 4 months ago

Whenever the updateTimeSpan function is called, now will make the correct checks (for end == nil) and force the two new times to be times with seconds set to 00.

Please let me know if the formatting etc. needs to be changed.

I have tested with go test ./... as well as the Makefile and all tests from GO + UI passed.

It also had the desired effect (in my local testing) that if I had a 25min or multiple therefore Interval it always counted correctly.

This should close/fix #110

MaximilianSoerenPollak commented 4 months ago

Makes sense. I'm not too familiar with React / TS.

I have tried fixing the bug first in this section of code This is the ui/src/timespan/TimeSpan.tsx file. Lines starting at 179.

   //...
                    <DateTimeSelector
                        popoverOpen={dateSelectorOpen}
                        selectedDate={from}
                        onSelectDate={(newFrom) => {
                            if (!newFrom.isValid()) {
                                return;
                            }
                            if (to && moment(newFrom).isAfter(to)) {
                                const newTo = moment(newFrom).add(15, 'minute');
                                noteAwareUpdateTimeSpan({
                                    variables: {
                                        oldStart: oldFrom,
                                        id,
                                        start: inUserTz(newFrom).format(),
                                        end: inUserTz(newTo).format(),
                                        tags: toInputTags(selectedEntries),
                                    },
                                }).then(() => rangeChange({from: newFrom, to: newTo}));
                               //...

The fix I tried was this:

// old
const newTo = moment(newFrom).add(15, 'minute')
// new
const newTo = moment(newFrom).add(15, 'minute').set({seconds: 0})

But that then did not yield the results we wanted.

Would you be so kind and point me to the correct place or file to change it?

Thanks

jmattheis commented 4 months ago

This is the code branch when the from date is after the end date. you have to adjust the else branch too. It should probably be enough to do newFrom.set({seconds: 0}) at the start of the onSelectDate callback after the isValid check.

MaximilianSoerenPollak commented 4 months ago

I have changed both, just didn't want to put both here to make the example shorter. Okay well let me try that again and give it a go. Appreciate the directions

:) will report back as soon as I got something

jmattheis commented 4 months ago

Keep in mind there are two callbacks for changing the start and the end date.

MaximilianSoerenPollak commented 4 months ago

Had no luck so far, but will try again on in the comming days :)

MaximilianSoerenPollak commented 4 months ago

@jmattheis so I have changed every instance I think that is relevant for the 'update' timestamp. But it's still taking the seconds from the 'old' start/end time whichever was changed.

Could you be so generous and have a quick look whenever you have time and point me to the place where I'm missing something? I have the feeling it's something obvious but i'm just blind right now.

jmattheis commented 3 months ago

Superseded by #175