Repository: mattermost/mattermost-plugin-github
Pull request #321 (2020-07-07 18:08:44)

#GH-272/enable comment unread message to reroute to comment url
<!-- Thank you for contributing a pull request! Here are a few tips to help you:

1. If this is your first contribution, make sure you've read the Contribution Checklist https://developers.mattermost.com/contribute/getting-started/contribution-checklist/
2. Read our blog post about "Submitting Great PRs" https://developers.mattermost.com/blog/2019-01-24-submitting-great-prs
3. Take a look at other repository specific documentation at https://developers.mattermost.com/contribute
-->

#### Summary
This pull request modifies the URL that reroutes comment unread messages in order for the unread message link in the RHS to send you specifically to the comment.

#### Ticket Link
https://github.com/mattermost/mattermost-plugin-github/issues/272



codecov-commenter: # [Codecov](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/321?src=pr&el=h1) Report
> Merging [#321](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/321?src=pr&el=desc) into [master](https://codecov.io/gh/mattermost/mattermost-plugin-github/commit/63ef1c07e952d881d9bab72d2edbda016acd1425&el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `20.00%`.

[![Impacted file tree graph](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/321/graphs/tree.svg?width=650&height=150&src=pr&token=0uHZTjTrgl)](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/321?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #321      +/-   ##
==========================================
- Coverage   18.69%   18.69%   -0.01%     
==========================================
  Files          10       10              
  Lines        2455     2461       +6     
==========================================
+ Hits          459      460       +1     
- Misses       1971     1976       +5     
  Partials       25       25              
```


| [Impacted Files](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/321?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [server/api.go](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/321/diff?src=pr&el=tree#diff-c2VydmVyL2FwaS5nbw==) | `5.58% <0.00%> (-0.04%)` | :arrow_down: |
| [server/plugin.go](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/321/diff?src=pr&el=tree#diff-c2VydmVyL3BsdWdpbi5nbw==) | `4.27% <0.00%> (ø)` | |
| [server/utils.go](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/321/diff?src=pr&el=tree#diff-c2VydmVyL3V0aWxzLmdv) | `61.53% <100.00%> (+0.21%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/321?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/321?src=pr&el=footer). Last update [63ef1c0...27fc369](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/321?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/server/plugin/api.go b/server/plugin/api.go
index 815772b..2dcf830 100644
--- a/server/plugin/api.go
+++ b/server/plugin/api.go
@@ -21,7 +21,7 @@ import (
 
 const (
 	apiErrorIDNotConnected = "not_connected"
-	// the OAuth token expiry in seconds
+	// TokenTTL is the OAuth token expiry duration in seconds
 	TokenTTL = 10 * 60
 )
 
@@ -50,8 +50,19 @@ type PRDetails struct {
 	Reviews            []*github.PullRequestReview `json:"reviews"`
 }
 
+// HTTPHandlerFuncWithUser is http.HandleFunc but userID is already exported
 type HTTPHandlerFuncWithUser func(w http.ResponseWriter, r *http.Request, userID string)
 
+// ResponseType indicates type of response returned by api
+type ResponseType string
+
+const (
+	// ResponseTypeJSON indicates that response type is json
+	ResponseTypeJSON ResponseType = "JSON_RESPONSE"
+	// ResponseTypePlain indicates that response type is text plain
+	ResponseTypePlain ResponseType = "TEXT_RESPONSE"
+)
+
 func (p *Plugin) writeJSON(w http.ResponseWriter, v interface{}) {
 	b, err := json.Marshal(v)
 	if err != nil {
@@ -93,38 +104,41 @@ func (p *Plugin) initializeAPI() {
 
 	p.router.HandleFunc("/webhook", p.handleWebhook).Methods(http.MethodPost)
 
-	oauthRouter.HandleFunc("/connect", p.extractUserMiddleWare(p.connectUserToGitHub, false)).Methods(http.MethodGet)
-	oauthRouter.HandleFunc("/complete", p.extractUserMiddleWare(p.completeConnectUserToGitHub, false)).Methods(http.MethodGet)
+	oauthRouter.HandleFunc("/connect", p.extractUserMiddleWare(p.connectUserToGitHub, ResponseTypePlain)).Methods(http.MethodGet)
+	oauthRouter.HandleFunc("/complete", p.extractUserMiddleWare(p.completeConnectUserToGitHub, ResponseTypePlain)).Methods(http.MethodGet)
 
 	apiRouter.HandleFunc("/connected", p.getConnected).Methods(http.MethodGet)
-	apiRouter.HandleFunc("/todo", p.extractUserMiddleWare(p.postToDo, true)).Methods(http.MethodPost)
-	apiRouter.HandleFunc("/reviews", p.extractUserMiddleWare(p.getReviews, false)).Methods(http.MethodGet)
-	apiRouter.HandleFunc("/yourprs", p.extractUserMiddleWare(p.getYourPrs, false)).Methods(http.MethodGet)
-	apiRouter.HandleFunc("/prsdetails", p.extractUserMiddleWare(p.getPrsDetails, false)).Methods(http.MethodPost)
-	apiRouter.HandleFunc("/searchissues", p.extractUserMiddleWare(p.searchIssues, false)).Methods(http.MethodGet)
-	apiRouter.HandleFunc("/yourassignments", p.extractUserMiddleWare(p.getYourAssignments, false)).Methods(http.MethodGet)
-	apiRouter.HandleFunc("/createissue", p.extractUserMiddleWare(p.createIssue, false)).Methods(http.MethodPost)
-	apiRouter.HandleFunc("/createissuecomment", p.extractUserMiddleWare(p.createIssueComment, false)).Methods(http.MethodPost)
-	apiRouter.HandleFunc("/mentions", p.extractUserMiddleWare(p.getMentions, false)).Methods(http.MethodGet)
-	apiRouter.HandleFunc("/unreads", p.extractUserMiddleWare(p.getUnreads, false)).Methods(http.MethodGet)
-	apiRouter.HandleFunc("/repositories", p.extractUserMiddleWare(p.getRepositories, false)).Methods(http.MethodGet)
-	apiRouter.HandleFunc("/settings", p.extractUserMiddleWare(p.updateSettings, false)).Methods(http.MethodPost)
-	apiRouter.HandleFunc("/user", p.extractUserMiddleWare(p.getGitHubUser, true)).Methods(http.MethodPost)
-	apiRouter.HandleFunc("/issue", p.extractUserMiddleWare(p.getIssueByNumber, false)).Methods(http.MethodGet)
-	apiRouter.HandleFunc("/pr", p.extractUserMiddleWare(p.getPrByNumber, false)).Methods(http.MethodGet)
+	apiRouter.HandleFunc("/todo", p.extractUserMiddleWare(p.postToDo, ResponseTypeJSON)).Methods(http.MethodPost)
+	apiRouter.HandleFunc("/reviews", p.extractUserMiddleWare(p.getReviews, ResponseTypePlain)).Methods(http.MethodGet)
+	apiRouter.HandleFunc("/yourprs", p.extractUserMiddleWare(p.getYourPrs, ResponseTypePlain)).Methods(http.MethodGet)
+	apiRouter.HandleFunc("/prsdetails", p.extractUserMiddleWare(p.getPrsDetails, ResponseTypePlain)).Methods(http.MethodPost)
+	apiRouter.HandleFunc("/searchissues", p.extractUserMiddleWare(p.searchIssues, ResponseTypePlain)).Methods(http.MethodGet)
+	apiRouter.HandleFunc("/yourassignments", p.extractUserMiddleWare(p.getYourAssignments, ResponseTypePlain)).Methods(http.MethodGet)
+	apiRouter.HandleFunc("/createissue", p.extractUserMiddleWare(p.createIssue, ResponseTypePlain)).Methods(http.MethodPost)
+	apiRouter.HandleFunc("/createissuecomment", p.extractUserMiddleWare(p.createIssueComment, ResponseTypePlain)).Methods(http.MethodPost)
+	apiRouter.HandleFunc("/mentions", p.extractUserMiddleWare(p.getMentions, ResponseTypePlain)).Methods(http.MethodGet)
+	apiRouter.HandleFunc("/unreads", p.extractUserMiddleWare(p.getUnreads, ResponseTypePlain)).Methods(http.MethodGet)
+	apiRouter.HandleFunc("/repositories", p.extractUserMiddleWare(p.getRepositories, ResponseTypePlain)).Methods(http.MethodGet)
+	apiRouter.HandleFunc("/settings", p.extractUserMiddleWare(p.updateSettings, ResponseTypePlain)).Methods(http.MethodPost)
+	apiRouter.HandleFunc("/user", p.extractUserMiddleWare(p.getGitHubUser, ResponseTypeJSON)).Methods(http.MethodPost)
+	apiRouter.HandleFunc("/issue", p.extractUserMiddleWare(p.getIssueByNumber, ResponseTypePlain)).Methods(http.MethodGet)
+	apiRouter.HandleFunc("/pr", p.extractUserMiddleWare(p.getPrByNumber, ResponseTypePlain)).Methods(http.MethodGet)
 
 	apiRouter.HandleFunc("/config", checkPluginRequest(p.getConfig)).Methods(http.MethodGet)
 	apiRouter.HandleFunc("/token", checkPluginRequest(p.getToken)).Methods(http.MethodGet)
 }
 
-func (p *Plugin) extractUserMiddleWare(handler HTTPHandlerFuncWithUser, jsonResponse bool) http.HandlerFunc {
+func (p *Plugin) extractUserMiddleWare(handler HTTPHandlerFuncWithUser, responseType ResponseType) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		userID := r.Header.Get("Mattermost-User-ID")
 		if userID == "" {
-			if jsonResponse {
+			switch responseType {
+			case ResponseTypeJSON:
 				p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "Not authorized.", StatusCode: http.StatusUnauthorized})
-			} else {
+			case ResponseTypePlain:
 				http.Error(w, "Not authorized", http.StatusUnauthorized)
+			default:
+				p.API.LogError("Unknown ResponseType detected")
 			}
 			return
 		}
@@ -506,9 +520,17 @@ func (p *Plugin) getUnreads(w http.ResponseWriter, r *http.Request, userID strin
 			continue
 		}
 
+		issueURL := n.GetSubject().GetURL()
+		issueNumIndex := strings.LastIndex(issueURL, "/")
+		issueNum := issueURL[issueNumIndex+1:]
+		subjectURL := n.GetSubject().GetURL()
+		if n.GetSubject().GetLatestCommentURL() != "" {
+			subjectURL = n.GetSubject().GetLatestCommentURL()
+		}
+
 		filteredNotifications = append(filteredNotifications, &filteredNotification{
 			Notification: *n,
-			HTMLUrl:      fixGithubNotificationSubjectURL(n.GetSubject().GetURL()),
+			HTMLUrl:      fixGithubNotificationSubjectURL(subjectURL, issueNum),
 		})
 	}
 
@@ -924,7 +946,7 @@ func (p *Plugin) getIssueByNumber(w http.ResponseWriter, r *http.Request, userID
 		p.writeAPIError(w, &APIErrorResponse{Message: "Could not get issue", StatusCode: http.StatusInternalServerError})
 		return
 	}
-
+	*result.Body = mdCommentRegex.ReplaceAllString(result.GetBody(), "")
 	p.writeJSON(w, result)
 }
 
@@ -961,7 +983,7 @@ func (p *Plugin) getPrByNumber(w http.ResponseWriter, r *http.Request, userID st
 		p.writeAPIError(w, &APIErrorResponse{Message: "Could not get pull request", StatusCode: http.StatusInternalServerError})
 		return
 	}
-
+	*result.Body = mdCommentRegex.ReplaceAllString(result.GetBody(), "")
 	p.writeJSON(w, result)
 }
 
diff --git a/server/plugin/command.go b/server/plugin/command.go
index 72fca8e..5d1852f 100644
--- a/server/plugin/command.go
+++ b/server/plugin/command.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"strings"
+	"unicode"
 
 	"github.com/google/go-github/v31/github"
 	"github.com/mattermost/mattermost-server/v5/mlog"
@@ -155,7 +156,16 @@ func (p *Plugin) handleSubscribe(_ *plugin.Context, args *model.CommandArgs, par
 		return err.Error()
 	}
 
-	return fmt.Sprintf("Successfully subscribed to %s.", repo)
+	msg := fmt.Sprintf("Successfully subscribed to %s.", repo)
+
+	ghRepo, _, err := githubClient.Repositories.Get(ctx, owner, repo)
+	if err != nil {
+		p.API.LogWarn("Failed to fetch repository", "error", err.Error())
+	} else if ghRepo != nil && ghRepo.GetPrivate() {
+		msg += "\n\n**Warning:** You subscribed to a private repository. Anyone with access to this channel will be able to read the events getting posted here."
+	}
+
+	return msg
 }
 
 func (p *Plugin) handleUnsubscribe(_ *plugin.Context, args *model.CommandArgs, parameters []string, _ *GitHubUserInfo) string {
@@ -258,16 +268,7 @@ func (p *Plugin) handleSettings(_ *plugin.Context, _ *model.CommandArgs, paramet
 type CommandHandleFunc func(c *plugin.Context, args *model.CommandArgs, parameters []string, userInfo *GitHubUserInfo) string
 
 func (p *Plugin) ExecuteCommand(c *plugin.Context, args *model.CommandArgs) (*model.CommandResponse, *model.AppError) {
-	split := strings.Fields(args.Command)
-	command := split[0]
-	var parameters []string
-	action := ""
-	if len(split) > 1 {
-		action = split[1]
-	}
-	if len(split) > 2 {
-		parameters = split[2:]
-	}
+	command, action, parameters := parseCommand(args.Command)
 
 	if command != "/github" {
 		return &model.CommandResponse{}, nil
@@ -323,3 +324,54 @@ func (p *Plugin) ExecuteCommand(c *plugin.Context, args *model.CommandArgs) (*mo
 	p.postCommandResponse(args, fmt.Sprintf("Unknown action %v", action))
 	return &model.CommandResponse{}, nil
 }
+
+// parseCommand parses the entire command input string and retrieves the command, action and parameters
+func parseCommand(input string) (command, action string, parameters []string) {
+	split := make([]string, 0)
+	current := ""
+	inQuotes := false
+
+	for _, char := range input {
+		if unicode.IsSpace(char) {
+			// keep whitespaces that are inside double qoutes
+			if inQuotes {
+				current += " "
+				continue
+			}
+
+			// ignore successive whitespaces that are outside of double quotes
+			if len(current) == 0 && !inQuotes {
+				continue
+			}
+
+			// append the current word to the list & move on to the next word/expression
+			split = append(split, current)
+			current = ""
+			continue
+		}
+
+		// append the current character to the current word
+		current += string(char)
+
+		if char == '"' {
+			inQuotes = !inQuotes
+		}
+	}
+
+	// append the last word/expression to the list
+	if len(current) > 0 {
+		split = append(split, current)
+	}
+
+	command = split[0]
+
+	if len(split) > 1 {
+		action = split[1]
+	}
+
+	if len(split) > 2 {
+		parameters = split[2:]
+	}
+
+	return command, action, parameters
+}
diff --git a/server/plugin/plugin.go b/server/plugin/plugin.go
index 85c6ab1..1b5c078 100644
--- a/server/plugin/plugin.go
+++ b/server/plugin/plugin.go
@@ -422,11 +422,19 @@ func (p *Plugin) GetToDo(ctx context.Context, username string, githubClient *git
 		notificationType := notificationSubject.GetType()
 		switch notificationType {
 		case "RepositoryVulnerabilityAlert":
-			message := fmt.Sprintf("[Vulnerability Alert for %v](%v)", n.GetRepository().GetFullName(), fixGithubNotificationSubjectURL(n.GetSubject().GetURL()))
+			message := fmt.Sprintf("[Vulnerability Alert for %v](%v)", n.GetRepository().GetFullName(), fixGithubNotificationSubjectURL(n.GetSubject().GetURL(), ""))
 			notificationContent += fmt.Sprintf("* %v\n", message)
 		default:
+			issueURL := n.GetSubject().GetURL()
+			issueNumIndex := strings.LastIndex(issueURL, "/")
+			issueNum := issueURL[issueNumIndex+1:]
+			subjectURL := n.GetSubject().GetURL()
+			if n.GetSubject().GetLatestCommentURL() != "" {
+				subjectURL = n.GetSubject().GetLatestCommentURL()
+			}
+
 			notificationTitle := notificationSubject.GetTitle()
-			notificationURL := fixGithubNotificationSubjectURL(notificationSubject.GetURL())
+			notificationURL := fixGithubNotificationSubjectURL(subjectURL, issueNum)
 			notificationContent += getToDoDisplayText(baseURL, notificationTitle, notificationURL, notificationType)
 		}
 
diff --git a/server/plugin/template.go b/server/plugin/template.go
index ddb549c..50cdb34 100644
--- a/server/plugin/template.go
+++ b/server/plugin/template.go
@@ -2,6 +2,7 @@ package plugin
 
 import (
 	"bytes"
+	"net/url"
 	"regexp"
 	"strings"
 	"text/template"
@@ -81,6 +82,25 @@ func init() {
 		return ">" + strings.ReplaceAll(body, "\n", "\n>")
 	}
 
+	// Escape characters not allowed in URL path
+	funcMap["pathEscape"] = url.PathEscape
+
+	// Transform multiple variables to dictionary
+	funcMap["dict"] = func(values ...interface{}) (map[string]interface{}, error) {
+		if len(values)%2 != 0 {
+			return nil, errors.New("invalid dict call, exactly one value is required for every key")
+		}
+		dict := make(map[string]interface{}, len(values)/2)
+		for i := 0; i < len(values); i += 2 {
+			key, ok := values[i].(string)
+			if !ok {
+				return nil, errors.New("dict keys must be strings")
+			}
+			dict[key] = values[i+1]
+		}
+		return dict, nil
+	}
+
 	masterTemplate = template.Must(template.New("master").Funcs(funcMap).Parse(""))
 
 	// The user template links to the corresponding GitHub user. If the GitHub user is a known
@@ -149,10 +169,24 @@ func init() {
 		`{{template "eventRepoIssueFullLink" .}} - {{.GetIssue.GetTitle}}`,
 	))
 
+	template.Must(masterTemplate.New("labels").Funcs(funcMap).Parse(`
+{{- if .Labels }}
+Labels: {{range $i, $el := .Labels -}}` + "{{- if $i}}, {{end}}[`{{ $el.Name }}`]({{ $.RepositoryURL }}/labels/{{ $el.Name | pathEscape }})" + `{{end -}}
+{{ end -}}
+`))
+
+	template.Must(masterTemplate.New("assignee").Funcs(funcMap).Parse(`
+{{- if .Assignees }}
+Assignees: {{range $i, $el := .Assignees -}} {{- if $i}}, {{end}}{{template "user" $el}}{{end -}}
+{{- end -}}
+`))
+
 	template.Must(masterTemplate.New("newPR").Funcs(funcMap).Parse(`
 #### {{.GetPullRequest.GetTitle}}
 ##### {{template "eventRepoPullRequest" .}}
 #new-pull-request by {{template "user" .GetSender}}
+{{- template "labels" dict "Labels" .GetPullRequest.Labels "RepositoryURL" .GetRepo.GetHTMLURL  }}
+{{- template "assignee" .GetPullRequest }}
 
 {{.GetPullRequest.GetBody | removeComments | replaceAllGitHubUsernames}}
 `))
@@ -178,6 +212,8 @@ func init() {
 #### {{.GetIssue.GetTitle}}
 ##### {{template "eventRepoIssue" .}}
 #new-issue by {{template "user" .GetSender}}
+{{- template "labels" dict "Labels" .GetIssue.Labels "RepositoryURL" .GetRepo.GetHTMLURL  }}
+{{- template "assignee" .GetIssue }}
 
 {{.GetIssue.GetBody | removeComments | replaceAllGitHubUsernames}}
 `))
@@ -296,7 +332,8 @@ func init() {
 		"    * `deletes` - includes branch and tag deletions\n" +
 		"    * `issue_comments` - includes new issue comments\n" +
 		"    * `pull_reviews` - includes pull request reviews\n" +
-		"    * `label:<labelname>` - must include `pulls` or `issues` in feature list when using a label. Defaults to `pulls,issues,creates,deletes`\n" +
+		"    * `label:<labelname>` - limit pull request and issue events to only this label. Must include `pulls` or `issues` in feature list when using a label.\n" +
+		"    * Defaults to `pulls,issues,creates,deletes`\n" +
 		"  * `flags` currently supported:\n" +
 		"    * `--exclude-org-member` - events triggered by organization members will not be delivered (the GitHub organization config should be set, otherwise this flag has not effect)\n" +
 		"* `/github unsubscribe owner/repo` - Unsubscribe the current channel from a repository\n" +
diff --git a/server/plugin/utils.go b/server/plugin/utils.go
index ca40843..5ed0b47 100644
--- a/server/plugin/utils.go
+++ b/server/plugin/utils.go
@@ -160,12 +160,13 @@ func parseGitHubUsernamesFromText(text string) []string {
 	return usernames
 }
 
-func fixGithubNotificationSubjectURL(url string) string {
+func fixGithubNotificationSubjectURL(url, issueNum string) string {
 	url = strings.Replace(url, "api.", "", 1)
 	url = strings.Replace(url, "repos/", "", 1)
 	url = strings.Replace(url, "/pulls/", "/pull/", 1)
 	url = strings.Replace(url, "/commits/", "/commit/", 1)
 	url = strings.Replace(url, "/api/v3", "", 1)
+	url = strings.Replace(url, "comments/", issueNum+"#issuecomment-", 1)
 	return url
 }
 
diff --git a/webapp/src/components/link_tooltip/tooltip.css b/webapp/src/components/link_tooltip/tooltip.css
index 81e10c9..ba6abe2 100644
--- a/webapp/src/components/link_tooltip/tooltip.css
+++ b/webapp/src/components/link_tooltip/tooltip.css
@@ -72,6 +72,7 @@
     max-height: 150px;
     line-height: 1.25;
     overflow: hidden;
+    word-break: break-all;
     font-size: 12px;
 }
 


Test patch:
diff --git a/server/plugin/command_test.go b/server/plugin/command_test.go
index d4791e7..8130833 100644
--- a/server/plugin/command_test.go
+++ b/server/plugin/command_test.go
@@ -44,32 +44,32 @@ func TestValidateFeatures(t *testing.T) {
 		},
 		{
 			name: "all features valid with label but issues and pulls missing",
-			args: []string{"pushes", "label:\"ruby\""},
+			args: []string{"pushes", `label:"ruby"`},
 			want: output{false, []string{}},
 		},
 		{
 			name: "all features valid with label and issues in features",
-			args: []string{"issues", "label:\"ruby\""},
+			args: []string{"issues", `label:"ruby"`},
 			want: output{true, []string{}},
 		},
 		{
 			name: "all features valid with label and pulls in features",
-			args: []string{"pulls", "label:\"ruby\""},
+			args: []string{"pulls", `label:"ruby"`},
 			want: output{true, []string{}},
 		},
 		{
 			name: "multiple features invalid with label but issues and pulls missing",
-			args: []string{"issue", "push", "label:\"ruby\""},
+			args: []string{"issue", "push", `label:"ruby"`},
 			want: output{false, []string{"issue", "push"}},
 		},
 		{
 			name: "multiple features invalid with label and issues in features",
-			args: []string{"issues", "push", "create", "label:\"ruby\""},
+			args: []string{"issues", "push", "create", `label:"ruby"`},
 			want: output{false, []string{"push", "create"}},
 		},
 		{
 			name: "multiple features invalid with label and pulls in features",
-			args: []string{"pulls", "push", "create", "label:\"ruby\""},
+			args: []string{"pulls", "push", "create", `label:"ruby"`},
 			want: output{false, []string{"push", "create"}},
 		},
 	}
@@ -83,3 +83,117 @@ func TestValidateFeatures(t *testing.T) {
 		})
 	}
 }
+
+func TestParseCommand(t *testing.T) {
+	type output struct {
+		command    string
+		action     string
+		parameters []string
+	}
+
+	tt := []struct {
+		name  string
+		input string
+		want  output
+	}{
+		{
+			name:  "no parameters",
+			input: "/github subscribe",
+			want: output{
+				"/github",
+				"subscribe",
+				[]string(nil),
+			},
+		},
+		{
+			name:  "no action and no parameters",
+			input: "/github",
+			want: output{
+				"/github",
+				"",
+				[]string(nil),
+			},
+		},
+		{
+			name:  "simple one-word label",
+			input: `/github subscribe DHaussermann/hello-world issues,label:"Help"`,
+			want: output{
+				"/github",
+				"subscribe",
+				[]string{"DHaussermann/hello-world", `issues,label:"Help"`},
+			},
+		},
+		{
+			name:  "two-word label",
+			input: `/github subscribe DHaussermann/hello-world issues,label:"Help Wanted"`,
+			want: output{
+				"/github",
+				"subscribe",
+				[]string{"DHaussermann/hello-world", `issues,label:"Help Wanted"`},
+			},
+		},
+		{
+			name:  "multi-word label",
+			input: `/github subscribe DHaussermann/hello-world issues,label:"Good First Issue"`,
+			want: output{
+				"/github",
+				"subscribe",
+				[]string{"DHaussermann/hello-world", `issues,label:"Good First Issue"`},
+			},
+		},
+		{
+			name:  "multiple spaces inside double-quotes",
+			input: `/github subscribe DHaussermann/hello-world issues,label:"Help    Wanted"`,
+			want: output{
+				"/github",
+				"subscribe",
+				[]string{"DHaussermann/hello-world", `issues,label:"Help    Wanted"`},
+			},
+		},
+		{
+			name:  "multiple spaces outside of double-quotes",
+			input: `  /github    subscribe     DHaussermann/hello-world issues,label:"Help Wanted"`,
+			want: output{
+				"/github",
+				"subscribe",
+				[]string{"DHaussermann/hello-world", `issues,label:"Help Wanted"`},
+			},
+		},
+		{
+			name:  "trailing whitespaces",
+			input: `/github subscribe DHaussermann/hello-world issues,label:"Help Wanted" `,
+			want: output{
+				"/github",
+				"subscribe",
+				[]string{"DHaussermann/hello-world", `issues,label:"Help Wanted"`},
+			},
+		},
+		{
+			name:  "non-ASCII characters",
+			input: `/github subscribe طماطم issues,label:"日本語"`,
+			want: output{
+				"/github",
+				"subscribe",
+				[]string{"طماطم", `issues,label:"日本語"`},
+			},
+		},
+		{
+			name:  "line breaks",
+			input: "/github \nsubscribe\nDHaussermann/hello-world\nissues,label:\"Good First Issue\"",
+			want: output{
+				"/github",
+				"subscribe",
+				[]string{"DHaussermann/hello-world", `issues,label:"Good First Issue"`},
+			},
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			command, action, parameters := parseCommand(tc.input)
+			got := output{command, action, parameters}
+			errMsg := fmt.Sprintf("validateFeatures() = %v, want %v", got, tc.want)
+			assert.EqualValues(t, tc.want, got, errMsg)
+		})
+	}
+}
diff --git a/server/plugin/template_test.go b/server/plugin/template_test.go
index bdc9b5a..c08de39 100644
--- a/server/plugin/template_test.go
+++ b/server/plugin/template_test.go
@@ -19,6 +19,21 @@ var pushEventRepository = github.PushEventRepository{
 	HTMLURL:  sToP("https://github.com/mattermost/mattermost-plugin-github"),
 }
 
+var singleLabel = []*github.Label{
+	{
+		Name: sToP("Help Wanted"),
+	},
+}
+
+var labels = []*github.Label{
+	{
+		Name: sToP("Help Wanted"),
+	},
+	{
+		Name: sToP("Tech/Go"),
+	},
+}
+
 var pullRequest = github.PullRequest{
 	Number:    iToP(42),
 	HTMLURL:   sToP("https://github.com/mattermost/mattermost-plugin-github/pull/42"),
@@ -46,6 +61,36 @@ var pullRequestWithMentions = github.PullRequest{
 -->`),
 }
 
+var pullRequestWithLabelAndAssignee = github.PullRequest{
+	Number:    iToP(42),
+	HTMLURL:   sToP("https://github.com/mattermost/mattermost-plugin-github/pull/42"),
+	Title:     sToP("Leverage git-get-head"),
+	CreatedAt: tToP(time.Date(2019, 04, 01, 02, 03, 04, 0, time.UTC)),
+	UpdatedAt: tToP(time.Date(2019, 05, 01, 02, 03, 04, 0, time.UTC)),
+	Body: sToP(`<!-- Thank you for opening this pull request-->git-get-head gets the non-sent upstream heads inside the stashed non-cleaned applied areas, and after pruning bases to many archives, you can initialize the origin of the bases.
+<!-- Please make sure you have done the following :
+- Added tests
+- Removed console logs
+-->`),
+	Labels:    singleLabel,
+	Assignees: []*github.User{&user},
+}
+
+var pullRequestWithMultipleLabelsAndAssignees = github.PullRequest{
+	Number:    iToP(42),
+	HTMLURL:   sToP("https://github.com/mattermost/mattermost-plugin-github/pull/42"),
+	Title:     sToP("Leverage git-get-head"),
+	CreatedAt: tToP(time.Date(2019, 04, 01, 02, 03, 04, 0, time.UTC)),
+	UpdatedAt: tToP(time.Date(2019, 05, 01, 02, 03, 04, 0, time.UTC)),
+	Body: sToP(`<!-- Thank you for opening this pull request-->git-get-head gets the non-sent upstream heads inside the stashed non-cleaned applied areas, and after pruning bases to many archives, you can initialize the origin of the bases.
+<!-- Please make sure you have done the following :
+- Added tests
+- Removed console logs
+-->`),
+	Labels:    labels,
+	Assignees: []*github.User{&user, &user},
+}
+
 var mergedPullRequest = github.PullRequest{
 	Number:    iToP(42),
 	HTMLURL:   sToP("https://github.com/mattermost/mattermost-plugin-github/pull/42"),
@@ -79,6 +124,29 @@ var issueWithMentions = github.Issue{
 ` + gitHubMentions),
 }
 
+var issueWithLabelAndAssignee = github.Issue{
+	Number:    iToP(1),
+	HTMLURL:   sToP("https://github.com/mattermost/mattermost-plugin-github/issues/1"),
+	Title:     sToP("Implement git-get-head"),
+	CreatedAt: tToP(time.Date(2019, 04, 01, 02, 03, 04, 0, time.UTC)),
+	UpdatedAt: tToP(time.Date(2019, 05, 01, 02, 03, 04, 0, time.UTC)),
+	Body:      sToP(`<!-- Thank you for opening this issue-->git-get-head sounds like a great feature we should support`),
+	Labels:    singleLabel,
+	Assignee:  &user,
+	Assignees: []*github.User{&user},
+}
+
+var issueWithMultipleLabelsAndAssignee = github.Issue{
+	Number:    iToP(1),
+	HTMLURL:   sToP("https://github.com/mattermost/mattermost-plugin-github/issues/1"),
+	Title:     sToP("Implement git-get-head"),
+	CreatedAt: tToP(time.Date(2019, 04, 01, 02, 03, 04, 0, time.UTC)),
+	UpdatedAt: tToP(time.Date(2019, 05, 01, 02, 03, 04, 0, time.UTC)),
+	Body:      sToP(`<!-- Thank you for opening this issue-->git-get-head sounds like a great feature we should support`),
+	Labels:    labels,
+	Assignees: []*github.User{&user, &user},
+}
+
 var user = github.User{
 	Login:   sToP("panda"),
 	HTMLURL: sToP("https://github.com/panda"),
@@ -198,6 +266,48 @@ git-get-head gets the non-sent upstream heads inside the stashed non-cleaned app
 		require.NoError(t, err)
 		require.Equal(t, expected, actual)
 	}))
+
+	t.Run("with single label and assignee", func(t *testing.T) {
+		expected := `
+#### Leverage git-get-head
+##### [mattermost-plugin-github#42](https://github.com/mattermost/mattermost-plugin-github/pull/42)
+#new-pull-request by [panda](https://github.com/panda)
+Labels: ` + "[`Help Wanted`](https://github.com/mattermost/mattermost-plugin-github/labels/Help%20Wanted)" + `
+Assignees: [panda](https://github.com/panda)
+
+git-get-head gets the non-sent upstream heads inside the stashed non-cleaned applied areas, and after pruning bases to many archives, you can initialize the origin of the bases.
+
+`
+
+		actual, err := renderTemplate("newPR", &github.PullRequestEvent{
+			Repo:        &repo,
+			PullRequest: &pullRequestWithLabelAndAssignee,
+			Sender:      &user,
+		})
+		require.NoError(t, err)
+		require.Equal(t, expected, actual)
+	})
+
+	t.Run("with multiple labels and assignees", func(t *testing.T) {
+		expected := `
+#### Leverage git-get-head
+##### [mattermost-plugin-github#42](https://github.com/mattermost/mattermost-plugin-github/pull/42)
+#new-pull-request by [panda](https://github.com/panda)
+Labels: ` + "[`Help Wanted`](https://github.com/mattermost/mattermost-plugin-github/labels/Help%20Wanted), [`Tech/Go`](https://github.com/mattermost/mattermost-plugin-github/labels/Tech%2FGo)" + `
+Assignees: [panda](https://github.com/panda), [panda](https://github.com/panda)
+
+git-get-head gets the non-sent upstream heads inside the stashed non-cleaned applied areas, and after pruning bases to many archives, you can initialize the origin of the bases.
+
+`
+
+		actual, err := renderTemplate("newPR", &github.PullRequestEvent{
+			Repo:        &repo,
+			PullRequest: &pullRequestWithMultipleLabelsAndAssignees,
+			Sender:      &user,
+		})
+		require.NoError(t, err)
+		require.Equal(t, expected, actual)
+	})
 }
 
 func TestClosedPRMessageTemplate(t *testing.T) {
@@ -286,6 +396,46 @@ git-get-head sounds like a great feature we should support
 		require.NoError(t, err)
 		require.Equal(t, expected, actual)
 	}))
+
+	t.Run("with single label and assignee", func(t *testing.T) {
+		expected := `
+#### Implement git-get-head
+##### [mattermost-plugin-github#1](https://github.com/mattermost/mattermost-plugin-github/issues/1)
+#new-issue by [panda](https://github.com/panda)
+Labels: ` + "[`Help Wanted`](https://github.com/mattermost/mattermost-plugin-github/labels/Help%20Wanted)" + `
+Assignees: [panda](https://github.com/panda)
+
+git-get-head sounds like a great feature we should support
+`
+
+		actual, err := renderTemplate("newIssue", &github.IssuesEvent{
+			Repo:   &repo,
+			Issue:  &issueWithLabelAndAssignee,
+			Sender: &user,
+		})
+		require.NoError(t, err)
+		require.Equal(t, expected, actual)
+	})
+
+	t.Run("with multiple labels and assignees", func(t *testing.T) {
+		expected := `
+#### Implement git-get-head
+##### [mattermost-plugin-github#1](https://github.com/mattermost/mattermost-plugin-github/issues/1)
+#new-issue by [panda](https://github.com/panda)
+Labels: ` + "[`Help Wanted`](https://github.com/mattermost/mattermost-plugin-github/labels/Help%20Wanted), [`Tech/Go`](https://github.com/mattermost/mattermost-plugin-github/labels/Tech%2FGo)" + `
+Assignees: [panda](https://github.com/panda), [panda](https://github.com/panda)
+
+git-get-head sounds like a great feature we should support
+`
+
+		actual, err := renderTemplate("newIssue", &github.IssuesEvent{
+			Repo:   &repo,
+			Issue:  &issueWithMultipleLabelsAndAssignee,
+			Sender: &user,
+		})
+		require.NoError(t, err)
+		require.Equal(t, expected, actual)
+	})
 }
 
 func TestClosedIssueTemplate(t *testing.T) {
diff --git a/server/plugin/utils_test.go b/server/plugin/utils_test.go
index b6bd976..46ca183 100644
--- a/server/plugin/utils_test.go
+++ b/server/plugin/utils_test.go
@@ -39,16 +39,18 @@ func TestFixGithubNotificationSubjectURL(t *testing.T) {
 	tcs := []struct {
 		Text     string
 		Expected string
+		IssueNum string
 	}{
 		{Text: "https://api.github.com/repos/jwilander/mattermost-webapp/issues/123", Expected: "https://github.com/jwilander/mattermost-webapp/issues/123"},
 		{Text: "https://api.github.com/repos/jwilander/mattermost-webapp/pulls/123", Expected: "https://github.com/jwilander/mattermost-webapp/pull/123"},
 		{Text: "https://enterprise.github.com/api/v3/jwilander/mattermost-webapp/issues/123", Expected: "https://enterprise.github.com/jwilander/mattermost-webapp/issues/123"},
 		{Text: "https://enterprise.github.com/api/v3/jwilander/mattermost-webapp/pull/123", Expected: "https://enterprise.github.com/jwilander/mattermost-webapp/pull/123"},
 		{Text: "https://api.github.com/repos/mattermost/mattermost-server/commits/cc6c385d3e8903546fc6fc856bf468ad09b70913", Expected: "https://github.com/mattermost/mattermost-server/commit/cc6c385d3e8903546fc6fc856bf468ad09b70913"},
+		{Text: "https://api.github.com/repos/user/rate_my_cakes/issues/comments/655139214", Expected: "https://github.com/user/rate_my_cakes/issues/4#issuecomment-655139214", IssueNum: "4"},
 	}
 
 	for _, tc := range tcs {
-		assert.Equal(t, tc.Expected, fixGithubNotificationSubjectURL(tc.Text))
+		assert.Equal(t, tc.Expected, fixGithubNotificationSubjectURL(tc.Text, tc.IssueNum))
 	}
 }