Issue content should not be updated when closing with comment (#2833)

Author: lafriks

integrations/issue_test.go

@@ -107,7 +107,7 @@ func TestNoLoginViewIssue(t *testing.T) {
 	MakeRequest(t, req, http.StatusOK)
 }
 
-func testNewIssue(t *testing.T, session *TestSession, user, repo, title string) {
+func testNewIssue(t *testing.T, session *TestSession, user, repo, title, content string) string {
 
 	req := NewRequest(t, "GET", path.Join(user, repo, "issues", "new"))
 	resp := session.MakeRequest(t, req, http.StatusOK)
@@ -116,17 +116,70 @@ func testNewIssue(t *testing.T, session *TestSession, user, repo, title string)
 	link, exists := htmlDoc.doc.Find("form.ui.form").Attr("action")
 	assert.True(t, exists, "The template has changed")
 	req = NewRequestWithValues(t, "POST", link, map[string]string{
-		"_csrf": htmlDoc.GetCSRF(),
-		"title": title,
+		"_csrf":   htmlDoc.GetCSRF(),
+		"title":   title,
+		"content": content,
+	})
+	resp = session.MakeRequest(t, req, http.StatusFound)
+
+	issueURL := RedirectURL(t, resp)
+	req = NewRequest(t, "GET", issueURL)
+	resp = session.MakeRequest(t, req, http.StatusOK)
+
+	htmlDoc = NewHTMLParser(t, resp.Body)
+	val := htmlDoc.doc.Find("#issue-title").Text()
+	assert.Equal(t, title, val)
+	val = htmlDoc.doc.Find(".comment-list .comments .comment .render-content p").First().Text()
+	assert.Equal(t, content, val)
+
+	return issueURL
+}
+
+func testIssueAddComment(t *testing.T, session *TestSession, issueURL, content, status string) {
+
+	req := NewRequest(t, "GET", issueURL)
+	resp := session.MakeRequest(t, req, http.StatusOK)
+
+	htmlDoc := NewHTMLParser(t, resp.Body)
+	link, exists := htmlDoc.doc.Find("#comment-form").Attr("action")
+	assert.True(t, exists, "The template has changed")
+
+	commentCount := htmlDoc.doc.Find(".comment-list .comments .comment .render-content").Length()
+
+	req = NewRequestWithValues(t, "POST", link, map[string]string{
+		"_csrf":   htmlDoc.GetCSRF(),
+		"content": content,
+		"status":  status,
 	})
 	resp = session.MakeRequest(t, req, http.StatusFound)
 
 	req = NewRequest(t, "GET", RedirectURL(t, resp))
 	resp = session.MakeRequest(t, req, http.StatusOK)
+
+	htmlDoc = NewHTMLParser(t, resp.Body)
+
+	val := htmlDoc.doc.Find(".comment-list .comments .comment .render-content p").Eq(commentCount).Text()
+	assert.Equal(t, content, val)
 }
 
 func TestNewIssue(t *testing.T) {
 	prepareTestEnv(t)
 	session := loginUser(t, "user2")
-	testNewIssue(t, session, "user2", "repo1", "Title")
+	testNewIssue(t, session, "user2", "repo1", "Title", "Description")
+}
+
+func TestIssueCommentClose(t *testing.T) {
+	prepareTestEnv(t)
+	session := loginUser(t, "user2")
+	issueURL := testNewIssue(t, session, "user2", "repo1", "Title", "Description")
+	testIssueAddComment(t, session, issueURL, "Test comment 1", "")
+	testIssueAddComment(t, session, issueURL, "Test comment 2", "")
+	testIssueAddComment(t, session, issueURL, "Test comment 3", "close")
+
+	// Validate that issue content has not been updated
+	req := NewRequest(t, "GET", issueURL)
+	resp := session.MakeRequest(t, req, http.StatusOK)
+	htmlDoc := NewHTMLParser(t, resp.Body)
+	val := htmlDoc.doc.Find(".comment-list .comments .comment .render-content p").First().Text()
+	assert.Equal(t, "Description", val)
 }

integrations/repo_activity_test.go

@@ -31,9 +31,9 @@ func TestRepoActivity(t *testing.T) {
 	testPullCreate(t, session, "user1", "repo1", "feat/much_better_readme")
 
 	// Create issues (3 new issues)
-	testNewIssue(t, session, "user2", "repo1", "Issue 1")
-	testNewIssue(t, session, "user2", "repo1", "Issue 2")
-	testNewIssue(t, session, "user2", "repo1", "Issue 3")
+	testNewIssue(t, session, "user2", "repo1", "Issue 1", "Description 1")
+	testNewIssue(t, session, "user2", "repo1", "Issue 2", "Description 2")
+	testNewIssue(t, session, "user2", "repo1", "Issue 3", "Description 3")
 
 	// Create releases (1 new release)
 	createNewRelease(t, session, "/user2/repo1", "v1.0.0", "v1.0.0", false, false)

models/issue_comment.go

@@ -272,15 +272,15 @@ func (c *Comment) MailParticipants(e Engine, opType ActionType, issue *Issue) (e
 		return fmt.Errorf("UpdateIssueMentions [%d]: %v", c.IssueID, err)
 	}
 
+	content := c.Content
+
 	switch opType {
-	case ActionCommentIssue:
-		issue.Content = c.Content
 	case ActionCloseIssue:
-		issue.Content = fmt.Sprintf("Closed #%d", issue.Index)
+		content = fmt.Sprintf("Closed #%d", issue.Index)
 	case ActionReopenIssue:
-		issue.Content = fmt.Sprintf("Reopened #%d", issue.Index)
+		content = fmt.Sprintf("Reopened #%d", issue.Index)
 	}
-	if err = mailIssueCommentToParticipants(e, issue, c.Poster, c, mentions); err != nil {
+	if err = mailIssueCommentToParticipants(e, issue, c.Poster, content, c, mentions); err != nil {
 		log.Error(4, "mailIssueCommentToParticipants: %v", err)
 	}
 

models/issue_mail.go

@@ -22,7 +22,7 @@ func (issue *Issue) mailSubject() string {
 // This function sends two list of emails:
 // 1. Repository watchers and users who are participated in comments.
 // 2. Users who are not in 1. but get mentioned in current issue/comment.
-func mailIssueCommentToParticipants(e Engine, issue *Issue, doer *User, comment *Comment, mentions []string) error {
+func mailIssueCommentToParticipants(e Engine, issue *Issue, doer *User, content string, comment *Comment, mentions []string) error {
 	if !setting.Service.EnableNotifyMail {
 		return nil
 	}
@@ -80,7 +80,7 @@ func mailIssueCommentToParticipants(e Engine, issue *Issue, doer *User, comment
 		names = append(names, participants[i].Name)
 	}
 
-	SendIssueCommentMail(issue, doer, comment, tos)
+	SendIssueCommentMail(issue, doer, content, comment, tos)
 
 	// Mail mentioned people and exclude watchers.
 	names = append(names, doer.Name)
@@ -92,7 +92,7 @@ func mailIssueCommentToParticipants(e Engine, issue *Issue, doer *User, comment
 
 		tos = append(tos, mentions[i])
 	}
-	SendIssueMentionMail(issue, doer, comment, getUserEmailsByNames(e, tos))
+	SendIssueMentionMail(issue, doer, content, comment, getUserEmailsByNames(e, tos))
 
 	return nil
 }
@@ -109,7 +109,7 @@ func (issue *Issue) mailParticipants(e Engine) (err error) {
 		return fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err)
 	}
 
-	if err = mailIssueCommentToParticipants(e, issue, issue.Poster, nil, mentions); err != nil {
+	if err = mailIssueCommentToParticipants(e, issue, issue.Poster, issue.Content, nil, mentions); err != nil {
 		log.Error(4, "mailIssueCommentToParticipants: %v", err)
 	}
 

models/mail.go

@@ -149,9 +149,9 @@ func composeTplData(subject, body, link string) map[string]interface{} {
 	return data
 }
 
-func composeIssueCommentMessage(issue *Issue, doer *User, comment *Comment, tplName base.TplName, tos []string, info string) *mailer.Message {
+func composeIssueCommentMessage(issue *Issue, doer *User, content string, comment *Comment, tplName base.TplName, tos []string, info string) *mailer.Message {
 	subject := issue.mailSubject()
-	body := string(markup.RenderByType(markdown.MarkupName, []byte(issue.Content), issue.Repo.HTMLURL(), issue.Repo.ComposeMetas()))
+	body := string(markup.RenderByType(markdown.MarkupName, []byte(content), issue.Repo.HTMLURL(), issue.Repo.ComposeMetas()))
 
 	data := make(map[string]interface{}, 10)
 	if comment != nil {
@@ -161,30 +161,30 @@ func composeIssueCommentMessage(issue *Issue, doer *User, comment *Comment, tplN
 	}
 	data["Doer"] = doer
 
-	var content bytes.Buffer
+	var mailBody bytes.Buffer
 
-	if err := templates.ExecuteTemplate(&content, string(tplName), data); err != nil {
+	if err := templates.ExecuteTemplate(&mailBody, string(tplName), data); err != nil {
 		log.Error(3, "Template: %v", err)
 	}
 
-	msg := mailer.NewMessageFrom(tos, doer.DisplayName(), setting.MailService.FromEmail, subject, content.String())
+	msg := mailer.NewMessageFrom(tos, doer.DisplayName(), setting.MailService.FromEmail, subject, mailBody.String())
 	msg.Info = fmt.Sprintf("Subject: %s, %s", subject, info)
 	return msg
 }
 
 // SendIssueCommentMail composes and sends issue comment emails to target receivers.
-func SendIssueCommentMail(issue *Issue, doer *User, comment *Comment, tos []string) {
+func SendIssueCommentMail(issue *Issue, doer *User, content string, comment *Comment, tos []string) {
 	if len(tos) == 0 {
 		return
 	}
 
-	mailer.SendAsync(composeIssueCommentMessage(issue, doer, comment, mailIssueComment, tos, "issue comment"))
+	mailer.SendAsync(composeIssueCommentMessage(issue, doer, content, comment, mailIssueComment, tos, "issue comment"))
 }
 
 // SendIssueMentionMail composes and sends issue mention emails to target receivers.
-func SendIssueMentionMail(issue *Issue, doer *User, comment *Comment, tos []string) {
+func SendIssueMentionMail(issue *Issue, doer *User, content string, comment *Comment, tos []string) {
 	if len(tos) == 0 {
 		return
 	}
-	mailer.SendAsync(composeIssueCommentMessage(issue, doer, comment, mailIssueMention, tos, "issue mention"))
+	mailer.SendAsync(composeIssueCommentMessage(issue, doer, content, comment, mailIssueMention, tos, "issue mention"))
 }