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

Merge FEATURE-create-issue-modal into master #239

Merged
merged 36 commits into from
May 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
96f8770
Adding create issue modal (#123)
dhadiseputro Mar 20, 2020
763816b
fix linting error (#221)
jfrerich Mar 24, 2020
e7c21fa
[GH-219] After creating issue, reply to post in MM with the Gith… (#222)
jfrerich Mar 24, 2020
f6c2182
Add user validation for repo and title fields (#228)
jfrerich Mar 26, 2020
e81835b
Add attach and create post actions (#229)
jfrerich Mar 27, 2020
ca37b05
Complete the Create Issue Modal issues (#232)
jfrerich Apr 14, 2020
1aa6563
Merge branch 'master' into FEATURE-create-issue-modal
jfrerich Apr 14, 2020
1194c2d
PR Review changes
jfrerich Apr 16, 2020
fc19f20
Allow user to edit the Description of the ticket. Continue to initialze
jfrerich Apr 16, 2020
4e7e89e
fix the spinner in the form_button.
jfrerich Apr 16, 2020
8b6718b
Append created issue message with a mention that issue was created from
jfrerich Apr 23, 2020
65788bd
Remove team from permalink function. Not needed with the redirect link
jfrerich Apr 27, 2020
722c7ac
s/createIssueModalVisible/isCreateIssueModalVisible/
jfrerich Apr 27, 2020
d2a2076
send the repo value from the client to be of "owner/repo". Split this
jfrerich Apr 27, 2020
ce678a8
check issue.PostId not empty
jfrerich Apr 27, 2020
f8f4fdd
use lowercase for string comparison
jfrerich Apr 27, 2020
0cabd95
if create issue submission fails with an error, show the error in the…
jfrerich Apr 27, 2020
aa1f643
fix lint error
jfrerich Apr 27, 2020
c1d7b37
remove occurrences of currentTeam from front and backends. This was
jfrerich Apr 27, 2020
e4ed032
Merge remote-tracking branch 'origin/master' into FEATURE-create-issu…
hanzei Apr 29, 2020
8dad1cd
Drop currentTeam
hanzei Apr 29, 2020
529e46e
Simplify getPermaLink
hanzei Apr 29, 2020
a7b9d3c
Update to github.com/google/go-github/v31
hanzei Apr 29, 2020
f4a1cc3
correctly fetch all repos
hanzei Apr 29, 2020
7644332
Fix test
hanzei Apr 29, 2020
4bcb03f
Fix permalink
hanzei Apr 29, 2020
dea3276
Only send down fields that are really needed
hanzei May 7, 2020
495aae1
Link MM account it it's connected
hanzei May 7, 2020
0f9ec54
Wording
hanzei May 8, 2020
de0e8b9
Spelling
hanzei May 8, 2020
003d748
Spelling fix 2
hanzei May 8, 2020
fb1609e
Make code independent of plugin id
hanzei May 8, 2020
40378f2
Simplify props
hanzei May 8, 2020
eab01f5
Return early if modal is not visible
hanzei May 9, 2020
85b7fcb
Check SiteURL on startup
hanzei May 14, 2020
ef63090
Merge remote-tracking branch 'origin/master' into FEATURE-create-issu…
hanzei May 16, 2020
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
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ Once connected, you'll have access to the following features:

* __Daily reminders__ - the first time you log in to Mattermost each day, get a post letting you know what issues and pull requests need your attention
* __Notifications__ - get a direct message in Mattermost when someone mentions you, requests your review, comments on or modifies one of your pull requests/issues, or assigns you on GitHub
* __Post actions__ - create a GitHub issue from a post or attach a post message to an issue
* hover over a post to reveal the post actions menu and click the __More Actions (...)__ option
* __Sidebar buttons__ - stay up-to-date with how many reviews, unread messages, assignments and open pull requests you have with buttons in the Mattermost sidebar
* __Slash commands__ - interact with the GitHub plugin using the `/github` slash command
* __Subscribe to a respository__ - Use `/github subscribe` to subscribe a Mattermost channel to receive notifications for new pull requests, issues, branch creation and more in a GitHub repository.
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go 1.12

require (
github.com/Masterminds/sprig/v3 v3.0.2
github.com/google/go-github/v25 v25.1.1
github.com/google/go-github/v31 v31.0.0
github.com/gorilla/mux v1.7.4
github.com/mattermost/mattermost-server/v5 v5.18.0
github.com/pkg/errors v0.8.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5a
github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
github.com/google/go-github v17.0.0+incompatible h1:N0LgJ1j65A7kfXrZnUDaYCs/Sf4rEjNlfyDHW9dolSY=
github.com/google/go-github v17.0.0+incompatible/go.mod h1:zLgOLi98H3fifZn+44m+umXrS52loVEgC2AApnigrVQ=
github.com/google/go-github/v25 v25.1.1 h1:6eW++i/CXcR5GKfYaaJT7oJJtHNU+/iiw55noEPNVao=
github.com/google/go-github/v25 v25.1.1/go.mod h1:6z5pC69qHtrPJ0sXPsj4BLnd82b+r6sLB7qcBoRZqpw=
github.com/google/go-github/v31 v31.0.0 h1:JJUxlP9lFK+ziXKimTCprajMApV1ecWD4NB6CCb0plo=
github.com/google/go-github/v31 v31.0.0/go.mod h1:NQPZol8/1sMoWYGN2yaALIBytu17gAWfhbweiEed3pM=
github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk=
github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck=
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
Expand Down
245 changes: 206 additions & 39 deletions server/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,21 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/pkg/errors"
"net/http"
"strconv"
"strings"
"sync"
"time"

"github.com/pkg/errors"

"github.com/gorilla/mux"

"github.com/mattermost/mattermost-server/v5/mlog"
"github.com/mattermost/mattermost-server/v5/model"
"github.com/mattermost/mattermost-server/v5/plugin"

"github.com/google/go-github/v25/github"
"github.com/google/go-github/v31/github"
"golang.org/x/oauth2"
)

Expand All @@ -39,6 +40,10 @@ type APIErrorResponse struct {
StatusCode int `json:"status_code"`
}

func (e *APIErrorResponse) Error() string {
return e.Message
}

type PRDetails struct {
URL string `json:"url"`
Number int `json:"number"`
Expand Down Expand Up @@ -73,9 +78,11 @@ func (p *Plugin) initialiseAPI() {
apiRouter.HandleFunc("/prsdetails", p.extractUserMiddleWare(p.getPrsDetails, false)).Methods("POST")
apiRouter.HandleFunc("/searchissues", p.extractUserMiddleWare(p.searchIssues, false)).Methods("GET")
apiRouter.HandleFunc("/yourassignments", p.extractUserMiddleWare(p.getYourAssignments, false)).Methods("GET")
apiRouter.HandleFunc("/createissue", p.extractUserMiddleWare(p.createIssue, false)).Methods("POST")
apiRouter.HandleFunc("/createissuecomment", p.extractUserMiddleWare(p.createIssueComment, false)).Methods("POST")
apiRouter.HandleFunc("/mentions", p.extractUserMiddleWare(p.getMentions, false)).Methods("GET")
apiRouter.HandleFunc("/unreads", p.extractUserMiddleWare(p.getUnreads, false)).Methods("GET")
apiRouter.HandleFunc("/repositories", p.extractUserMiddleWare(p.getRepositories, false)).Methods("GET")
apiRouter.HandleFunc("/settings", p.extractUserMiddleWare(p.updateSettings, false)).Methods("POST")
apiRouter.HandleFunc("/user", p.extractUserMiddleWare(p.getGitHubUser, true)).Methods("POST")
}
Expand Down Expand Up @@ -283,12 +290,11 @@ type ConnectedResponse struct {
}

type CreateIssueCommentRequest struct {
PostId string `json:"post_id"`
Owner string `json:"owner"`
Repo string `json:"repo"`
Number int `json:"number"`
Comment string `json:"comment"`
CurrentTeam string `json:"current_team"`
PostId string `json:"post_id"`
Owner string `json:"owner"`
Repo string `json:"repo"`
Number int `json:"number"`
Comment string `json:"comment"`
}

type GitHubUserRequest struct {
Expand Down Expand Up @@ -644,8 +650,10 @@ func (p *Plugin) searchIssues(w http.ResponseWriter, r *http.Request, userID str
w.Write(resp)
}

func getPermaLink(siteUrl string, postId string, currentTeam string) string {
return fmt.Sprintf("%v/%v/pl/%v", siteUrl, currentTeam, postId)
func (p *Plugin) getPermaLink(postId string) string {
siteUrl := *p.API.GetConfig().ServiceSettings.SiteURL
hanzei marked this conversation as resolved.
Show resolved Hide resolved

return fmt.Sprintf("%v/_redirect/pl/%v", siteUrl, postId)
}

func getFailReason(code int, repo string, username string) string {
Expand All @@ -669,8 +677,7 @@ func getFailReason(code int, repo string, username string) string {

func (p *Plugin) createIssueComment(w http.ResponseWriter, r *http.Request, userID string) {
req := &CreateIssueCommentRequest{}
dec := json.NewDecoder(r.Body)
if err := dec.Decode(&req); err != nil {
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
mlog.Error("Error decoding JSON body", mlog.Err(err))
writeAPIError(w, &APIErrorResponse{ID: "", Message: "Please provide a JSON object.", StatusCode: http.StatusBadRequest})
return
Expand Down Expand Up @@ -701,11 +708,6 @@ func (p *Plugin) createIssueComment(w http.ResponseWriter, r *http.Request, user
return
}

if req.CurrentTeam == "" {
writeAPIError(w, &APIErrorResponse{ID: "", Message: "Please provide a valid team", StatusCode: http.StatusBadRequest})
return
}

ctx := context.Background()

var githubClient *github.Client
Expand All @@ -728,22 +730,10 @@ func (p *Plugin) createIssueComment(w http.ResponseWriter, r *http.Request, user
return
}

commentUsername := ""

if info, apiEr := p.getGitHubUserInfo(post.UserId); apiEr != nil {
if apiEr.ID == API_ERROR_ID_NOT_CONNECTED {
commentUser, appEr := api.GetUser(post.UserId)
if appEr != nil {
writeAPIError(w, &APIErrorResponse{ID: "", Message: "failed to load post.UserID " + post.UserId + ": not found", StatusCode: http.StatusInternalServerError})
return
}
commentUsername = commentUser.Username
} else {
writeAPIError(w, apiEr)
return
}
} else {
commentUsername = info.GitHubUsername
commentUsername, err := p.getUsername(post.UserId)
if err != nil {
writeAPIError(w, &APIErrorResponse{ID: "", Message: "failed to get username", StatusCode: http.StatusInternalServerError})
return
}

currentUsername := ""
Expand All @@ -754,11 +744,8 @@ func (p *Plugin) createIssueComment(w http.ResponseWriter, r *http.Request, user
currentUsername = info.GitHubUsername
}

siteUrl := api.GetConfig().ServiceSettings.SiteURL

permalink := getPermaLink(*siteUrl, req.PostId, req.CurrentTeam)
hanzei marked this conversation as resolved.
Show resolved Hide resolved

permalinkMessage := fmt.Sprintf("*@%s attached a* [message](%s) *from @%s*\n", currentUsername, permalink, commentUsername)
permalink := p.getPermaLink(req.PostId)
permalinkMessage := fmt.Sprintf("*@%s attached a* [message](%s) *from %s*\n\n", currentUsername, permalink, commentUsername)

req.Comment = permalinkMessage + req.Comment
comment := &github.IssueComment{
Expand All @@ -776,7 +763,7 @@ func (p *Plugin) createIssueComment(w http.ResponseWriter, r *http.Request, user
rootId = post.RootId
}

permalinkReplyMessage := fmt.Sprintf("Message attached to [#%v](https://github.com/%v/%v/issues/%v)", req.Number, req.Owner, req.Repo, req.Number)
permalinkReplyMessage := fmt.Sprintf("[Message](%v) attached to GitHub issue [#%v](%v)", permalink, req.Number, result.GetHTMLURL())
reply := &model.Post{
Message: permalinkReplyMessage,
ChannelId: post.ChannelId,
Expand Down Expand Up @@ -869,3 +856,183 @@ func (p *Plugin) updateSettings(w http.ResponseWriter, r *http.Request, userID s
resp, _ := json.Marshal(info.Settings)
w.Write(resp)
}

func (p *Plugin) getRepositories(w http.ResponseWriter, r *http.Request, userID string) {
info, err := p.getGitHubUserInfo(userID)
if err != nil {
writeAPIError(w, err)
return
}

githubClient := p.githubConnect(*info.Token)

ctx := context.Background()
org := p.getConfiguration().GitHubOrg

var allRepos []*github.Repository
opt := github.ListOptions{PerPage: 50}

if org == "" {
for {
repos, resp, err := githubClient.Repositories.List(ctx, "", &github.RepositoryListOptions{ListOptions: opt})
if err != nil {
mlog.Error(err.Error())
return
Copy link
Member

Choose a reason for hiding this comment

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

We are exiting this function without writing to the response. Is this intended? It also happens in the else block below.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no consistent error handling in the code. Returning there just lead to an empty response, which is not wrong. There is room for improvement, but I need to think about it and there are more important issues.

Copy link
Member

Choose a reason for hiding this comment

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

@hanzei I want to know why this is the case. If we don't call Write or WriteHeader (or an equivalent), how does the response get closed? Is it something that the server does automatically for us? Maybe it's my mis-understanding of how http requests are handled in Go.

Copy link
Contributor

Choose a reason for hiding this comment

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

That is part of the http package. Once ServeHTTP terminates and not header has been written, 200 gets send as a response.

Mini example:

	http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {})
	log.Fatal(http.ListenAndServe(":8080", nil))

}
allRepos = append(allRepos, repos...)
if resp.NextPage == 0 {
break
}
opt.Page = resp.NextPage
}
} else {
for {
repos, resp, err := githubClient.Repositories.ListByOrg(ctx, org, &github.RepositoryListByOrgOptions{Sort: "full_name", ListOptions: opt})
if err != nil {
mlog.Error(err.Error())
return
}
allRepos = append(allRepos, repos...)
if resp.NextPage == 0 {
break
}
opt.Page = resp.NextPage
}
}

// Only send down fields to client that are needed
type RepositoryResponse struct {
Name string `json:"name,omitempty"`
FullName string `json:"full_name,omitempty"`
}

response := make([]RepositoryResponse, len(allRepos))
for i, r := range allRepos {
response[i].Name = r.GetName()
response[i].FullName = r.GetFullName()
}

resp, _ := json.Marshal(response)
w.Write(resp)
}

func (p *Plugin) createIssue(w http.ResponseWriter, r *http.Request, userID string) {
type IssueRequest struct {
Title string `json:"title"`
Body string `json:"body"`
Repo string `json:"repo"`
PostId string `json:"post_id"`
}

// get data for the issue from the request body and fill IssueRequest object
issue := &IssueRequest{}
if err := json.NewDecoder(r.Body).Decode(&issue); err != nil {
mickmister marked this conversation as resolved.
Show resolved Hide resolved
mlog.Error("Error decoding JSON body", mlog.Err(err))
writeAPIError(w, &APIErrorResponse{ID: "", Message: "Please provide a JSON object.", StatusCode: http.StatusBadRequest})
return
}

if issue.Title == "" {
writeAPIError(w, &APIErrorResponse{ID: "", Message: "Please provide a valid issue title.", StatusCode: http.StatusBadRequest})
return
}

if issue.Repo == "" {
writeAPIError(w, &APIErrorResponse{ID: "", Message: "Please provide a valid repo name.", StatusCode: http.StatusBadRequest})
return
}

if issue.PostId == "" {
writeAPIError(w, &APIErrorResponse{ID: "", Message: "Please provide a postID", StatusCode: http.StatusBadRequest})
return
}

// Make sure user has a connected github account
info, apiErr := p.getGitHubUserInfo(userID)
if apiErr != nil {
writeAPIError(w, apiErr)
return
}

post, appErr := p.API.GetPost(issue.PostId)
larkox marked this conversation as resolved.
Show resolved Hide resolved
if appErr != nil {
writeAPIError(w, &APIErrorResponse{ID: "", Message: "failed to load post " + issue.PostId, StatusCode: http.StatusInternalServerError})
return
}
if post == nil {
writeAPIError(w, &APIErrorResponse{ID: "", Message: "failed to load post " + issue.PostId + ": not found", StatusCode: http.StatusNotFound})
return
}

username, err := p.getUsername(post.UserId)
if err != nil {
writeAPIError(w, &APIErrorResponse{ID: "", Message: "failed to get username", StatusCode: http.StatusInternalServerError})
return
}

ghIssue := &github.IssueRequest{
Title: &issue.Title,
Body: &issue.Body,
}

permalink := p.getPermaLink(issue.PostId)

mmMessage := fmt.Sprintf("_Issue created from a [Mattermost message](%v) *by %s*._", permalink, username)

if ghIssue.GetBody() != "" {
mmMessage = "\n\n" + mmMessage
}
*ghIssue.Body = ghIssue.GetBody() + mmMessage

currentUser, appErr := p.API.GetUser(userID)
if appErr != nil {
writeAPIError(w, &APIErrorResponse{ID: "", Message: "failed to load current user", StatusCode: http.StatusInternalServerError})
return
}

splittedRepo := strings.Split(issue.Repo, "/")
owner := splittedRepo[0]
repoName := splittedRepo[1]

githubClient := p.githubConnect(*info.Token)
result, resp, err := githubClient.Issues.Create(context.Background(), owner, repoName, ghIssue)
if err != nil {
writeAPIError(w,
&APIErrorResponse{
ID: "",
Message: "failed to create issue: " + getFailReason(resp.StatusCode,
issue.Repo,
currentUser.Username),
StatusCode: resp.StatusCode,
})
return
}

if resp.Response.StatusCode == http.StatusGone {
writeAPIError(w, &APIErrorResponse{ID: "", Message: "Issues are disabled on this repository.", StatusCode: http.StatusMethodNotAllowed})
return
}

rootId := issue.PostId
if post.RootId != "" {
rootId = post.RootId
}

message := fmt.Sprintf("Created GitHub issue [#%v](%v) from a [message](%s)", result.GetNumber(), result.GetHTMLURL(), permalink)
reply := &model.Post{
Message: message,
ChannelId: post.ChannelId,
RootId: rootId,
ParentId: rootId,
UserId: userID,
}

_, appErr = p.API.CreatePost(reply)
if appErr != nil {
writeAPIError(w, &APIErrorResponse{ID: "", Message: "failed to create notification post " + issue.PostId, StatusCode: http.StatusInternalServerError})
return
}

json_r, _ := json.Marshal(result)
w.Write(json_r)
}
2 changes: 1 addition & 1 deletion server/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/mattermost/mattermost-server/v5/mlog"
"github.com/mattermost/mattermost-server/v5/plugin"

"github.com/google/go-github/v25/github"
"github.com/google/go-github/v31/github"
"github.com/mattermost/mattermost-server/v5/model"
)

Expand Down
2 changes: 1 addition & 1 deletion server/permalinks.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"strings"
"time"

"github.com/google/go-github/v25/github"
"github.com/google/go-github/v31/github"
)

// maxPermalinkReplacements sets the maximum limit to the number of
Expand Down
2 changes: 1 addition & 1 deletion server/permalinks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"net/url"
"testing"

"github.com/google/go-github/v25/github"
"github.com/google/go-github/v31/github"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
Expand Down
Loading