Skip to content

Commit bd5553a

Browse files
authored
Anchor batch action dtstart to midnight (#663)
1 parent a845ede commit bd5553a

5 files changed

Lines changed: 46 additions & 17 deletions

File tree

broker/patron_request/service/statemodels/state-models.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -700,7 +700,7 @@
700700
"actionName": "email-pullslips",
701701
"title": "Email pull slips ready to ship",
702702
"batchQuery": "side = lending and state = WILL_SUPPLY",
703-
"schedule": "FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR;BYHOUR=6;BYMINUTE=0;BYSECOND=0",
703+
"schedule": "FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR;BYHOUR=6;BYMINUTE=0",
704704
"actionParams": {
705705
"to": [
706706
"staff@example.com"
@@ -724,7 +724,7 @@
724724
"actionName": "request-aging",
725725
"title": "Request aging of rush requests",
726726
"batchQuery": "(state = VALIDATED or state = WILL_SUPPLY or state = CONDITION_PENDING or state = NEEDS_REVIEW) and service_level = Rush",
727-
"schedule": "FREQ=HOURLY;BYMINUTE=0;BYSECOND=0",
727+
"schedule": "FREQ=HOURLY;BYMINUTE=0",
728728
"actionParams": {
729729
"interval": "24h"
730730
}
@@ -733,7 +733,7 @@
733733
"actionName": "request-aging",
734734
"title": "Request aging of all requests",
735735
"batchQuery": "state = VALIDATED or state = WILL_SUPPLY or state = CONDITION_PENDING or state = NEEDS_REVIEW",
736-
"schedule": "FREQ=DAILY;BYHOUR=5;BYMINUTE=0;BYSECOND=0",
736+
"schedule": "FREQ=DAILY;BYHOUR=5;BYMINUTE=0",
737737
"actionParams": {
738738
"interval": "72h"
739739
}

broker/scheduler/api/api_handler_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,15 +233,17 @@ func TestPostBatchActions_OK(t *testing.T) {
233233
repo.AssertExpectations(t)
234234
}
235235

236-
func TestPostBatchActions_ValidDailySchedule_ComputesDynamicRunAt(t *testing.T) {
236+
func TestPostBatchActions_ValidDailySchedule_ComputesMidnightRunAt(t *testing.T) {
237237
repo := new(MockSchedRepo)
238238
before := time.Now().UTC()
239239

240240
repo.On("SaveScheduledTask", mock.MatchedBy(func(p sched_db.SaveScheduledTaskParams) bool {
241+
runAt := p.RunAt.Time
241242
return p.Schedule == "FREQ=DAILY" &&
242243
p.RunAt.Valid &&
243-
p.RunAt.Time.After(before.Add(23*time.Hour)) &&
244-
p.RunAt.Time.Before(before.Add(25*time.Hour))
244+
runAt.After(before) &&
245+
!runAt.After(before.Add(24*time.Hour+time.Second)) &&
246+
runAt.Equal(runAt.UTC().Truncate(24*time.Hour))
245247
})).Return(saveScheduledTaskReturn, nil)
246248

247249
h := newHandler(repo)

broker/scheduler/service/scheduler.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,9 @@ func nextScheduleTimeAt(schedule string, now time.Time) (pgtype.Timestamptz, err
286286
if err != nil {
287287
return pgtype.Timestamptz{}, fmt.Errorf("invalid rrule string %q: %w", schedule, err)
288288
}
289-
options.Dtstart = now
289+
// Anchor at today's UTC midnight so omitted time components inherit zero and
290+
// interval rules have a deterministic daily phase.
291+
options.Dtstart = now.Truncate(24 * time.Hour)
290292

291293
rule, err := rrule.NewRRule(*options)
292294
if err != nil {

broker/scheduler/service/scheduler_test.go

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,30 +117,55 @@ func TestNextScheduleTime_SpecificSchedule(t *testing.T) {
117117
assert.True(t, ts.Time.After(time.Now()))
118118
}
119119

120-
func TestNextScheduleTime_UsesDynamicDTStartDefaults(t *testing.T) {
120+
func TestNextScheduleTime_MidnightAnchoredDefaults(t *testing.T) {
121121
tests := []struct {
122122
name string
123123
schedule string
124124
now time.Time
125125
expected time.Time
126126
}{
127127
{
128-
name: "daily defaults to current time of day",
128+
name: "daily defaults to midnight",
129129
schedule: "FREQ=DAILY",
130130
now: time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC),
131-
expected: time.Date(2026, 1, 3, 3, 4, 5, 0, time.UTC),
131+
expected: time.Date(2026, 1, 3, 0, 0, 0, 0, time.UTC),
132132
},
133133
{
134-
name: "weekly defaults to current weekday and time",
134+
name: "weekly defaults to anchor weekday at midnight",
135135
schedule: "FREQ=WEEKLY",
136136
now: time.Date(2026, 1, 7, 10, 11, 12, 0, time.UTC),
137-
expected: time.Date(2026, 1, 14, 10, 11, 12, 0, time.UTC),
137+
expected: time.Date(2026, 1, 14, 0, 0, 0, 0, time.UTC),
138138
},
139139
{
140-
name: "monthly defaults to current day of month and time",
140+
name: "monthly defaults to anchor day at midnight",
141141
schedule: "FREQ=MONTHLY",
142142
now: time.Date(2026, 1, 15, 8, 30, 0, 0, time.UTC),
143-
expected: time.Date(2026, 2, 15, 8, 30, 0, 0, time.UTC),
143+
expected: time.Date(2026, 2, 15, 0, 0, 0, 0, time.UTC),
144+
},
145+
{
146+
name: "minutely interval has a midnight-aligned phase",
147+
schedule: "FREQ=MINUTELY;INTERVAL=15",
148+
now: time.Date(2026, 1, 2, 10, 7, 44, 0, time.UTC),
149+
expected: time.Date(2026, 1, 2, 10, 15, 0, 0, time.UTC),
150+
},
151+
{
152+
name: "hourly defaults minute and second to zero",
153+
schedule: "FREQ=HOURLY",
154+
now: time.Date(2026, 1, 2, 10, 7, 44, 0, time.UTC),
155+
expected: time.Date(2026, 1, 2, 11, 0, 0, 0, time.UTC),
156+
},
157+
{
158+
name: "explicit minute overrides the anchor default",
159+
schedule: "FREQ=HOURLY;BYMINUTE=30",
160+
now: time.Date(2026, 1, 2, 10, 7, 44, 0, time.UTC),
161+
expected: time.Date(2026, 1, 2, 10, 30, 0, 0, time.UTC),
162+
},
163+
{
164+
name: "calendar rule inherits a zero second",
165+
schedule: "FREQ=WEEKLY;BYDAY=MO;BYHOUR=9;BYMINUTE=0",
166+
// Monday 08:59:45 — next run is 09:00:00 sharp, not 09:00:45.
167+
now: time.Date(2026, 1, 12, 8, 59, 45, 0, time.UTC),
168+
expected: time.Date(2026, 1, 12, 9, 0, 0, 0, time.UTC),
144169
},
145170
}
146171

misc/state-models.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ batchActionDefaults:
476476
- actionName: email-pullslips
477477
title: Email pull slips ready to ship
478478
batchQuery: side = lending and state = WILL_SUPPLY
479-
schedule: "FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR;BYHOUR=6;BYMINUTE=0;BYSECOND=0"
479+
schedule: "FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR;BYHOUR=6;BYMINUTE=0"
480480
actionParams:
481481
to:
482482
- staff@example.com
@@ -495,14 +495,14 @@ batchActionDefaults:
495495
- actionName: request-aging
496496
title: Request aging of rush requests
497497
batchQuery: "(state = VALIDATED or state = WILL_SUPPLY or state = CONDITION_PENDING or state = NEEDS_REVIEW) and service_level = Rush"
498-
schedule: "FREQ=HOURLY;BYMINUTE=0;BYSECOND=0"
498+
schedule: "FREQ=HOURLY;BYMINUTE=0"
499499
actionParams:
500500
interval: 24h
501501

502502
- actionName: request-aging
503503
title: Request aging of all requests
504504
batchQuery: state = VALIDATED or state = WILL_SUPPLY or state = CONDITION_PENDING or state = NEEDS_REVIEW
505-
schedule: "FREQ=DAILY;BYHOUR=5;BYMINUTE=0;BYSECOND=0"
505+
schedule: "FREQ=DAILY;BYHOUR=5;BYMINUTE=0"
506506
actionParams:
507507
interval: 72h
508508

0 commit comments

Comments
 (0)