Skip to content

Commit c1cb55b

Browse files
orltomorltom
authored
Merge pull request #12 from orltom/10-scheduler-error
Failure detection when no on-call duty is available for the shift
2 parents 1fb8e56 + c363702 commit c1cb55b

File tree

6 files changed

+196
-66
lines changed

6 files changed

+196
-66
lines changed

‎cmd/create_command.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,11 @@ func RunCreateShiftPlan(arguments []string) error {
102102
return fmt.Errorf("%w: invalid team config file: %w", ErrInvalidArgument, err)
103103
}
104104

105-
plan := shiftplan.NewDefaultShiftPlanner(team.Employees).Plan(*str, *end, time.Duration(*duration)*time.Hour)
105+
plan, err := shiftplan.NewDefaultShiftPlanner(team.Employees).Plan(*str, *end, time.Duration(*duration)*time.Hour)
106+
if err != nil {
107+
return fmt.Errorf("can not create on-call schedule: %w", err)
108+
}
109+
106110
if err := converters[transform]().Write(plan, os.Stdout); err != nil {
107111
return fmt.Errorf("unexpecting error: %w", err)
108112
}

‎internal/shiftplan/default_rules.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ type DefaultRule struct {
1414
fn func(e apis.Employee, _ []apis.Shift, start time.Time, end time.Time) bool
1515
}
1616

17-
func (d *DefaultRule) Match(primary apis.Employee, shifts []apis.Shift, start time.Time, end time.Time) bool {
18-
return d.fn(primary, shifts, start, end)
17+
func (d *DefaultRule) Match(employee apis.Employee, shifts []apis.Shift, start time.Time, end time.Time) bool {
18+
return d.fn(employee, shifts, start, end)
1919
}
2020

2121
func VacationConflict() *DefaultRule {

‎internal/shiftplan/default_rules_test.go

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
package shiftplan
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
"github.com/orltom/on-call-schedule/pkg/apis"
8+
)
9+
10+
func TestVacationConflict(t *testing.T) {
11+
type args struct {
12+
employee apis.Employee
13+
start time.Time
14+
end time.Time
15+
}
16+
tests := []struct {
17+
name string
18+
args args
19+
want bool
20+
}{
21+
{
22+
name: "Should detect conflict if employee has holiday between scheduled shift",
23+
args: args{
24+
employee: apis.Employee{ID: "a", Name: "a", VacationDays: []string{"2024-01-01"}},
25+
start: date("2024-01-01"),
26+
end: date("2024-01-02"),
27+
},
28+
want: true,
29+
},
30+
{
31+
name: "Should not detect conflict if employee has no holidays",
32+
args: args{
33+
employee: apis.Employee{ID: "a", Name: "a", VacationDays: nil},
34+
start: date("2024-01-01"),
35+
end: date("2024-01-02"),
36+
},
37+
want: false,
38+
},
39+
{
40+
name: "Should not detect conflict if employee has holiday outside scheduled shift",
41+
args: args{
42+
employee: apis.Employee{ID: "a", Name: "a", VacationDays: []string{"2024-01-03"}},
43+
start: date("2024-01-01"),
44+
end: date("2024-01-02"),
45+
},
46+
want: false,
47+
},
48+
}
49+
for _, tt := range tests {
50+
t.Run(tt.name, func(t *testing.T) {
51+
d := VacationConflict()
52+
if got := d.Match(tt.args.employee, nil, tt.args.start, tt.args.end); got != tt.want {
53+
t.Errorf("Match() = %v, want %v", got, tt.want)
54+
}
55+
})
56+
}
57+
}
58+
59+
func TestInvolvedInLastSift(t *testing.T) {
60+
type args struct {
61+
employee apis.Employee
62+
shifts []apis.Shift
63+
}
64+
tests := []struct {
65+
name string
66+
args args
67+
want bool
68+
}{
69+
{
70+
name: "Should not detect conflict when start with the first schedule shift",
71+
args: args{
72+
employee: apis.Employee{ID: "a", Name: "a", VacationDays: nil},
73+
shifts: nil,
74+
},
75+
want: false,
76+
},
77+
{
78+
name: "Should not detect conflict if employee was not on last shift",
79+
args: args{
80+
employee: apis.Employee{ID: "a", Name: "a", VacationDays: nil},
81+
shifts: []apis.Shift{{
82+
Primary: "b",
83+
Secondary: "c",
84+
}},
85+
},
86+
want: false,
87+
},
88+
{
89+
name: "Should detect conflict if employee was on last shift as primary",
90+
args: args{
91+
employee: apis.Employee{ID: "a", Name: "a", VacationDays: nil},
92+
shifts: []apis.Shift{{
93+
Primary: "a",
94+
Secondary: "b",
95+
}},
96+
},
97+
want: true,
98+
},
99+
{
100+
name: "Should detect conflict if employee was on last shift as secondary",
101+
args: args{
102+
employee: apis.Employee{ID: "a", Name: "a", VacationDays: nil},
103+
shifts: []apis.Shift{{
104+
Primary: "c",
105+
Secondary: "a",
106+
}},
107+
},
108+
want: true,
109+
},
110+
}
111+
for _, tt := range tests {
112+
t.Run(tt.name, func(t *testing.T) {
113+
d := InvolvedInLastSift()
114+
if got := d.Match(tt.args.employee, tt.args.shifts, time.Now(), time.Now()); got != tt.want {
115+
t.Errorf("Match() = %v, want %v", got, tt.want)
116+
}
117+
})
118+
}
119+
}

‎internal/shiftplan/planner.go

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package shiftplan
22

33
import (
4+
"fmt"
45
"slices"
56
"time"
67

@@ -16,8 +17,8 @@ type ShiftPlanner struct {
1617
func NewDefaultShiftPlanner(team []apis.Employee) *ShiftPlanner {
1718
return NewShiftPlanner(
1819
team,
19-
[]apis.Rule{VacationConflict(), InvolvedInLastSift()},
20-
[]apis.Rule{VacationConflict(), InvolvedInLastSift()},
20+
[]apis.Rule{VacationConflict()},
21+
[]apis.Rule{VacationConflict()},
2122
)
2223
}
2324

@@ -29,16 +30,22 @@ func NewShiftPlanner(team []apis.Employee, primaryConflictCheckers []apis.Rule,
2930
}
3031
}
3132

32-
func (p *ShiftPlanner) Plan(start time.Time, end time.Time, rotation time.Duration) []apis.Shift {
33+
func (p *ShiftPlanner) Plan(start time.Time, end time.Time, rotation time.Duration) ([]apis.Shift, error) {
3334
var plan []apis.Shift
3435

3536
for s := start; s.Before(end); s = s.Add(rotation) {
3637
e := s.Add(rotation)
3738

38-
primary := p.findPrimary(plan, s, e)
39+
primary, err := p.findPrimary(plan, s, e)
40+
if err != nil {
41+
return nil, err
42+
}
3943
p.team = remove(p.team, primary)
4044

41-
secondary := p.findSecondary(plan, s, e)
45+
secondary, err := p.findSecondary(plan, s, e)
46+
if err != nil {
47+
return nil, err
48+
}
4249
p.team = remove(p.team, secondary)
4350

4451
shift := apis.Shift{Start: s, End: e, Primary: primary.ID, Secondary: secondary.ID}
@@ -47,27 +54,27 @@ func (p *ShiftPlanner) Plan(start time.Time, end time.Time, rotation time.Durati
4754
p.team = append(p.team, secondary, primary)
4855
}
4956

50-
return plan
57+
return plan, nil
5158
}
5259

53-
func (p *ShiftPlanner) findPrimary(shifts []apis.Shift, start time.Time, end time.Time) apis.Employee {
60+
func (p *ShiftPlanner) findPrimary(shifts []apis.Shift, start time.Time, end time.Time) (apis.Employee, error) {
5461
return p.find(shifts, start, end, p.primaryConflictCheckers)
5562
}
5663

57-
func (p *ShiftPlanner) findSecondary(shifts []apis.Shift, start time.Time, end time.Time) apis.Employee {
64+
func (p *ShiftPlanner) findSecondary(shifts []apis.Shift, start time.Time, end time.Time) (apis.Employee, error) {
5865
return p.find(shifts, start, end, p.secondaryConflictChecker)
5966
}
6067

61-
func (p *ShiftPlanner) find(shifts []apis.Shift, start time.Time, end time.Time, checkers []apis.Rule) apis.Employee {
68+
func (p *ShiftPlanner) find(shifts []apis.Shift, start time.Time, end time.Time, checkers []apis.Rule) (apis.Employee, error) {
6269
for idx := range p.team {
6370
if p.hasConflict(p.team[idx], checkers, shifts, start, end) {
6471
continue
6572
}
6673

67-
return p.team[idx]
74+
return p.team[idx], nil
6875
}
6976

70-
return p.team[0]
77+
return apis.Employee{}, fmt.Errorf("could not find available duty between %s and %s", start, end)
7178
}
7279

7380
func (p *ShiftPlanner) hasConflict(e apis.Employee, conflictCheckers []apis.Rule, shifts []apis.Shift, start time.Time, end time.Time) bool {

‎internal/shiftplan/planner_test.go

Lines changed: 51 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,13 @@ func TestPlanner_Plan(t *testing.T) {
1919
employees []apis.Employee
2020
args args
2121
want []apis.Shift
22+
wantErr bool
2223
}{
2324
{
2425
name: "Without declared holidays, a daily schedule should be generated according to the order of the employees",
2526
employees: []apis.Employee{
26-
{
27-
ID: "a@test.ch",
28-
Name: "a",
29-
VacationDays: nil,
30-
},
31-
{
32-
ID: "b@test.ch",
33-
Name: "b",
34-
VacationDays: nil,
35-
},
27+
{ID: "a@test.ch", Name: "a", VacationDays: nil},
28+
{ID: "b@test.ch", Name: "b", VacationDays: nil},
3629
},
3730
args: args{
3831
start: date("2020-04-01"),
@@ -59,30 +52,15 @@ func TestPlanner_Plan(t *testing.T) {
5952
Secondary: "b@test.ch",
6053
},
6154
},
55+
wantErr: false,
6256
},
6357
{
6458
name: "Should not schedule Primary on holiday days",
6559
employees: []apis.Employee{
66-
{
67-
ID: "a@test.ch",
68-
Name: "a",
69-
VacationDays: []string{"2020-04-01"},
70-
},
71-
{
72-
ID: "b@test.ch",
73-
Name: "b",
74-
VacationDays: nil,
75-
},
76-
{
77-
ID: "c@test.ch",
78-
Name: "c",
79-
VacationDays: nil,
80-
},
81-
{
82-
ID: "d@test.ch",
83-
Name: "d",
84-
VacationDays: nil,
85-
},
60+
{ID: "a@test.ch", Name: "a", VacationDays: []string{"2020-04-01"}},
61+
{ID: "b@test.ch", Name: "b", VacationDays: nil},
62+
{ID: "c@test.ch", Name: "c", VacationDays: nil},
63+
{ID: "d@test.ch", Name: "d", VacationDays: nil},
8664
},
8765
args: args{
8866
start: date("2020-04-01"),
@@ -115,30 +93,15 @@ func TestPlanner_Plan(t *testing.T) {
11593
Secondary: "a@test.ch",
11694
},
11795
},
96+
wantErr: false,
11897
},
11998
{
12099
name: "Should not schedule Secondary on holiday days",
121100
employees: []apis.Employee{
122-
{
123-
ID: "a@test.ch",
124-
Name: "a",
125-
VacationDays: nil,
126-
},
127-
{
128-
ID: "b@test.ch",
129-
Name: "b",
130-
VacationDays: []string{"2020-04-01"},
131-
},
132-
{
133-
ID: "c@test.ch",
134-
Name: "c",
135-
VacationDays: nil,
136-
},
137-
{
138-
ID: "d@test.ch",
139-
Name: "d",
140-
VacationDays: nil,
141-
},
101+
{ID: "a@test.ch", Name: "a", VacationDays: nil},
102+
{ID: "b@test.ch", Name: "b", VacationDays: []string{"2020-04-01"}},
103+
{ID: "c@test.ch", Name: "c", VacationDays: nil},
104+
{ID: "d@test.ch", Name: "d", VacationDays: nil},
142105
},
143106
args: args{
144107
start: date("2020-04-01"),
@@ -171,12 +134,49 @@ func TestPlanner_Plan(t *testing.T) {
171134
Secondary: "b@test.ch",
172135
},
173136
},
137+
wantErr: false,
138+
},
139+
{
140+
name: "Should return an error if can not find next available duty",
141+
employees: []apis.Employee{
142+
{ID: "a@test.ch", Name: "a", VacationDays: []string{"2020-04-01"}},
143+
{ID: "b@test.ch", Name: "b", VacationDays: []string{"2020-04-01"}},
144+
{ID: "c@test.ch", Name: "c", VacationDays: []string{"2020-04-01"}},
145+
{ID: "d@test.ch", Name: "d", VacationDays: []string{"2020-04-01"}},
146+
},
147+
args: args{
148+
start: date("2020-04-01"),
149+
end: date("2020-04-04"),
150+
rotation: 24 * time.Hour,
151+
},
152+
want: nil,
153+
wantErr: true,
154+
},
155+
{
156+
name: "Should return an error if can not find next secondary",
157+
employees: []apis.Employee{
158+
{ID: "a@test.ch", Name: "a", VacationDays: nil},
159+
{ID: "b@test.ch", Name: "b", VacationDays: []string{"2020-04-01"}},
160+
{ID: "c@test.ch", Name: "c", VacationDays: []string{"2020-04-01"}},
161+
{ID: "d@test.ch", Name: "d", VacationDays: []string{"2020-04-01"}},
162+
},
163+
args: args{
164+
start: date("2020-04-01"),
165+
end: date("2020-04-04"),
166+
rotation: 24 * time.Hour,
167+
},
168+
want: nil,
169+
wantErr: true,
174170
},
175171
}
176172
for _, tt := range tests {
177173
t.Run(tt.name, func(t *testing.T) {
178174
planner := NewDefaultShiftPlanner(tt.employees)
179-
if got := planner.Plan(tt.args.start, tt.args.end, tt.args.rotation); !reflect.DeepEqual(got, tt.want) {
175+
got, err := planner.Plan(tt.args.start, tt.args.end, tt.args.rotation)
176+
if (err != nil) != tt.wantErr {
177+
t.Errorf("Plan() error = %v, wantErr %v", err, tt.wantErr)
178+
}
179+
if !reflect.DeepEqual(got, tt.want) {
180180
t.Errorf("Plan() = %v, want %v", got, tt.want)
181181
}
182182
})

‎pkg/apis/plan_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ type Shift struct {
1313
}
1414

1515
type Rule interface {
16-
Match(primary Employee, shifts []Shift, start time.Time, end time.Time) bool
16+
Match(employee Employee, shifts []Shift, start time.Time, end time.Time) bool
1717
}
1818

1919
type Exporter interface {

0 commit comments

Comments
 (0)