Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions web/app/assignment_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,108 @@ func AssignmentBranchSchema() []FieldMeta {
return branchFields
}

// AssignmentApprovalSettingsSchema returns the metadata for the mergeRequest
// approval settings. These are tri-state: an ENUM whose empty option means
// "not set" (inherit), so `an`/`aus` map to *bool true/false and "" to nil.
func AssignmentApprovalSettingsSchema() []FieldMeta {
return approvalSettingsFields
}

// AssignmentApprovalRuleSchema returns the metadata for one approval rule — a row
// of the repeat-group `approvals.rules` list.
func AssignmentApprovalRuleSchema() []FieldMeta {
return approvalRuleFields
}

var triStateOptions = []FieldOption{
{Value: "true", Label: "an", Description: "aktiviert"},
{Value: "false", Label: "aus", Description: "deaktiviert"},
}

var approvalSettingsFields = []FieldMeta{
{
Key: "preventApprovalByMergeRequestCreator",
Label: "Ersteller darf nicht selbst freigeben",
Description: "Die MR-Erstellerin/der Ersteller kann den eigenen MR nicht approven.",
Kind: KindEnum,
Options: triStateOptions,
},
{
Key: "preventApprovalsByUsersWhoAddCommits",
Label: "Committer dürfen nicht freigeben",
Description: "Wer Commits hinzufügt, kann den MR nicht approven.",
Kind: KindEnum,
Options: triStateOptions,
},
{
Key: "preventEditingApprovalRulesInMergeRequests",
Label: "Approval-Regeln im MR sperren",
Description: "Approval-Regeln können im einzelnen MR nicht geändert werden.",
Kind: KindEnum,
Options: triStateOptions,
},
{
Key: "requireUserReauthenticationToApprove",
Label: "Re-Authentifizierung zum Freigeben",
Description: "Approven erfordert erneute Authentifizierung.",
Kind: KindEnum,
Options: triStateOptions,
},
{
Key: "whenCommitAdded",
Label: "Bei neuem Commit",
Description: "Was mit bestehenden Approvals passiert, wenn ein Commit hinzukommt (leer = GitLab-Default).",
Kind: KindEnum,
Options: []FieldOption{
{Value: "keepApprovals", Label: "Behalten", Description: "Approvals bleiben bestehen."},
{Value: "removeAllApprovals", Label: "Alle entfernen", Description: "Alle Approvals werden zurückgesetzt."},
{Value: "removeCodeOwnerApprovalsIfTheirFilesChanged", Label: "Code-Owner (geänderte Dateien)", Description: "Nur Code-Owner-Approvals entfernen, wenn deren Dateien geändert wurden."},
},
},
}

var approvalRuleFields = []FieldMeta{
{
Key: "name",
Label: "Regelname",
Description: "Name der Approval-Regel.",
Kind: KindString,
Required: true,
Example: "Tutoren",
},
{
Key: "requiredApprovals",
Label: "Erforderliche Freigaben",
Description: "Anzahl nötiger Approvals für diese Regel.",
Kind: KindInt,
Example: "1",
},
{
Key: "usernames",
Label: "Usernames",
Description: "GitLab-Usernames, die freigeben dürfen (kommagetrennt).",
Kind: KindStringList,
},
{
Key: "groups",
Label: "Gruppen",
Description: "GitLab-Gruppen(pfade), die freigeben dürfen (kommagetrennt).",
Kind: KindStringList,
},
{
Key: "branches",
Label: "Branches",
Description: "Branches, für die die Regel gilt (kommagetrennt; leer = alle).",
Kind: KindStringList,
},
{
Key: "multiMemberGroupsOnly",
Label: "Nur mehrköpfige Gruppen",
Description: "Nur Gruppen mit mehr als einem Mitglied zählen.",
Kind: KindBool,
},
}

var branchFields = []FieldMeta{
{
Key: "name",
Expand Down
137 changes: 132 additions & 5 deletions web/app/assignments.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,61 @@ func assignmentOwn(src *config.AssignmentSource) []FieldValue {
FieldValue{Key: p + "codeOwnerApprovalRequired", Value: strconv.FormatBool(b.CodeOwnerApprovalRequired)},
)
}

// mergeRequest.approvals (settings tri-state + rules repeat group), keyed
// flat as approvals.* so they render as their own section.
var appr *config.ApprovalsSource
if src.MergeRequest != nil {
appr = src.MergeRequest.Approvals
}
var settings *config.ApprovalSettingsSource
if appr != nil {
settings = appr.Settings
}
own = append(own,
FieldValue{Key: "approvals.settings.preventApprovalByMergeRequestCreator", Value: triBool(settings, func(s *config.ApprovalSettingsSource) *bool { return s.PreventApprovalByMergeRequestCreator })},
FieldValue{Key: "approvals.settings.preventApprovalsByUsersWhoAddCommits", Value: triBool(settings, func(s *config.ApprovalSettingsSource) *bool { return s.PreventApprovalsByUsersWhoAddCommits })},
FieldValue{Key: "approvals.settings.preventEditingApprovalRulesInMergeRequests", Value: triBool(settings, func(s *config.ApprovalSettingsSource) *bool { return s.PreventEditingApprovalRulesInMergeRequests })},
FieldValue{Key: "approvals.settings.requireUserReauthenticationToApprove", Value: triBool(settings, func(s *config.ApprovalSettingsSource) *bool { return s.RequireUserReauthenticationToApprove })},
FieldValue{Key: "approvals.settings.whenCommitAdded", Value: optStr(settings)},
)
var rules []config.ApprovalRuleSource
if appr != nil {
rules = appr.Rules
}
own = append(own, FieldValue{Key: "approvals.rules.count", Value: strconv.Itoa(len(rules))})
for i, r := range rules {
p := fmt.Sprintf("approvals.rules.%d.", i)
own = append(own,
FieldValue{Key: p + "name", Value: r.Name},
FieldValue{Key: p + "requiredApprovals", Value: strconv.Itoa(r.RequiredApprovals)},
FieldValue{Key: p + "usernames", Value: strings.Join(r.Usernames, ", ")},
FieldValue{Key: p + "groups", Value: strings.Join(r.Groups, ", ")},
FieldValue{Key: p + "branches", Value: strings.Join(r.Branches, ", ")},
FieldValue{Key: p + "multiMemberGroupsOnly", Value: strconv.FormatBool(r.MultiMemberGroupsOnly)},
)
}
return own
}

func triBool(s *config.ApprovalSettingsSource, f func(*config.ApprovalSettingsSource) *bool) string {
if s == nil {
return ""
}
b := f(s)
if b == nil {
return ""
}
return strconv.FormatBool(*b)
}

func optStr(s *config.ApprovalSettingsSource) string {
if s == nil || s.WhenCommitAdded == nil {
return ""
}
return *s.WhenCommitAdded
}

func issuesBool(i *config.IssuesSource, f func(*config.IssuesSource) bool) string {
if i == nil {
return "false"
Expand Down Expand Up @@ -292,14 +344,13 @@ func splitIntList(s string) []int {
return out
}

// applyMergeRequestDraft rebuilds the mergeRequest block from the draft's
// mergeRequest.* keys into a NEW struct (never mutating the shared original) and
// nils it when every editable field is empty AND there is no approvals block.
// The (not-yet-editable) approvals block is carried over untouched.
// applyMergeRequestDraft rebuilds the mergeRequest block (scalars + approvals)
// from the draft into a NEW struct (never mutating the shared original) and nils
// it when every field is empty and there is no approvals block.
func applyMergeRequestDraft(orig *config.MergeRequestSource, draft map[string]string) *config.MergeRequestSource {
touched := false
for k := range draft {
if strings.HasPrefix(k, "mergeRequest.") {
if strings.HasPrefix(k, "mergeRequest.") || strings.HasPrefix(k, "approvals.") {
touched = true
break
}
Expand Down Expand Up @@ -328,6 +379,8 @@ func applyMergeRequestDraft(orig *config.MergeRequestSource, draft map[string]st
mr.StatusChecksMustSucceed = v == "true"
}
}
mr.Approvals = buildApprovals(mr.Approvals, draft)

if mr.MergeMethod == "" && mr.SquashOption == "" && !mr.Pipeline &&
!mr.SkippedPipelinesAreSuccessful && !mr.AllThreadsMustBeResolved &&
!mr.StatusChecksMustSucceed && mr.Approvals == nil {
Expand All @@ -336,6 +389,80 @@ func applyMergeRequestDraft(orig *config.MergeRequestSource, draft map[string]st
return &mr
}

// buildApprovals rebuilds the approvals block from the draft's approvals.* keys
// into a NEW struct; returns the original when the draft does not address
// approvals, and nil when there are neither settings nor rules.
func buildApprovals(orig *config.ApprovalsSource, draft map[string]string) *config.ApprovalsSource {
touched := false
for k := range draft {
if strings.HasPrefix(k, "approvals.") {
touched = true
break
}
}
if !touched {
return orig
}
settings := buildApprovalSettings(draft)
rules := buildApprovalRules(draft)
if settings == nil && len(rules) == 0 {
return nil
}
return &config.ApprovalsSource{Settings: settings, Rules: rules}
}

func buildApprovalSettings(draft map[string]string) *config.ApprovalSettingsSource {
s := config.ApprovalSettingsSource{}
any := false
setTri := func(key string, dst **bool) {
if v, ok := draft[key]; ok && v != "" {
b := v == "true"
*dst = &b
any = true
}
}
setTri("approvals.settings.preventApprovalByMergeRequestCreator", &s.PreventApprovalByMergeRequestCreator)
setTri("approvals.settings.preventApprovalsByUsersWhoAddCommits", &s.PreventApprovalsByUsersWhoAddCommits)
setTri("approvals.settings.preventEditingApprovalRulesInMergeRequests", &s.PreventEditingApprovalRulesInMergeRequests)
setTri("approvals.settings.requireUserReauthenticationToApprove", &s.RequireUserReauthenticationToApprove)
if v, ok := draft["approvals.settings.whenCommitAdded"]; ok {
if w := strings.TrimSpace(v); w != "" {
s.WhenCommitAdded = &w
any = true
}
}
if !any {
return nil
}
return &s
}

func buildApprovalRules(draft map[string]string) []config.ApprovalRuleSource {
countStr, ok := draft["approvals.rules.count"]
if !ok {
return nil
}
n, _ := strconv.Atoi(countStr)
var rules []config.ApprovalRuleSource
for i := 0; i < n; i++ {
p := fmt.Sprintf("approvals.rules.%d.", i)
name := strings.TrimSpace(draft[p+"name"])
if name == "" {
continue
}
ra, _ := strconv.Atoi(strings.TrimSpace(draft[p+"requiredApprovals"]))
rules = append(rules, config.ApprovalRuleSource{
Name: name,
Branches: splitList(draft[p+"branches"]),
Usernames: splitList(draft[p+"usernames"]),
Groups: splitList(draft[p+"groups"]),
MultiMemberGroupsOnly: draft[p+"multiMemberGroupsOnly"] == "true",
RequiredApprovals: ra,
})
}
return rules
}

// applyStartercodeDraft rebuilds the startercode block from the draft's
// startercode.* keys. It always returns a NEW struct (never mutates the shared
// original) when the draft touches startercode, and nil when every startercode
Expand Down
69 changes: 69 additions & 0 deletions web/app/assignments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,75 @@ func TestSetAssignment_branchesRepeatGroup(t *testing.T) {
}
}

func TestSetAssignment_approvals(t *testing.T) {
const owner = "prof@hm.edu"
fs := newFakeStore()
fs.courses[owner+"/tc"] = storedCourse(t, owner, tcCourse)
a := &App{db: fs, gitlabHost: "https://gl"}
ctx := ctxAs(owner)

view, err := a.SetAssignment(ctx, "tc", "blatt1", map[string]string{
"mergeRequest.mergeMethod": "merge",
"approvals.settings.preventApprovalByMergeRequestCreator": "true",
"approvals.settings.requireUserReauthenticationToApprove": "false",
"approvals.settings.whenCommitAdded": "keepApprovals",
"approvals.rules.count": "1",
"approvals.rules.0.name": "Tutoren",
"approvals.rules.0.requiredApprovals": "2",
"approvals.rules.0.usernames": "a, b",
"approvals.rules.0.multiMemberGroupsOnly": "true",
})
if err != nil {
t.Fatalf("set: %v", err)
}
mr := fs.saved.Source.Assignments["blatt1"].MergeRequest
if mr == nil || mr.Approvals == nil {
t.Fatalf("approvals missing: %+v", mr)
}
s := mr.Approvals.Settings
if s == nil || s.PreventApprovalByMergeRequestCreator == nil || !*s.PreventApprovalByMergeRequestCreator {
t.Errorf("prevent-creator tri-state should be true: %+v", s)
}
if s.RequireUserReauthenticationToApprove == nil || *s.RequireUserReauthenticationToApprove {
t.Errorf("reauth tri-state should be false")
}
if s.PreventApprovalsByUsersWhoAddCommits != nil {
t.Errorf("unset tri-state should stay nil")
}
if s.WhenCommitAdded == nil || *s.WhenCommitAdded != "keepApprovals" {
t.Errorf("whenCommitAdded = %v", s.WhenCommitAdded)
}
if len(mr.Approvals.Rules) != 1 {
t.Fatalf("want 1 rule, got %+v", mr.Approvals.Rules)
}
r := mr.Approvals.Rules[0]
if r.Name != "Tutoren" || r.RequiredApprovals != 2 || len(r.Usernames) != 2 || !r.MultiMemberGroupsOnly {
t.Errorf("rule = %+v", r)
}

own := ownMap(view.Own)
if own["approvals.settings.preventApprovalByMergeRequestCreator"] != "true" ||
own["approvals.settings.preventApprovalsByUsersWhoAddCommits"] != "" ||
own["approvals.rules.count"] != "1" || own["approvals.rules.0.usernames"] != "a, b" {
t.Errorf("own approval keys wrong: %v", own)
}

// Clearing settings + rules removes the approvals block; the MR itself stays.
if _, err := a.SetAssignment(ctx, "tc", "blatt1", map[string]string{
"mergeRequest.mergeMethod": "merge",
"approvals.settings.preventApprovalByMergeRequestCreator": "",
"approvals.settings.requireUserReauthenticationToApprove": "",
"approvals.settings.whenCommitAdded": "",
"approvals.rules.count": "0",
}); err != nil {
t.Fatalf("set (clear): %v", err)
}
mr2 := fs.saved.Source.Assignments["blatt1"].MergeRequest
if mr2 == nil || mr2.Approvals != nil {
t.Errorf("approvals should be cleared while the MR is kept: %+v", mr2)
}
}

func TestSetAssignment_rejectsUnresolvable(t *testing.T) {
const owner = "prof@hm.edu"
fs := newFakeStore()
Expand Down
4 changes: 4 additions & 0 deletions web/graph/assignments.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ extend type Query {
assignmentSchema: [FieldMeta!]!
"Field metadata for one branch rule — a row of the repeat-group `branches` list."
branchRuleSchema: [FieldMeta!]!
"Field metadata for the mergeRequest approval settings (tri-state; empty ENUM option means unset)."
approvalSettingsSchema: [FieldMeta!]!
"Field metadata for one approval rule — a row of the repeat-group `approvals.rules` list."
approvalRuleSchema: [FieldMeta!]!
"One assignment of one of the caller's courses: source values, resolved preview. Null when there is no such assignment."
assignment(course: String!, name: String!): AssignmentView
"Validate a draft assignment against the real resolver without saving."
Expand Down
10 changes: 10 additions & 0 deletions web/graph/assignments.resolvers.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading