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
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
@@ -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")
|
||||
@@ -21,6 +20,7 @@ var (
|
||||
type DayService struct {
|
||||
entries *store.EntryStore
|
||||
closedDays *store.ClosedDayStore
|
||||
closedWeeks *store.ClosedWeekStore
|
||||
settings *store.SettingsStore
|
||||
tz *time.Location
|
||||
}
|
||||
@@ -28,17 +28,55 @@ type DayService struct {
|
||||
func NewDayService(
|
||||
entries *store.EntryStore,
|
||||
closedDays *store.ClosedDayStore,
|
||||
closedWeeks *store.ClosedWeekStore,
|
||||
settings *store.SettingsStore,
|
||||
tz *time.Location,
|
||||
) *DayService {
|
||||
return &DayService{
|
||||
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.
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user