diff --git a/models/error.go b/models/error.go
index 16be512139..a17ff5f9d0 100644
--- a/models/error.go
+++ b/models/error.go
@@ -1121,6 +1121,38 @@ func (err ErrNewIssueInsert) Error() string {
return err.OriginalError.Error()
}
+// ErrIssueWasClosed is used when close a closed issue
+type ErrIssueWasClosed struct {
+ ID int64
+ Index int64
+}
+
+// IsErrIssueWasClosed checks if an error is a ErrIssueWasClosed.
+func IsErrIssueWasClosed(err error) bool {
+ _, ok := err.(ErrIssueWasClosed)
+ return ok
+}
+
+func (err ErrIssueWasClosed) Error() string {
+ return fmt.Sprintf("Issue [%d] %d was already closed", err.ID, err.Index)
+}
+
+// ErrPullWasClosed is used close a closed pull request
+type ErrPullWasClosed struct {
+ ID int64
+ Index int64
+}
+
+// IsErrPullWasClosed checks if an error is a ErrErrPullWasClosed.
+func IsErrPullWasClosed(err error) bool {
+ _, ok := err.(ErrPullWasClosed)
+ return ok
+}
+
+func (err ErrPullWasClosed) Error() string {
+ return fmt.Sprintf("Pull request [%d] %d was already closed", err.ID, err.Index)
+}
+
// ErrForbiddenIssueReaction is used when a forbidden reaction was try to created
type ErrForbiddenIssueReaction struct {
Reaction string
diff --git a/models/issue.go b/models/issue.go
index 0a08a97fdd..48974b279e 100644
--- a/models/issue.go
+++ b/models/issue.go
@@ -600,16 +600,23 @@ func updateIssueCols(e Engine, issue *Issue, cols ...string) error {
return nil
}
-func (issue *Issue) changeStatus(e *xorm.Session, doer *User, isClosed bool) (err error) {
+func (issue *Issue) changeStatus(e *xorm.Session, doer *User, isClosed bool) (*Comment, error) {
// Reload the issue
currentIssue, err := getIssueByID(e, issue.ID)
if err != nil {
- return err
+ return nil, err
}
// Nothing should be performed if current status is same as target status
if currentIssue.IsClosed == isClosed {
- return nil
+ if !issue.IsPull {
+ return nil, ErrIssueWasClosed{
+ ID: issue.ID,
+ }
+ }
+ return nil, ErrPullWasClosed{
+ ID: issue.ID,
+ }
}
// Check for open dependencies
@@ -617,11 +624,11 @@ func (issue *Issue) changeStatus(e *xorm.Session, doer *User, isClosed bool) (er
// only check if dependencies are enabled and we're about to close an issue, otherwise reopening an issue would fail when there are unsatisfied dependencies
noDeps, err := issueNoDependenciesLeft(e, issue)
if err != nil {
- return err
+ return nil, err
}
if !noDeps {
- return ErrDependenciesLeft{issue.ID}
+ return nil, ErrDependenciesLeft{issue.ID}
}
}
@@ -633,22 +640,22 @@ func (issue *Issue) changeStatus(e *xorm.Session, doer *User, isClosed bool) (er
}
if err = updateIssueCols(e, issue, "is_closed", "closed_unix"); err != nil {
- return err
+ return nil, err
}
// Update issue count of labels
if err = issue.getLabels(e); err != nil {
- return err
+ return nil, err
}
for idx := range issue.Labels {
if err = updateLabel(e, issue.Labels[idx]); err != nil {
- return err
+ return nil, err
}
}
// Update issue count of milestone
if err := updateMilestoneClosedNum(e, issue.MilestoneID); err != nil {
- return err
+ return nil, err
}
// New action comment
@@ -657,43 +664,39 @@ func (issue *Issue) changeStatus(e *xorm.Session, doer *User, isClosed bool) (er
cmtType = CommentTypeReopen
}
- var opts = &CreateCommentOptions{
+ return createCommentWithNoAction(e, &CreateCommentOptions{
Type: cmtType,
Doer: doer,
Repo: issue.Repo,
Issue: issue,
- }
- comment, err := createCommentWithNoAction(e, opts)
- if err != nil {
- return err
- }
- return sendCreateCommentAction(e, opts, comment)
+ })
}
// ChangeStatus changes issue status to open or closed.
-func (issue *Issue) ChangeStatus(doer *User, isClosed bool) (err error) {
+func (issue *Issue) ChangeStatus(doer *User, isClosed bool) (*Comment, error) {
sess := x.NewSession()
defer sess.Close()
- if err = sess.Begin(); err != nil {
- return err
+ if err := sess.Begin(); err != nil {
+ return nil, err
}
- if err = issue.loadRepo(sess); err != nil {
- return err
+ if err := issue.loadRepo(sess); err != nil {
+ return nil, err
}
- if err = issue.loadPoster(sess); err != nil {
- return err
+ if err := issue.loadPoster(sess); err != nil {
+ return nil, err
}
- if err = issue.changeStatus(sess, doer, isClosed); err != nil {
- return err
+ comment, err := issue.changeStatus(sess, doer, isClosed)
+ if err != nil {
+ return nil, err
}
if err = sess.Commit(); err != nil {
- return fmt.Errorf("Commit: %v", err)
+ return nil, fmt.Errorf("Commit: %v", err)
}
- return nil
+ return comment, nil
}
// ChangeTitle changes the title of this issue, as the given user.
diff --git a/models/issue_comment.go b/models/issue_comment.go
index c68df028e0..9ca7aaeb01 100644
--- a/models/issue_comment.go
+++ b/models/issue_comment.go
@@ -750,10 +750,6 @@ func CreateComment(opts *CreateCommentOptions) (comment *Comment, err error) {
return nil, err
}
- if err = sendCreateCommentAction(sess, opts, comment); err != nil {
- return nil, err
- }
-
if err = sess.Commit(); err != nil {
return nil, err
}
diff --git a/models/issue_dependency_test.go b/models/issue_dependency_test.go
index ede9e008eb..bf323abb98 100644
--- a/models/issue_dependency_test.go
+++ b/models/issue_dependency_test.go
@@ -45,7 +45,7 @@ func TestCreateIssueDependency(t *testing.T) {
assert.False(t, left)
// Close #2 and check again
- err = issue2.ChangeStatus(user1, true)
+ _, err = issue2.ChangeStatus(user1, true)
assert.NoError(t, err)
left, err = IssueNoDependenciesLeft(issue1)
diff --git a/models/issue_xref_test.go b/models/issue_xref_test.go
index d8defd99c6..936d1124be 100644
--- a/models/issue_xref_test.go
+++ b/models/issue_xref_test.go
@@ -94,7 +94,8 @@ func TestXRef_ResolveCrossReferences(t *testing.T) {
i1 := testCreateIssue(t, 1, 2, "title1", "content1", false)
i2 := testCreateIssue(t, 1, 2, "title2", "content2", false)
i3 := testCreateIssue(t, 1, 2, "title3", "content3", false)
- assert.NoError(t, i3.ChangeStatus(d, true))
+ _, err := i3.ChangeStatus(d, true)
+ assert.NoError(t, err)
pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index))
rp := AssertExistsAndLoadBean(t, &Comment{IssueID: i1.ID, RefIssueID: pr.Issue.ID, RefCommentID: 0}).(*Comment)
diff --git a/models/pull.go b/models/pull.go
index 3af51112d4..d703c9a2ee 100644
--- a/models/pull.go
+++ b/models/pull.go
@@ -460,7 +460,7 @@ func (pr *PullRequest) SetMerged() (err error) {
return err
}
- if err = pr.Issue.changeStatus(sess, pr.Merger, true); err != nil {
+ if _, err = pr.Issue.changeStatus(sess, pr.Merger, true); err != nil {
return fmt.Errorf("Issue.changeStatus: %v", err)
}
if _, err = sess.ID(pr.ID).Cols("has_merged, status, merged_commit_id, merger_id, merged_unix").Update(pr); err != nil {
diff --git a/modules/notification/action/action.go b/modules/notification/action/action.go
index 9caeb5aac0..7441de638a 100644
--- a/modules/notification/action/action.go
+++ b/modules/notification/action/action.go
@@ -53,6 +53,60 @@ func (a *actionNotifier) NotifyNewIssue(issue *models.Issue) {
}
}
+// NotifyIssueChangeStatus notifies close or reopen issue to notifiers
+func (a *actionNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, actionComment *models.Comment, closeOrReopen bool) {
+ // Compose comment action, could be plain comment, close or reopen issue/pull request.
+ // This object will be used to notify watchers in the end of function.
+ act := &models.Action{
+ ActUserID: doer.ID,
+ ActUser: doer,
+ Content: fmt.Sprintf("%d|%s", issue.Index, ""),
+ RepoID: issue.Repo.ID,
+ Repo: issue.Repo,
+ Comment: actionComment,
+ CommentID: actionComment.ID,
+ IsPrivate: issue.Repo.IsPrivate,
+ }
+ // Check comment type.
+ if closeOrReopen {
+ act.OpType = models.ActionCloseIssue
+ if issue.IsPull {
+ act.OpType = models.ActionClosePullRequest
+ }
+ } else {
+ act.OpType = models.ActionReopenIssue
+ if issue.IsPull {
+ act.OpType = models.ActionReopenPullRequest
+ }
+ }
+
+ // Notify watchers for whatever action comes in, ignore if no action type.
+ if err := models.NotifyWatchers(act); err != nil {
+ log.Error("NotifyWatchers: %v", err)
+ }
+}
+
+// NotifyCreateIssueComment notifies comment on an issue to notifiers
+func (a *actionNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
+ issue *models.Issue, comment *models.Comment) {
+ act := &models.Action{
+ OpType: models.ActionCommentIssue,
+ ActUserID: doer.ID,
+ ActUser: doer,
+ Content: fmt.Sprintf("%d|%s", issue.Index, comment.Content),
+ RepoID: issue.Repo.ID,
+ Repo: issue.Repo,
+ Comment: comment,
+ CommentID: comment.ID,
+ IsPrivate: issue.Repo.IsPrivate,
+ }
+
+ // Notify watchers for whatever action comes in, ignore if no action type.
+ if err := models.NotifyWatchers(act); err != nil {
+ log.Error("NotifyWatchers: %v", err)
+ }
+}
+
func (a *actionNotifier) NotifyNewPullRequest(pull *models.PullRequest) {
if err := pull.LoadIssue(); err != nil {
log.Error("pull.LoadIssue: %v", err)
diff --git a/modules/notification/base/notifier.go b/modules/notification/base/notifier.go
index c8c89d22bd..934ee80aa7 100644
--- a/modules/notification/base/notifier.go
+++ b/modules/notification/base/notifier.go
@@ -21,7 +21,7 @@ type Notifier interface {
NotifyTransferRepository(doer *models.User, repo *models.Repository, oldOwnerName string)
NotifyNewIssue(*models.Issue)
- NotifyIssueChangeStatus(*models.User, *models.Issue, bool)
+ NotifyIssueChangeStatus(*models.User, *models.Issue, *models.Comment, bool)
NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue, oldMilestoneID int64)
NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment)
NotifyIssueChangeContent(doer *models.User, issue *models.Issue, oldContent string)
diff --git a/modules/notification/base/null.go b/modules/notification/base/null.go
index 79e9f7f7fc..a04d0e8caa 100644
--- a/modules/notification/base/null.go
+++ b/modules/notification/base/null.go
@@ -31,7 +31,7 @@ func (*NullNotifier) NotifyNewIssue(issue *models.Issue) {
}
// NotifyIssueChangeStatus places a place holder function
-func (*NullNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, isClosed bool) {
+func (*NullNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, actionComment *models.Comment, isClosed bool) {
}
// NotifyNewPullRequest places a place holder function
diff --git a/modules/notification/mail/mail.go b/modules/notification/mail/mail.go
index 558c9a6243..38146cef2e 100644
--- a/modules/notification/mail/mail.go
+++ b/modules/notification/mail/mail.go
@@ -51,7 +51,7 @@ func (m *mailNotifier) NotifyNewIssue(issue *models.Issue) {
}
}
-func (m *mailNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, isClosed bool) {
+func (m *mailNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, actionComment *models.Comment, isClosed bool) {
var actionType models.ActionType
if issue.IsPull {
if isClosed {
diff --git a/modules/notification/notification.go b/modules/notification/notification.go
index 71d6e79e6d..ab671fa291 100644
--- a/modules/notification/notification.go
+++ b/modules/notification/notification.go
@@ -53,9 +53,9 @@ func NotifyNewIssue(issue *models.Issue) {
}
// NotifyIssueChangeStatus notifies close or reopen issue to notifiers
-func NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, closeOrReopen bool) {
+func NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, actionComment *models.Comment, closeOrReopen bool) {
for _, notifier := range notifiers {
- notifier.NotifyIssueChangeStatus(doer, issue, closeOrReopen)
+ notifier.NotifyIssueChangeStatus(doer, issue, actionComment, closeOrReopen)
}
}
diff --git a/modules/notification/ui/ui.go b/modules/notification/ui/ui.go
index bfe497f866..f58ebce6d7 100644
--- a/modules/notification/ui/ui.go
+++ b/modules/notification/ui/ui.go
@@ -62,7 +62,7 @@ func (ns *notificationService) NotifyNewIssue(issue *models.Issue) {
}
}
-func (ns *notificationService) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, isClosed bool) {
+func (ns *notificationService) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, actionComment *models.Comment, isClosed bool) {
ns.issueQueue <- issueNotificationOpts{
issueID: issue.ID,
notificationAuthorID: doer.ID,
diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go
index 4ef60fef84..ee91a29f02 100644
--- a/modules/notification/webhook/webhook.go
+++ b/modules/notification/webhook/webhook.go
@@ -211,7 +211,7 @@ func (m *webhookNotifier) NotifyIssueChangeTitle(doer *models.User, issue *model
}
}
-func (m *webhookNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, isClosed bool) {
+func (m *webhookNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, actionComment *models.Comment, isClosed bool) {
mode, _ := models.AccessLevel(issue.Poster, issue.Repo)
var err error
if issue.IsPull {
diff --git a/modules/repofiles/action.go b/modules/repofiles/action.go
index 517c145659..a5a5e151cb 100644
--- a/modules/repofiles/action.go
+++ b/modules/repofiles/action.go
@@ -31,7 +31,7 @@ func getIssueFromRef(repo *models.Repository, index int64) (*models.Issue, error
return issue, nil
}
-func changeIssueStatus(repo *models.Repository, issue *models.Issue, doer *models.User, status bool) error {
+func changeIssueStatus(repo *models.Repository, issue *models.Issue, doer *models.User, closed bool) error {
stopTimerIfAvailable := func(doer *models.User, issue *models.Issue) error {
if models.StopwatchExists(doer.ID, issue.ID) {
@@ -44,7 +44,8 @@ func changeIssueStatus(repo *models.Repository, issue *models.Issue, doer *model
}
issue.Repo = repo
- if err := issue.ChangeStatus(doer, status); err != nil {
+ comment, err := issue.ChangeStatus(doer, closed)
+ if err != nil {
// Don't return an error when dependencies are open as this would let the push fail
if models.IsErrDependenciesLeft(err) {
return stopTimerIfAvailable(doer, issue)
@@ -52,6 +53,8 @@ func changeIssueStatus(repo *models.Repository, issue *models.Issue, doer *model
return err
}
+ notification.NotifyIssueChangeStatus(doer, issue, comment, closed)
+
return stopTimerIfAvailable(doer, issue)
}
diff --git a/services/issue/status.go b/services/issue/status.go
index 0df08eafd1..b01ce4bbb8 100644
--- a/services/issue/status.go
+++ b/services/issue/status.go
@@ -11,11 +11,11 @@ import (
// ChangeStatus changes issue status to open or closed.
func ChangeStatus(issue *models.Issue, doer *models.User, isClosed bool) (err error) {
- err = issue.ChangeStatus(doer, isClosed)
+ comment, err := issue.ChangeStatus(doer, isClosed)
if err != nil {
return
}
- notification.NotifyIssueChangeStatus(doer, issue, isClosed)
+ notification.NotifyIssueChangeStatus(doer, issue, comment, isClosed)
return nil
}
diff --git a/services/pull/check.go b/services/pull/check.go
index 7344f071ac..db25361b6d 100644
--- a/services/pull/check.go
+++ b/services/pull/check.go
@@ -19,6 +19,7 @@ import (
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/graceful"
"code.gitea.io/gitea/modules/log"
+ "code.gitea.io/gitea/modules/notification"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/sync"
"code.gitea.io/gitea/modules/timeutil"
@@ -145,6 +146,15 @@ func manuallyMerged(pr *models.PullRequest) bool {
log.Error("PullRequest[%d].setMerged : %v", pr.ID, err)
return false
}
+
+ baseGitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
+ if err != nil {
+ log.Error("OpenRepository[%s] : %v", pr.BaseRepo.RepoPath(), err)
+ return false
+ }
+
+ notification.NotifyMergePullRequest(pr, merger, baseGitRepo)
+
log.Info("manuallyMerged[%d]: Marked as manually merged into %s/%s by commit id: %s", pr.ID, pr.BaseRepo.Name, pr.BaseBranch, commit.ID.String())
return true
}