dmitri.shuralyov.com/service/change/...

gerritapi: Handle comments with multiple labels.

Previously, the review state wasn't correctly parsed when there
was more than 1 label (e.g., "Run-TryBot+1 Code-Review+2").

Example of an affected review is https://go-review.googlesource.com/c/go/+/112935#message-45e0d04e5f087880438b34c77b6b683f0beffaa3.
dmitshur committed 3 months ago commit edf95a0d92762102d986315f7a564b09d12e5637
gerritapi/gerritapi.go
@@ -265,23 +265,14 @@ func (s service) ListTimeline(ctx context.Context, _ string, id uint64, opt *cha
 					},
 				})
 			}
 			continue
 		}
-		label, body, ok := parseMessage(message.Message)
+		labels, body, ok := parseMessage(message.Message)
 		if !ok {
 			continue
 		}
-		var state change.ReviewState
-		switch label {
-		default:
-			state = change.Commented
-		case "Code-Review+2":
-			state = change.Approved
-		case "Code-Review-2":
-			state = change.ChangesRequested
-		}
 		var cs []change.InlineComment
 		for file, comments := range *comments {
 			for _, c := range comments {
 				if c.Updated.Equal(message.Date.Time) {
 					cs = append(cs, change.InlineComment{
@@ -294,20 +285,20 @@ func (s service) ListTimeline(ctx context.Context, _ string, id uint64, opt *cha
 		}
 		timeline = append(timeline, change.Review{
 			ID:        fmt.Sprint(idx), // TODO: message.ID is not uint64; e.g., "bfba753d015916303152305cee7152ea7a112fe0".
 			User:      s.gerritUser(message.Author),
 			CreatedAt: message.Date.Time,
-			State:     state,
+			State:     reviewState(labels),
 			Body:      body,
 			Editable:  false,
 			Comments:  cs,
 		})
 	}
 	return timeline, nil
 }
 
-func parseMessage(m string) (label string, body string, ok bool) {
+func parseMessage(m string) (labels string, body string, ok bool) {
 	// "Patch Set ".
 	if !strings.HasPrefix(m, "Patch Set ") {
 		return "", "", false
 	}
 	m = m[len("Patch Set "):]
@@ -325,33 +316,45 @@ func parseMessage(m string) (label string, body string, ok bool) {
 	}
 	m = m[1:]
 
 	switch i = strings.IndexByte(m, '\n'); i {
 	case -1:
-		label = m
+		labels = m
 	default:
-		label = m[:i]
+		labels = m[:i]
 		body = m[i+1:]
 	}
 
-	if label != "" {
+	if labels != "" {
 		// " ".
-		if len(label) < 1 || label[0] != ' ' {
+		if len(labels) < 1 || labels[0] != ' ' {
 			return "", "", false
 		}
-		label = label[1:]
+		labels = labels[1:]
 	}
 
 	if body != "" {
 		// "\n".
 		if len(body) < 1 || body[0] != '\n' {
 			return "", "", false
 		}
 		body = body[1:]
 	}
 
-	return label, body, true
+	return labels, body, true
+}
+
+func reviewState(labels string) change.ReviewState {
+	for _, label := range strings.Split(labels, " ") {
+		switch label {
+		case "Code-Review+2":
+			return change.Approved
+		case "Code-Review-2":
+			return change.ChangesRequested
+		}
+	}
+	return change.Commented
 }
 
 func (service) EditComment(_ context.Context, repo string, id uint64, cr change.CommentRequest) (change.Comment, error) {
 	return change.Comment{}, fmt.Errorf("EditComment: not implemented")
 }
gerritapi/gerritapi_test.go
@@ -4,37 +4,42 @@ import (
 	"testing"
 )
 
 func TestParseMessage(t *testing.T) {
 	tests := []struct {
-		in        string
-		wantLabel string
-		wantBody  string
+		in         string
+		wantLabels string
+		wantBody   string
 	}{
 		{
-			in:        "Patch Set 2: Code-Review+2",
-			wantLabel: "Code-Review+2",
-			wantBody:  "",
+			in:         "Patch Set 2: Code-Review+2",
+			wantLabels: "Code-Review+2",
+			wantBody:   "",
 		},
 		{
-			in:        "Patch Set 2: Code-Review+2\n\nThanks.",
-			wantLabel: "Code-Review+2",
-			wantBody:  "Thanks.",
+			in:         "Patch Set 3: Run-TryBot+1 Code-Review+2",
+			wantLabels: "Run-TryBot+1 Code-Review+2",
+			wantBody:   "",
 		},
 		{
-			in:        "Patch Set 1:\n\nFirst contribution — trying to get my feet wet. Please review.",
-			wantLabel: "",
-			wantBody:  "First contribution — trying to get my feet wet. Please review.",
+			in:         "Patch Set 2: Code-Review+2\n\nThanks.",
+			wantLabels: "Code-Review+2",
+			wantBody:   "Thanks.",
+		},
+		{
+			in:         "Patch Set 1:\n\nFirst contribution — trying to get my feet wet. Please review.",
+			wantLabels: "",
+			wantBody:   "First contribution — trying to get my feet wet. Please review.",
 		},
 	}
 	for i, tc := range tests {
-		gotLabel, gotBody, ok := parseMessage(tc.in)
+		gotLabels, gotBody, ok := parseMessage(tc.in)
 		if !ok {
 			t.Fatalf("%d: not ok", i)
 		}
-		if gotLabel != tc.wantLabel {
-			t.Errorf("%d: got label: %q, want: %q", i, gotLabel, tc.wantLabel)
+		if gotLabels != tc.wantLabels {
+			t.Errorf("%d: got labels: %q, want: %q", i, gotLabels, tc.wantLabels)
 		}
 		if gotBody != tc.wantBody {
 			t.Errorf("%d: got body: %q, want: %q", i, gotBody, tc.wantBody)
 		}
 	}