Merge pull request #131 from matrix-org/kegan/finer-gh-controls

Break apart 'issues' and 'pull_request' events

src/github.com/matrix-org/go-neb/services/github/github_webhook.go

@@ -36,7 +36,7 @@ const WebhookServiceType = "github-webhook"
 //           "!qmElAGdFYCHoCJuaNt:localhost": {
 //               Repos: {
 //                   "matrix-org/go-neb": {
-//                       Events: ["push", "issues", "pull_request"]
+//                       Events: ["push", "issues", "pull_request", "labels"]
 //                   }
 //               }
 //           }
@@ -57,10 +57,13 @@ type WebhookService struct {
 			// The webhook events to listen for. Currently supported:
 			//    push : When users push to this repository.
 			//    pull_request : When a pull request is made to this repository.
-			//    issues : When an issue is opened/closed.
+			//    issues : When an issue is opened/edited/closed/reopened.
 			//    issue_comment : When an issue or pull request is commented on.
 			//    pull_request_review_comment : When a line comment is made on a pull request.
-			// Full list: https://developer.github.com/webhooks/#events
+			//    labels : When any issue or pull request is labeled/unlabeled. Unique to Go-NEB.
+			//    milestones : When any issue or pull request is milestoned/demilestoned. Unique to Go-NEB.
+			//    assignments : When any issue or pull request is assigned/unassigned. Unique to Go-NEB.
+			// Most of these events are directly from: https://developer.github.com/webhooks/#events
 			Events []string
 		}
 	}

src/github.com/matrix-org/go-neb/services/github/webhook/webhook.go

@@ -61,14 +61,15 @@ func OnReceiveRequest(r *http.Request, secretToken string) (string, *github.Repo
 		return "", nil, nil, &errors.HTTPError{nil, "pong", 200}
 	}
 
-	htmlStr, repo, err := parseGithubEvent(eventType, content)
+	htmlStr, repo, refinedType, err := parseGithubEvent(eventType, content)
 	if err != nil {
 		log.WithError(err).Print("Failed to parse github event")
 		return "", nil, nil, &errors.HTTPError{nil, "Failed to parse github event", 500}
 	}
 
 	msg := matrix.GetHTMLMessage("m.notice", htmlStr)
-	return eventType, repo, &msg, nil
+
+	return refinedType, repo, &msg, nil
 }
 
 // checkMAC reports whether messageMAC is a valid HMAC tag for message.
@@ -80,24 +81,26 @@ func checkMAC(message, messageMAC, key []byte) bool {
 }
 
 // parseGithubEvent parses a github event type and JSON data and returns an explanatory
-// HTML string and the github repository this event affects, or an error.
-func parseGithubEvent(eventType string, data []byte) (string, *github.Repository, error) {
+// HTML string, the github repository and the refined event type, or an error.
+func parseGithubEvent(eventType string, data []byte) (string, *github.Repository, string, error) {
 	if eventType == "pull_request" {
 		var ev github.PullRequestEvent
 		if err := json.Unmarshal(data, &ev); err != nil {
-			return "", nil, err
+			return "", nil, eventType, err
 		}
-		return pullRequestHTMLMessage(ev), ev.Repo, nil
+		refinedEventType := refineEventType(eventType, ev.Action)
+		return pullRequestHTMLMessage(ev), ev.Repo, refinedEventType, nil
 	} else if eventType == "issues" {
 		var ev github.IssuesEvent
 		if err := json.Unmarshal(data, &ev); err != nil {
-			return "", nil, err
+			return "", nil, eventType, err
 		}
-		return issueHTMLMessage(ev), ev.Repo, nil
+		refinedEventType := refineEventType(eventType, ev.Action)
+		return issueHTMLMessage(ev), ev.Repo, refinedEventType, nil
 	} else if eventType == "push" {
 		var ev github.PushEvent
 		if err := json.Unmarshal(data, &ev); err != nil {
-			return "", nil, err
+			return "", nil, eventType, err
 		}
 
 		// The 'push' event repository format is subtly different from normal, so munge the bits we need.
@@ -109,21 +112,36 @@ func parseGithubEvent(eventType string, data []byte) (string, *github.Repository
 			Name:     ev.Repo.Name,
 			FullName: &fullName,
 		}
-		return pushHTMLMessage(ev), &repo, nil
+		return pushHTMLMessage(ev), &repo, eventType, nil
 	} else if eventType == "issue_comment" {
 		var ev github.IssueCommentEvent
 		if err := json.Unmarshal(data, &ev); err != nil {
-			return "", nil, err
+			return "", nil, eventType, err
 		}
-		return issueCommentHTMLMessage(ev), ev.Repo, nil
+		return issueCommentHTMLMessage(ev), ev.Repo, eventType, nil
 	} else if eventType == "pull_request_review_comment" {
 		var ev github.PullRequestReviewCommentEvent
 		if err := json.Unmarshal(data, &ev); err != nil {
-			return "", nil, err
+			return "", nil, eventType, err
+		}
+		return prReviewCommentHTMLMessage(ev), ev.Repo, eventType, nil
+	}
+	return "", nil, eventType, fmt.Errorf("Unrecognized event type")
+}
+
+func refineEventType(eventType string, action *string) string {
+	if action == nil {
+		return eventType
 	}
-		return prReviewCommentHTMLMessage(ev), ev.Repo, nil
+	a := *action
+	if a == "assigned" || a == "unassigned" {
+		return "assignments"
+	} else if a == "milestoned" || a == "demilestoned" {
+		return "milestones"
+	} else if a == "labeled" || a == "unlabeled" {
+		return "labels"
 	}
-	return "", nil, fmt.Errorf("Unrecognized event type")
+	return eventType
 }
 
 func pullRequestHTMLMessage(p github.PullRequestEvent) string {

src/github.com/matrix-org/go-neb/services/github/webhook/webhook_test.go

@@ -10,6 +10,7 @@ var ghtests = []struct {
 	jsonBody    string
 	outHTML     string
 	outFullRepo string
+	outType     string
 }{
 	{"issues",
 		`{
@@ -165,7 +166,7 @@ var ghtests = []struct {
 		  }
 		}`,
 		`[<u>DummyAccount/reponame</u>] DummyAccount closed <b>issue #15</b>: aaaaaa [closed] - https://github.com/DummyAccount/reponame/issues/15`,
-		"DummyAccount/reponame"},
+		"DummyAccount/reponame", "issues"},
 	// ==================================================================
 	{
 		"issue_comment",
@@ -350,7 +351,7 @@ var ghtests = []struct {
 		  }
 		}`,
 		"[<u>DummyAccount/arepo</u>] DummyAccount commented on DummyAccount's <b>issue #15</b>: aaaaaa - https://github.com/DummyAccount/arepo/issues/15",
-		"DummyAccount/arepo",
+		"DummyAccount/arepo", "issue_comment",
 	},
 	// ==================================================================
 	{
@@ -561,7 +562,7 @@ var ghtests = []struct {
 		  }
 		}`,
 		"[<u>matrix-org/sytest</u>] NegativeMjark pushed 2 commits to <b>develop</b>: https://github.com/matrix-org/sytest/commit/4a05c601f6b806110e63160cf7cf41b37787461f<br>NegativeMjark: Fix arguments to postgres connector to work with go<br>NegativeMjark: Add necessary info to the second postgres db",
-		"matrix-org/sytest",
+		"matrix-org/sytest", "push",
 	},
 	// ==================================================================
 	{
@@ -1032,7 +1033,7 @@ var ghtests = []struct {
 		  }
 		}`,
 		"[<u>matrix-org/matrix-react-sdk</u>] richvdh assigned <b>pull request #303</b>: Factor out common parts of room creation [open] to dbkr - https://github.com/matrix-org/matrix-react-sdk/pull/303",
-		"matrix-org/matrix-react-sdk",
+		"matrix-org/matrix-react-sdk", "assignments",
 	},
 	// ==================================================================
 	{
@@ -1500,25 +1501,28 @@ var ghtests = []struct {
 		  }
 		}`,
 		"[<u>matrix-org/synapse</u>] erikjohnston made a line comment on negzi's <b>pull request #860</b> (assignee: None): Fix a bug caused by a change in auth_handler function - https://github.com/matrix-org/synapse/pull/860#discussion_r66413356",
-		"matrix-org/synapse",
+		"matrix-org/synapse", "pull_request_review_comment",
 	},
 }
 
 func TestParseGithubEvent(t *testing.T) {
 	for _, gh := range ghtests {
-		outHTML, outRepo, outErr := parseGithubEvent(gh.eventType, []byte(gh.jsonBody))
+		outHTML, outRepo, outType, outErr := parseGithubEvent(gh.eventType, []byte(gh.jsonBody))
 		if outErr != nil {
 			t.Fatal(outErr)
 		}
 		if strings.TrimSpace(outHTML) != strings.TrimSpace(gh.outHTML) {
-			t.Fatalf("ParseGithubEvent(%s) => HTML output does not match. Got:\n%s\n\nExpected:\n%s", gh.eventType,
+			t.Errorf("ParseGithubEvent(%s) => HTML output does not match. Got:\n%s\n\nExpected:\n%s", gh.eventType,
 				strings.TrimSpace(outHTML), strings.TrimSpace(gh.outHTML))
 		}
 		if outRepo == nil {
-			t.Fatalf("ParseGithubEvent(%s) => Repo is nil", gh.eventType)
+			t.Errorf("ParseGithubEvent(%s) => Repo is nil", gh.eventType)
 		}
 		if *outRepo.FullName != gh.outFullRepo {
-			t.Fatalf("ParseGithubEvent(%s) => Repo: Want %s got %s", gh.eventType, gh.outFullRepo, *outRepo.FullName)
+			t.Errorf("ParseGithubEvent(%s) => Repo: Want %s got %s", gh.eventType, gh.outFullRepo, *outRepo.FullName)
+		}
+		if outType != gh.outType {
+			t.Errorf("ParseGithubEvent(%s) => Event type: Want %s got %s", gh.eventType, gh.outType, outType)
 		}
 	}
 }