Repository: mattermost/mattermost-plugin-github
Pull request #729 (2024-01-10 09:47:01)

[GH-719] Pushed commits events: add option to show Author instead of committer
#### Summary

Pushed commits events: add option to show Author instead of Committer.

#### Screenshot

![image](https://github.com/mattermost/mattermost-plugin-github/assets/167134/0c693e18-83ff-4e76-bd84-1676ab501d18)

#### What to test?

Before starting, some Mattermost channel should be subscribed to pushed commits notifications for a repo.

1. Go to plugin settings and set "Pushed commits events: show Author instead of Committer:" to true.
1. Create a git commit with committer != author. For example, "git am" a patch from someone else, or "git commit --author 'Test author <test@example.com>'" (example: https://github.com/FOULAB/eigma-test/commit/65ed77834f659a0498c5d02cc319e38bd12c50f3)
2. Push commit to GitHub
3. Observe the pushed commits notification.

#### Ticket Link
Fixes https://github.com/mattermost/mattermost-plugin-github/issues/719


mattermost-build: Hello @cpatulea,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process [here](https://developers.mattermost.com/contribute/getting-started/code-review/).

Patch:
diff --git a/plugin.json b/plugin.json
index 03a837a..0dd7b64 100644
--- a/plugin.json
+++ b/plugin.json
@@ -118,6 +118,13 @@
                 "type": "bool",
                 "help_text": "Allow the plugin to log the webhook event. The log level needs to be set to DEBUG.",
                 "default": false
+            },
+            {
+                "key": "ShowAuthorInCommitNotification",
+                "display_name": "Show Author in commit notification:",
+                "type": "bool",
+                "help_text": "In `Pushes` event notification, show commit author instead of commit committer.",
+                "default": false
             }
         ],
         "footer": "* To report an issue, make a suggestion or a contribution, [check the repository](https://github.com/mattermost/mattermost-plugin-github)."
diff --git a/server/plugin/configuration.go b/server/plugin/configuration.go
index d320c7d..5f25cff 100644
--- a/server/plugin/configuration.go
+++ b/server/plugin/configuration.go
@@ -25,19 +25,20 @@ import (
 // If you add non-reference types to your configuration struct, be sure to rewrite Clone as a deep
 // copy appropriate for your types.
 type Configuration struct {
-	GitHubOrg                   string `json:"githuborg"`
-	GitHubOAuthClientID         string `json:"githuboauthclientid"`
-	GitHubOAuthClientSecret     string `json:"githuboauthclientsecret"`
-	WebhookSecret               string `json:"webhooksecret"`
-	EnableLeftSidebar           bool   `json:"enableleftsidebar"`
-	EnablePrivateRepo           bool   `json:"enableprivaterepo"`
-	ConnectToPrivateByDefault   bool   `json:"connecttoprivatebydefault"`
-	EncryptionKey               string `json:"encryptionkey"`
-	EnterpriseBaseURL           string `json:"enterprisebaseurl"`
-	EnterpriseUploadURL         string `json:"enterpriseuploadurl"`
-	EnableCodePreview           string `json:"enablecodepreview"`
-	EnableWebhookEventLogging   bool   `json:"enablewebhookeventlogging"`
-	UsePreregisteredApplication bool   `json:"usepreregisteredapplication"`
+	GitHubOrg                      string `json:"githuborg"`
+	GitHubOAuthClientID            string `json:"githuboauthclientid"`
+	GitHubOAuthClientSecret        string `json:"githuboauthclientsecret"`
+	WebhookSecret                  string `json:"webhooksecret"`
+	EnableLeftSidebar              bool   `json:"enableleftsidebar"`
+	EnablePrivateRepo              bool   `json:"enableprivaterepo"`
+	ConnectToPrivateByDefault      bool   `json:"connecttoprivatebydefault"`
+	EncryptionKey                  string `json:"encryptionkey"`
+	EnterpriseBaseURL              string `json:"enterprisebaseurl"`
+	EnterpriseUploadURL            string `json:"enterpriseuploadurl"`
+	EnableCodePreview              string `json:"enablecodepreview"`
+	EnableWebhookEventLogging      bool   `json:"enablewebhookeventlogging"`
+	UsePreregisteredApplication    bool   `json:"usepreregisteredapplication"`
+	ShowAuthorInCommitNotification bool   `json:"showauthorincommitnotification"`
 }
 
 func (c *Configuration) ToMap() (map[string]interface{}, error) {
diff --git a/server/plugin/graphql/lhs_query.go b/server/plugin/graphql/lhs_query.go
index 29c17a7..3de215e 100644
--- a/server/plugin/graphql/lhs_query.go
+++ b/server/plugin/graphql/lhs_query.go
@@ -15,6 +15,11 @@ type (
 		Login githubv4.String
 	}
 
+	labelNode struct {
+		Name  githubv4.String
+		Color githubv4.String
+	}
+
 	prSearchNodes struct {
 		PullRequest struct {
 			Body              githubv4.String
@@ -27,6 +32,12 @@ type (
 			Title             githubv4.String
 			Author            authorQuery
 			URL               githubv4.URI
+			Labels            struct {
+				Nodes []labelNode
+			} `graphql:"labels(first:100)"`
+			Milestone struct {
+				Title githubv4.String
+			}
 		} `graphql:"... on PullRequest"`
 	}
 )
@@ -44,6 +55,12 @@ type (
 			Title             githubv4.String
 			Author            authorQuery
 			URL               githubv4.URI
+			Labels            struct {
+				Nodes []labelNode
+			} `graphql:"labels(first:100)"`
+			Milestone struct {
+				Title githubv4.String
+			}
 		} `graphql:"... on Issue"`
 
 		PullRequest struct {
@@ -57,6 +74,12 @@ type (
 			Title             githubv4.String
 			Author            authorQuery
 			URL               githubv4.URI
+			Labels            struct {
+				Nodes []labelNode
+			} `graphql:"labels(first:100)"`
+			Milestone struct {
+				Title githubv4.String
+			}
 		} `graphql:"... on PullRequest"`
 	}
 )
diff --git a/server/plugin/graphql/lhs_request.go b/server/plugin/graphql/lhs_request.go
index 8c7d2db..2860986 100644
--- a/server/plugin/graphql/lhs_request.go
+++ b/server/plugin/graphql/lhs_request.go
@@ -64,7 +64,7 @@ func (c *Client) GetLHSData(ctx context.Context) ([]*github.Issue, []*github.Iss
 		if !allAssignmentsFetched {
 			for i := range mainQuery.Assignments.Nodes {
 				resp := mainQuery.Assignments.Nodes[i]
-				issue := getIssue(&resp)
+				issue := newIssueFromAssignmentResponse(&resp)
 				resultAssignee = append(resultAssignee, issue)
 			}
 
@@ -95,21 +95,38 @@ func (c *Client) GetLHSData(ctx context.Context) ([]*github.Issue, []*github.Iss
 
 func getPR(prResp *prSearchNodes) *github.Issue {
 	resp := prResp.PullRequest
+	labels := getGithubLabels(resp.Labels.Nodes)
 
-	return getGithubIssue(resp.Number, resp.Title, resp.Author.Login, resp.Repository.URL, resp.URL, resp.CreatedAt, resp.UpdatedAt)
+	return newGithubIssue(resp.Number, resp.Title, resp.Author.Login, resp.Repository.URL, resp.URL, resp.CreatedAt, resp.UpdatedAt, labels, resp.Milestone.Title)
 }
 
-func getIssue(assignmentResp *assignmentSearchNodes) *github.Issue {
+func newIssueFromAssignmentResponse(assignmentResp *assignmentSearchNodes) *github.Issue {
 	resp := assignmentResp.PullRequest
+	labels := getGithubLabels(resp.Labels.Nodes)
 
-	return getGithubIssue(resp.Number, resp.Title, resp.Author.Login, resp.Repository.URL, resp.URL, resp.CreatedAt, resp.UpdatedAt)
+	return newGithubIssue(resp.Number, resp.Title, resp.Author.Login, resp.Repository.URL, resp.URL, resp.CreatedAt, resp.UpdatedAt, labels, resp.Milestone.Title)
 }
 
-func getGithubIssue(prNumber githubv4.Int, title, login githubv4.String, repositoryURL, htmlURL githubv4.URI, createdAt, updatedAt githubv4.DateTime) *github.Issue {
+func getGithubLabels(labels []labelNode) []*github.Label {
+	githubLabels := []*github.Label{}
+	for _, label := range labels {
+		name := (string)(label.Name)
+		color := (string)(label.Color)
+		githubLabels = append(githubLabels, &github.Label{
+			Color: &color,
+			Name:  &name,
+		})
+	}
+
+	return githubLabels
+}
+
+func newGithubIssue(prNumber githubv4.Int, title, login githubv4.String, repositoryURL, htmlURL githubv4.URI, createdAt, updatedAt githubv4.DateTime, labels []*github.Label, milestone githubv4.String) *github.Issue {
 	number := int(prNumber)
 	repoURL := repositoryURL.String()
 	issuetitle := string(title)
-	userLogin := (string)(login)
+	userLogin := string(login)
+	milestoneTitle := string(milestone)
 	url := htmlURL.String()
 	createdAtTime := createdAt.Time
 	updatedAtTime := updatedAt.Time
@@ -123,6 +140,10 @@ func getGithubIssue(prNumber githubv4.Int, title, login githubv4.String, reposit
 		User: &github.User{
 			Login: &userLogin,
 		},
+		Milestone: &github.Milestone{
+			Title: &milestoneTitle,
+		},
 		HTMLURL: &url,
+		Labels:  labels,
 	}
 }
diff --git a/server/plugin/plugin.go b/server/plugin/plugin.go
index d3b1e3f..a1b6120 100644
--- a/server/plugin/plugin.go
+++ b/server/plugin/plugin.go
@@ -642,7 +642,11 @@ func (p *Plugin) storeGitHubToUserIDMapping(githubUsername, userID string) error
 
 func (p *Plugin) getGitHubToUserIDMapping(githubUsername string) string {
 	var data []byte
-	_ = p.client.KV.Get(githubUsername+githubUsernameKey, data)
+	err := p.client.KV.Get(githubUsername+githubUsernameKey, &data)
+	if err != nil {
+		p.API.LogWarn("Error occurred while getting the user ID from KV store using the Github username", "Error", err.Error())
+		return ""
+	}
 
 	return string(data)
 }
@@ -748,7 +752,7 @@ func (p *Plugin) StoreDailySummaryText(userID, summaryText string) error {
 
 func (p *Plugin) GetDailySummaryText(userID string) (string, error) {
 	var summaryByte []byte
-	err := p.client.KV.Get(userID+dailySummary, summaryByte)
+	err := p.client.KV.Get(userID+dailySummary, &summaryByte)
 	if err != nil {
 		return "", err
 	}
diff --git a/server/plugin/template.go b/server/plugin/template.go
index 46769ba..82b47fd 100644
--- a/server/plugin/template.go
+++ b/server/plugin/template.go
@@ -8,6 +8,7 @@ import (
 	"text/template"
 
 	"github.com/Masterminds/sprig/v3"
+	"github.com/google/go-github/v41/github"
 	"github.com/pkg/errors"
 )
 
@@ -32,6 +33,7 @@ var mdCommentRegex = regexp.MustCompile(mdCommentRegexPattern)
 var gitHubUsernameRegex = regexp.MustCompile(gitHubUsernameRegexPattern)
 var masterTemplate *template.Template
 var gitHubToUsernameMappingCallback func(string) string
+var showAuthorInCommitNotification bool
 
 func init() {
 	var funcMap = sprig.TxtFuncMap()
@@ -103,6 +105,14 @@ func init() {
 		return dict, nil
 	}
 
+	funcMap["commitAuthor"] = func(commit *github.HeadCommit) *github.CommitAuthor {
+		if showAuthorInCommitNotification {
+			return commit.GetAuthor()
+		}
+
+		return commit.GetCommitter()
+	}
+
 	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
@@ -273,7 +283,7 @@ Assignees: {{range $i, $el := .Assignees -}} {{- if $i}}, {{end}}{{template "use
 	template.Must(masterTemplate.New("pushedCommits").Funcs(funcMap).Parse(`
 {{template "user" .GetSender}} {{if .GetForced}}force-{{end}}pushed [{{len .Commits}} new commit{{if ne (len .Commits) 1}}s{{end}}]({{.GetCompare}}) to [\[{{.GetRepo.GetFullName}}:{{.GetRef | trimRef}}\]]({{.GetRepo.GetHTMLURL}}/tree/{{.GetRef | trimRef}}):
 {{range .Commits -}}
-[` + "`{{.GetID | substr 0 6}}`" + `]({{.GetURL}}) {{.GetMessage}} - {{.GetCommitter.GetName}}
+[` + "`{{.GetID | substr 0 6}}`" + `]({{.GetURL}}) {{.GetMessage}} - {{with . | commitAuthor}}{{.GetName}}{{end}}
 {{end -}}
 `))
 
@@ -433,6 +443,10 @@ func lookupMattermostUsername(githubUsername string) string {
 	return gitHubToUsernameMappingCallback(githubUsername)
 }
 
+func setShowAuthorInCommitNotification(value bool) {
+	showAuthorInCommitNotification = value
+}
+
 func renderTemplate(name string, data interface{}) (string, error) {
 	var output bytes.Buffer
 	t := masterTemplate.Lookup(name)
diff --git a/server/plugin/webhook.go b/server/plugin/webhook.go
index c2225e8..6e1126a 100644
--- a/server/plugin/webhook.go
+++ b/server/plugin/webhook.go
@@ -657,6 +657,7 @@ func (p *Plugin) postPushEvent(event *github.PushEvent) {
 		return
 	}
 
+	setShowAuthorInCommitNotification(p.configuration.ShowAuthorInCommitNotification)
 	pushedCommitsMessage, err := renderTemplate("pushedCommits", event)
 	if err != nil {
 		p.client.Log.Warn("Failed to render template", "error", err.Error())
diff --git a/webapp/src/components/link_tooltip/link_tooltip.jsx b/webapp/src/components/link_tooltip/link_tooltip.jsx
index 4ba3dc9..806f79c 100644
--- a/webapp/src/components/link_tooltip/link_tooltip.jsx
+++ b/webapp/src/components/link_tooltip/link_tooltip.jsx
@@ -7,6 +7,8 @@ import ReactMarkdown from 'react-markdown';
 import Client from 'client';
 import {getLabelFontColor, hexToRGB} from '../../utils/styles';
 
+const maxTicketDescriptionLength = 160;
+
 export const LinkTooltip = ({href, connected, show, theme}) => {
     const [data, setData] = useState(null);
     useEffect(() => {
@@ -87,6 +89,14 @@ export const LinkTooltip = ({href, connected, show, theme}) => {
         let date = new Date(data.created_at);
         date = date.toDateString();
 
+        let description = '';
+        if (data.body) {
+            description = data.body.substring(0, maxTicketDescriptionLength).trim();
+            if (data.body.length > maxTicketDescriptionLength) {
+                description += '...';
+            }
+        }
+
         return (
             <div className='github-tooltip'>
                 <div
@@ -118,10 +128,7 @@ export const LinkTooltip = ({href, connected, show, theme}) => {
                                 <span>{'#' + data.number}</span>
                             </a>
                             <div className='markdown-text mt-1 mb-1'>
-                                <ReactMarkdown
-                                    source={data.body}
-                                    linkTarget='_blank'
-                                />
+                                <ReactMarkdown linkTarget='_blank'>{description}</ReactMarkdown>
                             </div>
 
                             {/* base <- head */}
@@ -141,14 +148,6 @@ export const LinkTooltip = ({href, connected, show, theme}) => {
                                 </div>
                             )}
 
-                            <div className='see-more mt-1'>
-                                <a
-                                    href={href}
-                                    target='_blank'
-                                    rel='noopener noreferrer'
-                                >{'See more'}</a>
-                            </div>
-
                             {/* Labels */}
                             <div className='labels mt-3'>
                                 {data.labels && data.labels.map((label, idx) => {
diff --git a/webapp/src/components/sidebar_right/github_items.tsx b/webapp/src/components/sidebar_right/github_items.tsx
index 0c41e1a..3810918 100644
--- a/webapp/src/components/sidebar_right/github_items.tsx
+++ b/webapp/src/components/sidebar_right/github_items.tsx
@@ -167,7 +167,7 @@ function GithubItems(props: GithubItemsProps) {
         }
 
         let milestone: JSX.Element | null = null;
-        if (item.milestone) {
+        if (item.milestone?.title) {
             milestone = (
                 <span
                     title={item.milestone.title}
@@ -372,7 +372,7 @@ function getGithubLabels(labels: Label[]) {
     return labels.map((label) => {
         return (
             <Badge
-                key={label.id}
+                key={label.name}
                 aria-label={label.name}
                 role={'note'}
                 style={{...itemStyle, ...{backgroundColor: `#${label.color}`, color: getLabelFontColor(label.color)}}}


Test patch:
diff --git a/server/plugin/template_test.go b/server/plugin/template_test.go
index 5545c31..929dd9a 100644
--- a/server/plugin/template_test.go
+++ b/server/plugin/template_test.go
@@ -750,6 +750,41 @@ func TestPushedCommitsTemplate(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, expected, actual)
 	})
+
+	t.Run("single commit, with 'Show Author in commit notifications'", func(t *testing.T) {
+		showAuthorInCommitNotification = true
+		t.Cleanup(func() {
+			showAuthorInCommitNotification = false
+		})
+
+		expected := `
+[panda](https://github.com/panda) pushed [1 new commit](https://github.com/mattermost/mattermost-plugin-github/compare/master...branch) to [\[mattermost-plugin-github:branch\]](https://github.com/mattermost/mattermost-plugin-github/tree/branch):
+[` + "`a10867`" + `](https://github.com/mattermost/mattermost-plugin-github/commit/a10867b14bb761a232cd80139fbd4c0d33264240) Leverage git-get-head - lion
+`
+
+		actual, err := renderTemplate("pushedCommits", &github.PushEvent{
+			Repo:   &pushEventRepository,
+			Sender: &user,
+			Forced: bToP(false),
+			Commits: []*github.HeadCommit{
+				{
+					ID:      sToP("a10867b14bb761a232cd80139fbd4c0d33264240"),
+					URL:     sToP("https://github.com/mattermost/mattermost-plugin-github/commit/a10867b14bb761a232cd80139fbd4c0d33264240"),
+					Message: sToP("Leverage git-get-head"),
+					Committer: &github.CommitAuthor{
+						Name: sToP("panda"),
+					},
+					Author: &github.CommitAuthor{
+						Name: sToP("lion"),
+					},
+				},
+			},
+			Compare: sToP("https://github.com/mattermost/mattermost-plugin-github/compare/master...branch"),
+			Ref:     sToP("refs/heads/branch"),
+		})
+		require.NoError(t, err)
+		require.Equal(t, expected, actual)
+	})
 }
 
 func TestCreateMessageTemplate(t *testing.T) {