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 5 years ago commit edf95a0d92762102d986315f7a564b09d12e5637
Collapse all
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)
		}
	}