diff options
| -rw-r--r-- | models/issue_xref.go | 103 | ||||
| -rw-r--r-- | models/issue_xref_test.go | 164 | ||||
| -rw-r--r-- | modules/markup/html.go | 12 | ||||
| -rw-r--r-- | modules/references/references.go | 5 | ||||
| -rw-r--r-- | options/locale/locale_en-US.ini | 11 | ||||
| -rw-r--r-- | services/pull/merge.go | 22 | ||||
| -rw-r--r-- | templates/repo/issue/view_content/comments.tmpl | 29 |
7 files changed, 300 insertions, 46 deletions
diff --git a/models/issue_xref.go b/models/issue_xref.go index 787a0e5119..41937dc675 100644 --- a/models/issue_xref.go +++ b/models/issue_xref.go @@ -5,6 +5,8 @@ package models import ( + "fmt" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/references" @@ -27,13 +29,9 @@ type crossReferencesContext struct { func neuterCrossReferences(e Engine, issueID int64, commentID int64) error { active := make([]*Comment, 0, 10) - sess := e.Where("`ref_action` IN (?, ?, ?)", references.XRefActionNone, references.XRefActionCloses, references.XRefActionReopens) - if issueID != 0 { - sess = sess.And("`ref_issue_id` = ?", issueID) - } - if commentID != 0 { - sess = sess.And("`ref_comment_id` = ?", commentID) - } + sess := e.Where("`ref_action` IN (?, ?, ?)", references.XRefActionNone, references.XRefActionCloses, references.XRefActionReopens). + And("`ref_issue_id` = ?", issueID). + And("`ref_comment_id` = ?", commentID) if err := sess.Find(&active); err != nil || len(active) == 0 { return err } @@ -87,7 +85,7 @@ func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesC RefIssueID: ctx.OrigIssue.ID, RefCommentID: refCommentID, RefAction: xref.Action, - RefIsPull: xref.Issue.IsPull, + RefIsPull: ctx.OrigIssue.IsPull, }); err != nil { return err } @@ -98,9 +96,10 @@ func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesC func (issue *Issue) getCrossReferences(e *xorm.Session, ctx *crossReferencesContext, plaincontent, mdcontent string) ([]*crossReference, error) { xreflist := make([]*crossReference, 0, 5) var ( - refRepo *Repository - refIssue *Issue - err error + refRepo *Repository + refIssue *Issue + refAction references.XRefAction + err error ) allrefs := append(references.FindAllIssueReferences(plaincontent), references.FindAllIssueReferencesMarkdown(mdcontent)...) @@ -122,15 +121,13 @@ func (issue *Issue) getCrossReferences(e *xorm.Session, ctx *crossReferencesCont return nil, err } } - if refIssue, err = ctx.OrigIssue.findReferencedIssue(e, ctx, refRepo, ref.Index); err != nil { + if refIssue, refAction, err = ctx.OrigIssue.verifyReferencedIssue(e, ctx, refRepo, ref); err != nil { return nil, err } if refIssue != nil { xreflist = ctx.OrigIssue.updateCrossReferenceList(xreflist, &crossReference{ - Issue: refIssue, - // FIXME: currently ignore keywords - // Action: ref.Action, - Action: references.XRefActionNone, + Issue: refIssue, + Action: refAction, }) } } @@ -153,25 +150,42 @@ func (issue *Issue) updateCrossReferenceList(list []*crossReference, xref *cross return append(list, xref) } -func (issue *Issue) findReferencedIssue(e Engine, ctx *crossReferencesContext, repo *Repository, index int64) (*Issue, error) { - refIssue := &Issue{RepoID: repo.ID, Index: index} +// verifyReferencedIssue will check if the referenced issue exists, and whether the doer has permission to do what +func (issue *Issue) verifyReferencedIssue(e Engine, ctx *crossReferencesContext, repo *Repository, + ref references.IssueReference) (*Issue, references.XRefAction, error) { + + refIssue := &Issue{RepoID: repo.ID, Index: ref.Index} + refAction := ref.Action + if has, _ := e.Get(refIssue); !has { - return nil, nil + return nil, references.XRefActionNone, nil } if err := refIssue.loadRepo(e); err != nil { - return nil, err + return nil, references.XRefActionNone, err } - // Check user permissions - if refIssue.RepoID != ctx.OrigIssue.RepoID { + + // Close/reopen actions can only be set from pull requests to issues + if refIssue.IsPull || !issue.IsPull { + refAction = references.XRefActionNone + } + + // Check doer permissions; set action to None if the doer can't change the destination + if refIssue.RepoID != ctx.OrigIssue.RepoID || ref.Action != references.XRefActionNone { perm, err := getUserRepoPermission(e, refIssue.Repo, ctx.Doer) if err != nil { - return nil, err + return nil, references.XRefActionNone, err } if !perm.CanReadIssuesOrPulls(refIssue.IsPull) { - return nil, nil + return nil, references.XRefActionNone, nil + } + if ref.Action != references.XRefActionNone && + ctx.Doer.ID != refIssue.PosterID && + !perm.CanWriteIssuesOrPulls(refIssue.IsPull) { + refAction = references.XRefActionNone } } - return refIssue, nil + + return refIssue, refAction, nil } func (issue *Issue) neuterCrossReferences(e Engine) error { @@ -203,7 +217,7 @@ func (comment *Comment) addCrossReferences(e *xorm.Session, doer *User) error { } func (comment *Comment) neuterCrossReferences(e Engine) error { - return neuterCrossReferences(e, 0, comment.ID) + return neuterCrossReferences(e, comment.IssueID, comment.ID) } // LoadRefComment loads comment that created this reference from database @@ -268,3 +282,40 @@ func (comment *Comment) RefIssueIdent() string { // FIXME: check this name for cross-repository references (#7901 if it gets merged) return "#" + com.ToStr(comment.RefIssue.Index) } + +// __________ .__ .__ __________ __ +// \______ \__ __| | | |\______ \ ____ ________ __ ____ _______/ |_ +// | ___/ | \ | | | | _// __ \/ ____/ | \_/ __ \ / ___/\ __\ +// | | | | / |_| |_| | \ ___< <_| | | /\ ___/ \___ \ | | +// |____| |____/|____/____/____|_ /\___ >__ |____/ \___ >____ > |__| +// \/ \/ |__| \/ \/ + +// ResolveCrossReferences will return the list of references to close/reopen by this PR +func (pr *PullRequest) ResolveCrossReferences() ([]*Comment, error) { + unfiltered := make([]*Comment, 0, 5) + if err := x. + Where("ref_repo_id = ? AND ref_issue_id = ?", pr.Issue.RepoID, pr.Issue.ID). + In("ref_action", []references.XRefAction{references.XRefActionCloses, references.XRefActionReopens}). + OrderBy("id"). + Find(&unfiltered); err != nil { + return nil, fmt.Errorf("get reference: %v", err) + } + + refs := make([]*Comment, 0, len(unfiltered)) + for _, ref := range unfiltered { + found := false + for i, r := range refs { + if r.IssueID == ref.IssueID { + // Keep only the latest + refs[i] = ref + found = true + break + } + } + if !found { + refs = append(refs, ref) + } + } + + return refs, nil +} diff --git a/models/issue_xref_test.go b/models/issue_xref_test.go new file mode 100644 index 0000000000..4fc6011d78 --- /dev/null +++ b/models/issue_xref_test.go @@ -0,0 +1,164 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "fmt" + "testing" + + "code.gitea.io/gitea/modules/references" + + "github.com/stretchr/testify/assert" +) + +func TestXRef_AddCrossReferences(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + // Issue #1 to test against + itarget := testCreateIssue(t, 1, 2, "title1", "content1", false) + + // PR to close issue #1 + content := fmt.Sprintf("content2, closes #%d", itarget.Index) + pr := testCreateIssue(t, 1, 2, "title2", content, true) + ref := AssertExistsAndLoadBean(t, &Comment{IssueID: itarget.ID, RefIssueID: pr.ID, RefCommentID: 0}).(*Comment) + assert.Equal(t, CommentTypePullRef, ref.Type) + assert.Equal(t, pr.RepoID, ref.RefRepoID) + assert.Equal(t, true, ref.RefIsPull) + assert.Equal(t, references.XRefActionCloses, ref.RefAction) + + // Comment on PR to reopen issue #1 + content = fmt.Sprintf("content2, reopens #%d", itarget.Index) + c := testCreateComment(t, 1, 2, pr.ID, content) + ref = AssertExistsAndLoadBean(t, &Comment{IssueID: itarget.ID, RefIssueID: pr.ID, RefCommentID: c.ID}).(*Comment) + assert.Equal(t, CommentTypeCommentRef, ref.Type) + assert.Equal(t, pr.RepoID, ref.RefRepoID) + assert.Equal(t, true, ref.RefIsPull) + assert.Equal(t, references.XRefActionReopens, ref.RefAction) + + // Issue mentioning issue #1 + content = fmt.Sprintf("content3, mentions #%d", itarget.Index) + i := testCreateIssue(t, 1, 2, "title3", content, false) + ref = AssertExistsAndLoadBean(t, &Comment{IssueID: itarget.ID, RefIssueID: i.ID, RefCommentID: 0}).(*Comment) + assert.Equal(t, CommentTypeIssueRef, ref.Type) + assert.Equal(t, pr.RepoID, ref.RefRepoID) + assert.Equal(t, false, ref.RefIsPull) + assert.Equal(t, references.XRefActionNone, ref.RefAction) + + // Issue #4 to test against + itarget = testCreateIssue(t, 3, 3, "title4", "content4", false) + + // Cross-reference to issue #4 by admin + content = fmt.Sprintf("content5, mentions user3/repo3#%d", itarget.Index) + i = testCreateIssue(t, 2, 1, "title5", content, false) + ref = AssertExistsAndLoadBean(t, &Comment{IssueID: itarget.ID, RefIssueID: i.ID, RefCommentID: 0}).(*Comment) + assert.Equal(t, CommentTypeIssueRef, ref.Type) + assert.Equal(t, i.RepoID, ref.RefRepoID) + assert.Equal(t, false, ref.RefIsPull) + assert.Equal(t, references.XRefActionNone, ref.RefAction) + + // Cross-reference to issue #4 with no permission + content = fmt.Sprintf("content6, mentions user3/repo3#%d", itarget.Index) + i = testCreateIssue(t, 4, 5, "title6", content, false) + AssertNotExistsBean(t, &Comment{IssueID: itarget.ID, RefIssueID: i.ID, RefCommentID: 0}) +} + +func TestXRef_NeuterCrossReferences(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + // Issue #1 to test against + itarget := testCreateIssue(t, 1, 2, "title1", "content1", false) + + // Issue mentioning issue #1 + title := fmt.Sprintf("title2, mentions #%d", itarget.Index) + i := testCreateIssue(t, 1, 2, title, "content2", false) + ref := AssertExistsAndLoadBean(t, &Comment{IssueID: itarget.ID, RefIssueID: i.ID, RefCommentID: 0}).(*Comment) + assert.Equal(t, CommentTypeIssueRef, ref.Type) + assert.Equal(t, references.XRefActionNone, ref.RefAction) + + d := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + i.Title = "title2, no mentions" + assert.NoError(t, i.ChangeTitle(d, title)) + + ref = AssertExistsAndLoadBean(t, &Comment{IssueID: itarget.ID, RefIssueID: i.ID, RefCommentID: 0}).(*Comment) + assert.Equal(t, CommentTypeIssueRef, ref.Type) + assert.Equal(t, references.XRefActionNeutered, ref.RefAction) +} + +func TestXRef_ResolveCrossReferences(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + d := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + + i1 := testCreateIssue(t, 1, 2, "title1", "content1", false) + i2 := testCreateIssue(t, 1, 2, "title2", "content2", false) + i3 := testCreateIssue(t, 1, 2, "title3", "content3", false) + assert.NoError(t, i3.ChangeStatus(d, true)) + + pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index)) + rp := AssertExistsAndLoadBean(t, &Comment{IssueID: i1.ID, RefIssueID: pr.Issue.ID, RefCommentID: 0}).(*Comment) + + c1 := testCreateComment(t, 1, 2, pr.Issue.ID, fmt.Sprintf("closes #%d", i2.Index)) + r1 := AssertExistsAndLoadBean(t, &Comment{IssueID: i2.ID, RefIssueID: pr.Issue.ID, RefCommentID: c1.ID}).(*Comment) + + // Must be ignored + c2 := testCreateComment(t, 1, 2, pr.Issue.ID, fmt.Sprintf("mentions #%d", i2.Index)) + AssertExistsAndLoadBean(t, &Comment{IssueID: i2.ID, RefIssueID: pr.Issue.ID, RefCommentID: c2.ID}) + + // Must be superseded by c4/r4 + c3 := testCreateComment(t, 1, 2, pr.Issue.ID, fmt.Sprintf("reopens #%d", i3.Index)) + AssertExistsAndLoadBean(t, &Comment{IssueID: i3.ID, RefIssueID: pr.Issue.ID, RefCommentID: c3.ID}) + + c4 := testCreateComment(t, 1, 2, pr.Issue.ID, fmt.Sprintf("closes #%d", i3.Index)) + r4 := AssertExistsAndLoadBean(t, &Comment{IssueID: i3.ID, RefIssueID: pr.Issue.ID, RefCommentID: c4.ID}).(*Comment) + + refs, err := pr.ResolveCrossReferences() + assert.NoError(t, err) + assert.Len(t, refs, 3) + assert.Equal(t, rp.ID, refs[0].ID, "bad ref rp: %+v", refs[0]) + assert.Equal(t, r1.ID, refs[1].ID, "bad ref r1: %+v", refs[1]) + assert.Equal(t, r4.ID, refs[2].ID, "bad ref r4: %+v", refs[2]) +} + +func testCreateIssue(t *testing.T, repo, doer int64, title, content string, ispull bool) *Issue { + r := AssertExistsAndLoadBean(t, &Repository{ID: repo}).(*Repository) + d := AssertExistsAndLoadBean(t, &User{ID: doer}).(*User) + i := &Issue{RepoID: r.ID, PosterID: d.ID, Poster: d, Title: title, Content: content, IsPull: ispull} + + sess := x.NewSession() + defer sess.Close() + assert.NoError(t, sess.Begin()) + _, err := sess.SetExpr("`index`", "coalesce(MAX(`index`),0)+1").Where("repo_id=?", repo).Insert(i) + assert.NoError(t, err) + i, err = getIssueByID(sess, i.ID) + assert.NoError(t, err) + assert.NoError(t, i.addCrossReferences(sess, d)) + assert.NoError(t, sess.Commit()) + return i +} + +func testCreatePR(t *testing.T, repo, doer int64, title, content string) *PullRequest { + r := AssertExistsAndLoadBean(t, &Repository{ID: repo}).(*Repository) + d := AssertExistsAndLoadBean(t, &User{ID: doer}).(*User) + i := &Issue{RepoID: r.ID, PosterID: d.ID, Poster: d, Title: title, Content: content, IsPull: true} + pr := &PullRequest{HeadRepoID: repo, BaseRepoID: repo, HeadBranch: "head", BaseBranch: "base"} + assert.NoError(t, NewPullRequest(r, i, nil, nil, pr, nil)) + pr.Issue = i + return pr +} + +func testCreateComment(t *testing.T, repo, doer, issue int64, content string) *Comment { + d := AssertExistsAndLoadBean(t, &User{ID: doer}).(*User) + i := AssertExistsAndLoadBean(t, &Issue{ID: issue}).(*Issue) + c := &Comment{Type: CommentTypeComment, PosterID: doer, Poster: d, IssueID: issue, Issue: i, Content: content} + + sess := x.NewSession() + defer sess.Close() + assert.NoError(t, sess.Begin()) + _, err := sess.Insert(c) + assert.NoError(t, err) + assert.NoError(t, c.addCrossReferences(sess, d)) + assert.NoError(t, sess.Commit()) + return c +} diff --git a/modules/markup/html.go b/modules/markup/html.go index 1ff7a41cbb..924d0089a5 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -659,8 +659,16 @@ func issueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { return } - // Decorate action keywords - keyword := createKeyword(node.Data[ref.ActionLocation.Start:ref.ActionLocation.End]) + // Decorate action keywords if actionable + var keyword *html.Node + if references.IsXrefActionable(ref.Action) { + keyword = createKeyword(node.Data[ref.ActionLocation.Start:ref.ActionLocation.End]) + } else { + keyword = &html.Node{ + Type: html.TextNode, + Data: node.Data[ref.ActionLocation.Start:ref.ActionLocation.End], + } + } spaces := &html.Node{ Type: html.TextNode, Data: node.Data[ref.ActionLocation.End:ref.RefLocation.Start], diff --git a/modules/references/references.go b/modules/references/references.go index af0fe1aa0d..17e9ec2c91 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -350,3 +350,8 @@ func findActionKeywords(content []byte, start int) (XRefAction, *RefSpan) { } return XRefActionNone, nil } + +// IsXrefActionable returns true if the xref action is actionable (i.e. produces a result when resolved) +func IsXrefActionable(a XRefAction) bool { + return a == XRefActionCloses || a == XRefActionReopens +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 883b9222f5..4d22aa2fd9 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -876,10 +876,13 @@ issues.create_comment = Comment issues.closed_at = `closed <a id="%[1]s" href="#%[1]s">%[2]s</a>` issues.reopened_at = `reopened <a id="%[1]s" href="#%[1]s">%[2]s</a>` issues.commit_ref_at = `referenced this issue from a commit <a id="%[1]s" href="#%[1]s">%[2]s</a>` -issues.ref_issue_at = `referenced this issue %[1]s` -issues.ref_pull_at = `referenced this pull request %[1]s` -issues.ref_issue_ext_at = `referenced this issue from %[1]s %[2]s` -issues.ref_pull_ext_at = `referenced this pull request from %[1]s %[2]s` +issues.ref_issue_from = `<a href="%[3]s">referenced this issue %[4]s</a> <a id="%[1]s" href="#%[1]s">%[2]s</a>` +issues.ref_pull_from = `<a href="%[3]s">referenced this pull request %[4]s</a> <a id="%[1]s" href="#%[1]s">%[2]s</a>` +issues.ref_closing_from = `<a href="%[3]s">referenced a pull request %[4]s that will close this issue</a> <a id="%[1]s" href="#%[1]s">%[2]s</a>` +issues.ref_reopening_from = `<a href="%[3]s">referenced a pull request %[4]s that will reopen this issue</a> <a id="%[1]s" href="#%[1]s">%[2]s</a>` +issues.ref_closed_from = `<a href="%[3]s">closed this issue %[4]s</a> <a id="%[1]s" href="#%[1]s">%[2]s</a>` +issues.ref_reopened_from = `<a href="%[3]s">reopened this issue %[4]s</a> <a id="%[1]s" href="#%[1]s">%[2]s</a>` +issues.ref_from = `from %[1]s` issues.poster = Poster issues.collaborator = Collaborator issues.owner = Owner diff --git a/services/pull/merge.go b/services/pull/merge.go index 7dc3c07338..8ae5f029e8 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -21,8 +21,10 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification" + "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" + issue_service "code.gitea.io/gitea/services/issue" "github.com/mcuadros/go-version" ) @@ -447,6 +449,26 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor notification.NotifyIssueChangeStatus(doer, pr.Issue, true) + // Resolve cross references + refs, err := pr.ResolveCrossReferences() + if err != nil { + log.Error("ResolveCrossReferences: %v", err) + return nil + } + + for _, ref := range refs { + if err = ref.LoadIssue(); err != nil { + return err + } + if err = ref.Issue.LoadRepo(); err != nil { + return err + } + close := (ref.RefAction == references.XRefActionCloses) + if err = issue_service.ChangeStatus(ref.Issue, doer, close); err != nil { + return err + } + } + return nil } diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 5a3d4026c6..86860fe8f4 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -88,7 +88,20 @@ </a> <span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.GetDisplayName}}</a> {{$.i18n.Tr "repo.issues.closed_at" .EventTag $createdStr | Safe}}</span> </div> - {{else if or (eq .Type 3) (eq .Type 5) (eq .Type 6)}} + {{else if eq .Type 3 5 6}} + {{ $refFrom:= "" }} + {{if ne .RefRepoID .Issue.RepoID}} + {{ $refFrom = $.i18n.Tr "repo.issues.ref_from" .RefRepo.FullName }} + {{end}} + {{ $refTr := "repo.issues.ref_issue_from" }} + {{if .Issue.IsPull}} + {{ $refTr = "repo.issues.ref_pull_from" }} + {{else if eq .RefAction 1 }} + {{ $refTr = "repo.issues.ref_closing_from" }} + {{else if eq .RefAction 2 }} + {{ $refTr = "repo.issues.ref_reopening_from" }} + {{end}} + {{ $createdStr:= TimeSinceUnix .CreatedUnix $.Lang }} <div class="event" id="{{.HashTag}}"> <span class="octicon octicon-bookmark"></span> <a class="ui avatar image" href="{{.Poster.HomeLink}}"> @@ -96,19 +109,7 @@ </a> {{if eq .RefAction 3}}<del>{{end}} <span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.GetDisplayName}}</a> - {{if (eq .RefRepoID .Issue.RepoID)}} - {{if .Issue.IsPull}} - <a href="{{.RefCommentHTMLURL}}">{{$.i18n.Tr "repo.issues.ref_pull_at" $createdStr | Safe}}</a> - {{else}} - <a href="{{.RefCommentHTMLURL}}">{{$.i18n.Tr "repo.issues.ref_issue_at" $createdStr | Safe}}</a> - {{end}} - {{else}} - {{if .Issue.IsPull}} - <a href="{{.RefCommentHTMLURL}}">{{$.i18n.Tr "repo.issues.ref_pull_ext_at" .RefRepo.FullName $createdStr | Safe}}</a> - {{else}} - <a href="{{.RefCommentHTMLURL}}">{{$.i18n.Tr "repo.issues.ref_issue_ext_at" .RefRepo.FullName $createdStr | Safe}}</a> - {{end}} - {{end}} + {{$.i18n.Tr $refTr .EventTag $createdStr .RefCommentHTMLURL $refFrom | Safe}} </span> {{if eq .RefAction 3}}</del>{{end}} |