From 74774a41c322db8c78cc0dc9ce92402e673bda13 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Thu, 4 May 2017 17:58:40 +0200 Subject: [PATCH 01/22] Added comment's hashtag to url for mail notifications. Signed-off-by: Jonas --- models/issue_comment.go | 2 +- models/issue_mail.go | 84 ++++++++++++++++++++++++++++++----------- models/mail.go | 40 ++++++++++++++++++-- 3 files changed, 100 insertions(+), 26 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index e133cc049bdc..430bbdfa2650 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -289,7 +289,7 @@ func (c *Comment) MailParticipants(e Engine, opType ActionType, issue *Issue) (e case ActionReopenIssue: issue.Content = fmt.Sprintf("Reopened #%d", issue.Index) } - if err = mailIssueCommentToParticipants(issue, c.Poster, mentions); err != nil { + if err = mailIssueCommentToParticipants(issue, c.Poster, c, mentions); err != nil { log.Error(4, "mailIssueCommentToParticipants: %v", err) } diff --git a/models/issue_mail.go b/models/issue_mail.go index 4b6542ddb4c1..1ff407e0f293 100644 --- a/models/issue_mail.go +++ b/models/issue_mail.go @@ -18,22 +18,73 @@ func (issue *Issue) mailSubject() string { return fmt.Sprintf("[%s] %s (#%d)", issue.Repo.Name, issue.Title, issue.Index) } -// mailIssueCommentToParticipants can be used for both new issue creation and comment. + +// mailIssueCommentToParticipants can be used for only for comment. // 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(issue *Issue, doer *User, mentions []string) error { +func mailIssueCommentToParticipants(issue *Issue, doer *User, comment *Comment, mentions []string) error { + + names, tos, err := prepareMailToParticipants(issue, doer) + + if(err != nil) { + return fmt.Errorf("PrepareMailToParticipants: %v", err) + } + + SendIssueCommentMail(issue, doer, comment, tos) + + // Mail mentioned people and exclude watchers. + names = append(names, doer.Name) + tos = make([]string, 0, len(mentions)) // list of user names. + for i := range mentions { + if com.IsSliceContainsStr(names, mentions[i]) { + continue + } + + tos = append(tos, mentions[i]) + } + SendIssueMentionMail(issue, doer, comment, GetUserEmailsByNames(tos)) + + return nil +} + +func mailIssueActionToParticipants(issue *Issue, doer *User, mentions []string) error { + names, tos, err := prepareMailToParticipants(issue, doer) + + if(err != nil) { + return fmt.Errorf("PrepareMailToParticipants: %v", err) + } + + SendIssueActionMail(issue, doer, tos) + + // Mail mentioned people and exclude watchers. + names = append(names, doer.Name) + tos = make([]string, 0, len(mentions)) // list of user names. + for i := range mentions { + if com.IsSliceContainsStr(names, mentions[i]) { + continue + } + + tos = append(tos, mentions[i]) + } + SendIssueMentionInActionMail(issue, doer, GetUserEmailsByNames(tos)) + + return nil +} + +// prepareMailToParticipants creates the tos and names list for an issue and the issue's creator. +func prepareMailToParticipants(issue *Issue, doer *User) ([]string, []string, error) { if !setting.Service.EnableNotifyMail { - return nil + return nil, nil, nil } watchers, err := GetWatchers(issue.RepoID) if err != nil { - return fmt.Errorf("GetWatchers [repo_id: %d]: %v", issue.RepoID, err) + return nil, nil, err } participants, err := GetParticipantsByIssueID(issue.ID) if err != nil { - return fmt.Errorf("GetParticipantsByIssueID [issue_id: %d]: %v", issue.ID, err) + return nil, nil, err } // In case the issue poster is not watching the repository, @@ -51,7 +102,7 @@ func mailIssueCommentToParticipants(issue *Issue, doer *User, mentions []string) to, err := GetUserByID(watchers[i].UserID) if err != nil { - return fmt.Errorf("GetUserByID [%d]: %v", watchers[i].UserID, err) + return nil, nil, err } if to.IsOrganization() { continue @@ -70,23 +121,10 @@ func mailIssueCommentToParticipants(issue *Issue, doer *User, mentions []string) tos = append(tos, participants[i].Email) names = append(names, participants[i].Name) } - SendIssueCommentMail(issue, doer, tos) - - // Mail mentioned people and exclude watchers. - names = append(names, doer.Name) - tos = make([]string, 0, len(mentions)) // list of user names. - for i := range mentions { - if com.IsSliceContainsStr(names, mentions[i]) { - continue - } - - tos = append(tos, mentions[i]) - } - SendIssueMentionMail(issue, doer, GetUserEmailsByNames(tos)) - - return nil + return tos, names, nil } + // MailParticipants sends new issue thread created emails to repository watchers // and mentioned people. func (issue *Issue) MailParticipants() (err error) { @@ -95,9 +133,11 @@ func (issue *Issue) MailParticipants() (err error) { return fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err) } - if err = mailIssueCommentToParticipants(issue, issue.Poster, mentions); err != nil { + if err = mailIssueActionToParticipants(issue, issue.Poster, mentions); err != nil { log.Error(4, "mailIssueCommentToParticipants: %v", err) } return nil } + + diff --git a/models/mail.go b/models/mail.go index 16e8c9e2efbb..165db65698cd 100644 --- a/models/mail.go +++ b/models/mail.go @@ -148,6 +148,24 @@ 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 { + subject := issue.mailSubject() + body := string(markdown.RenderString(issue.Content, issue.Repo.HTMLURL(), issue.Repo.ComposeMetas())) + data := composeTplData(subject, body, issue.HTMLURL() + comment.HashTag()) + + data["Doer"] = doer + + var content bytes.Buffer + + if err := templates.ExecuteTemplate(&content, string(tplName), data); err != nil { + log.Error(3, "Template: %v", err) + } + + msg := mailer.NewMessageFrom(tos, fmt.Sprintf(`"%s" <%s>`, doer.DisplayName(), setting.MailService.FromEmail), subject, content.String()) + msg.Info = fmt.Sprintf("Subject: %s, %s", subject, info) + return msg +} + func composeIssueMessage(issue *Issue, doer *User, tplName base.TplName, tos []string, info string) *mailer.Message { subject := issue.mailSubject() body := string(markdown.RenderString(issue.Content, issue.Repo.HTMLURL(), issue.Repo.ComposeMetas())) @@ -166,16 +184,32 @@ func composeIssueMessage(issue *Issue, doer *User, tplName base.TplName, tos []s } // SendIssueCommentMail composes and sends issue comment emails to target receivers. -func SendIssueCommentMail(issue *Issue, doer *User, tos []string) { +func SendIssueCommentMail(issue *Issue, doer *User, comment *Comment, tos []string) { if len(tos) == 0 { return } - mailer.SendAsync(composeIssueMessage(issue, doer, mailIssueComment, tos, "issue comment")) + mailer.SendAsync(composeIssueCommentMessage(issue, doer, comment, mailIssueComment, tos, "issue comment")) } // SendIssueMentionMail composes and sends issue mention emails to target receivers. -func SendIssueMentionMail(issue *Issue, doer *User, tos []string) { +func SendIssueMentionMail(issue *Issue, doer *User, comment *Comment, tos []string) { + if len(tos) == 0 { + return + } + mailer.SendAsync(composeIssueCommentMessage(issue, doer, comment, mailIssueMention, tos, "issue mention")) +} +// SendIssueActionMail composes and sends issue action emails to target receivers. +func SendIssueActionMail(issue *Issue, doer *User, tos []string) { + if len(tos) == 0 { + return + } + + mailer.SendAsync(composeIssueMessage(issue, doer, mailIssueComment, tos, "issue comment")) +} + +// SendIssueMentionInActionMail composes and sends issue mention emails to target receivers. Only applies if no comment is given. +func SendIssueMentionInActionMail(issue *Issue, doer *User, tos []string) { if len(tos) == 0 { return } From f385727aef6f0a777d7fedba56d701ff8b2a0b9e Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Thu, 4 May 2017 18:00:40 +0200 Subject: [PATCH 02/22] Added comment's hashtag to url for mail notifications. Added explanation to return statement. Signed-off-by: Jonas --- models/issue_mail.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue_mail.go b/models/issue_mail.go index 1ff407e0f293..1fabde489a65 100644 --- a/models/issue_mail.go +++ b/models/issue_mail.go @@ -73,7 +73,7 @@ func mailIssueActionToParticipants(issue *Issue, doer *User, mentions []string) } // prepareMailToParticipants creates the tos and names list for an issue and the issue's creator. -func prepareMailToParticipants(issue *Issue, doer *User) ([]string, []string, error) { +func prepareMailToParticipants(issue *Issue, doer *User) (tos []string, names []string, error error) { if !setting.Service.EnableNotifyMail { return nil, nil, nil } From e05eb960a89631c6986c3f4a5096ff6a35b45c38 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Thu, 4 May 2017 18:13:31 +0200 Subject: [PATCH 03/22] Added comment's hashtag to url for mail notifications. Added explanation to return statement + documentation. Signed-off-by: Jonas --- models/issue_mail.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/models/issue_mail.go b/models/issue_mail.go index 1fabde489a65..bb41f228834e 100644 --- a/models/issue_mail.go +++ b/models/issue_mail.go @@ -19,7 +19,7 @@ func (issue *Issue) mailSubject() string { } -// mailIssueCommentToParticipants can be used for only for comment. +// mailIssueCommentToParticipants can be used only for comment. // 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. @@ -48,6 +48,10 @@ func mailIssueCommentToParticipants(issue *Issue, doer *User, comment *Comment, return nil } +// mailIssueActionToParticipants can be used for creation or pull requests. +// 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 mailIssueActionToParticipants(issue *Issue, doer *User, mentions []string) error { names, tos, err := prepareMailToParticipants(issue, doer) @@ -93,8 +97,8 @@ func prepareMailToParticipants(issue *Issue, doer *User) (tos []string, names [] participants = append(participants, issue.Poster) } - tos := make([]string, 0, len(watchers)) // List of email addresses. - names := make([]string, 0, len(watchers)) + tos = make([]string, 0, len(watchers)) // List of email addresses. + names = make([]string, 0, len(watchers)) for i := range watchers { if watchers[i].UserID == doer.ID { continue From 5cb36ca7bd650fda84a361febf3c26c77ef7f753 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Thu, 4 May 2017 18:30:37 +0200 Subject: [PATCH 04/22] Added comment's hashtag to url for mail notifications. Signed-off-by: Jonas Franz --- models/mail.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/mail.go b/models/mail.go index 165db65698cd..8ac941a26747 100644 --- a/models/mail.go +++ b/models/mail.go @@ -151,7 +151,7 @@ func composeTplData(subject, body, link string) map[string]interface{} { func composeIssueCommentMessage(issue *Issue, doer *User, comment *Comment, tplName base.TplName, tos []string, info string) *mailer.Message { subject := issue.mailSubject() body := string(markdown.RenderString(issue.Content, issue.Repo.HTMLURL(), issue.Repo.ComposeMetas())) - data := composeTplData(subject, body, issue.HTMLURL() + comment.HashTag()) + data := composeTplData(subject, body, issue.HTMLURL() + "#" + comment.HashTag()) data["Doer"] = doer From 2dd674e51dc120c70ca04f3153d246af35d5080b Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Thu, 4 May 2017 21:29:55 +0200 Subject: [PATCH 05/22] Replacing in-line link generation with HTMLURL. (+gofmt) Signed-off-by: Jonas Franz --- models/issue_mail.go | 10 +++------- models/mail.go | 3 ++- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/models/issue_mail.go b/models/issue_mail.go index bb41f228834e..fc58b6f61379 100644 --- a/models/issue_mail.go +++ b/models/issue_mail.go @@ -18,7 +18,6 @@ func (issue *Issue) mailSubject() string { return fmt.Sprintf("[%s] %s (#%d)", issue.Repo.Name, issue.Title, issue.Index) } - // mailIssueCommentToParticipants can be used only for comment. // This function sends two list of emails: // 1. Repository watchers and users who are participated in comments. @@ -27,7 +26,7 @@ func mailIssueCommentToParticipants(issue *Issue, doer *User, comment *Comment, names, tos, err := prepareMailToParticipants(issue, doer) - if(err != nil) { + if err != nil { return fmt.Errorf("PrepareMailToParticipants: %v", err) } @@ -55,7 +54,7 @@ func mailIssueCommentToParticipants(issue *Issue, doer *User, comment *Comment, func mailIssueActionToParticipants(issue *Issue, doer *User, mentions []string) error { names, tos, err := prepareMailToParticipants(issue, doer) - if(err != nil) { + if err != nil { return fmt.Errorf("PrepareMailToParticipants: %v", err) } @@ -77,7 +76,7 @@ func mailIssueActionToParticipants(issue *Issue, doer *User, mentions []string) } // prepareMailToParticipants creates the tos and names list for an issue and the issue's creator. -func prepareMailToParticipants(issue *Issue, doer *User) (tos []string, names []string, error error) { +func prepareMailToParticipants(issue *Issue, doer *User) (tos []string, names []string, error error) { if !setting.Service.EnableNotifyMail { return nil, nil, nil } @@ -128,7 +127,6 @@ func prepareMailToParticipants(issue *Issue, doer *User) (tos []string, names [] return tos, names, nil } - // MailParticipants sends new issue thread created emails to repository watchers // and mentioned people. func (issue *Issue) MailParticipants() (err error) { @@ -143,5 +141,3 @@ func (issue *Issue) MailParticipants() (err error) { return nil } - - diff --git a/models/mail.go b/models/mail.go index 8ac941a26747..4b1b32b1e9e9 100644 --- a/models/mail.go +++ b/models/mail.go @@ -151,7 +151,7 @@ func composeTplData(subject, body, link string) map[string]interface{} { func composeIssueCommentMessage(issue *Issue, doer *User, comment *Comment, tplName base.TplName, tos []string, info string) *mailer.Message { subject := issue.mailSubject() body := string(markdown.RenderString(issue.Content, issue.Repo.HTMLURL(), issue.Repo.ComposeMetas())) - data := composeTplData(subject, body, issue.HTMLURL() + "#" + comment.HashTag()) + data := composeTplData(subject, body, comment.HTMLURL()) data["Doer"] = doer @@ -199,6 +199,7 @@ func SendIssueMentionMail(issue *Issue, doer *User, comment *Comment, tos []stri } mailer.SendAsync(composeIssueCommentMessage(issue, doer, comment, mailIssueMention, tos, "issue mention")) } + // SendIssueActionMail composes and sends issue action emails to target receivers. func SendIssueActionMail(issue *Issue, doer *User, tos []string) { if len(tos) == 0 { From 716bf811b4249215a49ac151f66d626cc32c4e44 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Thu, 4 May 2017 22:28:01 +0200 Subject: [PATCH 06/22] Replaced action-based model with nil-based model. (+gofmt) Signed-off-by: Jonas Franz --- models/issue_comment.go | 2 +- models/issue_mail.go | 91 ++++++++++++++--------------------------- models/mail.go | 39 +++--------------- 3 files changed, 36 insertions(+), 96 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index 430bbdfa2650..a65abfe0124c 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -153,7 +153,7 @@ func (c *Comment) HTMLURL() string { log.Error(4, "GetIssueByID(%d): %v", c.IssueID, err) return "" } - return fmt.Sprintf("%s#issuecomment-%d", issue.HTMLURL(), c.ID) + return fmt.Sprintf("%s#%s", issue.HTMLURL(), c.HashTag()) } // IssueURL formats a URL-string to the issue diff --git a/models/issue_mail.go b/models/issue_mail.go index fc58b6f61379..528a629f3d06 100644 --- a/models/issue_mail.go +++ b/models/issue_mail.go @@ -23,71 +23,17 @@ func (issue *Issue) mailSubject() string { // 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(issue *Issue, doer *User, comment *Comment, mentions []string) error { - - names, tos, err := prepareMailToParticipants(issue, doer) - - if err != nil { - return fmt.Errorf("PrepareMailToParticipants: %v", err) - } - - SendIssueCommentMail(issue, doer, comment, tos) - - // Mail mentioned people and exclude watchers. - names = append(names, doer.Name) - tos = make([]string, 0, len(mentions)) // list of user names. - for i := range mentions { - if com.IsSliceContainsStr(names, mentions[i]) { - continue - } - - tos = append(tos, mentions[i]) - } - SendIssueMentionMail(issue, doer, comment, GetUserEmailsByNames(tos)) - - return nil -} - -// mailIssueActionToParticipants can be used for creation or pull requests. -// 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 mailIssueActionToParticipants(issue *Issue, doer *User, mentions []string) error { - names, tos, err := prepareMailToParticipants(issue, doer) - - if err != nil { - return fmt.Errorf("PrepareMailToParticipants: %v", err) - } - - SendIssueActionMail(issue, doer, tos) - - // Mail mentioned people and exclude watchers. - names = append(names, doer.Name) - tos = make([]string, 0, len(mentions)) // list of user names. - for i := range mentions { - if com.IsSliceContainsStr(names, mentions[i]) { - continue - } - - tos = append(tos, mentions[i]) - } - SendIssueMentionInActionMail(issue, doer, GetUserEmailsByNames(tos)) - - return nil -} - -// prepareMailToParticipants creates the tos and names list for an issue and the issue's creator. -func prepareMailToParticipants(issue *Issue, doer *User) (tos []string, names []string, error error) { if !setting.Service.EnableNotifyMail { - return nil, nil, nil + return nil } watchers, err := GetWatchers(issue.RepoID) if err != nil { - return nil, nil, err + return fmt.Errorf("GetWatchers [repo_id: %d]: %v", issue.RepoID, err) } participants, err := GetParticipantsByIssueID(issue.ID) if err != nil { - return nil, nil, err + return fmt.Errorf("GetParticipantsByIssueID [issue_id: %d]: %v", issue.ID, err) } // In case the issue poster is not watching the repository, @@ -96,8 +42,8 @@ func prepareMailToParticipants(issue *Issue, doer *User) (tos []string, names [] participants = append(participants, issue.Poster) } - tos = make([]string, 0, len(watchers)) // List of email addresses. - names = make([]string, 0, len(watchers)) + tos := make([]string, 0, len(watchers)) // List of email addresses. + names := make([]string, 0, len(watchers)) for i := range watchers { if watchers[i].UserID == doer.ID { continue @@ -105,7 +51,7 @@ func prepareMailToParticipants(issue *Issue, doer *User) (tos []string, names [] to, err := GetUserByID(watchers[i].UserID) if err != nil { - return nil, nil, err + return fmt.Errorf("GetUserByID [%d]: %v", watchers[i].UserID, err) } if to.IsOrganization() { continue @@ -124,7 +70,30 @@ func prepareMailToParticipants(issue *Issue, doer *User) (tos []string, names [] tos = append(tos, participants[i].Email) names = append(names, participants[i].Name) } - return tos, names, nil + + SendIssueCommentMail(issue, doer, comment, tos) + + // Mail mentioned people and exclude watchers. + names = append(names, doer.Name) + tos = make([]string, 0, len(mentions)) // list of user names. + for i := range mentions { + if com.IsSliceContainsStr(names, mentions[i]) { + continue + } + + tos = append(tos, mentions[i]) + } + SendIssueMentionMail(issue, doer, comment, GetUserEmailsByNames(tos)) + + return nil +} + +// mailIssueActionToParticipants can be used for creation or pull requests. +// 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 mailIssueActionToParticipants(issue *Issue, doer *User, mentions []string) error { + return mailIssueCommentToParticipants(issue, doer, nil, mentions) } // MailParticipants sends new issue thread created emails to repository watchers diff --git a/models/mail.go b/models/mail.go index 4b1b32b1e9e9..43536380d8e1 100644 --- a/models/mail.go +++ b/models/mail.go @@ -151,25 +151,13 @@ func composeTplData(subject, body, link string) map[string]interface{} { func composeIssueCommentMessage(issue *Issue, doer *User, comment *Comment, tplName base.TplName, tos []string, info string) *mailer.Message { subject := issue.mailSubject() body := string(markdown.RenderString(issue.Content, issue.Repo.HTMLURL(), issue.Repo.ComposeMetas())) - data := composeTplData(subject, body, comment.HTMLURL()) - data["Doer"] = doer - - var content bytes.Buffer - - if err := templates.ExecuteTemplate(&content, string(tplName), data); err != nil { - log.Error(3, "Template: %v", err) + data := make(map[string]interface{}, 10) + if comment != nil { + data = composeTplData(subject, body, issue.HTMLURL()+"#"+comment.HashTag()) + } else { + data = composeTplData(subject, body, issue.HTMLURL()) } - - msg := mailer.NewMessageFrom(tos, fmt.Sprintf(`"%s" <%s>`, doer.DisplayName(), setting.MailService.FromEmail), subject, content.String()) - msg.Info = fmt.Sprintf("Subject: %s, %s", subject, info) - return msg -} - -func composeIssueMessage(issue *Issue, doer *User, tplName base.TplName, tos []string, info string) *mailer.Message { - subject := issue.mailSubject() - body := string(markdown.RenderString(issue.Content, issue.Repo.HTMLURL(), issue.Repo.ComposeMetas())) - data := composeTplData(subject, body, issue.HTMLURL()) data["Doer"] = doer var content bytes.Buffer @@ -199,20 +187,3 @@ func SendIssueMentionMail(issue *Issue, doer *User, comment *Comment, tos []stri } mailer.SendAsync(composeIssueCommentMessage(issue, doer, comment, mailIssueMention, tos, "issue mention")) } - -// SendIssueActionMail composes and sends issue action emails to target receivers. -func SendIssueActionMail(issue *Issue, doer *User, tos []string) { - if len(tos) == 0 { - return - } - - mailer.SendAsync(composeIssueMessage(issue, doer, mailIssueComment, tos, "issue comment")) -} - -// SendIssueMentionInActionMail composes and sends issue mention emails to target receivers. Only applies if no comment is given. -func SendIssueMentionInActionMail(issue *Issue, doer *User, tos []string) { - if len(tos) == 0 { - return - } - mailer.SendAsync(composeIssueMessage(issue, doer, mailIssueMention, tos, "issue mention")) -} From edaa5761a6811eedde033d97f62c06aa96d591b1 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Thu, 4 May 2017 22:35:45 +0200 Subject: [PATCH 07/22] Replaced mailIssueActionToParticipants with mailIssueCommentToParticipants. Signed-off-by: Jonas Franz --- models/issue_mail.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/models/issue_mail.go b/models/issue_mail.go index 528a629f3d06..a9c5570c298c 100644 --- a/models/issue_mail.go +++ b/models/issue_mail.go @@ -88,14 +88,6 @@ func mailIssueCommentToParticipants(issue *Issue, doer *User, comment *Comment, return nil } -// mailIssueActionToParticipants can be used for creation or pull requests. -// 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 mailIssueActionToParticipants(issue *Issue, doer *User, mentions []string) error { - return mailIssueCommentToParticipants(issue, doer, nil, mentions) -} - // MailParticipants sends new issue thread created emails to repository watchers // and mentioned people. func (issue *Issue) MailParticipants() (err error) { @@ -104,7 +96,7 @@ func (issue *Issue) MailParticipants() (err error) { return fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err) } - if err = mailIssueActionToParticipants(issue, issue.Poster, mentions); err != nil { + if err = mailIssueCommentToParticipants(issue, issue.Poster, nil, mentions); err != nil { log.Error(4, "mailIssueCommentToParticipants: %v", err) } From b0e22aba71e4988c42d6598c5da18d3110e64cd3 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Fri, 5 May 2017 00:06:21 +0200 Subject: [PATCH 08/22] Updating comment for mailIssueCommentToParticipants Signed-off-by: Jonas Franz --- models/issue_mail.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue_mail.go b/models/issue_mail.go index a9c5570c298c..615aa82f0609 100644 --- a/models/issue_mail.go +++ b/models/issue_mail.go @@ -18,7 +18,7 @@ func (issue *Issue) mailSubject() string { return fmt.Sprintf("[%s] %s (#%d)", issue.Repo.Name, issue.Title, issue.Index) } -// mailIssueCommentToParticipants can be used only for comment. +// mailIssueCommentToParticipants can be used for both new issue creation and comment. // 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. From 36ae73298333d068bf5c69dcf064db459806be09 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Thu, 15 Jun 2017 17:13:09 +0200 Subject: [PATCH 09/22] Added link to comment in "Dashboard" Deleting feed entry if a comment is going to be deleted Signed-off-by: Jonas Franz --- models/action.go | 24 ++++++++++++++++++++++++ models/issue_comment.go | 12 +++++++++++- routers/user/home.go | 16 ++++++++++++++++ templates/user/dashboard/feeds.tmpl | 2 +- 4 files changed, 52 insertions(+), 2 deletions(-) diff --git a/models/action.go b/models/action.go index bf3ce027cf3b..3b47a7163b42 100644 --- a/models/action.go +++ b/models/action.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "strconv" ) // ActionType represents the type of an action. @@ -77,6 +78,8 @@ type Action struct { ActUser *User `xorm:"-"` RepoID int64 `xorm:"INDEX"` Repo *Repository `xorm:"-"` + CommentID int64 `xorm:"INDEX"` + Comment *Comment `xorm:"-"` RefName string IsPrivate bool `xorm:"INDEX NOT NULL DEFAULT false"` Content string `xorm:"TEXT"` @@ -191,6 +194,27 @@ func (a *Action) GetRepoLink() string { return "/" + a.GetRepoPath() } +// GetCommentLink returns link to action comment. +func (a *Action) GetCommentLink() string { + if a.Comment == nil { + //Return link to issue if comment is not set. + if len(a.GetIssueInfos()) > 1 { + issueIdString := a.GetIssueInfos()[0] + if issueId, err := strconv.ParseInt(issueIdString, 10, 64); err != nil { + issue, err := GetIssueByID(issueId) + if err != nil { + return "#" + } + return issue.HTMLURL() + }else { + return "#" + } + } + return "#" + } + return a.Comment.HTMLURL() +} + // GetBranch returns the action's repository branch. func (a *Action) GetBranch() string { return a.RefName diff --git a/models/issue_comment.go b/models/issue_comment.go index 69edf28f5432..5aec25995583 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -318,7 +318,8 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err OldTitle: opts.OldTitle, NewTitle: opts.NewTitle, } - if _, err = e.Insert(comment); err != nil { + var commentId int64 + if commentId, err = e.Insert(comment); err != nil { return nil, err } @@ -334,6 +335,8 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err Content: fmt.Sprintf("%d|%s", opts.Issue.Index, strings.Split(opts.Content, "\n")[0]), RepoID: opts.Repo.ID, Repo: opts.Repo, + Comment: comment, + CommentID: commentId, IsPrivate: opts.Repo.IsPrivate, } @@ -641,5 +644,12 @@ func DeleteComment(comment *Comment) error { } } + //Delete associated Action + if _, err := sess.Delete(&Action{ + CommentID: comment.ID, + }); err != nil { + return err + } + return sess.Commit() } diff --git a/routers/user/home.go b/routers/user/home.go index 9ff03cf04f21..2b5779be632e 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -74,6 +74,7 @@ func retrieveFeeds(ctx *context.Context, user *models.User, includePrivate, isPr if ctx.User != nil { userCache[ctx.User.ID] = ctx.User } + commentCache := map[int64]*models.Comment{} repoCache := map[int64]*models.Repository{} for _, act := range actions { // Cache results to reduce queries. @@ -104,6 +105,20 @@ func retrieveFeeds(ctx *context.Context, user *models.User, includePrivate, isPr } act.Repo = repo + + comment, ok := commentCache[act.CommentID] + if !ok { + comment, err = models.GetCommentByID(act.CommentID) + if err != nil { + if models.IsErrCommentNotExist(err) { + continue + } + ctx.Handle(500, "GetCommentByID", err) + return + } + } + act.Comment = comment + repoOwner, ok := userCache[repo.OwnerID] if !ok { repoOwner, err = models.GetUserByID(repo.OwnerID) @@ -115,6 +130,7 @@ func retrieveFeeds(ctx *context.Context, user *models.User, includePrivate, isPr return } } + repo.Owner = repoOwner } ctx.Data["Feeds"] = actions diff --git a/templates/user/dashboard/feeds.tmpl b/templates/user/dashboard/feeds.tmpl index 12768bca5285..855a79786a4b 100644 --- a/templates/user/dashboard/feeds.tmpl +++ b/templates/user/dashboard/feeds.tmpl @@ -63,7 +63,7 @@ {{else if eq .GetOpType 7}} {{index .GetIssueInfos 1}} {{else if eq .GetOpType 10}} - {{.GetIssueTitle}} + {{.GetIssueTitle}}

{{index .GetIssueInfos 1}}

{{else if eq .GetOpType 11}}

{{index .GetIssueInfos 1}}

From 1826b2b03326c25d18076ef4050e38a58ada0b5d Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sat, 17 Jun 2017 13:30:08 +0200 Subject: [PATCH 10/22] Added migration Signed-off-by: Jonas Franz --- models/migrations/migrations.go | 2 ++ models/migrations/v35.go | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 models/migrations/v35.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 2a54c497c1f6..923984ac061f 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -118,6 +118,8 @@ var migrations = []Migration{ NewMigration("remove columns from action", removeActionColumns), // v34 -> v35 NewMigration("give all units to owner teams", giveAllUnitsToOwnerTeams), + // v35 -> v36 + NewMigration("adds comment to an action", addCommentIdToAction), } // Migrate database to current version diff --git a/models/migrations/v35.go b/models/migrations/v35.go new file mode 100644 index 000000000000..50450cd56738 --- /dev/null +++ b/models/migrations/v35.go @@ -0,0 +1,46 @@ +// Copyright 2017 The Gogs 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 migrations + +import ( + "fmt" + + "github.com/go-xorm/xorm" + "time" +) + +func addCommentIdToAction(x *xorm.Engine) error { + // Action see models/action.go + type Action struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"INDEX"` // Receiver user id. + ActUserID int64 `xorm:"INDEX"` // Action user id. + RepoID int64 `xorm:"INDEX"` + CommentID int64 `xorm:"INDEX"` + RefName string + IsPrivate bool `xorm:"INDEX NOT NULL DEFAULT false"` + Content string `xorm:"TEXT"` + Created time.Time `xorm:"-"` + CreatedUnix int64 `xorm:"INDEX"` + } + + var actions []*Action + if err := x.Find(&actions); err != nil { + return fmt.Errorf("Find: %v", err) + } + + if err := x.DropTables(new(Action)); err != nil { + return fmt.Errorf("DropTables: %v", err) + } + + if err := x.Sync2(new(Action)); err != nil { + return fmt.Errorf("Sync2: %v", err) + } + + if _, err := x.Insert(actions); err != nil { + return fmt.Errorf("Insert: %v", err) + } + return nil +} From bb33f2eac22794e35eb569611311d26a0a06098d Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sat, 17 Jun 2017 15:04:08 +0200 Subject: [PATCH 11/22] Added improved migration to add a CommentID column to action. Added improved links to comments in feed entries. Signed-off-by: Jonas Franz --- models/action.go | 41 ++++++++++++++++++----------- models/issue_comment.go | 5 ++-- models/migrations/v35.go | 23 +--------------- templates/user/dashboard/feeds.tmpl | 2 +- 4 files changed, 30 insertions(+), 41 deletions(-) diff --git a/models/action.go b/models/action.go index 3b47a7163b42..65a4ed5626b4 100644 --- a/models/action.go +++ b/models/action.go @@ -196,23 +196,34 @@ func (a *Action) GetRepoLink() string { // GetCommentLink returns link to action comment. func (a *Action) GetCommentLink() string { - if a.Comment == nil { - //Return link to issue if comment is not set. - if len(a.GetIssueInfos()) > 1 { - issueIdString := a.GetIssueInfos()[0] - if issueId, err := strconv.ParseInt(issueIdString, 10, 64); err != nil { - issue, err := GetIssueByID(issueId) - if err != nil { - return "#" - } - return issue.HTMLURL() - }else { - return "#" - } - } + if a == nil { + return "#" + } + if a.Comment == nil && a.CommentID != 0 { + a.Comment, _ = GetCommentByID(a.CommentID) + } + if a.Comment != nil { + return a.Comment.HTMLURL() + } + if len(a.GetIssueInfos()) == 0 { + return "#" + } + //Return link to issue + issueIdString := a.GetIssueInfos()[0] + issueId, err := strconv.ParseInt(issueIdString, 10, 64) + + if err != nil { return "#" } - return a.Comment.HTMLURL() + + issue, err := GetIssueByID(issueId) + + if err != nil { + return "#" + } + + return issue.HTMLURL() + } // GetBranch returns the action's repository branch. diff --git a/models/issue_comment.go b/models/issue_comment.go index 5aec25995583..22286e0958f0 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -318,8 +318,7 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err OldTitle: opts.OldTitle, NewTitle: opts.NewTitle, } - var commentId int64 - if commentId, err = e.Insert(comment); err != nil { + if _, err = e.Insert(comment); err != nil { return nil, err } @@ -336,7 +335,7 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err RepoID: opts.Repo.ID, Repo: opts.Repo, Comment: comment, - CommentID: commentId, + CommentID: comment.ID, IsPrivate: opts.Repo.IsPrivate, } diff --git a/models/migrations/v35.go b/models/migrations/v35.go index 50450cd56738..daef9c49ffb7 100644 --- a/models/migrations/v35.go +++ b/models/migrations/v35.go @@ -8,39 +8,18 @@ import ( "fmt" "github.com/go-xorm/xorm" - "time" ) func addCommentIdToAction(x *xorm.Engine) error { // Action see models/action.go type Action struct { - ID int64 `xorm:"pk autoincr"` - UserID int64 `xorm:"INDEX"` // Receiver user id. - ActUserID int64 `xorm:"INDEX"` // Action user id. - RepoID int64 `xorm:"INDEX"` - CommentID int64 `xorm:"INDEX"` - RefName string - IsPrivate bool `xorm:"INDEX NOT NULL DEFAULT false"` - Content string `xorm:"TEXT"` - Created time.Time `xorm:"-"` - CreatedUnix int64 `xorm:"INDEX"` + CommentID int64 `xorm:"INDEX"` } - var actions []*Action - if err := x.Find(&actions); err != nil { - return fmt.Errorf("Find: %v", err) - } - - if err := x.DropTables(new(Action)); err != nil { - return fmt.Errorf("DropTables: %v", err) - } if err := x.Sync2(new(Action)); err != nil { return fmt.Errorf("Sync2: %v", err) } - if _, err := x.Insert(actions); err != nil { - return fmt.Errorf("Insert: %v", err) - } return nil } diff --git a/templates/user/dashboard/feeds.tmpl b/templates/user/dashboard/feeds.tmpl index 855a79786a4b..9431d75c356c 100644 --- a/templates/user/dashboard/feeds.tmpl +++ b/templates/user/dashboard/feeds.tmpl @@ -68,7 +68,7 @@ {{else if eq .GetOpType 11}}

{{index .GetIssueInfos 1}}

{{else if (or (or (eq .GetOpType 12) (eq .GetOpType 13)) (or (eq .GetOpType 14) (eq .GetOpType 15)))}} - {{.GetIssueTitle}} + {{.GetIssueTitle}} {{end}}

{{TimeSince .GetCreate $.i18n.Lang}}

From 3f1c85c40703a777223d7136ffa915938fb18f4b Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sat, 17 Jun 2017 15:04:37 +0200 Subject: [PATCH 12/22] Added improved migration to add a CommentID column to action. Added improved links to comments in feed entries. Signed-off-by: Jonas Franz --- models/action.go | 4 ++-- models/migrations/v35.go | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/models/action.go b/models/action.go index 65a4ed5626b4..4ab4746d1c36 100644 --- a/models/action.go +++ b/models/action.go @@ -78,8 +78,8 @@ type Action struct { ActUser *User `xorm:"-"` RepoID int64 `xorm:"INDEX"` Repo *Repository `xorm:"-"` - CommentID int64 `xorm:"INDEX"` - Comment *Comment `xorm:"-"` + CommentID int64 `xorm:"INDEX"` + Comment *Comment `xorm:"-"` RefName string IsPrivate bool `xorm:"INDEX NOT NULL DEFAULT false"` Content string `xorm:"TEXT"` diff --git a/models/migrations/v35.go b/models/migrations/v35.go index daef9c49ffb7..b5f1f1c11dbc 100644 --- a/models/migrations/v35.go +++ b/models/migrations/v35.go @@ -13,10 +13,9 @@ import ( func addCommentIdToAction(x *xorm.Engine) error { // Action see models/action.go type Action struct { - CommentID int64 `xorm:"INDEX"` + CommentID int64 `xorm:"INDEX"` } - if err := x.Sync2(new(Action)); err != nil { return fmt.Errorf("Sync2: %v", err) } From 5791853b077ebf3a21b9a465a0bace5c9fe12dfa Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sat, 17 Jun 2017 15:13:27 +0200 Subject: [PATCH 13/22] Added improved migration to add a CommentID column to action. Added improved links to comments in feed entries. Signed-off-by: Jonas Franz --- models/action.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/action.go b/models/action.go index 4ab4746d1c36..4ef070dbae57 100644 --- a/models/action.go +++ b/models/action.go @@ -209,14 +209,14 @@ func (a *Action) GetCommentLink() string { return "#" } //Return link to issue - issueIdString := a.GetIssueInfos()[0] - issueId, err := strconv.ParseInt(issueIdString, 10, 64) + issueIDString := a.GetIssueInfos()[0] + issueID, err := strconv.ParseInt(issueIDString, 10, 64) if err != nil { return "#" } - issue, err := GetIssueByID(issueId) + issue, err := GetIssueByID(issueID) if err != nil { return "#" From deddcdaa1b5efe53f52ff1ded1404987757b40df Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sat, 17 Jun 2017 15:14:17 +0200 Subject: [PATCH 14/22] Added improved migration to add a CommentID column to action. Added improved links to comments in feed entries. (+ gofmt) Signed-off-by: Jonas Franz --- routers/user/home.go | 1 - 1 file changed, 1 deletion(-) diff --git a/routers/user/home.go b/routers/user/home.go index 2b5779be632e..315301fc7026 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -105,7 +105,6 @@ func retrieveFeeds(ctx *context.Context, user *models.User, includePrivate, isPr } act.Repo = repo - comment, ok := commentCache[act.CommentID] if !ok { comment, err = models.GetCommentByID(act.CommentID) From 70d8dd910b5ec8e42f113b3dd0c7f557517e1055 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sat, 17 Jun 2017 15:17:15 +0200 Subject: [PATCH 15/22] Added improved migration to add a CommentID column to action. Added improved links to comments in feed entries. (+ gofmt) Signed-off-by: Jonas Franz --- models/migrations/migrations.go | 2 +- models/migrations/v35.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 923984ac061f..87f1850a6db8 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -119,7 +119,7 @@ var migrations = []Migration{ // v34 -> v35 NewMigration("give all units to owner teams", giveAllUnitsToOwnerTeams), // v35 -> v36 - NewMigration("adds comment to an action", addCommentIdToAction), + NewMigration("adds comment to an action", addCommentIDToAction), } // Migrate database to current version diff --git a/models/migrations/v35.go b/models/migrations/v35.go index b5f1f1c11dbc..c716438f48aa 100644 --- a/models/migrations/v35.go +++ b/models/migrations/v35.go @@ -10,7 +10,7 @@ import ( "github.com/go-xorm/xorm" ) -func addCommentIdToAction(x *xorm.Engine) error { +func addCommentIDToAction(x *xorm.Engine) error { // Action see models/action.go type Action struct { CommentID int64 `xorm:"INDEX"` From 3b997264e903ba05ed0211f9ff8f2f5ca6b12a36 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sat, 17 Jun 2017 15:24:13 +0200 Subject: [PATCH 16/22] Added improved migration to add a CommentID column to action. Added improved links to comments in feed entries. (+ gofmt) Signed-off-by: Jonas Franz --- models/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/action.go b/models/action.go index 4ef070dbae57..db3592536021 100644 --- a/models/action.go +++ b/models/action.go @@ -9,6 +9,7 @@ import ( "fmt" "path" "regexp" + "strconv" "strings" "time" "unicode" @@ -22,7 +23,6 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" - "strconv" ) // ActionType represents the type of an action. From 9e18513430cdba60fb3402c4602f504931f68b72 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sat, 17 Jun 2017 16:00:00 +0200 Subject: [PATCH 17/22] Added improved migration to add a CommentID column to action. Added improved links to comments in feed entries. (+ gofmt) Signed-off-by: Jonas Franz --- models/migrations/v35.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/v35.go b/models/migrations/v35.go index c716438f48aa..0bfcbc7aaa0b 100644 --- a/models/migrations/v35.go +++ b/models/migrations/v35.go @@ -1,4 +1,4 @@ -// Copyright 2017 The Gogs Authors. All rights reserved. +// Copyright 2017 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. From 0e20e81957ce4436d441dfeb70b9686204bb1c66 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sat, 17 Jun 2017 18:26:54 +0200 Subject: [PATCH 18/22] Added improved migration to add a CommentID column to action. Added improved links to comments in feed entries. (+ gofmt) Signed-off-by: Jonas Franz --- routers/user/home.go | 14 -------------- templates/user/dashboard/feeds.tmpl | 2 +- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/routers/user/home.go b/routers/user/home.go index 315301fc7026..3867d01a6337 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -74,7 +74,6 @@ func retrieveFeeds(ctx *context.Context, user *models.User, includePrivate, isPr if ctx.User != nil { userCache[ctx.User.ID] = ctx.User } - commentCache := map[int64]*models.Comment{} repoCache := map[int64]*models.Repository{} for _, act := range actions { // Cache results to reduce queries. @@ -105,19 +104,6 @@ func retrieveFeeds(ctx *context.Context, user *models.User, includePrivate, isPr } act.Repo = repo - comment, ok := commentCache[act.CommentID] - if !ok { - comment, err = models.GetCommentByID(act.CommentID) - if err != nil { - if models.IsErrCommentNotExist(err) { - continue - } - ctx.Handle(500, "GetCommentByID", err) - return - } - } - act.Comment = comment - repoOwner, ok := userCache[repo.OwnerID] if !ok { repoOwner, err = models.GetUserByID(repo.OwnerID) diff --git a/templates/user/dashboard/feeds.tmpl b/templates/user/dashboard/feeds.tmpl index 9431d75c356c..855a79786a4b 100644 --- a/templates/user/dashboard/feeds.tmpl +++ b/templates/user/dashboard/feeds.tmpl @@ -68,7 +68,7 @@ {{else if eq .GetOpType 11}}

{{index .GetIssueInfos 1}}

{{else if (or (or (eq .GetOpType 12) (eq .GetOpType 13)) (or (eq .GetOpType 14) (eq .GetOpType 15)))}} - {{.GetIssueTitle}} + {{.GetIssueTitle}} {{end}}

{{TimeSince .GetCreate $.i18n.Lang}}

From 8818bc7bd48773974a7d7a3c9c67d6ca267685bb Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sun, 18 Jun 2017 09:44:34 +0200 Subject: [PATCH 19/22] Added improved links to comments in feed entries. (+ gofmt) Signed-off-by: Jonas Franz --- routers/user/home.go | 1 - 1 file changed, 1 deletion(-) diff --git a/routers/user/home.go b/routers/user/home.go index 3867d01a6337..9ff03cf04f21 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -115,7 +115,6 @@ func retrieveFeeds(ctx *context.Context, user *models.User, includePrivate, isPr return } } - repo.Owner = repoOwner } ctx.Data["Feeds"] = actions From 3e1241c373bd4a9bad735394ab9d99ff11dbdb0a Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Thu, 22 Jun 2017 20:21:01 +0200 Subject: [PATCH 20/22] Fixes #1956 by filtering for deleted comments that are referenced in actions. Signed-off-by: Jonas Franz --- models/action.go | 17 ++++++++++++----- models/action_test.go | 27 +++++++++++++++------------ models/issue_comment.go | 7 ------- routers/user/home.go | 13 +++++++------ routers/user/profile.go | 2 +- 5 files changed, 35 insertions(+), 31 deletions(-) diff --git a/models/action.go b/models/action.go index db3592536021..e1d51f5d6f25 100644 --- a/models/action.go +++ b/models/action.go @@ -223,7 +223,6 @@ func (a *Action) GetCommentLink() string { } return issue.HTMLURL() - } // GetBranch returns the action's repository branch. @@ -709,10 +708,11 @@ func MergePullRequestAction(actUser *User, repo *Repository, pull *Issue) error // GetFeedsOptions options for retrieving feeds type GetFeedsOptions struct { - RequestedUser *User - RequestingUserID int64 - IncludePrivate bool // include private actions - OnlyPerformedBy bool // only actions performed by requested user + RequestedUser *User + RequestingUserID int64 + IncludePrivate bool // include private actions + OnlyPerformedBy bool // only actions performed by requested user + IncludeDeletedComments bool // include comments that reference a deleted comment } // GetFeeds returns actions according to the provided options @@ -741,5 +741,12 @@ func GetFeeds(opts GetFeedsOptions) ([]*Action, error) { if opts.RequestedUser.IsOrganization() { sess.In("repo_id", repoIDs) } + + if !opts.IncludeDeletedComments { + // commentID != 0 AND comment exist + sess.And("(comment_id ISNULL OR comment_id = 0 OR EXISTS(SELECT NULL FROM comment c WHERE comment_id = c.id))") + + } + return actions, sess.Find(&actions) } diff --git a/models/action_test.go b/models/action_test.go index debc884b37f5..a3e382555e53 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -306,10 +306,11 @@ func TestGetFeeds(t *testing.T) { user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) actions, err := GetFeeds(GetFeedsOptions{ - RequestedUser: user, - RequestingUserID: user.ID, - IncludePrivate: true, - OnlyPerformedBy: false, + RequestedUser: user, + RequestingUserID: user.ID, + IncludePrivate: true, + OnlyPerformedBy: false, + IncludeDeletedComments: true, }) assert.NoError(t, err) assert.Len(t, actions, 1) @@ -333,10 +334,11 @@ func TestGetFeeds2(t *testing.T) { userID := AssertExistsAndLoadBean(t, &OrgUser{OrgID: org.ID, IsOwner: true}).(*OrgUser).UID actions, err := GetFeeds(GetFeedsOptions{ - RequestedUser: org, - RequestingUserID: userID, - IncludePrivate: true, - OnlyPerformedBy: false, + RequestedUser: org, + RequestingUserID: userID, + IncludePrivate: true, + OnlyPerformedBy: false, + IncludeDeletedComments: true, }) assert.NoError(t, err) assert.Len(t, actions, 1) @@ -344,10 +346,11 @@ func TestGetFeeds2(t *testing.T) { assert.EqualValues(t, org.ID, actions[0].UserID) actions, err = GetFeeds(GetFeedsOptions{ - RequestedUser: org, - RequestingUserID: userID, - IncludePrivate: false, - OnlyPerformedBy: false, + RequestedUser: org, + RequestingUserID: userID, + IncludePrivate: false, + OnlyPerformedBy: false, + IncludeDeletedComments: true, }) assert.NoError(t, err) assert.Len(t, actions, 0) diff --git a/models/issue_comment.go b/models/issue_comment.go index 22286e0958f0..8ed6c1eb01cd 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -643,12 +643,5 @@ func DeleteComment(comment *Comment) error { } } - //Delete associated Action - if _, err := sess.Delete(&Action{ - CommentID: comment.ID, - }); err != nil { - return err - } - return sess.Commit() } diff --git a/routers/user/home.go b/routers/user/home.go index 9ff03cf04f21..c39e31bc6423 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -54,16 +54,17 @@ func getDashboardContextUser(ctx *context.Context) *models.User { } // retrieveFeeds loads feeds for the specified user -func retrieveFeeds(ctx *context.Context, user *models.User, includePrivate, isProfile bool) { +func retrieveFeeds(ctx *context.Context, user *models.User, includePrivate, isProfile bool, includeDeletedComments bool) { var requestingID int64 if ctx.User != nil { requestingID = ctx.User.ID } actions, err := models.GetFeeds(models.GetFeedsOptions{ - RequestedUser: user, - RequestingUserID: requestingID, - IncludePrivate: includePrivate, - OnlyPerformedBy: isProfile, + RequestedUser: user, + RequestingUserID: requestingID, + IncludePrivate: includePrivate, + OnlyPerformedBy: isProfile, + IncludeDeletedComments: includeDeletedComments, }) if err != nil { ctx.Handle(500, "GetFeeds", err) @@ -186,7 +187,7 @@ func Dashboard(ctx *context.Context) { ctx.Data["MirrorCount"] = len(mirrors) ctx.Data["Mirrors"] = mirrors - retrieveFeeds(ctx, ctxUser, true, false) + retrieveFeeds(ctx, ctxUser, true, false, false) if ctx.Written() { return } diff --git a/routers/user/profile.go b/routers/user/profile.go index 1768cec7b10b..c7d761052e85 100644 --- a/routers/user/profile.go +++ b/routers/user/profile.go @@ -138,7 +138,7 @@ func Profile(ctx *context.Context) { ctx.Data["Keyword"] = keyword switch tab { case "activity": - retrieveFeeds(ctx, ctxUser, showPrivate, true) + retrieveFeeds(ctx, ctxUser, showPrivate, true, false) if ctx.Written() { return } From 3ee3dc3768fc74da52278790e0283c7f660ab278 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Fri, 23 Jun 2017 20:26:20 +0200 Subject: [PATCH 21/22] Fixes #1956 by filtering for deleted comments. Signed-off-by: Jonas Franz --- models/action.go | 34 ++++++++++++++++------------------ models/issue_comment.go | 2 ++ models/migrations/v35.go | 7 ++++++- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/models/action.go b/models/action.go index e1d51f5d6f25..a499f704ca95 100644 --- a/models/action.go +++ b/models/action.go @@ -71,20 +71,21 @@ func init() { // repository. It implemented interface base.Actioner so that can be // used in template render. type Action struct { - ID int64 `xorm:"pk autoincr"` - UserID int64 `xorm:"INDEX"` // Receiver user id. - OpType ActionType - ActUserID int64 `xorm:"INDEX"` // Action user id. - ActUser *User `xorm:"-"` - RepoID int64 `xorm:"INDEX"` - Repo *Repository `xorm:"-"` - CommentID int64 `xorm:"INDEX"` - Comment *Comment `xorm:"-"` - RefName string - IsPrivate bool `xorm:"INDEX NOT NULL DEFAULT false"` - Content string `xorm:"TEXT"` - Created time.Time `xorm:"-"` - CreatedUnix int64 `xorm:"INDEX"` + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"INDEX"` // Receiver user id. + OpType ActionType + ActUserID int64 `xorm:"INDEX"` // Action user id. + ActUser *User `xorm:"-"` + RepoID int64 `xorm:"INDEX"` + Repo *Repository `xorm:"-"` + CommentID int64 `xorm:"INDEX"` + Comment *Comment `xorm:"-"` + CommentDeleted bool `xorm:"default 0 not null"` + RefName string + IsPrivate bool `xorm:"INDEX NOT NULL DEFAULT false"` + Content string `xorm:"TEXT"` + Created time.Time `xorm:"-"` + CreatedUnix int64 `xorm:"INDEX"` } // BeforeInsert will be invoked by XORM before inserting a record @@ -211,13 +212,11 @@ func (a *Action) GetCommentLink() string { //Return link to issue issueIDString := a.GetIssueInfos()[0] issueID, err := strconv.ParseInt(issueIDString, 10, 64) - if err != nil { return "#" } issue, err := GetIssueByID(issueID) - if err != nil { return "#" } @@ -743,8 +742,7 @@ func GetFeeds(opts GetFeedsOptions) ([]*Action, error) { } if !opts.IncludeDeletedComments { - // commentID != 0 AND comment exist - sess.And("(comment_id ISNULL OR comment_id = 0 OR EXISTS(SELECT NULL FROM comment c WHERE comment_id = c.id))") + sess.And("comment_deleted = ?", false) } diff --git a/models/issue_comment.go b/models/issue_comment.go index 8ed6c1eb01cd..ecff06941dea 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -643,5 +643,7 @@ func DeleteComment(comment *Comment) error { } } + sess.Exec("UPDATE `action` SET comment_deleted = 1 WHERE comment_id = ?", comment.ID) + return sess.Commit() } diff --git a/models/migrations/v35.go b/models/migrations/v35.go index 0bfcbc7aaa0b..e5089f7a9650 100644 --- a/models/migrations/v35.go +++ b/models/migrations/v35.go @@ -13,12 +13,17 @@ import ( func addCommentIDToAction(x *xorm.Engine) error { // Action see models/action.go type Action struct { - CommentID int64 `xorm:"INDEX"` + CommentID int64 `xorm:"INDEX"` + CommentDeleted bool `xorm:"default 0 not null"` } if err := x.Sync2(new(Action)); err != nil { return fmt.Errorf("Sync2: %v", err) } + x.Where("comment_id = 0").Update(&Action{ + CommentDeleted: false, + }) + return nil } From 56a64e3e34f3187c167a594407ebade02aaa95cb Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sun, 25 Jun 2017 18:17:50 +0200 Subject: [PATCH 22/22] Fixes #1956 by filtering for deleted comments. Introducing "IsDeleted" column to action. Signed-off-by: Jonas Franz --- models/action.go | 44 ++++++++++++++++++++-------------------- models/action_test.go | 30 +++++++++++++-------------- models/issue_comment.go | 3 +-- models/migrations/v35.go | 8 ++------ routers/user/home.go | 10 ++++----- 5 files changed, 45 insertions(+), 50 deletions(-) diff --git a/models/action.go b/models/action.go index a499f704ca95..9a12e9229bcf 100644 --- a/models/action.go +++ b/models/action.go @@ -71,21 +71,21 @@ func init() { // repository. It implemented interface base.Actioner so that can be // used in template render. type Action struct { - ID int64 `xorm:"pk autoincr"` - UserID int64 `xorm:"INDEX"` // Receiver user id. - OpType ActionType - ActUserID int64 `xorm:"INDEX"` // Action user id. - ActUser *User `xorm:"-"` - RepoID int64 `xorm:"INDEX"` - Repo *Repository `xorm:"-"` - CommentID int64 `xorm:"INDEX"` - Comment *Comment `xorm:"-"` - CommentDeleted bool `xorm:"default 0 not null"` - RefName string - IsPrivate bool `xorm:"INDEX NOT NULL DEFAULT false"` - Content string `xorm:"TEXT"` - Created time.Time `xorm:"-"` - CreatedUnix int64 `xorm:"INDEX"` + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"INDEX"` // Receiver user id. + OpType ActionType + ActUserID int64 `xorm:"INDEX"` // Action user id. + ActUser *User `xorm:"-"` + RepoID int64 `xorm:"INDEX"` + Repo *Repository `xorm:"-"` + CommentID int64 `xorm:"INDEX"` + Comment *Comment `xorm:"-"` + IsDeleted bool `xorm:"INDEX NOT NULL DEFAULT false"` + RefName string + IsPrivate bool `xorm:"INDEX NOT NULL DEFAULT false"` + Content string `xorm:"TEXT"` + Created time.Time `xorm:"-"` + CreatedUnix int64 `xorm:"INDEX"` } // BeforeInsert will be invoked by XORM before inserting a record @@ -707,11 +707,11 @@ func MergePullRequestAction(actUser *User, repo *Repository, pull *Issue) error // GetFeedsOptions options for retrieving feeds type GetFeedsOptions struct { - RequestedUser *User - RequestingUserID int64 - IncludePrivate bool // include private actions - OnlyPerformedBy bool // only actions performed by requested user - IncludeDeletedComments bool // include comments that reference a deleted comment + RequestedUser *User + RequestingUserID int64 + IncludePrivate bool // include private actions + OnlyPerformedBy bool // only actions performed by requested user + IncludeDeleted bool // include deleted actions } // GetFeeds returns actions according to the provided options @@ -741,8 +741,8 @@ func GetFeeds(opts GetFeedsOptions) ([]*Action, error) { sess.In("repo_id", repoIDs) } - if !opts.IncludeDeletedComments { - sess.And("comment_deleted = ?", false) + if !opts.IncludeDeleted { + sess.And("is_deleted = ?", false) } diff --git a/models/action_test.go b/models/action_test.go index a3e382555e53..96ee9a5f73bc 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -306,11 +306,11 @@ func TestGetFeeds(t *testing.T) { user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) actions, err := GetFeeds(GetFeedsOptions{ - RequestedUser: user, - RequestingUserID: user.ID, - IncludePrivate: true, - OnlyPerformedBy: false, - IncludeDeletedComments: true, + RequestedUser: user, + RequestingUserID: user.ID, + IncludePrivate: true, + OnlyPerformedBy: false, + IncludeDeleted: true, }) assert.NoError(t, err) assert.Len(t, actions, 1) @@ -334,11 +334,11 @@ func TestGetFeeds2(t *testing.T) { userID := AssertExistsAndLoadBean(t, &OrgUser{OrgID: org.ID, IsOwner: true}).(*OrgUser).UID actions, err := GetFeeds(GetFeedsOptions{ - RequestedUser: org, - RequestingUserID: userID, - IncludePrivate: true, - OnlyPerformedBy: false, - IncludeDeletedComments: true, + RequestedUser: org, + RequestingUserID: userID, + IncludePrivate: true, + OnlyPerformedBy: false, + IncludeDeleted: true, }) assert.NoError(t, err) assert.Len(t, actions, 1) @@ -346,11 +346,11 @@ func TestGetFeeds2(t *testing.T) { assert.EqualValues(t, org.ID, actions[0].UserID) actions, err = GetFeeds(GetFeedsOptions{ - RequestedUser: org, - RequestingUserID: userID, - IncludePrivate: false, - OnlyPerformedBy: false, - IncludeDeletedComments: true, + RequestedUser: org, + RequestingUserID: userID, + IncludePrivate: false, + OnlyPerformedBy: false, + IncludeDeleted: true, }) assert.NoError(t, err) assert.Len(t, actions, 0) diff --git a/models/issue_comment.go b/models/issue_comment.go index ecff06941dea..19bda2834027 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -642,8 +642,7 @@ func DeleteComment(comment *Comment) error { return err } } - - sess.Exec("UPDATE `action` SET comment_deleted = 1 WHERE comment_id = ?", comment.ID) + sess.Where("comment_id = ?", comment.ID).Cols("is_deleted").Update(&Action{IsDeleted: true}) return sess.Commit() } diff --git a/models/migrations/v35.go b/models/migrations/v35.go index e5089f7a9650..7746663a40a8 100644 --- a/models/migrations/v35.go +++ b/models/migrations/v35.go @@ -13,17 +13,13 @@ import ( func addCommentIDToAction(x *xorm.Engine) error { // Action see models/action.go type Action struct { - CommentID int64 `xorm:"INDEX"` - CommentDeleted bool `xorm:"default 0 not null"` + CommentID int64 `xorm:"INDEX"` + IsDeleted bool `xorm:"INDEX NOT NULL DEFAULT false"` } if err := x.Sync2(new(Action)); err != nil { return fmt.Errorf("Sync2: %v", err) } - x.Where("comment_id = 0").Update(&Action{ - CommentDeleted: false, - }) - return nil } diff --git a/routers/user/home.go b/routers/user/home.go index c39e31bc6423..71288906908f 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -60,11 +60,11 @@ func retrieveFeeds(ctx *context.Context, user *models.User, includePrivate, isPr requestingID = ctx.User.ID } actions, err := models.GetFeeds(models.GetFeedsOptions{ - RequestedUser: user, - RequestingUserID: requestingID, - IncludePrivate: includePrivate, - OnlyPerformedBy: isProfile, - IncludeDeletedComments: includeDeletedComments, + RequestedUser: user, + RequestingUserID: requestingID, + IncludePrivate: includePrivate, + OnlyPerformedBy: isProfile, + IncludeDeleted: includeDeletedComments, }) if err != nil { ctx.Handle(500, "GetFeeds", err)