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-15439 Restores 1.0 compatibility for webhook filtering #72

Merged
merged 8 commits into from
May 24, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
12 changes: 8 additions & 4 deletions server/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,15 @@ func parseJIRAIssuesFromText(text string, keys []string) []string {
return issues
}

func getIssueURL(i *JIRAWebhookIssue) string {
u, _ := url.Parse(i.Self)
return u.Scheme + "://" + u.Host + "/browse/" + i.Key
func getIssueURL(issue *jira.Issue) string {
if issue == nil {
return ""
}
u, _ := url.Parse(issue.Self)
return u.Scheme + "://" + u.Host + "/browse/" + issue.Key
}

func getUserURL(issue *JIRAWebhookIssue, user *jira.User) string {
func getUserURL(user *jira.User) string {
// TODO is this right?
Copy link
Member

Choose a reason for hiding this comment

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

TODO? What's this about?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is not used in 2.0, and will likely be deprecated with @cpoile's implementation of /jira settings notify.

I can cut a separate PR and simply remove all "notify" functionality from v2.0 since it's not used. This function will disappear then.

return user.Self
}
138 changes: 100 additions & 38 deletions server/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,56 +18,76 @@ import (
"github.com/mattermost/mattermost-server/model"
)

type JIRAWebhookIssue struct {
Self string
Key string
Fields struct {
Assignee *jira.User
Reporter *jira.User
Summary string
Description string
Priority *struct {
Id string
Name string
IconURL string
}
IssueType struct {
Name string
IconURL string
}
Resolution *struct {
Id string
}
Status struct {
Id string
}
Labels []string
}
const (
eventCreated = uint64(1 << iota)
eventCreatedComment
eventDeleted
eventDeletedComment
eventDeletedUnresolved
eventUpdatedAssignee
eventUpdatedAttachment
eventUpdatedComment
eventUpdatedDescription
eventUpdatedLabels
eventUpdatedPriority
eventUpdatedRank
eventUpdatedReopened
eventUpdatedResolved
eventUpdatedSprint
eventUpdatedStatus
eventUpdatedSummary
eventMax = iota
)

const maskLegacy = eventCreated |
eventUpdatedReopened |
eventUpdatedResolved |
eventDeletedUnresolved

const maskComments = eventCreatedComment |
eventDeletedComment |
eventUpdatedComment

const maskDefault = maskLegacy |
eventUpdatedAssignee |
maskComments

// The keys listed here can be used in the Webhook URL to control what events
// are posted to Mattermost. A matching parameter with a non-empty value must
// be added to turn on the event display.
var eventParamMasks = map[string]uint64{
"updated_attachment": eventUpdatedAttachment, // updated attachments
"updated_description": eventUpdatedDescription, // issue description edited
"updated_labels": eventUpdatedLabels, // updated labels
"updated_prioity": eventUpdatedPriority, // changes in priority
"updated_rank": eventUpdatedRank, // ranked higher or lower
"updated_sprint": eventUpdatedSprint, // assigned to a different sprint
"updated_status": eventUpdatedStatus, // transitions like Done, In Progress
"updated_summary": eventUpdatedSummary, // issue renamed
"updated_all": ^(-1 << eventMax), // all events
}

type JIRAWebhook struct {
WebhookEvent string
Issue JIRAWebhookIssue
User jira.User
Comment struct {
Body string
UpdateAuthor jira.User
}
ChangeLog struct {
WebhookEvent string `json:"webhookEvent,omitempty"`
Issue jira.Issue `json:"issue,omitempty"`
User jira.User `json:"user,omitempty"`
Comment jira.Comment `json:"comment,omitempty"`
ChangeLog struct {
Items []struct {
From string
FromString string
To string
ToString string
Field string
}
}
} `json:"changelog,omitempty"`
IssueEventTypeName string `json:"issue_event_type_name"`
}

type parsedJIRAWebhook struct {
*JIRAWebhook
RawJSON string
events uint64
headline string
details string
text string
Expand Down Expand Up @@ -130,11 +150,24 @@ func httpWebhook(p *Plugin, w http.ResponseWriter, r *http.Request) (int, error)
return appErr.StatusCode, fmt.Errorf(appErr.Message)
}

initPost, err := AsSlackAttachment(r.Body)
parsed, err := parse(r.Body, nil)
if err != nil {
return http.StatusBadRequest, err
}

eventMask := maskDefault
for key, paramMask := range eventParamMasks {
if r.FormValue(key) == "" {
continue
}
eventMask = eventMask | paramMask
}
if parsed.events&eventMask == 0 {
p.debugf("skipping: %q", parsed.headline)
return http.StatusOK, nil
}

slackAttachment := newSlackAttachment(parsed)
post := &model.Post{
ChannelId: channel.Id,
UserId: user.Id,
Expand All @@ -143,7 +176,7 @@ func httpWebhook(p *Plugin, w http.ResponseWriter, r *http.Request) (int, error)
"use_user_icon": "true",
},
}
initPost(post)
model.ParseSlackAttachment(post, []*model.SlackAttachment{slackAttachment})

_, appErr = p.API.CreatePost(post)
if appErr != nil {
Expand Down Expand Up @@ -175,6 +208,9 @@ func parse(in io.Reader, linkf func(w *JIRAWebhook) string) (*parsedJIRAWebhook,
if webhook.WebhookEvent == "" {
return nil, fmt.Errorf("No webhook event")
}
if webhook.Issue.Fields == nil {
return nil, fmt.Errorf("Invalid webhook event")
}

parsed := parsedJIRAWebhook{
JIRAWebhook: &webhook,
Expand All @@ -192,43 +228,52 @@ func parse(in io.Reader, linkf func(w *JIRAWebhook) string) (*parsedJIRAWebhook,
issue := parsed.mdIssueType() + " " + linkf(parsed.JIRAWebhook)
switch parsed.WebhookEvent {
case "jira:issue_created":
parsed.event(eventCreated)
Copy link
Member

Choose a reason for hiding this comment

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

I like the encapsulation, maybe consider completely encapsulating the functionality of the bitset? Or to save time use a library like: https://github.com/willf/bitset

The other thing is there is a mix of concerns in this function. We are creating the bitset but also deriving a headline and other stuff. Maybe split this into multiple functions? Like createEventsBitset and createHeadline, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do a separate PR for 2.1 for this. I am inclined not to bring in an extra dependency for what I consider temporary feature flags. If exposing the bit-logic is not sufficiently intention-revealing, I'd rather just use a go map, it's not a material performance difference in the context.

Copy link
Member

Choose a reason for hiding this comment

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

Alright.

parsed.style = mdRootStyle
headline = fmt.Sprintf("created %v", issue)
parsed.details = parsed.mdIssueCreatedDetails()
parsed.text = parsed.mdIssueDescription()
case "jira:issue_deleted":
parsed.event(eventDeleted)
if parsed.Issue.Fields != nil && parsed.Issue.Fields.Resolution == nil {
parsed.event(eventDeletedUnresolved)
}
headline = fmt.Sprintf("deleted %v", issue)
case "jira:issue_updated":
switch parsed.IssueEventTypeName {
case "issue_assigned":
parsed.event(eventUpdatedAssignee)
headline = fmt.Sprintf("assigned %v to %v", issue, parsed.mdIssueAssignee())

case "issue_updated", "issue_generic":
// text summary, description, updated priority, status, etc.
headline, parsed.text = parsed.fromChangeLog(issue)
}
case "comment_deleted":
parsed.event(eventDeletedComment)
user = &parsed.Comment.UpdateAuthor
headline = fmt.Sprintf("removed a comment from %v", issue)

case "comment_updated":
parsed.event(eventUpdatedComment)
user = &parsed.Comment.UpdateAuthor
headline = fmt.Sprintf("edited a comment in %v", issue)
parsed.text = truncate(parsed.Comment.Body, 3000)

case "comment_created":
parsed.event(eventCreatedComment)
user = &parsed.Comment.UpdateAuthor
headline = fmt.Sprintf("commented on %v", issue)
parsed.text = truncate(parsed.Comment.Body, 3000)
}
if headline == "" {
return nil, fmt.Errorf("Unsupported webhook data: %v", parsed.WebhookEvent)
}
parsed.headline = fmt.Sprintf("%v %v %v", mdUser(user), headline, parsed.mdIssueHashtags())
parsed.headline = fmt.Sprintf("%v %v", mdUser(user), headline)

parsed.authorDisplayName = user.DisplayName
parsed.authorUsername = user.Name
parsed.authorURL = getUserURL(&parsed.Issue, user)
parsed.authorURL = getUserURL(user)
if parsed.Issue.Fields.Assignee != nil {
parsed.assigneeUsername = parsed.Issue.Fields.Assignee.Name
}
Expand All @@ -244,49 +289,66 @@ func (p *parsedJIRAWebhook) fromChangeLog(issue string) (string, string) {
from := item.FromString
switch {
case item.Field == "resolution" && to == "" && from != "":
p.event(eventUpdatedReopened)
return fmt.Sprintf("reopened %v", issue), ""

case item.Field == "resolution" && to != "" && from == "":
p.event(eventUpdatedResolved)
return fmt.Sprintf("resolved %v", issue), ""

case item.Field == "status" && to == "Backlog":
p.event(eventUpdatedStatus)
return fmt.Sprintf("moved %v to backlog", issue), ""

case item.Field == "status" && to == "In Progress":
p.event(eventUpdatedStatus)
return fmt.Sprintf("started working on %v", issue), ""

case item.Field == "status" && to == "Selected for Development":
p.event(eventUpdatedStatus)
return fmt.Sprintf("selected %v for development", issue), ""

case item.Field == "priority" && item.From > item.To:
p.event(eventUpdatedPriority)
return fmt.Sprintf("raised priority of %v to %v", issue, to), ""

case item.Field == "priority" && item.From < item.To:
p.event(eventUpdatedPriority)
return fmt.Sprintf("lowered priority of %v to %v", issue, to), ""

case item.Field == "summary":
p.event(eventUpdatedSummary)
return fmt.Sprintf("renamed %v to %v", issue, p.mdIssueSummary()), ""

case item.Field == "description":
p.event(eventUpdatedDescription)
return fmt.Sprintf("edited description of %v", issue),
p.mdIssueDescription()

case item.Field == "Sprint" && len(to) > 0:
p.event(eventUpdatedSprint)
return fmt.Sprintf("moved %v to %v", issue, to), ""

case item.Field == "Rank" && len(to) > 0:
p.event(eventUpdatedRank)
return fmt.Sprintf("%v %v", strings.ToLower(to), issue), ""

case item.Field == "Attachment":
p.event(eventUpdatedAttachment)
return fmt.Sprintf("%v %v", mdAddRemove(from, to, "attached", "removed attachments"), issue), ""

case item.Field == "labels":
p.event(eventUpdatedLabels)
return fmt.Sprintf("%v %v", mdAddRemove(from, to, "added labels", "removed labels"), issue), ""
}
}
return "", ""
}

func (parsed *parsedJIRAWebhook) event(event uint64) {
parsed.events = parsed.events | event
}

func (p *Plugin) notify(ji Instance, parsed *parsedJIRAWebhook, text string) {
if parsed.authorUsername == "" {
return
Expand Down
33 changes: 19 additions & 14 deletions server/webhook_markdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,47 +65,62 @@ func (w *JIRAWebhook) mdIssueCreatedDetails() string {
}

func (w *JIRAWebhook) mdIssueSummary() string {
if w.Issue.Fields == nil {
return ""
}
return truncate(w.Issue.Fields.Summary, 80)
}

func (w *JIRAWebhook) mdIssueDescription() string {
if w.Issue.Fields == nil {
return ""
}
return truncate(w.Issue.Fields.Description, 3000)
}

func (w *JIRAWebhook) mdIssueAssignee() string {
if w.Issue.Fields == nil {
return ""
}
if w.Issue.Fields.Assignee == nil {
return "_nobody_"
}
return mdUser(w.Issue.Fields.Assignee)
}

func (w *JIRAWebhook) mdIssueAssignedTo() string {
if w.Issue.Fields.Assignee == nil {
if w.Issue.Fields == nil || w.Issue.Fields.Assignee == nil {
return ""
}
return "Assigned to: " + mdBOLD(w.mdIssueAssignee())
}

func (w *JIRAWebhook) mdIssueReportedBy() string {
if w.Issue.Fields.Reporter == nil {
if w.Issue.Fields == nil || w.Issue.Fields.Reporter == nil {
return ""
}
return "Reported by: " + mdBOLD(mdUser(w.Issue.Fields.Reporter))
}

func (w *JIRAWebhook) mdIssueLabels() string {
if len(w.Issue.Fields.Labels) == 0 {
if w.Issue.Fields == nil || len(w.Issue.Fields.Labels) == 0 {
return ""
}
return "Labels: " + strings.Join(w.Issue.Fields.Labels, ",")
}

func (w *JIRAWebhook) mdIssuePriority() string {
if w.Issue.Fields == nil || w.Issue.Fields.Priority == nil {
return ""
}
return "Priority: " + mdBOLD(w.Issue.Fields.Priority.Name)
}

func (w *JIRAWebhook) mdIssueType() string {
return strings.ToLower(w.Issue.Fields.IssueType.Name)
if w.Issue.Fields == nil {
return ""
}
return strings.ToLower(w.Issue.Fields.Type.Name)
}

func (w *JIRAWebhook) mdIssueLongLink() string {
Expand All @@ -116,16 +131,6 @@ func (w *JIRAWebhook) mdIssueLink() string {
return fmt.Sprintf("[%v](%v/browse/%v)", w.Issue.Key, w.jiraURL(), w.Issue.Key)
}

func (w *JIRAWebhook) mdIssueHashtags() string {
s := "("
if w.WebhookEvent == "jira:issue_created" {
s += "#jira-new "
}
s += "#" + w.Issue.Key
s += ")"
return s
}

func mdAddRemove(from, to, add, remove string) string {
added := mdDiff(from, to)
removed := mdDiff(to, from)
Expand Down
Loading