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

Fix #302. Show referencing commits in issue.
This commit is contained in:
无闻 2014-07-24 04:25:00 -04:00
commit a76a948a02
6 changed files with 254 additions and 42 deletions

View file

@ -8,8 +8,10 @@ import (
"encoding/json" "encoding/json"
"errors" "errors"
"fmt" "fmt"
"regexp"
"strings" "strings"
"time" "time"
"unicode"
"github.com/gogits/git" "github.com/gogits/git"
@ -32,6 +34,20 @@ const (
OP_COMMENT_ISSUE 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., // Action represents user operation type and other information to repository.,
// it implemented interface base.Actioner so that can be used in template render. // it implemented interface base.Actioner so that can be used in template render.
type Action struct { type Action struct {
@ -78,6 +94,81 @@ func (a Action) GetContent() string {
return a.Content 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. // CommitRepoAction adds new action for committing repository.
func CommitRepoAction(userId, repoUserId int64, userName, actEmail string, func CommitRepoAction(userId, repoUserId int64, userName, actEmail string,
repoId int64, repoUserName, repoName string, refFullName string, commit *base.PushCommits) error { 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()) 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, if err = NotifyWatchers(&Action{ActUserId: userId, ActUserName: userName, ActEmail: actEmail,
OpType: opType, Content: string(bs), RepoId: repoId, RepoUserName: repoUserName, OpType: opType, Content: string(bs), RepoId: repoId, RepoUserName: repoUserName,
RepoName: repoName, RefName: refName, RepoName: repoName, RefName: refName,

View file

@ -7,6 +7,8 @@ package models
import ( import (
"bytes" "bytes"
"errors" "errors"
"html/template"
"strconv"
"strings" "strings"
"time" "time"
@ -20,6 +22,7 @@ var (
ErrLabelNotExist = errors.New("Label does not exist") ErrLabelNotExist = errors.New("Label does not exist")
ErrMilestoneNotExist = errors.New("Milestone does not exist") ErrMilestoneNotExist = errors.New("Milestone does not exist")
ErrWrongIssueCounter = errors.New("Invalid number of issues for this milestone") 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. // Issue represents an issue or pull request of repository.
@ -122,6 +125,29 @@ func NewIssue(issue *Issue) (err error) {
return 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. // GetIssueByIndex returns issue by given index in repository.
func GetIssueByIndex(rid, index int64) (*Issue, error) { func GetIssueByIndex(rid, index int64) (*Issue, error) {
issue := &Issue{RepoId: rid, Index: index} issue := &Issue{RepoId: rid, Index: index}
@ -400,6 +426,11 @@ func GetUserIssueStats(uid int64, filterMode int) *IssueStats {
// UpdateIssue updates information of issue. // UpdateIssue updates information of issue.
func UpdateIssue(issue *Issue) error { func UpdateIssue(issue *Issue) error {
_, err := x.Id(issue.Id).AllCols().Update(issue) _, err := x.Id(issue.Id).AllCols().Update(issue)
if err != nil {
return err
}
return err return err
} }
@ -670,6 +701,32 @@ func ChangeMilestoneStatus(m *Milestone, isClosed bool) (err error) {
return sess.Commit() 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. // ChangeMilestoneAssign changes assignment of milestone for issue.
func ChangeMilestoneAssign(oldMid, mid int64, issue *Issue) (err error) { func ChangeMilestoneAssign(oldMid, mid int64, issue *Issue) (err error) {
sess := x.NewSession() sess := x.NewSession()
@ -693,6 +750,7 @@ func ChangeMilestoneAssign(oldMid, mid int64, issue *Issue) (err error) {
} else { } else {
m.Completeness = 0 m.Completeness = 0
} }
if _, err = sess.Id(m.Id).Update(m); err != nil { if _, err = sess.Id(m.Id).Update(m); err != nil {
sess.Rollback() sess.Rollback()
return err return err
@ -710,6 +768,7 @@ func ChangeMilestoneAssign(oldMid, mid int64, issue *Issue) (err error) {
if err != nil { if err != nil {
return err return err
} }
m.NumIssues++ m.NumIssues++
if issue.IsClosed { if issue.IsClosed {
m.NumClosedIssues++ m.NumClosedIssues++
@ -731,6 +790,7 @@ func ChangeMilestoneAssign(oldMid, mid int64, issue *Issue) (err error) {
return err return err
} }
} }
return sess.Commit() 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 ( const (
IT_PLAIN = iota // Pure comment. // Plain comment, can be associated with a commit (CommitId > 0) and a line (Line > 0)
IT_REOPEN // Issue reopen status change prompt. COMMENT CommentType = iota
IT_CLOSE // Issue close status change prompt.
// 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. // Comment represents a comment in commit and issue page.
type Comment struct { type Comment struct {
Id int64 Id int64
Type int Type CommentType
PosterId int64 PosterId int64
Poster *User `xorm:"-"` Poster *User `xorm:"-"`
IssueId int64 IssueId int64
@ -795,7 +871,7 @@ type Comment struct {
} }
// CreateComment creates comment of issue or commit. // 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() sess := x.NewSession()
defer sess.Close() defer sess.Close()
if err := sess.Begin(); err != nil { if err := sess.Begin(); err != nil {
@ -810,19 +886,19 @@ func CreateComment(userId, repoId, issueId, commitId, line int64, cmtType int, c
// Check comment type. // Check comment type.
switch cmtType { switch cmtType {
case IT_PLAIN: case COMMENT:
rawSql := "UPDATE `issue` SET num_comments = num_comments + 1 WHERE id = ?" rawSql := "UPDATE `issue` SET num_comments = num_comments + 1 WHERE id = ?"
if _, err := sess.Exec(rawSql, issueId); err != nil { if _, err := sess.Exec(rawSql, issueId); err != nil {
sess.Rollback() sess.Rollback()
return err return err
} }
case IT_REOPEN: case REOPEN:
rawSql := "UPDATE `repository` SET num_closed_issues = num_closed_issues - 1 WHERE id = ?" rawSql := "UPDATE `repository` SET num_closed_issues = num_closed_issues - 1 WHERE id = ?"
if _, err := sess.Exec(rawSql, repoId); err != nil { if _, err := sess.Exec(rawSql, repoId); err != nil {
sess.Rollback() sess.Rollback()
return err return err
} }
case IT_CLOSE: case CLOSE:
rawSql := "UPDATE `repository` SET num_closed_issues = num_closed_issues + 1 WHERE id = ?" rawSql := "UPDATE `repository` SET num_closed_issues = num_closed_issues + 1 WHERE id = ?"
if _, err := sess.Exec(rawSql, repoId); err != nil { if _, err := sess.Exec(rawSql, repoId); err != nil {
sess.Rollback() sess.Rollback()
@ -832,6 +908,10 @@ func CreateComment(userId, repoId, issueId, commitId, line int64, cmtType int, c
return sess.Commit() return sess.Commit()
} }
func (c *Comment) ContentHtml() template.HTML {
return template.HTML(c.Content)
}
// GetIssueComments returns list of comment by given issue id. // GetIssueComments returns list of comment by given issue id.
func GetIssueComments(issueId int64) ([]Comment, error) { func GetIssueComments(issueId int64) ([]Comment, error) {
comments := make([]Comment, 0, 10) comments := make([]Comment, 0, 10)

View file

@ -7,9 +7,9 @@ package models
import ( import (
"errors" "errors"
"fmt" "fmt"
"io/ioutil"
"html" "html"
"html/template" "html/template"
"io/ioutil"
"os" "os"
"path" "path"
"path/filepath" "path/filepath"
@ -43,6 +43,7 @@ var (
ErrRepoNameIllegal = errors.New("Repository name contains illegal characters") ErrRepoNameIllegal = errors.New("Repository name contains illegal characters")
ErrRepoFileNotLoaded = errors.New("Repository file not loaded") ErrRepoFileNotLoaded = errors.New("Repository file not loaded")
ErrMirrorNotExist = errors.New("Mirror does not exist") ErrMirrorNotExist = errors.New("Mirror does not exist")
ErrInvalidReference = errors.New("Invalid reference specified")
) )
var ( var (
@ -837,6 +838,26 @@ func DeleteRepository(userId, repoId int64, userName string) error {
return sess.Commit() 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. // GetRepositoryByName returns the repository by given name under user if exists.
func GetRepositoryByName(userId int64, repoName string) (*Repository, error) { func GetRepositoryByName(userId int64, repoName string) (*Repository, error) {
repo := &Repository{ repo := &Repository{

View file

@ -1258,9 +1258,16 @@ body {
} }
#issue .issue-child .panel-heading .user, #issue .issue-child .panel-heading .user,
#issue .issue-closed a.user, #issue .issue-closed a.user,
#issue .issue-opened a.user { #issue .issue-opened a.user,
#issue .issue-reference a.user {
font-weight: bold; font-weight: bold;
} }
#issue .issue-child .issue-content .user .avatar {
height: 21px;
width: 21px;
}
#issue .issue-line { #issue .issue-line {
border-color: #CCC; border-color: #CCC;
} }
@ -1280,17 +1287,25 @@ body {
width: 60%; width: 60%;
} }
#issue .issue-closed .issue-content, #issue .issue-closed .issue-content,
#issue .issue-opened .issue-content { #issue .issue-opened .issue-content,
#issue .issue-reference .issue-content {
line-height: 42px; line-height: 42px;
} }
#issue .issue-closed, #issue .issue-closed,
#issue .issue-opened { #issue .issue-opened,
#issue .issue-reference {
border-bottom: 2px solid #CCC; border-bottom: 2px solid #CCC;
margin-bottom: 24px; margin-bottom: 24px;
padding-bottom: 24px; padding-bottom: 24px;
} }
#issue .issue-reference {
padding-bottom: 6px;
}
#issue .issue-closed .label-danger, #issue .issue-closed .label-danger,
#issue .issue-opened .label-success { #issue .issue-opened .label-success,
#issue .issue-reference .label-primary {
margin: 0.8em; margin: 0.8em;
} }
#issue .milestone-item .actions { #issue .milestone-item .actions {

View file

@ -393,8 +393,11 @@ func ViewIssue(ctx *middleware.Context, params martini.Params) {
return return
} }
comments[i].Poster = u comments[i].Poster = u
if comments[i].Type == models.COMMENT {
comments[i].Content = string(base.RenderMarkdown([]byte(comments[i].Content), ctx.Repo.RepoLink)) comments[i].Content = string(base.RenderMarkdown([]byte(comments[i].Content), ctx.Repo.RepoLink))
} }
}
ctx.Data["Title"] = issue.Name ctx.Data["Title"] = issue.Name
ctx.Data["Issue"] = issue ctx.Data["Issue"] = issue
@ -644,30 +647,14 @@ func Comment(ctx *middleware.Context, params martini.Params) {
// Change open/closed issue counter for the associated milestone // Change open/closed issue counter for the associated milestone
if issue.MilestoneId > 0 { if issue.MilestoneId > 0 {
l, err := models.GetMilestoneById(issue.MilestoneId) if err = models.ChangeMilestoneIssueStats(issue); err != nil {
ctx.Handle(500, "issue.Comment(ChangeMilestoneIssueStats)", err)
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
} }
} }
cmtType := models.IT_CLOSE cmtType := models.CLOSE
if !issue.IsClosed { 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 { 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 { if len(content) > 0 {
switch params["action"] { switch params["action"] {
case "new": 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) ctx.Handle(500, "issue.Comment(create comment)", err)
return return
} }

View file

@ -49,6 +49,7 @@
</div> </div>
</div> </div>
{{range .Comments}} {{range .Comments}}
{{/* 0 = COMMENT, 1 = REOPEN, 2 = CLOSE, 3 = ISSUE, 4 = COMMIT, 5 = PULL */}}
{{if eq .Type 0}} {{if eq .Type 0}}
<div class="issue-child" id="issue-comment-{{.Id}}"> <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> <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> <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>
</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}}
{{end}} {{end}}
<hr class="issue-line"/> <hr class="issue-line"/>