From c675a7b01d2c74f71d99838008dfc3577af865bb Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 30 Apr 2026 17:59:04 +0200 Subject: [PATCH] fix: skip untracked workdays when closing a week Previously any past workday without a closed_days record blocked week close. Now only days that actually have entries require an explicit close. Empty workdays count as 0h worked, which is reflected in the weekly delta automatically. - WeekService.CloseWeek: after finding no closed_days record, check whether the day has any entries; only error if it does - NewWeekService: takes EntryStore to support the above check - Updated TestCloseWeekMissingDayFails to reflect the new semantic (test now creates entries on Friday but leaves it unclosed) --- cmd/wotra/main.go | 2 +- internal/service/week_service.go | 28 ++++++++++++++++++++------- internal/service/week_service_test.go | 21 ++++++++++++++++---- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/cmd/wotra/main.go b/cmd/wotra/main.go index 5dc878b..f06f2b1 100644 --- a/cmd/wotra/main.go +++ b/cmd/wotra/main.go @@ -48,7 +48,7 @@ func main() { entrySvc := service.NewEntryService(entryStore, closedDayStore, settingsStore, tz) daySvc := service.NewDayService(entryStore, closedDayStore, settingsStore, tz) settingsSvc := service.NewSettingsService(settingsStore) - weekSvc := service.NewWeekService(closedDayStore, closedWeekStore, settingsStore, db, tz) + weekSvc := service.NewWeekService(closedDayStore, closedWeekStore, entryStore, settingsStore, db, tz) // Background goroutine: auto-stop entries that cross midnight ctx, cancel := context.WithCancel(context.Background()) diff --git a/internal/service/week_service.go b/internal/service/week_service.go index 94b3869..262c39f 100644 --- a/internal/service/week_service.go +++ b/internal/service/week_service.go @@ -15,6 +15,7 @@ import ( type WeekService struct { closedDays *store.ClosedDayStore closedWeeks *store.ClosedWeekStore + entries *store.EntryStore settings *store.SettingsStore db interface { QueryRowContext(ctx context.Context, query string, args ...interface{}) *sql.Row @@ -26,6 +27,7 @@ type WeekService struct { func NewWeekService( closedDays *store.ClosedDayStore, closedWeeks *store.ClosedWeekStore, + entries *store.EntryStore, settings *store.SettingsStore, rawDB *sql.DB, tz *time.Location, @@ -33,6 +35,7 @@ func NewWeekService( return &WeekService{ closedDays: closedDays, closedWeeks: closedWeeks, + entries: entries, settings: settings, rawDB: rawDB, tz: tz, @@ -88,8 +91,9 @@ func (s *WeekService) CloseWeek(ctx context.Context, weekKey string) (*domain.Cl // Compute expected ms for the week (from settings frozen at week start) expectedMs := int64(set.HoursPerWeek * 3_600_000) - // Verify all workdays up to and including today are closed; collect worked ms. - // Future workdays in the week (not yet reached) are skipped. + // Verify all past workdays that have entries are closed; collect worked ms. + // Past workdays with no entries at all are skipped (they contribute 0h). + // Future workdays in the week are always skipped. todayKey := time.Now().In(s.tz).Format("2006-01-02") var totalWorkedMs int64 for _, dk := range dayKeys { @@ -101,13 +105,23 @@ func (s *WeekService) CloseWeek(ctx context.Context, weekKey string) (*domain.Cl continue // future workday — skip } cd, err := s.closedDays.GetByDayKey(ctx, dk) - if err != nil { - if errors.Is(err, sql.ErrNoRows) { - return nil, fmt.Errorf("%w: %s", ErrWeekHasUnclosedDays, dk) - } + if err != nil && !errors.Is(err, sql.ErrNoRows) { return nil, err } - totalWorkedMs += cd.WorkedMs + if cd != nil { + totalWorkedMs += cd.WorkedMs + continue + } + // No closed_days record — check whether the day has any entries. + dayEntries, err := s.entries.ListByDayKey(ctx, dk) + if err != nil { + return nil, err + } + if len(dayEntries) > 0 { + // Day has tracked time but was never closed — require explicit close. + return nil, fmt.Errorf("%w: %s", ErrWeekHasUnclosedDays, dk) + } + // No entries, no closed record → untracked day, counts as 0h. } now := time.Now().UnixMilli() diff --git a/internal/service/week_service_test.go b/internal/service/week_service_test.go index 9ae4ce8..ae0ad35 100644 --- a/internal/service/week_service_test.go +++ b/internal/service/week_service_test.go @@ -27,7 +27,7 @@ func newFullServices(t *testing.T) (*service.EntryService, *service.DayService, entrySvc := service.NewEntryService(entryStore, closedDayStore, settingsStore, tz) daySvc := service.NewDayService(entryStore, closedDayStore, settingsStore, tz) - weekSvc := service.NewWeekService(closedDayStore, closedWeekStore, settingsStore, db, tz) + weekSvc := service.NewWeekService(closedDayStore, closedWeekStore, entryStore, settingsStore, db, tz) settingsSvc := service.NewSettingsService(settingsStore) return entrySvc, daySvc, weekSvc, settingsSvc } @@ -88,19 +88,32 @@ 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() - _, daySvc, weekSvc, _ := newFullServices(t) + entrySvc, daySvc, weekSvc, _ := newFullServices(t) - // Only close Mon-Thu, leave Friday open — all are in the past + // 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. + 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{ + StartTime: fridayStart, + EndTime: fridayEnd, + }); err != nil { + t.Fatalf("CreateInterval: %v", err) + } + _, err := weekSvc.CloseWeek(ctx, "2024-W03") if err == nil { - t.Fatal("expected error closing week with unclosed past day") + t.Fatal("expected error: friday has entries but is not closed") } }