From c3637023b1611881abc6bb8e4f041a5b8c854702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Orlando=20Tom=C3=A1s?= Date: Mon, 30 Dec 2024 23:22:47 +0100 Subject: [PATCH] Failure detection when no on-call duty is available for the shift --- cmd/create_command.go | 6 +- internal/shiftplan/default_rules.go | 4 +- internal/shiftplan/default_rules_test.go | 119 +++++++++++++++++++++++ internal/shiftplan/planner.go | 29 +++--- internal/shiftplan/planner_test.go | 102 +++++++++---------- pkg/apis/plan_types.go | 2 +- 6 files changed, 196 insertions(+), 66 deletions(-) create mode 100644 internal/shiftplan/default_rules_test.go diff --git a/cmd/create_command.go b/cmd/create_command.go index 52a304a..91a111e 100644 --- a/cmd/create_command.go +++ b/cmd/create_command.go @@ -102,7 +102,11 @@ func RunCreateShiftPlan(arguments []string) error { return fmt.Errorf("%w: invalid team config file: %w", ErrInvalidArgument, err) } - plan := shiftplan.NewDefaultShiftPlanner(team.Employees).Plan(*str, *end, time.Duration(*duration)*time.Hour) + plan, err := shiftplan.NewDefaultShiftPlanner(team.Employees).Plan(*str, *end, time.Duration(*duration)*time.Hour) + if err != nil { + return fmt.Errorf("can not create on-call schedule: %w", err) + } + if err := converters[transform]().Write(plan, os.Stdout); err != nil { return fmt.Errorf("unexpecting error: %w", err) } diff --git a/internal/shiftplan/default_rules.go b/internal/shiftplan/default_rules.go index ab01bd6..01a8c8e 100644 --- a/internal/shiftplan/default_rules.go +++ b/internal/shiftplan/default_rules.go @@ -14,8 +14,8 @@ type DefaultRule struct { fn func(e apis.Employee, _ []apis.Shift, start time.Time, end time.Time) bool } -func (d *DefaultRule) Match(primary apis.Employee, shifts []apis.Shift, start time.Time, end time.Time) bool { - return d.fn(primary, shifts, start, end) +func (d *DefaultRule) Match(employee apis.Employee, shifts []apis.Shift, start time.Time, end time.Time) bool { + return d.fn(employee, shifts, start, end) } func VacationConflict() *DefaultRule { diff --git a/internal/shiftplan/default_rules_test.go b/internal/shiftplan/default_rules_test.go new file mode 100644 index 0000000..8869e4f --- /dev/null +++ b/internal/shiftplan/default_rules_test.go @@ -0,0 +1,119 @@ +package shiftplan + +import ( + "testing" + "time" + + "github.com/orltom/on-call-schedule/pkg/apis" +) + +func TestVacationConflict(t *testing.T) { + type args struct { + employee apis.Employee + start time.Time + end time.Time + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "Should detect conflict if employee has holiday between scheduled shift", + args: args{ + employee: apis.Employee{ID: "a", Name: "a", VacationDays: []string{"2024-01-01"}}, + start: date("2024-01-01"), + end: date("2024-01-02"), + }, + want: true, + }, + { + name: "Should not detect conflict if employee has no holidays", + args: args{ + employee: apis.Employee{ID: "a", Name: "a", VacationDays: nil}, + start: date("2024-01-01"), + end: date("2024-01-02"), + }, + want: false, + }, + { + name: "Should not detect conflict if employee has holiday outside scheduled shift", + args: args{ + employee: apis.Employee{ID: "a", Name: "a", VacationDays: []string{"2024-01-03"}}, + start: date("2024-01-01"), + end: date("2024-01-02"), + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := VacationConflict() + if got := d.Match(tt.args.employee, nil, tt.args.start, tt.args.end); got != tt.want { + t.Errorf("Match() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestInvolvedInLastSift(t *testing.T) { + type args struct { + employee apis.Employee + shifts []apis.Shift + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "Should not detect conflict when start with the first schedule shift", + args: args{ + employee: apis.Employee{ID: "a", Name: "a", VacationDays: nil}, + shifts: nil, + }, + want: false, + }, + { + name: "Should not detect conflict if employee was not on last shift", + args: args{ + employee: apis.Employee{ID: "a", Name: "a", VacationDays: nil}, + shifts: []apis.Shift{{ + Primary: "b", + Secondary: "c", + }}, + }, + want: false, + }, + { + name: "Should detect conflict if employee was on last shift as primary", + args: args{ + employee: apis.Employee{ID: "a", Name: "a", VacationDays: nil}, + shifts: []apis.Shift{{ + Primary: "a", + Secondary: "b", + }}, + }, + want: true, + }, + { + name: "Should detect conflict if employee was on last shift as secondary", + args: args{ + employee: apis.Employee{ID: "a", Name: "a", VacationDays: nil}, + shifts: []apis.Shift{{ + Primary: "c", + Secondary: "a", + }}, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := InvolvedInLastSift() + if got := d.Match(tt.args.employee, tt.args.shifts, time.Now(), time.Now()); got != tt.want { + t.Errorf("Match() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/shiftplan/planner.go b/internal/shiftplan/planner.go index 8783ca5..4087047 100644 --- a/internal/shiftplan/planner.go +++ b/internal/shiftplan/planner.go @@ -1,6 +1,7 @@ package shiftplan import ( + "fmt" "slices" "time" @@ -16,8 +17,8 @@ type ShiftPlanner struct { func NewDefaultShiftPlanner(team []apis.Employee) *ShiftPlanner { return NewShiftPlanner( team, - []apis.Rule{VacationConflict(), InvolvedInLastSift()}, - []apis.Rule{VacationConflict(), InvolvedInLastSift()}, + []apis.Rule{VacationConflict()}, + []apis.Rule{VacationConflict()}, ) } @@ -29,16 +30,22 @@ func NewShiftPlanner(team []apis.Employee, primaryConflictCheckers []apis.Rule, } } -func (p *ShiftPlanner) Plan(start time.Time, end time.Time, rotation time.Duration) []apis.Shift { +func (p *ShiftPlanner) Plan(start time.Time, end time.Time, rotation time.Duration) ([]apis.Shift, error) { var plan []apis.Shift for s := start; s.Before(end); s = s.Add(rotation) { e := s.Add(rotation) - primary := p.findPrimary(plan, s, e) + primary, err := p.findPrimary(plan, s, e) + if err != nil { + return nil, err + } p.team = remove(p.team, primary) - secondary := p.findSecondary(plan, s, e) + secondary, err := p.findSecondary(plan, s, e) + if err != nil { + return nil, err + } p.team = remove(p.team, secondary) 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 p.team = append(p.team, secondary, primary) } - return plan + return plan, nil } -func (p *ShiftPlanner) findPrimary(shifts []apis.Shift, start time.Time, end time.Time) apis.Employee { +func (p *ShiftPlanner) findPrimary(shifts []apis.Shift, start time.Time, end time.Time) (apis.Employee, error) { return p.find(shifts, start, end, p.primaryConflictCheckers) } -func (p *ShiftPlanner) findSecondary(shifts []apis.Shift, start time.Time, end time.Time) apis.Employee { +func (p *ShiftPlanner) findSecondary(shifts []apis.Shift, start time.Time, end time.Time) (apis.Employee, error) { return p.find(shifts, start, end, p.secondaryConflictChecker) } -func (p *ShiftPlanner) find(shifts []apis.Shift, start time.Time, end time.Time, checkers []apis.Rule) apis.Employee { +func (p *ShiftPlanner) find(shifts []apis.Shift, start time.Time, end time.Time, checkers []apis.Rule) (apis.Employee, error) { for idx := range p.team { if p.hasConflict(p.team[idx], checkers, shifts, start, end) { continue } - return p.team[idx] + return p.team[idx], nil } - return p.team[0] + return apis.Employee{}, fmt.Errorf("could not find available duty between %s and %s", start, end) } func (p *ShiftPlanner) hasConflict(e apis.Employee, conflictCheckers []apis.Rule, shifts []apis.Shift, start time.Time, end time.Time) bool { diff --git a/internal/shiftplan/planner_test.go b/internal/shiftplan/planner_test.go index c8421a0..59943d0 100644 --- a/internal/shiftplan/planner_test.go +++ b/internal/shiftplan/planner_test.go @@ -19,20 +19,13 @@ func TestPlanner_Plan(t *testing.T) { employees []apis.Employee args args want []apis.Shift + wantErr bool }{ { name: "Without declared holidays, a daily schedule should be generated according to the order of the employees", employees: []apis.Employee{ - { - ID: "a@test.ch", - Name: "a", - VacationDays: nil, - }, - { - ID: "b@test.ch", - Name: "b", - VacationDays: nil, - }, + {ID: "a@test.ch", Name: "a", VacationDays: nil}, + {ID: "b@test.ch", Name: "b", VacationDays: nil}, }, args: args{ start: date("2020-04-01"), @@ -59,30 +52,15 @@ func TestPlanner_Plan(t *testing.T) { Secondary: "b@test.ch", }, }, + wantErr: false, }, { name: "Should not schedule Primary on holiday days", employees: []apis.Employee{ - { - ID: "a@test.ch", - Name: "a", - VacationDays: []string{"2020-04-01"}, - }, - { - ID: "b@test.ch", - Name: "b", - VacationDays: nil, - }, - { - ID: "c@test.ch", - Name: "c", - VacationDays: nil, - }, - { - ID: "d@test.ch", - Name: "d", - VacationDays: nil, - }, + {ID: "a@test.ch", Name: "a", VacationDays: []string{"2020-04-01"}}, + {ID: "b@test.ch", Name: "b", VacationDays: nil}, + {ID: "c@test.ch", Name: "c", VacationDays: nil}, + {ID: "d@test.ch", Name: "d", VacationDays: nil}, }, args: args{ start: date("2020-04-01"), @@ -115,30 +93,15 @@ func TestPlanner_Plan(t *testing.T) { Secondary: "a@test.ch", }, }, + wantErr: false, }, { name: "Should not schedule Secondary on holiday days", employees: []apis.Employee{ - { - ID: "a@test.ch", - Name: "a", - VacationDays: nil, - }, - { - ID: "b@test.ch", - Name: "b", - VacationDays: []string{"2020-04-01"}, - }, - { - ID: "c@test.ch", - Name: "c", - VacationDays: nil, - }, - { - ID: "d@test.ch", - Name: "d", - VacationDays: nil, - }, + {ID: "a@test.ch", Name: "a", VacationDays: nil}, + {ID: "b@test.ch", Name: "b", VacationDays: []string{"2020-04-01"}}, + {ID: "c@test.ch", Name: "c", VacationDays: nil}, + {ID: "d@test.ch", Name: "d", VacationDays: nil}, }, args: args{ start: date("2020-04-01"), @@ -171,12 +134,49 @@ func TestPlanner_Plan(t *testing.T) { Secondary: "b@test.ch", }, }, + wantErr: false, + }, + { + name: "Should return an error if can not find next available duty", + employees: []apis.Employee{ + {ID: "a@test.ch", Name: "a", VacationDays: []string{"2020-04-01"}}, + {ID: "b@test.ch", Name: "b", VacationDays: []string{"2020-04-01"}}, + {ID: "c@test.ch", Name: "c", VacationDays: []string{"2020-04-01"}}, + {ID: "d@test.ch", Name: "d", VacationDays: []string{"2020-04-01"}}, + }, + args: args{ + start: date("2020-04-01"), + end: date("2020-04-04"), + rotation: 24 * time.Hour, + }, + want: nil, + wantErr: true, + }, + { + name: "Should return an error if can not find next secondary", + employees: []apis.Employee{ + {ID: "a@test.ch", Name: "a", VacationDays: nil}, + {ID: "b@test.ch", Name: "b", VacationDays: []string{"2020-04-01"}}, + {ID: "c@test.ch", Name: "c", VacationDays: []string{"2020-04-01"}}, + {ID: "d@test.ch", Name: "d", VacationDays: []string{"2020-04-01"}}, + }, + args: args{ + start: date("2020-04-01"), + end: date("2020-04-04"), + rotation: 24 * time.Hour, + }, + want: nil, + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { planner := NewDefaultShiftPlanner(tt.employees) - if got := planner.Plan(tt.args.start, tt.args.end, tt.args.rotation); !reflect.DeepEqual(got, tt.want) { + got, err := planner.Plan(tt.args.start, tt.args.end, tt.args.rotation) + if (err != nil) != tt.wantErr { + t.Errorf("Plan() error = %v, wantErr %v", err, tt.wantErr) + } + if !reflect.DeepEqual(got, tt.want) { t.Errorf("Plan() = %v, want %v", got, tt.want) } }) diff --git a/pkg/apis/plan_types.go b/pkg/apis/plan_types.go index e8f1b18..60eb900 100644 --- a/pkg/apis/plan_types.go +++ b/pkg/apis/plan_types.go @@ -13,7 +13,7 @@ type Shift struct { } type Rule interface { - Match(primary Employee, shifts []Shift, start time.Time, end time.Time) bool + Match(employee Employee, shifts []Shift, start time.Time, end time.Time) bool } type Exporter interface {