From 47c7a97d47c87dbeb6e7a5092def3449536af461 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 30 Apr 2026 18:16:22 +0200 Subject: [PATCH] fix: keep closed week snapshot in sync when days change When a day is closed, re-closed, or reopened, DayService now recomputes worked_ms and delta_ms on the closed week containing that day (if the week is already closed). This prevents stale delta values after editing entries and re-closing a day. - DayService.recomputeWeek: sums worked_ms from all closed_days in the week, updates closed_weeks row preserving expected_ms - NewDayService now takes ClosedWeekStore - WeekKeyForDayKey exported helper (used by DayService) - TestWeekSnapshotUpdatesWhenDayReopened regression test --- cmd/wotra/main.go | 2 +- internal/service/day_service.go | 67 +++++++++++++++++++---- internal/service/day_service_test.go | 3 +- internal/service/entry_service_test.go | 3 +- internal/service/week_service.go | 10 ++++ internal/service/week_service_test.go | 75 ++++++++++++++++++++------ 6 files changed, 132 insertions(+), 28 deletions(-) diff --git a/cmd/wotra/main.go b/cmd/wotra/main.go index f06f2b1..c57511d 100644 --- a/cmd/wotra/main.go +++ b/cmd/wotra/main.go @@ -46,7 +46,7 @@ func main() { syncStore := store.NewSyncStore(db) entrySvc := service.NewEntryService(entryStore, closedDayStore, settingsStore, tz) - daySvc := service.NewDayService(entryStore, closedDayStore, settingsStore, tz) + daySvc := service.NewDayService(entryStore, closedDayStore, closedWeekStore, settingsStore, tz) settingsSvc := service.NewSettingsService(settingsStore) weekSvc := service.NewWeekService(closedDayStore, closedWeekStore, entryStore, settingsStore, db, tz) diff --git a/internal/service/day_service.go b/internal/service/day_service.go index d7a8e3a..84f7bd5 100644 --- a/internal/service/day_service.go +++ b/internal/service/day_service.go @@ -9,7 +9,6 @@ import ( "github.com/wotra/wotra/internal/domain" "github.com/wotra/wotra/internal/store" ) - var ( ErrWeekAlreadyClosed = errors.New("week is already closed") ErrWeekNotClosed = errors.New("week is not closed") @@ -19,26 +18,65 @@ var ( // DayService handles closing and marking days. type DayService struct { - entries *store.EntryStore - closedDays *store.ClosedDayStore - settings *store.SettingsStore - tz *time.Location + entries *store.EntryStore + closedDays *store.ClosedDayStore + closedWeeks *store.ClosedWeekStore + settings *store.SettingsStore + tz *time.Location } func NewDayService( entries *store.EntryStore, closedDays *store.ClosedDayStore, + closedWeeks *store.ClosedWeekStore, settings *store.SettingsStore, tz *time.Location, ) *DayService { return &DayService{ - entries: entries, - closedDays: closedDays, - settings: settings, - tz: tz, + entries: entries, + closedDays: closedDays, + closedWeeks: closedWeeks, + settings: settings, + tz: tz, } } +// recomputeWeek updates worked_ms/delta_ms on the closed week containing dayKey, +// if that week is already closed. Called after any day close/reopen. +func (s *DayService) recomputeWeek(ctx context.Context, dayKey string) error { + weekKey, err := WeekKeyForDayKey(dayKey, s.tz) + if err != nil { + return err + } + cw, err := s.closedWeeks.GetByWeekKey(ctx, weekKey) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil // week not closed yet — nothing to do + } + return err + } + // Recompute worked_ms from all closed days in the week. + dayKeys, err := weekDayKeys(weekKey, s.tz) + if err != nil { + return err + } + var totalWorkedMs int64 + for _, dk := range dayKeys { + cd, err := s.closedDays.GetByDayKey(ctx, dk) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + continue + } + return err + } + totalWorkedMs += cd.WorkedMs + } + cw.WorkedMs = totalWorkedMs + cw.DeltaMs = totalWorkedMs - cw.ExpectedMs + cw.UpdatedAt = time.Now().UnixMilli() + return s.closedWeeks.Upsert(ctx, cw) +} + // CloseDay merges all completed entries for the given day key into a ClosedDay. // Returns ErrRunningEntryOnDay if a running entry exists. // Returns ErrDayAlreadyClosed if already closed. @@ -98,6 +136,9 @@ func (s *DayService) CloseDay(ctx context.Context, dayKey string) (*domain.Close if err := s.closedDays.Upsert(ctx, cd); err != nil { return nil, err } + if err := s.recomputeWeek(ctx, dayKey); err != nil { + return nil, err + } return cd, nil } @@ -134,6 +175,9 @@ func (s *DayService) MarkDay(ctx context.Context, dayKey string, kind domain.Day if err := s.closedDays.Upsert(ctx, cd); err != nil { return nil, err } + if err := s.recomputeWeek(ctx, dayKey); err != nil { + return nil, err + } return cd, nil } @@ -149,7 +193,10 @@ func (s *DayService) ReopenDay(ctx context.Context, dayKey string) error { if existing == nil { return ErrDayNotClosed } - return s.closedDays.Delete(ctx, dayKey) + if err := s.closedDays.Delete(ctx, dayKey); err != nil { + return err + } + return s.recomputeWeek(ctx, dayKey) } // GetDay returns the closed day for a given day key, or nil if open. diff --git a/internal/service/day_service_test.go b/internal/service/day_service_test.go index aca96ad..98916b6 100644 --- a/internal/service/day_service_test.go +++ b/internal/service/day_service_test.go @@ -20,11 +20,12 @@ func newTestDayServices(t *testing.T) (*service.EntryService, *service.DayServic entryStore := store.NewEntryStore(db) closedDayStore := store.NewClosedDayStore(db) + closedWeekStore := store.NewClosedWeekStore(db) settingsStore := store.NewSettingsStore(db) tz, _ := time.LoadLocation("UTC") entrySvc := service.NewEntryService(entryStore, closedDayStore, settingsStore, tz) - daySvc := service.NewDayService(entryStore, closedDayStore, settingsStore, tz) + daySvc := service.NewDayService(entryStore, closedDayStore, closedWeekStore, settingsStore, tz) settingsSvc := service.NewSettingsService(settingsStore) return entrySvc, daySvc, settingsSvc } diff --git a/internal/service/entry_service_test.go b/internal/service/entry_service_test.go index 34f8371..c6c4e0e 100644 --- a/internal/service/entry_service_test.go +++ b/internal/service/entry_service_test.go @@ -208,10 +208,11 @@ func TestUpdateRejectsClosedDay(t *testing.T) { entryStore := store.NewEntryStore(db) closedDayStore := store.NewClosedDayStore(db) + closedWeekStore := store.NewClosedWeekStore(db) settingsStore := store.NewSettingsStore(db) tz, _ := time.LoadLocation("UTC") svc := service.NewEntryService(entryStore, closedDayStore, settingsStore, tz) - daySvc := service.NewDayService(entryStore, closedDayStore, settingsStore, tz) + daySvc := service.NewDayService(entryStore, closedDayStore, closedWeekStore, settingsStore, tz) entry, _ := svc.Start(ctx, "") svc.Stop(ctx) diff --git a/internal/service/week_service.go b/internal/service/week_service.go index 262c39f..5634861 100644 --- a/internal/service/week_service.go +++ b/internal/service/week_service.go @@ -45,6 +45,16 @@ func NewWeekService( // WeekDayKeysExported is exported for testing. var WeekDayKeysExported = weekDayKeys +// WeekKeyForDayKey returns the ISO week key (YYYY-Www) for a given YYYY-MM-DD day key. +func WeekKeyForDayKey(dayKey string, tz *time.Location) (string, error) { + t, err := time.ParseInLocation("2006-01-02", dayKey, tz) + if err != nil { + return "", fmt.Errorf("invalid day_key %q: %w", dayKey, err) + } + year, week := t.ISOWeek() + return fmt.Sprintf("%d-W%02d", year, week), nil +} + // weekDayKeys returns the YYYY-MM-DD keys for Mon-Sun of the ISO week encoded as weekKey. // weekKey format: "YYYY-Www" (e.g. "2024-W03"). func weekDayKeys(weekKey string, tz *time.Location) ([]string, error) { diff --git a/internal/service/week_service_test.go b/internal/service/week_service_test.go index ae0ad35..a52781f 100644 --- a/internal/service/week_service_test.go +++ b/internal/service/week_service_test.go @@ -26,14 +26,13 @@ func newFullServices(t *testing.T) (*service.EntryService, *service.DayService, tz, _ := time.LoadLocation("UTC") entrySvc := service.NewEntryService(entryStore, closedDayStore, settingsStore, tz) - daySvc := service.NewDayService(entryStore, closedDayStore, settingsStore, tz) + daySvc := service.NewDayService(entryStore, closedDayStore, closedWeekStore, settingsStore, tz) weekSvc := service.NewWeekService(closedDayStore, closedWeekStore, entryStore, settingsStore, db, tz) settingsSvc := service.NewSettingsService(settingsStore) return entrySvc, daySvc, weekSvc, settingsSvc } func TestWeekDayKeys(t *testing.T) { - // 2024-W03 = Jan 15-21, 2024 tz, _ := time.LoadLocation("UTC") keys, err := service.WeekDayKeysExported("2024-W03", tz) if err != nil { @@ -54,11 +53,9 @@ func TestCloseWeekBasic(t *testing.T) { ctx := context.Background() entrySvc, daySvc, weekSvc, _ := newFullServices(t) - // Use a fixed Monday to make the test deterministic - monday := time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC) // 2024-W03, Monday + monday := time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC) weekKey := "2024-W03" - // Close Mon-Fri by marking as holiday (easiest — no entries needed) for i := 0; i < 5; i++ { dk := monday.AddDate(0, 0, i).Format("2006-01-02") _, err := daySvc.MarkDay(ctx, dk, domain.DayKindHoliday) @@ -71,11 +68,9 @@ func TestCloseWeekBasic(t *testing.T) { if err != nil { t.Fatalf("CloseWeek: %v", err) } - // Expected: 40h = 144000000 ms if cw.ExpectedMs != 40*3_600_000 { t.Errorf("expected 40h expected_ms, got %d ms", cw.ExpectedMs) } - // Worked: each holiday day = 8h = 5 * 8h = 40h if cw.WorkedMs != 40*3_600_000 { t.Errorf("expected 40h worked_ms, got %d ms", cw.WorkedMs) } @@ -88,20 +83,17 @@ func TestCloseWeekBasic(t *testing.T) { } func TestCloseWeekMissingDayFails(t *testing.T) { - // A past workday that HAS entries but was never closed should still block week close. ctx := context.Background() entrySvc, daySvc, weekSvc, _ := newFullServices(t) - // Use a fixed past week: 2024-W03 (Mon 2024-01-15 .. Sun 2024-01-21) monday := time.Date(2024, 1, 15, 0, 0, 0, 0, time.UTC) - // Close Mon–Thu as holiday. for i := 0; i < 4; i++ { dk := monday.AddDate(0, 0, i).Format("2006-01-02") daySvc.MarkDay(ctx, dk, domain.DayKindHoliday) } - // Friday (2024-01-19): add a completed entry but do NOT close the day. + // Friday: add an entry but don't close the day. fridayStart := time.Date(2024, 1, 19, 9, 0, 0, 0, time.UTC).UnixMilli() fridayEnd := time.Date(2024, 1, 19, 17, 0, 0, 0, time.UTC).UnixMilli() if _, err := entrySvc.CreateInterval(ctx, service.CreateIntervalInput{ @@ -148,7 +140,6 @@ func TestReopenWeek(t *testing.T) { if err := weekSvc.ReopenWeek(ctx, "2024-W03"); err != nil { t.Fatalf("ReopenWeek: %v", err) } - // Should be closeable again _, err := weekSvc.CloseWeek(ctx, "2024-W03") if err != nil { t.Fatalf("CloseWeek after reopen: %v", err) @@ -156,8 +147,6 @@ func TestReopenWeek(t *testing.T) { } func TestCloseWeekMidWeek(t *testing.T) { - // Regression: closing the current week mid-week should succeed when all - // past workdays are closed and future workdays are left open. ctx := context.Background() _, daySvc, weekSvc, _ := newFullServices(t) @@ -166,7 +155,6 @@ func TestCloseWeekMidWeek(t *testing.T) { isoYear, isoWeek := now.ISOWeek() weekKey := fmt.Sprintf("%d-W%02d", isoYear, isoWeek) - // Close every workday from Monday up to and including today. monday := now.AddDate(0, 0, -int(now.Weekday()-time.Monday)) today := now.Format("2006-01-02") for d := monday; d.Format("2006-01-02") <= today; d = d.AddDate(0, 0, 1) { @@ -184,3 +172,60 @@ func TestCloseWeekMidWeek(t *testing.T) { t.Fatalf("CloseWeek mid-week: %v", err) } } + +func TestWeekSnapshotUpdatesWhenDayReopened(t *testing.T) { + // Regression: closing a day after the week is already closed must update + // the frozen worked_ms/delta_ms on the closed week. + ctx := context.Background() + entrySvc, daySvc, weekSvc, _ := newFullServices(t) + + weekKey := "2024-W03" + monday := time.Date(2024, 1, 15, 0, 0, 0, 0, time.UTC) + + // Close Mon–Thu as holiday (8h each), leave Friday untracked. + for i := 0; i < 4; i++ { + dk := monday.AddDate(0, 0, i).Format("2006-01-02") + if _, err := daySvc.MarkDay(ctx, dk, domain.DayKindHoliday); err != nil { + t.Fatalf("MarkDay: %v", err) + } + } + + // Close week: 4 * 8h = 32h worked, expected 40h, delta -8h. + cw, err := weekSvc.CloseWeek(ctx, weekKey) + if err != nil { + t.Fatalf("CloseWeek: %v", err) + } + if cw.WorkedMs != 32*3_600_000 { + t.Errorf("initial worked_ms: want 32h, got %dms", cw.WorkedMs) + } + + // Reopen Thursday, add a 9h entry, re-close it. + thursdayKey := monday.AddDate(0, 0, 3).Format("2006-01-02") + if err := daySvc.ReopenDay(ctx, thursdayKey); err != nil { + t.Fatalf("ReopenDay: %v", err) + } + thuStart := time.Date(2024, 1, 18, 8, 0, 0, 0, time.UTC).UnixMilli() + thuEnd := time.Date(2024, 1, 18, 17, 0, 0, 0, time.UTC).UnixMilli() // 9h + if _, err := entrySvc.CreateInterval(ctx, service.CreateIntervalInput{ + StartTime: thuStart, EndTime: thuEnd, + }); err != nil { + t.Fatalf("CreateInterval: %v", err) + } + if _, err := daySvc.CloseDay(ctx, thursdayKey); err != nil { + t.Fatalf("CloseDay: %v", err) + } + + // Week snapshot must now reflect 3*8h + 9h = 33h worked. + cw2, err := weekSvc.GetWeek(ctx, weekKey) + if err != nil || cw2 == nil { + t.Fatalf("GetWeek: %v", err) + } + wantWorked := int64(3*8+9) * 3_600_000 + if cw2.WorkedMs != wantWorked { + t.Errorf("updated worked_ms: want %dms (%dh), got %dms", wantWorked, wantWorked/3_600_000, cw2.WorkedMs) + } + wantDelta := wantWorked - cw2.ExpectedMs + if cw2.DeltaMs != wantDelta { + t.Errorf("updated delta_ms: want %dms, got %dms", wantDelta, cw2.DeltaMs) + } +}