feat!: split startercode policy into branches and issues; apply branch rules during generate

config/assignment.go

@@ -47,6 +47,10 @@ func GetAssignmentConfig(course, assignment string, onlyForStudentsOrGroups ...s
 		containerRegistry = true
 	}
 
+	starter := startercode(assignmentKey)
+	branchRules := branches(assignmentKey, starter)
+	defaultCloneBranch := defaultBranch(branchRules, "main")
+
 	assignmentConfig := &AssignmentConfig{
 		Course:                course,
 		Name:                  assignment,
@@ -58,10 +62,12 @@ func GetAssignmentConfig(course, assignment string, onlyForStudentsOrGroups ...s
 		ContainerRegistry:     containerRegistry,
 		AccessLevel:           accessLevel(assignmentKey),
 		MergeRequest:          mergeRequest(assignmentKey),
+		Branches:              branchRules,
+		Issues:                issues(assignmentKey),
 		Students:              students(per, course, assignment, onlyForStudentsOrGroups...),
 		Groups:                groups(per, course, assignment, onlyForStudentsOrGroups...),
-		Startercode:           startercode(assignmentKey),
-		Clone:                 clone(assignmentKey),
+		Startercode:           starter,
+		Clone:                 clone(assignmentKey, defaultCloneBranch),
 		Release:               release,
 		Seeder:                seeder(assignmentKey),
 	}

config/repo.go

@@ -1,6 +1,8 @@
 package config
 
 import (
+	"strings"
+
 	"github.com/rs/zerolog/log"
 	"github.com/spf13/viper"
 )
@@ -29,40 +31,123 @@ func startercode(assignmentKey string) *Startercode {
 		toBranch = tB
 	}
 
-	devBranch := toBranch
-	if dB := viper.GetString(assignmentKey + ".startercode.devBranch"); len(dB) > 0 {
-		devBranch = dB
+	additionalBranches := viper.GetStringSlice(assignmentKey + ".startercode.additionalBranches")
+
+	return &Startercode{
+		URL:                url,
+		FromBranch:         fromBranch,
+		ToBranch:           toBranch,
+		AdditionalBranches: additionalBranches,
 	}
+}
 
-	additionalBranches := []string{}
-	if addB := viper.GetStringSlice(assignmentKey + ".startercode.additionalBranches"); len(addB) > 0 {
-		additionalBranches = addB
+func branches(assignmentKey string, starter *Startercode) []BranchRule {
+	var configured []BranchRule
+	if err := viper.UnmarshalKey(assignmentKey+".branches", &configured); err != nil {
+		log.Fatal().Err(err).Str("assignmentKey", assignmentKey).Msg("cannot parse branches config")
 	}
 
-	replicateIssue := viper.GetBool(assignmentKey + ".startercode.replicateIssue")
+	rules := make([]BranchRule, 0)
+	seen := make(map[string]int)
+	appendOrMerge := func(rule BranchRule) {
+		rule.Name = strings.TrimSpace(rule.Name)
+		if rule.Name == "" {
+			return
+		}
+		if idx, ok := seen[rule.Name]; ok {
+			rules[idx].Protect = rules[idx].Protect || rule.Protect
+			rules[idx].MergeOnly = rules[idx].MergeOnly || rule.MergeOnly
+			rules[idx].Default = rules[idx].Default || rule.Default
+			return
+		}
+		seen[rule.Name] = len(rules)
+		rules = append(rules, rule)
+	}
+
+	for _, rule := range configured {
+		appendOrMerge(rule)
+	}
+
+	if len(rules) == 0 {
+		if starter != nil {
+			appendOrMerge(BranchRule{Name: starter.ToBranch, Default: true})
+		}
+
+		// Legacy compatibility for old startercode-based branch config.
+		legacyDevBranch := viper.GetString(assignmentKey + ".startercode.devBranch")
+		if legacyDevBranch != "" {
+			appendOrMerge(BranchRule{Name: legacyDevBranch, Default: true})
+		}
+
+		for _, branchName := range viper.GetStringSlice(assignmentKey + ".startercode.additionalBranches") {
+			appendOrMerge(BranchRule{Name: branchName})
+		}
+
+		if viper.GetBool(assignmentKey+".startercode.protectToBranch") && starter != nil {
+			appendOrMerge(BranchRule{Name: starter.ToBranch, Protect: true})
+		}
 
-	var issueNumbers []int
-	if replicateIssue {
-		issueNumbers = []int{1}
-		if issueNums := viper.GetIntSlice(assignmentKey + ".startercode.issueNumbers"); len(issueNums) > 0 {
-			issueNumbers = issueNums
+		if viper.GetBool(assignmentKey + ".startercode.protectDevBranchMergeOnly") {
+			legacyTarget := legacyDevBranch
+			if legacyTarget == "" && starter != nil {
+				legacyTarget = starter.ToBranch
+			}
+			appendOrMerge(BranchRule{Name: legacyTarget, MergeOnly: true})
 		}
 	}
 
-	return &Startercode{
-		URL:                       url,
-		FromBranch:                fromBranch,
-		ToBranch:                  toBranch,
-		DevBranch:                 devBranch,
-		AdditionalBranches:        additionalBranches,
-		ProtectToBranch:           viper.GetBool(assignmentKey + ".startercode.protectToBranch"),
-		ProtectDevBranchMergeOnly: viper.GetBool(assignmentKey + ".startercode.protectDevBranchMergeOnly"),
-		ReplicateIssue:            replicateIssue,
-		IssueNumbers:              issueNumbers,
+	hasDefault := false
+	for _, rule := range rules {
+		if rule.Default {
+			hasDefault = true
+			break
+		}
+	}
+
+	if !hasDefault && len(rules) > 0 {
+		rules[0].Default = true
+	}
+
+	return rules
+}
+
+func defaultBranch(rules []BranchRule, fallback string) string {
+	for _, rule := range rules {
+		if rule.Default && rule.Name != "" {
+			return rule.Name
+		}
+	}
+	if fallback != "" {
+		return fallback
+	}
+	if len(rules) > 0 {
+		return rules[0].Name
+	}
+	return "main"
+}
+
+func issues(assignmentKey string) *IssueReplication {
+	replicate := viper.GetBool(assignmentKey + ".issues.replicateFromStartercode")
+	numbers := viper.GetIntSlice(assignmentKey + ".issues.issueNumbers")
+
+	// Legacy compatibility for old startercode issue replication config.
+	if !replicate && !viper.IsSet(assignmentKey+".issues") {
+		replicate = viper.GetBool(assignmentKey + ".startercode.replicateIssue")
+		numbers = viper.GetIntSlice(assignmentKey + ".startercode.issueNumbers")
+	}
+
+	if !replicate {
+		return &IssueReplication{ReplicateFromStartercode: false}
+	}
+
+	if len(numbers) == 0 {
+		numbers = []int{1}
 	}
+
+	return &IssueReplication{ReplicateFromStartercode: true, IssueNumbers: numbers}
 }
 
-func clone(assignmentKey string) *Clone {
+func clone(assignmentKey, defaultBranch string) *Clone {
 	cloneMap := viper.GetStringMapString(assignmentKey + ".clone")
 
 	localpath, ok := cloneMap["localpath"]
@@ -72,7 +157,7 @@ func clone(assignmentKey string) *Clone {
 
 	branch, ok := cloneMap["branch"]
 	if !ok {
-		branch = "main"
+		branch = defaultBranch
 	}
 
 	force := viper.GetBool(assignmentKey + ".clone.force")
@@ -89,10 +174,21 @@ func (cfg *AssignmentConfig) SetBranch(branch string) {
 }
 
 func (cfg *AssignmentConfig) SetProtectToBranch(branch string) {
-	if branch != "" {
-		cfg.Startercode.ToBranch = branch
+	if branch == "" && len(cfg.Branches) > 0 {
+		branch = cfg.Branches[0].Name
 	}
-	cfg.Startercode.ProtectToBranch = true
+	if branch == "" {
+		branch = "main"
+	}
+
+	for i := range cfg.Branches {
+		if cfg.Branches[i].Name == branch {
+			cfg.Branches[i].Protect = true
+			return
+		}
+	}
+
+	cfg.Branches = append(cfg.Branches, BranchRule{Name: branch, Protect: true})
 }
 
 func (cfg *AssignmentConfig) SetLocalpath(localpath string) {

config/repo_test.go

@@ -11,21 +11,14 @@ func TestStartercodeDefaultsAndReplication(t *testing.T) {
 	resetViper(t)
 	viper.Set("course.a1.startercode", map[string]string{"url": "git@example.org:starter.git"})
 	viper.Set("course.a1.startercode.url", "git@example.org:starter.git")
-	viper.Set("course.a1.startercode.replicateIssue", true)
 
 	s := startercode("course.a1")
 	if s == nil {
 		t.Fatal("startercode should not be nil")
 	}
-	if s.FromBranch != "main" || s.ToBranch != "main" || s.DevBranch != "main" {
+	if s.FromBranch != "main" || s.ToBranch != "main" {
 		t.Fatalf("unexpected startercode defaults: %#v", s)
 	}
-	if !s.ReplicateIssue {
-		t.Fatal("ReplicateIssue should be true")
-	}
-	if !reflect.DeepEqual(s.IssueNumbers, []int{1}) {
-		t.Fatalf("IssueNumbers = %#v, want [1]", s.IssueNumbers)
-	}
 }
 
 func TestStartercodeOverrides(t *testing.T) {
@@ -34,42 +27,101 @@ func TestStartercodeOverrides(t *testing.T) {
 	viper.Set("course.a1.startercode.url", "git@example.org:starter.git")
 	viper.Set("course.a1.startercode.fromBranch", "template")
 	viper.Set("course.a1.startercode.toBranch", "submission")
-	viper.Set("course.a1.startercode.devBranch", "develop")
 	viper.Set("course.a1.startercode.additionalBranches", []string{"release", "demo"})
-	viper.Set("course.a1.startercode.replicateIssue", true)
-	viper.Set("course.a1.startercode.issueNumbers", []int{4, 7})
-	viper.Set("course.a1.startercode.protectToBranch", true)
-	viper.Set("course.a1.startercode.protectDevBranchMergeOnly", true)
 
 	s := startercode("course.a1")
-	if s.FromBranch != "template" || s.ToBranch != "submission" || s.DevBranch != "develop" {
+	if s.FromBranch != "template" || s.ToBranch != "submission" {
 		t.Fatalf("startercode branches = %#v", s)
 	}
 	if !reflect.DeepEqual(s.AdditionalBranches, []string{"release", "demo"}) {
-		t.Fatalf("additional branches = %#v", s.AdditionalBranches)
+		t.Fatalf("startercode additional branches = %#v", s.AdditionalBranches)
 	}
-	if !reflect.DeepEqual(s.IssueNumbers, []int{4, 7}) {
-		t.Fatalf("IssueNumbers = %#v", s.IssueNumbers)
+}
+
+func TestBranches_DefaultsAndLegacyFallback(t *testing.T) {
+	resetViper(t)
+	viper.Set("course.a1.startercode", map[string]string{"url": "git@example.org:starter.git"})
+	viper.Set("course.a1.startercode.url", "git@example.org:starter.git")
+	viper.Set("course.a1.startercode.toBranch", "main")
+	viper.Set("course.a1.startercode.devBranch", "develop")
+	viper.Set("course.a1.startercode.additionalBranches", []string{"release"})
+	viper.Set("course.a1.startercode.protectToBranch", true)
+	viper.Set("course.a1.startercode.protectDevBranchMergeOnly", true)
+
+	b := branches("course.a1", startercode("course.a1"))
+	if len(b) != 3 {
+		t.Fatalf("len(branches) = %d, want 3", len(b))
+	}
+	if b[0].Name != "main" {
+		t.Fatalf("first branch = %#v", b[0])
 	}
-	if !s.ProtectToBranch || !s.ProtectDevBranchMergeOnly {
-		t.Fatalf("protect flags = %#v", s)
+	if !b[0].Protect {
+		t.Fatalf("main branch should be protected: %#v", b[0])
+	}
+	if b[1].Name != "develop" || !b[1].Default || !b[1].MergeOnly {
+		t.Fatalf("develop branch = %#v", b[1])
+	}
+}
+
+func TestBranches_ExplicitConfig(t *testing.T) {
+	resetViper(t)
+	viper.Set("course.a1.branches", []map[string]any{
+		{"name": "main", "protect": true},
+		{"name": "dev", "default": true, "mergeOnly": true},
+	})
+
+	b := branches("course.a1", nil)
+	if len(b) != 2 {
+		t.Fatalf("len(branches) = %d, want 2", len(b))
+	}
+	if b[0].Name != "main" || !b[0].Protect {
+		t.Fatalf("main branch = %#v", b[0])
+	}
+	if b[1].Name != "dev" || !b[1].Default || !b[1].MergeOnly {
+		t.Fatalf("dev branch = %#v", b[1])
+	}
+}
+
+func TestIssues_DefaultsAndLegacyFallback(t *testing.T) {
+	resetViper(t)
+	viper.Set("course.a1.startercode.replicateIssue", true)
+
+	i := issues("course.a1")
+	if i == nil || !i.ReplicateFromStartercode {
+		t.Fatalf("issues = %#v", i)
+	}
+	if !reflect.DeepEqual(i.IssueNumbers, []int{1}) {
+		t.Fatalf("IssueNumbers = %#v, want [1]", i.IssueNumbers)
+	}
+
+	viper.Set("course.a1.issues.replicateFromStartercode", true)
+	viper.Set("course.a1.issues.issueNumbers", []int{4, 7})
+	i = issues("course.a1")
+	if !reflect.DeepEqual(i.IssueNumbers, []int{4, 7}) {
+		t.Fatalf("IssueNumbers = %#v", i.IssueNumbers)
 	}
 }
 
 func TestCloneDefaultsAndOverrides(t *testing.T) {
 	resetViper(t)
 
-	c := clone("course.a1")
-	if c.LocalPath != "." || c.Branch != "main" || c.Force {
+	c := clone("course.a1", "develop")
+	if c.LocalPath != "." || c.Branch != "develop" || c.Force {
 		t.Fatalf("clone defaults = %#v", c)
 	}
 
+	resetViper(t)
+	c = clone("course.a1", "develop")
+	if c.Branch != "develop" {
+		t.Fatalf("clone default branch = %q, want %q", c.Branch, "develop")
+	}
+
 	viper.Set("course.a1.clone", map[string]string{"localpath": "/tmp/repos", "branch": "dev"})
 	viper.Set("course.a1.clone.localpath", "/tmp/repos")
 	viper.Set("course.a1.clone.branch", "dev")
 	viper.Set("course.a1.clone.force", true)
 
-	c = clone("course.a1")
+	c = clone("course.a1", "develop")
 	if c.LocalPath != "/tmp/repos" || c.Branch != "dev" || !c.Force {
 		t.Fatalf("clone overrides = %#v", c)
 	}

config/setters_test.go

@@ -19,25 +19,24 @@ func TestSetBranch_Empty(t *testing.T) {
 }
 
 func TestSetProtectToBranch_WithBranch(t *testing.T) {
-	cfg := &AssignmentConfig{Startercode: &Startercode{ToBranch: "main"}}
+	cfg := &AssignmentConfig{}
 	cfg.SetProtectToBranch("feature")
-	if cfg.Startercode.ToBranch != "feature" {
-		t.Fatalf("ToBranch = %q, want %q", cfg.Startercode.ToBranch, "feature")
+	if len(cfg.Branches) != 1 {
+		t.Fatalf("len(Branches) = %d, want 1", len(cfg.Branches))
 	}
-	if !cfg.Startercode.ProtectToBranch {
-		t.Fatal("ProtectToBranch should be true")
+	if cfg.Branches[0].Name != "feature" || !cfg.Branches[0].Protect {
+		t.Fatalf("branch config = %#v", cfg.Branches[0])
 	}
 }
 
 func TestSetProtectToBranch_EmptyBranch(t *testing.T) {
-	cfg := &AssignmentConfig{Startercode: &Startercode{ToBranch: "main"}}
+	cfg := &AssignmentConfig{Branches: []BranchRule{{Name: "main"}}}
 	cfg.SetProtectToBranch("")
-	// empty: ToBranch stays unchanged, ProtectToBranch is set to true
-	if cfg.Startercode.ToBranch != "main" {
-		t.Fatalf("ToBranch = %q, want %q", cfg.Startercode.ToBranch, "main")
+	if len(cfg.Branches) != 1 {
+		t.Fatalf("len(Branches) = %d, want 1", len(cfg.Branches))
 	}
-	if !cfg.Startercode.ProtectToBranch {
-		t.Fatal("ProtectToBranch should be true even with empty branch string")
+	if cfg.Branches[0].Name != "main" || !cfg.Branches[0].Protect {
+		t.Fatalf("branch config = %#v", cfg.Branches[0])
 	}
 }