Skip to content

Commit

Permalink
Reference issues from pull requests and other issues (#8137)
Browse files Browse the repository at this point in the history
* Update ref comment

* Generate comment on simple ref

* Make fmt + remove unneeded repo load

* Add TODO comments

* Add ref-check in issue creation; re-arrange template

* Make unit tests pass; rearrange code

* Make fmt

* Filter out xref comment if user can't see the referencing issue

* Add TODOs

* Add cross reference

* Rearrange code; add cross-repository references

* Striketrhough obsolete references

* Remove unnecesary TODO

* Add "not supported" note

* Support for edits and deletes, and issue title

* Revert changes to go.mod

* Fix fmt

* Add support for xref from API

* Add first integration test

* Add integration tests

* Correct formatting

* Fix add comment test

* Add migration

* Remove outdated comments; fix typo

* Some code refactoring and rearranging

* Rename findCrossReferences to createCrossReferences

* Delete xrefs when repository is deleted

* Corrections as suggested by @lafriks

* Prepare for merge

* Fix log for errors
  • Loading branch information
guillep2k authored and techknowlogick committed Sep 20, 2019
1 parent 8a0379d commit 2a2b46c
Show file tree
Hide file tree
Showing 10 changed files with 610 additions and 16 deletions.
104 changes: 103 additions & 1 deletion integrations/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package integrations

import (
"fmt"
"net/http"
"path"
"strconv"
Expand Down Expand Up @@ -136,7 +137,7 @@ func testNewIssue(t *testing.T, session *TestSession, user, repo, title, content
return issueURL
}

func testIssueAddComment(t *testing.T, session *TestSession, issueURL, content, status string) {
func testIssueAddComment(t *testing.T, session *TestSession, issueURL, content, status string) int64 {

req := NewRequest(t, "GET", issueURL)
resp := session.MakeRequest(t, req, http.StatusOK)
Expand All @@ -161,6 +162,13 @@ func testIssueAddComment(t *testing.T, session *TestSession, issueURL, content,

val := htmlDoc.doc.Find(".comment-list .comments .comment .render-content p").Eq(commentCount).Text()
assert.Equal(t, content, val)

idAttr, has := htmlDoc.doc.Find(".comment-list .comments .comment").Eq(commentCount).Attr("id")
idStr := idAttr[strings.LastIndexByte(idAttr, '-')+1:]
assert.True(t, has)
id, err := strconv.Atoi(idStr)
assert.NoError(t, err)
return int64(id)
}

func TestNewIssue(t *testing.T) {
Expand All @@ -184,3 +192,97 @@ func TestIssueCommentClose(t *testing.T) {
val := htmlDoc.doc.Find(".comment-list .comments .comment .render-content p").First().Text()
assert.Equal(t, "Description", val)
}

func TestIssueCrossReference(t *testing.T) {
prepareTestEnv(t)

// Issue that will be referenced
_, issueBase := testIssueWithBean(t, "user2", 1, "Title", "Description")

// Ref from issue title
issueRefURL, issueRef := testIssueWithBean(t, "user2", 1, fmt.Sprintf("Title ref #%d", issueBase.Index), "Description")
models.AssertExistsAndLoadBean(t, &models.Comment{
IssueID: issueBase.ID,
RefRepoID: 1,
RefIssueID: issueRef.ID,
RefCommentID: 0,
RefIsPull: false,
RefAction: models.XRefActionNone})

// Edit title, neuter ref
testIssueChangeInfo(t, "user2", issueRefURL, "title", "Title no ref")
models.AssertExistsAndLoadBean(t, &models.Comment{
IssueID: issueBase.ID,
RefRepoID: 1,
RefIssueID: issueRef.ID,
RefCommentID: 0,
RefIsPull: false,
RefAction: models.XRefActionNeutered})

// Ref from issue content
issueRefURL, issueRef = testIssueWithBean(t, "user2", 1, "TitleXRef", fmt.Sprintf("Description ref #%d", issueBase.Index))
models.AssertExistsAndLoadBean(t, &models.Comment{
IssueID: issueBase.ID,
RefRepoID: 1,
RefIssueID: issueRef.ID,
RefCommentID: 0,
RefIsPull: false,
RefAction: models.XRefActionNone})

// Edit content, neuter ref
testIssueChangeInfo(t, "user2", issueRefURL, "content", "Description no ref")
models.AssertExistsAndLoadBean(t, &models.Comment{
IssueID: issueBase.ID,
RefRepoID: 1,
RefIssueID: issueRef.ID,
RefCommentID: 0,
RefIsPull: false,
RefAction: models.XRefActionNeutered})

// Ref from a comment
session := loginUser(t, "user2")
commentID := testIssueAddComment(t, session, issueRefURL, fmt.Sprintf("Adding ref from comment #%d", issueBase.Index), "")
comment := &models.Comment{
IssueID: issueBase.ID,
RefRepoID: 1,
RefIssueID: issueRef.ID,
RefCommentID: commentID,
RefIsPull: false,
RefAction: models.XRefActionNone}
models.AssertExistsAndLoadBean(t, comment)

// Ref from a different repository
issueRefURL, issueRef = testIssueWithBean(t, "user12", 10, "TitleXRef", fmt.Sprintf("Description ref user2/repo1#%d", issueBase.Index))
models.AssertExistsAndLoadBean(t, &models.Comment{
IssueID: issueBase.ID,
RefRepoID: 10,
RefIssueID: issueRef.ID,
RefCommentID: 0,
RefIsPull: false,
RefAction: models.XRefActionNone})
}

func testIssueWithBean(t *testing.T, user string, repoID int64, title, content string) (string, *models.Issue) {
session := loginUser(t, user)
issueURL := testNewIssue(t, session, user, fmt.Sprintf("repo%d", repoID), title, content)
indexStr := issueURL[strings.LastIndexByte(issueURL, '/')+1:]
index, err := strconv.Atoi(indexStr)
assert.NoError(t, err, "Invalid issue href: %s", issueURL)
issue := &models.Issue{RepoID: repoID, Index: int64(index)}
models.AssertExistsAndLoadBean(t, issue)
return issueURL, issue
}

func testIssueChangeInfo(t *testing.T, user, issueURL, info string, value string) {
session := loginUser(t, user)

req := NewRequest(t, "GET", issueURL)
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)

req = NewRequestWithValues(t, "POST", path.Join(issueURL, info), map[string]string{
"_csrf": htmlDoc.GetCSRF(),
info: value,
})
_ = session.MakeRequest(t, req, http.StatusOK)
}
57 changes: 52 additions & 5 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -595,8 +595,9 @@ func (issue *Issue) ClearLabels(doer *User) (err error) {
if err = sess.Commit(); err != nil {
return fmt.Errorf("Commit: %v", err)
}
sess.Close()

if err = issue.loadPoster(x); err != nil {
if err = issue.LoadPoster(); err != nil {
return fmt.Errorf("loadPoster: %v", err)
}

Expand Down Expand Up @@ -870,9 +871,18 @@ func (issue *Issue) ChangeTitle(doer *User, title string) (err error) {
return fmt.Errorf("createChangeTitleComment: %v", err)
}

if err = issue.neuterCrossReferences(sess); err != nil {
return err
}

if err = issue.addCrossReferences(sess, doer); err != nil {
return err
}

if err = sess.Commit(); err != nil {
return err
}
sess.Close()

mode, _ := AccessLevel(issue.Poster, issue.Repo)
if issue.IsPull {
Expand Down Expand Up @@ -939,9 +949,26 @@ func (issue *Issue) ChangeContent(doer *User, content string) (err error) {
oldContent := issue.Content
issue.Content = content

if err = UpdateIssueCols(issue, "content"); err != nil {
sess := x.NewSession()
defer sess.Close()
if err = sess.Begin(); err != nil {
return err
}

if err = updateIssueCols(sess, issue, "content"); err != nil {
return fmt.Errorf("UpdateIssueCols: %v", err)
}
if err = issue.neuterCrossReferences(sess); err != nil {
return err
}
if err = issue.addCrossReferences(sess, doer); err != nil {
return err
}

if err = sess.Commit(); err != nil {
return err
}
sess.Close()

mode, _ := AccessLevel(issue.Poster, issue.Repo)
if issue.IsPull {
Expand Down Expand Up @@ -1171,8 +1198,10 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) {
}
}
}

return opts.Issue.loadAttributes(e)
if err = opts.Issue.loadAttributes(e); err != nil {
return err
}
return opts.Issue.addCrossReferences(e, doer)
}

// NewIssue creates new issue with labels for repository.
Expand All @@ -1199,6 +1228,7 @@ func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, assigneeIDs []in
if err = sess.Commit(); err != nil {
return fmt.Errorf("Commit: %v", err)
}
sess.Close()

if err = NotifyWatchers(&Action{
ActUserID: issue.Poster.ID,
Expand Down Expand Up @@ -1808,7 +1838,24 @@ func updateIssue(e Engine, issue *Issue) error {

// UpdateIssue updates all fields of given issue.
func UpdateIssue(issue *Issue) error {
return updateIssue(x, issue)
sess := x.NewSession()
defer sess.Close()
if err := sess.Begin(); err != nil {
return err
}
if err := updateIssue(sess, issue); err != nil {
return err
}
if err := issue.neuterCrossReferences(sess); err != nil {
return err
}
if err := issue.loadPoster(sess); err != nil {
return err
}
if err := issue.addCrossReferences(sess, issue.Poster); err != nil {
return err
}
return sess.Commit()
}

// UpdateIssueDeadline updates an issue deadline and adds comments. Setting a deadline to 0 means deleting it.
Expand Down
61 changes: 55 additions & 6 deletions models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,30 @@ type Comment struct {
Review *Review `xorm:"-"`
ReviewID int64 `xorm:"index"`
Invalidated bool

// Reference an issue or pull from another comment, issue or PR
// All information is about the origin of the reference
RefRepoID int64 `xorm:"index"` // Repo where the referencing
RefIssueID int64 `xorm:"index"`
RefCommentID int64 `xorm:"index"` // 0 if origin is Issue title or content (or PR's)
RefAction XRefAction `xorm:"SMALLINT"` // What hapens if RefIssueID resolves
RefIsPull bool

RefRepo *Repository `xorm:"-"`
RefIssue *Issue `xorm:"-"`
RefComment *Comment `xorm:"-"`
}

// LoadIssue loads issue from database
func (c *Comment) LoadIssue() (err error) {
return c.loadIssue(x)
}

func (c *Comment) loadIssue(e Engine) (err error) {
if c.Issue != nil {
return nil
}
c.Issue, err = GetIssueByID(c.IssueID)
c.Issue, err = getIssueByID(e, c.IssueID)
return
}

Expand Down Expand Up @@ -527,6 +543,11 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
TreePath: opts.TreePath,
ReviewID: opts.ReviewID,
Patch: opts.Patch,
RefRepoID: opts.RefRepoID,
RefIssueID: opts.RefIssueID,
RefCommentID: opts.RefCommentID,
RefAction: opts.RefAction,
RefIsPull: opts.RefIsPull,
}
if _, err = e.Insert(comment); err != nil {
return nil, err
Expand All @@ -540,6 +561,10 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
return nil, err
}

if err = comment.addCrossReferences(e, opts.Doer); err != nil {
return nil, err
}

return comment, nil
}

Expand Down Expand Up @@ -794,6 +819,11 @@ type CreateCommentOptions struct {
ReviewID int64
Content string
Attachments []string // UUIDs of attachments
RefRepoID int64
RefIssueID int64
RefCommentID int64
RefAction XRefAction
RefIsPull bool
}

// CreateComment creates comment of issue or commit.
Expand Down Expand Up @@ -934,21 +964,33 @@ func FindComments(opts FindCommentsOptions) ([]*Comment, error) {

// UpdateComment updates information of comment.
func UpdateComment(doer *User, c *Comment, oldContent string) error {
if _, err := x.ID(c.ID).AllCols().Update(c); err != nil {
sess := x.NewSession()
defer sess.Close()
if err := sess.Begin(); err != nil {
return err
}

if err := c.LoadPoster(); err != nil {
if _, err := sess.ID(c.ID).AllCols().Update(c); err != nil {
return err
}
if err := c.LoadIssue(); err != nil {
if err := c.loadIssue(sess); err != nil {
return err
}
if err := c.neuterCrossReferences(sess); err != nil {
return err
}
if err := c.addCrossReferences(sess, doer); err != nil {
return err
}
if err := sess.Commit(); err != nil {
return fmt.Errorf("Commit: %v", err)
}
sess.Close()

if err := c.Issue.LoadAttributes(); err != nil {
if err := c.LoadPoster(); err != nil {
return err
}
if err := c.loadPoster(x); err != nil {
if err := c.Issue.LoadAttributes(); err != nil {
return err
}

Expand Down Expand Up @@ -996,6 +1038,10 @@ func DeleteComment(doer *User, comment *Comment) error {
return err
}

if err := comment.neuterCrossReferences(sess); err != nil {
return err
}

if err := sess.Commit(); err != nil {
return err
}
Expand All @@ -1014,6 +1060,9 @@ func DeleteComment(doer *User, comment *Comment) error {
if err := comment.loadPoster(x); err != nil {
return err
}
if err := comment.neuterCrossReferences(x); err != nil {
return err
}

mode, _ := AccessLevel(doer, comment.Issue.Repo)

Expand Down
Loading

0 comments on commit 2a2b46c

Please sign in to comment.