Merge pull request #305 from nuss-justin/feature/commit-issue-fix

Fix #302. Show referencing commits in issue.

6 files changed, 254 insertions, 42 deletions

diff --git a/models/action.go b/models/action.go
index 55557da2ff..362b238f26 100644
--- a/models/action.go
+++ b/models/action.go
@@ -8,8 +8,10 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"regexp"
 	"strings"
 	"time"
+	"unicode"
 
 	"github.com/gogits/git"
 
@@ -32,6 +34,20 @@ const (
 	OP_COMMENT_ISSUE
 )
 
+var (
+	ErrNotImplemented = errors.New("Not implemented yet")
+)
+
+var (
+	// Same as Github. See https://help.github.com/articles/closing-issues-via-commit-messages
+	IssueKeywords    = []string{"close", "closes", "closed", "fix", "fixes", "fixed", "resolve", "resolves", "resolved"}
+	IssueKeywordsPat *regexp.Regexp
+)
+
+func init() {
+	IssueKeywordsPat = regexp.MustCompile(fmt.Sprintf(`(?i)(?:%s) \S+`, strings.Join(IssueKeywords, "|")))
+}
+
 // Action represents user operation type and other information to repository.,
 // it implemented interface base.Actioner so that can be used in template render.
 type Action struct {
@@ -78,6 +94,81 @@ func (a Action) GetContent() string {
 	return a.Content
 }
 
+func updateIssuesCommit(userId, repoId int64, repoUserName, repoName string, commits []*base.PushCommit) error {
+	for _, c := range commits {
+		refs := IssueKeywordsPat.FindAllString(c.Message, -1)
+
+		for _, ref := range refs {
+			ref := ref[strings.IndexByte(ref, byte(' '))+1:]
+			ref = strings.TrimRightFunc(ref, func(c rune) bool {
+				return !unicode.IsDigit(c)
+			})
+
+			if len(ref) == 0 {
+				continue
+			}
+
+			// Add repo name if missing
+			if ref[0] == '#' {
+				ref = fmt.Sprintf("%s/%s%s", repoUserName, repoName, ref)
+			} else if strings.Contains(ref, "/") == false {
+				// We don't support User#ID syntax yet
+				// return ErrNotImplemented
+
+				continue
+			}
+
+			issue, err := GetIssueByRef(ref)
+
+			if err != nil {
+				return err
+			}
+
+			url := fmt.Sprintf("/%s/%s/commit/%s", repoUserName, repoName, c.Sha1)
+			message := fmt.Sprintf(`<a href="%s">%s</a>`, url, c.Message)
+
+			if err = CreateComment(userId, issue.RepoId, issue.Id, 0, 0, COMMIT, message); err != nil {
+				return err
+			}
+
+			if issue.RepoId == repoId {
+				if issue.IsClosed {
+					continue
+				}
+
+				issue.IsClosed = true
+
+				if err = UpdateIssue(issue); err != nil {
+					return err
+				}
+
+				issue.Repo, err = GetRepositoryById(issue.RepoId)
+
+				if err != nil {
+					return err
+				}
+
+				issue.Repo.NumClosedIssues++
+
+				if err = UpdateRepository(issue.Repo); err != nil {
+					return err
+				}
+
+				if err = ChangeMilestoneIssueStats(issue); err != nil {
+					return err
+				}
+
+				// If commit happened in the referenced repository, it means the issue can be closed.
+				if err = CreateComment(userId, repoId, issue.Id, 0, 0, CLOSE, ""); err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	return nil
+}
+
 // CommitRepoAction adds new action for committing repository.
 func CommitRepoAction(userId, repoUserId int64, userName, actEmail string,
 	repoId int64, repoUserName, repoName string, refFullName string, commit *base.PushCommits) error {
@@ -107,6 +198,12 @@ func CommitRepoAction(userId, repoUserId int64, userName, actEmail string,
 		return errors.New("action.CommitRepoAction(UpdateRepository): " + err.Error())
 	}
 
+	err = updateIssuesCommit(userId, repoId, repoUserName, repoName, commit.Commits)
+
+	if err != nil {
+		log.Debug("action.CommitRepoAction(updateIssuesCommit): ", err)
+	}
+
 	if err = NotifyWatchers(&Action{ActUserId: userId, ActUserName: userName, ActEmail: actEmail,
 		OpType: opType, Content: string(bs), RepoId: repoId, RepoUserName: repoUserName,
 		RepoName: repoName, RefName: refName,
diff --git a/models/issue.go b/models/issue.go
index 6253837237..fb84ffa841 100644
--- a/models/issue.go
+++ b/models/issue.go
@@ -7,6 +7,8 @@ package models
 import (
 	"bytes"
 	"errors"
+	"html/template"
+	"strconv"
 	"strings"
 	"time"
 
@@ -16,10 +18,11 @@ import (
 )
 
 var (
-	ErrIssueNotExist     = errors.New("Issue does not exist")
-	ErrLabelNotExist     = errors.New("Label does not exist")
-	ErrMilestoneNotExist = errors.New("Milestone does not exist")
-	ErrWrongIssueCounter = errors.New("Invalid number of issues for this milestone")
+	ErrIssueNotExist      = errors.New("Issue does not exist")
+	ErrLabelNotExist      = errors.New("Label does not exist")
+	ErrMilestoneNotExist  = errors.New("Milestone does not exist")
+	ErrWrongIssueCounter  = errors.New("Invalid number of issues for this milestone")
+	ErrMissingIssueNumber = errors.New("No issue number specified")
 )
 
 // Issue represents an issue or pull request of repository.
@@ -122,6 +125,29 @@ func NewIssue(issue *Issue) (err error) {
 	return
 }
 
+// GetIssueByRef returns an Issue specified by a GFM reference.
+// See https://help.github.com/articles/writing-on-github#references for more information on the syntax.
+func GetIssueByRef(ref string) (issue *Issue, err error) {
+	var issueNumber int64
+	var repo *Repository
+
+	n := strings.IndexByte(ref, byte('#'))
+
+	if n == -1 {
+		return nil, ErrMissingIssueNumber
+	}
+
+	if issueNumber, err = strconv.ParseInt(ref[n+1:], 10, 64); err != nil {
+		return
+	}
+
+	if repo, err = GetRepositoryByRef(ref[:n]); err != nil {
+		return
+	}
+
+	return GetIssueByIndex(repo.Id, issueNumber)
+}
+
 // GetIssueByIndex returns issue by given index in repository.
 func GetIssueByIndex(rid, index int64) (*Issue, error) {
 	issue := &Issue{RepoId: rid, Index: index}
@@ -400,6 +426,11 @@ func GetUserIssueStats(uid int64, filterMode int) *IssueStats {
 // UpdateIssue updates information of issue.
 func UpdateIssue(issue *Issue) error {
 	_, err := x.Id(issue.Id).AllCols().Update(issue)
+
+	if err != nil {
+		return err
+	}
+
 	return err
 }
 
@@ -670,6 +701,32 @@ func ChangeMilestoneStatus(m *Milestone, isClosed bool) (err error) {
 	return sess.Commit()
 }
 
+// ChangeMilestoneIssueStats updates the open/closed issues counter and progress for the
+// milestone associated witht the given issue.
+func ChangeMilestoneIssueStats(issue *Issue) error {
+	if issue.MilestoneId == 0 {
+		return nil
+	}
+
+	m, err := GetMilestoneById(issue.MilestoneId)
+
+	if err != nil {
+		return err
+	}
+
+	if issue.IsClosed {
+		m.NumOpenIssues--
+		m.NumClosedIssues++
+	} else {
+		m.NumOpenIssues++
+		m.NumClosedIssues--
+	}
+
+	m.Completeness = m.NumClosedIssues * 100 / m.NumIssues
+
+	return UpdateMilestone(m)
+}
+
 // ChangeMilestoneAssign changes assignment of milestone for issue.
 func ChangeMilestoneAssign(oldMid, mid int64, issue *Issue) (err error) {
 	sess := x.NewSession()
@@ -693,6 +750,7 @@ func ChangeMilestoneAssign(oldMid, mid int64, issue *Issue) (err error) {
 		} else {
 			m.Completeness = 0
 		}
+
 		if _, err = sess.Id(m.Id).Update(m); err != nil {
 			sess.Rollback()
 			return err
@@ -710,6 +768,7 @@ func ChangeMilestoneAssign(oldMid, mid int64, issue *Issue) (err error) {
 		if err != nil {
 			return err
 		}
+
 		m.NumIssues++
 		if issue.IsClosed {
 			m.NumClosedIssues++
@@ -731,6 +790,7 @@ func ChangeMilestoneAssign(oldMid, mid int64, issue *Issue) (err error) {
 			return err
 		}
 	}
+
 	return sess.Commit()
 }
 
@@ -774,17 +834,33 @@ func DeleteMilestone(m *Milestone) (err error) {
 //  \______  /\____/|__|_|  /__|_|  /\___  >___|  /__|
 //         \/             \/      \/     \/     \/
 
-// Issue types.
+// CommentType defines whether a comment is just a simple comment, an action (like close) or a reference.
+type CommentType int
+
 const (
-	IT_PLAIN  = iota // Pure comment.
-	IT_REOPEN        // Issue reopen status change prompt.
-	IT_CLOSE         // Issue close status change prompt.
+	// Plain comment, can be associated with a commit (CommitId > 0) and a line (Line > 0)
+	COMMENT CommentType = iota
+
+	// Reopen action
+	REOPEN
+
+	// Close action
+	CLOSE
+
+	// Reference from another issue
+	ISSUE
+
+	// Reference from some commit (not part of a pull request)
+	COMMIT
+
+	// Reference from some pull request
+	PULL
 )
 
 // Comment represents a comment in commit and issue page.
 type Comment struct {
 	Id       int64
-	Type     int
+	Type     CommentType
 	PosterId int64
 	Poster   *User `xorm:"-"`
 	IssueId  int64
@@ -795,7 +871,7 @@ type Comment struct {
 }
 
 // CreateComment creates comment of issue or commit.
-func CreateComment(userId, repoId, issueId, commitId, line int64, cmtType int, content string) error {
+func CreateComment(userId, repoId, issueId, commitId, line int64, cmtType CommentType, content string) error {
 	sess := x.NewSession()
 	defer sess.Close()
 	if err := sess.Begin(); err != nil {
@@ -810,19 +886,19 @@ func CreateComment(userId, repoId, issueId, commitId, line int64, cmtType int, c
 
 	// Check comment type.
 	switch cmtType {
-	case IT_PLAIN:
+	case COMMENT:
 		rawSql := "UPDATE `issue` SET num_comments = num_comments + 1 WHERE id = ?"
 		if _, err := sess.Exec(rawSql, issueId); err != nil {
 			sess.Rollback()
 			return err
 		}
-	case IT_REOPEN:
+	case REOPEN:
 		rawSql := "UPDATE `repository` SET num_closed_issues = num_closed_issues - 1 WHERE id = ?"
 		if _, err := sess.Exec(rawSql, repoId); err != nil {
 			sess.Rollback()
 			return err
 		}
-	case IT_CLOSE:
+	case CLOSE:
 		rawSql := "UPDATE `repository` SET num_closed_issues = num_closed_issues + 1 WHERE id = ?"
 		if _, err := sess.Exec(rawSql, repoId); err != nil {
 			sess.Rollback()
@@ -832,6 +908,10 @@ func CreateComment(userId, repoId, issueId, commitId, line int64, cmtType int, c
 	return sess.Commit()
 }
 
+func (c *Comment) ContentHtml() template.HTML {
+	return template.HTML(c.Content)
+}
+
 // GetIssueComments returns list of comment by given issue id.
 func GetIssueComments(issueId int64) ([]Comment, error) {
 	comments := make([]Comment, 0, 10)
diff --git a/models/repo.go b/models/repo.go
index 845c1b75a9..8dda6f0d03 100644
--- a/models/repo.go
+++ b/models/repo.go
@@ -7,9 +7,9 @@ package models
 import (
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"html"
 	"html/template"
+	"io/ioutil"
 	"os"
 	"path"
 	"path/filepath"
@@ -43,6 +43,7 @@ var (
 	ErrRepoNameIllegal   = errors.New("Repository name contains illegal characters")
 	ErrRepoFileNotLoaded = errors.New("Repository file not loaded")
 	ErrMirrorNotExist    = errors.New("Mirror does not exist")
+	ErrInvalidReference  = errors.New("Invalid reference specified")
 )
 
 var (
@@ -837,6 +838,26 @@ func DeleteRepository(userId, repoId int64, userName string) error {
 	return sess.Commit()
 }
 
+// GetRepositoryByRef returns a Repository specified by a GFM reference.
+// See https://help.github.com/articles/writing-on-github#references for more information on the syntax.
+func GetRepositoryByRef(ref string) (*Repository, error) {
+	n := strings.IndexByte(ref, byte('/'))
+
+	if n < 2 {
+		return nil, ErrInvalidReference
+	}
+
+	userName, repoName := ref[:n], ref[n+1:]
+
+	user, err := GetUserByName(userName)
+
+	if err != nil {
+		return nil, err
+	}
+
+	return GetRepositoryByName(user.Id, repoName)
+}
+
 // GetRepositoryByName returns the repository by given name under user if exists.
 func GetRepositoryByName(userId int64, repoName string) (*Repository, error) {
 	repo := &Repository{
@@ -1017,4 +1038,4 @@ func IsWatching(uid, rid int64) bool {
 
 func ForkRepository(repoName string, uid int64) {
 
-}
\ No newline at end of file
+}
diff --git a/public/css/gogs.css b/public/css/gogs.css
index c6554b7803..710d0c20b1 100755
--- a/public/css/gogs.css
+++ b/public/css/gogs.css
@@ -1258,9 +1258,16 @@ body {
 }
 #issue .issue-child .panel-heading .user,
 #issue .issue-closed a.user,
-#issue .issue-opened a.user {
+#issue .issue-opened a.user,
+#issue .issue-reference a.user {
     font-weight: bold;
 }
+
+#issue .issue-child .issue-content .user .avatar {
+    height: 21px;
+    width: 21px;
+} 
+
 #issue .issue-line {
     border-color: #CCC;
 }
@@ -1280,18 +1287,26 @@ body {
     width: 60%;
 }
 #issue .issue-closed .issue-content,
-#issue .issue-opened .issue-content {
+#issue .issue-opened .issue-content,
+#issue .issue-reference .issue-content {
     line-height: 42px;
 }
 #issue .issue-closed,
-#issue .issue-opened {
+#issue .issue-opened,
+#issue .issue-reference {
     border-bottom: 2px solid #CCC;
     margin-bottom: 24px;
     padding-bottom: 24px;
 }
+
+#issue .issue-reference {
+    padding-bottom: 6px;
+}
+
 #issue .issue-closed .label-danger,
-#issue .issue-opened .label-success {
-    margin: 0 .8em;
+#issue .issue-opened .label-success,
+#issue .issue-reference .label-primary {
+    margin: 0.8em;
 }
 #issue .milestone-item .actions {
     margin-top: 10px;
diff --git a/routers/repo/issue.go b/routers/repo/issue.go
index e71c835fd8..2d1c23d4b9 100644
--- a/routers/repo/issue.go
+++ b/routers/repo/issue.go
@@ -393,7 +393,10 @@ func ViewIssue(ctx *middleware.Context, params martini.Params) {
 			return
 		}
 		comments[i].Poster = u
-		comments[i].Content = string(base.RenderMarkdown([]byte(comments[i].Content), ctx.Repo.RepoLink))
+
+		if comments[i].Type == models.COMMENT {
+			comments[i].Content = string(base.RenderMarkdown([]byte(comments[i].Content), ctx.Repo.RepoLink))
+		}
 	}
 
 	ctx.Data["Title"] = issue.Name
@@ -644,30 +647,14 @@ func Comment(ctx *middleware.Context, params martini.Params) {
 
 			// Change open/closed issue counter for the associated milestone
 			if issue.MilestoneId > 0 {
-				l, err := models.GetMilestoneById(issue.MilestoneId)
-
-				if err != nil {
-					ctx.Handle(500, "issue.Comment(GetLabelById)", err)
-					return
-				}
-
-				if issue.IsClosed {
-					l.NumOpenIssues = l.NumOpenIssues - 1
-					l.NumClosedIssues = l.NumClosedIssues + 1
-				} else {
-					l.NumOpenIssues = l.NumOpenIssues + 1
-					l.NumClosedIssues = l.NumClosedIssues - 1
-				}
-
-				if err = models.UpdateMilestone(l); err != nil {
-					ctx.Handle(500, "issue.Comment(UpdateLabel)", err)
-					return
+				if err = models.ChangeMilestoneIssueStats(issue); err != nil {
+					ctx.Handle(500, "issue.Comment(ChangeMilestoneIssueStats)", err)
 				}
 			}
 
-			cmtType := models.IT_CLOSE
+			cmtType := models.CLOSE
 			if !issue.IsClosed {
-				cmtType = models.IT_REOPEN
+				cmtType = models.REOPEN
 			}
 
 			if err = models.CreateComment(ctx.User.Id, ctx.Repo.Repository.Id, issue.Id, 0, 0, cmtType, ""); err != nil {
@@ -683,7 +670,7 @@ func Comment(ctx *middleware.Context, params martini.Params) {
 	if len(content) > 0 {
 		switch params["action"] {
 		case "new":
-			if err = models.CreateComment(ctx.User.Id, ctx.Repo.Repository.Id, issue.Id, 0, 0, models.IT_PLAIN, content); err != nil {
+			if err = models.CreateComment(ctx.User.Id, ctx.Repo.Repository.Id, issue.Id, 0, 0, models.COMMENT, content); err != nil {
 				ctx.Handle(500, "issue.Comment(create comment)", err)
 				return
 			}
diff --git a/templates/repo/issue/view.tmpl b/templates/repo/issue/view.tmpl
index d95fba40d8..653734767e 100644
--- a/templates/repo/issue/view.tmpl
+++ b/templates/repo/issue/view.tmpl
@@ -49,6 +49,7 @@
                         </div>
                     </div>
                     {{range .Comments}}
+                    {{/* 0 = COMMENT, 1 = REOPEN, 2 = CLOSE, 3 = ISSUE, 4 = COMMIT, 5 = PULL */}}
                     {{if eq .Type 0}}
                     <div class="issue-child" id="issue-comment-{{.Id}}">
                         <a class="user pull-left" href="/user/{{.Poster.Name}}"><img class="avatar" src="{{.Poster.AvatarLink}}" alt=""/></a>
@@ -78,6 +79,17 @@
                             <a class="user pull-left" href="/user/{{.Poster.Name}}">{{.Poster.Name}}</a> <span class="label label-danger">Closed</span> this issue <span class="time">{{TimeSince .Created}}</span>
                         </div>
                     </div>
+                    {{else if eq .Type 4}}
+                    <div class="issue-child issue-reference issue-reference-commit">
+                        <a class="user pull-left" href="/user/{{.Poster.Name}}"><img class="avatar" src="{{.Poster.AvatarLink}}" alt=""/></a>
+                        <div class="issue-content">
+                            <a class="user pull-left" href="/user/{{.Poster.Name}}">{{.Poster.Name}}</a> <span class="label label-primary">Referenced</span> this issue <span class="time">{{TimeSince .Created}}</span>
+                            <p>
+                                <a class="user pull-left" href="/user/{{.Poster.Name}}"><img class="avatar" src="{{.Poster.AvatarLink}}" alt=""/></a>
+                                {{.ContentHtml}}
+                            </p>
+                        </div>
+                    </div>
                     {{end}}
                     {{end}}
                     <hr class="issue-line"/>