Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MM-356] Add feature to subscribe to release and workflow events #765

Merged
merged 13 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@ require (
golang.org/x/oauth2 v0.17.0
)

require (
github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8 // indirect
github.com/cloudflare/circl v1.3.7 // indirect
)

require (
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
github.com/Masterminds/goutils v1.1.1 // indirect
github.com/Masterminds/semver/v3 v3.1.1 // indirect
github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8 // indirect
github.com/aymerick/douceur v0.2.0 // indirect
github.com/blang/semver/v4 v4.0.0 // indirect
github.com/cloudflare/circl v1.3.7 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/dyatlov/go-opengraph/opengraph v0.0.0-20220524092352-606d7b1e5f8a // indirect
github.com/fatih/color v1.16.0 // indirect
Expand Down
54 changes: 29 additions & 25 deletions server/plugin/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,37 +15,41 @@ import (
)

const (
featureIssueCreation = "issue_creations"
featureIssues = "issues"
featurePulls = "pulls"
featurePullsMerged = "pulls_merged"
featurePullsCreated = "pulls_created"
featurePushes = "pushes"
featureCreates = "creates"
featureDeletes = "deletes"
featureIssueComments = "issue_comments"
featurePullReviews = "pull_reviews"
featureStars = "stars"
featureReleases = "releases"
featureIssueCreation = "issue_creations"
featureIssues = "issues"
featurePulls = "pulls"
featurePullsMerged = "pulls_merged"
featurePullsCreated = "pulls_created"
featurePushes = "pushes"
featureCreates = "creates"
featureDeletes = "deletes"
featureIssueComments = "issue_comments"
featurePullReviews = "pull_reviews"
featureStars = "stars"
featureReleases = "releases"
featureWorkflowFailures = "workflow_failures"
featureWorkflowSuccess = "workflow_success"
)

const (
PerPageValue = 50
)

var validFeatures = map[string]bool{
featureIssueCreation: true,
featureIssues: true,
featurePulls: true,
featurePullsMerged: true,
featurePullsCreated: true,
featurePushes: true,
featureCreates: true,
featureDeletes: true,
featureIssueComments: true,
featurePullReviews: true,
featureStars: true,
featureReleases: true,
featureIssueCreation: true,
featureIssues: true,
featurePulls: true,
featurePullsMerged: true,
featurePullsCreated: true,
featurePushes: true,
featureCreates: true,
featureDeletes: true,
featureIssueComments: true,
featurePullReviews: true,
featureStars: true,
featureReleases: true,
featureWorkflowFailures: true,
featureWorkflowSuccess: true,
}

type Features string
Expand Down Expand Up @@ -896,7 +900,7 @@ func getAutocompleteData(config *Configuration) *model.AutocompleteData {

subscriptionsAdd := model.NewAutocompleteData("add", "[owner/repo] [features] [flags]", "Subscribe the current channel to receive notifications about opened pull requests and issues for an organization or repository. [features] and [flags] are optional arguments")
subscriptionsAdd.AddTextArgument("Owner/repo to subscribe to", "[owner/repo]", "")
subscriptionsAdd.AddNamedTextArgument("features", "Comma-delimited list of one or more of: issues, pulls, pulls_merged, pulls_created, pushes, creates, deletes, issue_creations, issue_comments, pull_reviews, releases, label:\"<labelname>\". Defaults to pulls,issues,creates,deletes", "", `/[^,-\s]+(,[^,-\s]+)*/`, false)
subscriptionsAdd.AddNamedTextArgument("features", "Comma-delimited list of one or more of: issues, pulls, pulls_merged, pulls_created, pushes, creates, deletes, issue_creations, issue_comments, pull_reviews, workflow_success, workflow_failures, label:\"<labelname>\". Defaults to pulls,issues,creates,deletes", "", `/[^,-\s]+(,[^,-\s]+)*/`, false)
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved

if config.GitHubOrg != "" {
subscriptionsAdd.AddNamedStaticListArgument("exclude-org-member", "Events triggered by organization members will not be delivered (the organization config should be set, otherwise this flag has not effect)", false, []model.AutocompleteListItem{
Expand Down
4 changes: 4 additions & 0 deletions server/plugin/subscriptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ func (s *Subscription) Stars() bool {
return strings.Contains(s.Features.String(), featureStars)
}

func (s *Subscription) Workflows() bool {
return strings.Contains(s.Features.String(), featureWorkflowFailures) || strings.Contains(s.Features.String(), featureWorkflowSuccess)
}

func (s *Subscription) Release() bool {
return strings.Contains(s.Features.String(), featureReleases)
}
Expand Down
22 changes: 22 additions & 0 deletions server/plugin/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,16 @@ func init() {
return commit.GetCommitter()
}

funcMap["workflowJobFailedStep"] = func(steps []*github.TaskStep) string {
for _, step := range steps {
if step.GetConclusion() == workflowJobFail {
return step.GetName()
}
}

return ""
}

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
Expand Down Expand Up @@ -158,6 +168,11 @@ func init() {
`[#{{.GetNumber}} {{.GetTitle}}]({{.GetHTMLURL}})`,
))

// The workflow job links to the corresponding workflow.
template.Must(masterTemplate.New("workflowJob").Parse(
`[{{.GetName}}]({{.GetHTMLURL}})`,
))

// The release links to the corresponding release.
template.Must(masterTemplate.New("release").Parse(
`[{{.GetTagName}}]({{.GetHTMLURL}})`,
Expand Down Expand Up @@ -413,6 +428,8 @@ Assignees: {{range $i, $el := .Assignees -}} {{- if $i}}, {{end}}{{template "use
" * `issue_comments` - includes new issue comments\n" +
" * `issue_creations` - includes new issues only \n" +
" * `pull_reviews` - includes pull request reviews\n" +
" * `workflow_failures` - includes workflow job failures\n" +
" * `workflow_success` - includes workflow job success\n" +
" * `releases` - includes release created and deleted\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\n" +
Expand All @@ -436,6 +453,11 @@ Assignees: {{range $i, $el := .Assignees -}} {{- if $i}}, {{end}}{{template "use
{{- end }} by {{template "user" .GetSender}}
It now has **{{.GetRepo.GetStargazersCount}}** stars.`))

template.Must(masterTemplate.New("newWorkflowJob").Funcs(funcMap).Parse(`
{{template "repo" .GetRepo}} {{.GetWorkflowJob.GetWorkflowName}} workflow {{if eq .GetWorkflowJob.GetConclusion "success"}}succeeded{{else}}failed{{end}} (triggered by {{template "user" .GetSender}})
{{if eq .GetWorkflowJob.GetConclusion "failure"}}Job failed: {{template "workflowJob" .GetWorkflowJob}}
Step failed: {{.GetWorkflowJob.Steps | workflowJobFailedStep}}{{end}}
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
Commit: {{.GetRepo.GetHTMLURL}}/commit/{{.GetWorkflowJob.GetHeadSHA}}`))
template.Must(masterTemplate.New("newReleaseEvent").Funcs(funcMap).Parse(`
{{template "repo" .GetRepo}} {{template "user" .GetSender}}
{{- if eq .GetAction "created" }} created a release {{template "release" .GetRelease}}
Expand Down
39 changes: 39 additions & 0 deletions server/plugin/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1481,6 +1481,45 @@ func TestGitHubUsernameRegex(t *testing.T) {
}
}

func TestWorkflowJobNotification(t *testing.T) {
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
t.Run("failed", func(t *testing.T) {
expected := `
[\[mattermost-plugin-github\]](https://github.com/mattermost/mattermost-plugin-github) mock-workflow-name workflow failed (triggered by [panda](https://github.com/panda))
Job failed: [mock-workflow-job](https://github.com/mattermost/mattermost-plugin-github/actions/runs/12345/job/67890)
Step failed: mock-job-2
Commit: https://github.com/mattermost/mattermost-plugin-github/commit/1234567890`

actual, err := renderTemplate("newWorkflowJob", &github.WorkflowJobEvent{
Repo: &repo,
Sender: &user,
Action: sToP(actionCompleted),
WorkflowJob: &github.WorkflowJob{
Conclusion: sToP("failure"),
Name: sToP("mock-workflow-job"),
HeadSHA: sToP("1234567890"),
HTMLURL: sToP("https://github.com/mattermost/mattermost-plugin-github/actions/runs/12345/job/67890"),
WorkflowName: sToP("mock-workflow-name"),
Steps: []*github.TaskStep{
{
Name: sToP("mock-job-1"),
Conclusion: sToP("success"),
},
{
Name: sToP("mock-job-2"),
Conclusion: sToP("failure"),
},
{
Name: sToP("mock-job-3"),
Conclusion: sToP("success"),
},
},
},
})
require.NoError(t, err)
require.Equal(t, expected, actual)
})
}

func sToP(s string) *string {
return &s
}
Expand Down
56 changes: 53 additions & 3 deletions server/plugin/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,13 @@ const (
actionLabeled = "labeled"
actionAssigned = "assigned"

actionCreated = "created"
actionDeleted = "deleted"
actionEdited = "edited"
actionCreated = "created"
actionDeleted = "deleted"
actionEdited = "edited"
actionCompleted = "completed"

workflowJobFail = "failure"
workflowJobSuccess = "success"

postPropGithubRepo = "gh_repo"
postPropGithubObjectID = "gh_object_id"
Expand Down Expand Up @@ -282,6 +286,11 @@ func (p *Plugin) handleWebhook(w http.ResponseWriter, r *http.Request) {
handler = func() {
p.postStarEvent(event)
}
case *github.WorkflowJobEvent:
repo = event.GetRepo()
handler = func() {
p.postWorkflowJobEvent(event)
}
case *github.ReleaseEvent:
repo = event.GetRepo()
handler = func() {
Expand Down Expand Up @@ -1339,6 +1348,47 @@ func (p *Plugin) postStarEvent(event *github.StarEvent) {
}
}

func (p *Plugin) postWorkflowJobEvent(event *github.WorkflowJobEvent) {
if event.GetAction() != actionCompleted {
return
}

// Create a post only when the workflow job is completed and has either failed or succeeded
if event.GetWorkflowJob().GetConclusion() != workflowJobFail && event.GetWorkflowJob().GetConclusion() != workflowJobSuccess {
return
}

repo := event.GetRepo()
subs := p.GetSubscribedChannelsForRepository(repo)

if len(subs) == 0 {
return
}

newWorkflowJobMessage, err := renderTemplate("newWorkflowJob", event)
if err != nil {
p.client.Log.Warn("Failed to render template", "Error", err.Error())
return
}

for _, sub := range subs {
if !sub.Workflows() {
continue
}
Comment on lines +1385 to +1388
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1/5 I think we should support (or require) a workflow parameter here (and maybe support job parameter as well). Maybe we can support both individual workflows and all workflow cases:

Just the ci workflow

/github subscriptions add owner/repo pulls,workflows:ci,issues

All workflows

/github subscriptions add owner/repo pulls,workflows:*,issues

@hanzei What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me 👍

Limiting the notifications to a specific branch seems like a common use case. Can we include that in there as well?

Copy link
Contributor Author

@ayusht2810 ayusht2810 May 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the ci workflow

@mickmister Are you suggesting here to add a subscription on the basis of the workflow name, which is ci in the above case, or have I misunderstood the comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Limiting the notifications to a specific branch seems like a common use case. Can we include that in there as well?

@mickmister @hanzei For the above case, what should be the slash command format? Should we keep something like: /github subscriptions add owner/repo pulls,workflows:branch-b1,b2;name-n1,n2;job-j1,j2? Please let me know if you have any better suggestions for the above case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The subscriptions add feature is quite overloaded at this point. Adding more complexity on top of it may not be the best thing to do. Created a ticket for breaking this out of the command #780

Mainly thinking of the complexity of the job parameter there. I'm thinking we should avoid the job parameter, since those jobs may not belong to all workflows. Then it's up to the actions workflow developer in that project to structure the workflows in a way to be something to subscribe to.


post := &model.Post{
UserId: p.BotUserID,
Type: "custom_git_workflow_job",
Message: newWorkflowJobMessage,
ChannelId: sub.ChannelID,
}

if err = p.client.Post.CreatePost(post); err != nil {
p.client.Log.Warn("Error webhook post", "Post", post, "Error", err.Error())
}
}
}

func (p *Plugin) makeBotPost(message, postType string) *model.Post {
return &model.Post{
UserId: p.BotUserID,
Expand Down
Loading
Loading