Reference issues from pull requests and other issues (#8137)

* 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

Author: guillep2k

integrations/issue_test.go

@@ -5,6 +5,7 @@
 package integrations
 
 import (
+	"fmt"
 	"net/http"
 	"path"
 	"strconv"
@@ -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)
@@ -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) {
@@ -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)
+}

models/issue.go

@@ -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)
 	}
 
@@ -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 {
@@ -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 {
@@ -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.
@@ -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,
@@ -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.

models/issue_comment.go

@@ -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
 }
 
@@ -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
@@ -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
 }
 
@@ -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.
@@ -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
 	}
 
@@ -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
 	}
@@ -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)