Make API EditIssue and EditPullRequest issue notifications (#11123)

* Make API EditIssue and EditPullRequest issue notifications

Restructure models.UpdateIssueByAPI and EditIssue/EditPullRequest
to issue notifications

Fix #10014

Signed-off-by: Andrew Thornton <art27@cantab.net>

* As per @6543

Signed-off-by: Andrew Thornton <art27@cantab.net>

* update status!

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
Co-authored-by: John Olheiser <john.olheiser@gmail.com>
Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>
Co-authored-by: Lauris BH <lauris@nix.lv>

models/issue.go

@@ -580,8 +580,13 @@ func (issue *Issue) changeStatus(e *xorm.Session, doer *User, isClosed, isMergeP
 		}
 	}
 
+	issue.IsClosed = isClosed
+	return issue.doChangeStatus(e, doer, isMergePull)
+}
+
+func (issue *Issue) doChangeStatus(e *xorm.Session, doer *User, isMergePull bool) (*Comment, error) {
 	// Check for open dependencies
-	if isClosed && issue.Repo.isDependenciesEnabled(e) {
+	if issue.IsClosed && issue.Repo.isDependenciesEnabled(e) {
 		// 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 {
@@ -593,23 +598,22 @@ func (issue *Issue) changeStatus(e *xorm.Session, doer *User, isClosed, isMergeP
 		}
 	}
 
-	issue.IsClosed = isClosed
+	if issue.IsClosed {
-	if isClosed {
 		issue.ClosedUnix = timeutil.TimeStampNow()
 	} else {
 		issue.ClosedUnix = 0
 	}
 
-	if err = updateIssueCols(e, issue, "is_closed", "closed_unix"); err != nil {
+	if err := updateIssueCols(e, issue, "is_closed", "closed_unix"); err != nil {
 		return nil, err
 	}
 
 	// Update issue count of labels
-	if err = issue.getLabels(e); err != nil {
+	if err := issue.getLabels(e); err != nil {
 		return nil, err
 	}
 	for idx := range issue.Labels {
-		if err = updateLabelCols(e, issue.Labels[idx], "num_issues", "num_closed_issue"); err != nil {
+		if err := updateLabelCols(e, issue.Labels[idx], "num_issues", "num_closed_issue"); err != nil {
 			return nil, err
 		}
 	}
@@ -1607,28 +1611,59 @@ func SearchIssueIDsByKeyword(kw string, repoIDs []int64, limit, start int) (int6
 }
 
 // UpdateIssueByAPI updates all allowed fields of given issue.
-func UpdateIssueByAPI(issue *Issue) error {
+// If the issue status is changed a statusChangeComment is returned
+// similarly if the title is changed the titleChanged bool is set to true
+func UpdateIssueByAPI(issue *Issue, doer *User) (statusChangeComment *Comment, titleChanged bool, err error) {
 	sess := x.NewSession()
 	defer sess.Close()
 	if err := sess.Begin(); err != nil {
-		return err
+		return nil, false, err
+	}
+
+	if err := issue.loadRepo(sess); err != nil {
+		return nil, false, fmt.Errorf("loadRepo: %v", err)
+	}
+
+	// Reload the issue
+	currentIssue, err := getIssueByID(sess, issue.ID)
+	if err != nil {
+		return nil, false, err
 	}
 
 	if _, err := sess.ID(issue.ID).Cols(
-		"name", "is_closed", "content", "milestone_id", "priority",
+		"name", "content", "milestone_id", "priority",
-		"deadline_unix", "updated_unix", "closed_unix", "is_locked").
+		"deadline_unix", "updated_unix", "is_locked").
 		Update(issue); err != nil {
-		return err
+		return nil, false, err
 	}
 
-	if err := issue.loadPoster(sess); err != nil {
+	titleChanged = currentIssue.Title != issue.Title
-		return err
+	if titleChanged {
+		var opts = &CreateCommentOptions{
+			Type:     CommentTypeChangeTitle,
+			Doer:     doer,
+			Repo:     issue.Repo,
+			Issue:    issue,
+			OldTitle: currentIssue.Title,
+			NewTitle: issue.Title,
+		}
+		_, err := createComment(sess, opts)
+		if err != nil {
+			return nil, false, fmt.Errorf("createComment: %v", err)
+		}
 	}
 
-	if err := issue.addCrossReferences(sess, issue.Poster, true); err != nil {
+	if currentIssue.IsClosed != issue.IsClosed {
-		return err
+		statusChangeComment, err = issue.doChangeStatus(sess, doer, false)
+		if err != nil {
+			return nil, false, err
 		}
-	return sess.Commit()
+	}
+
+	if err := issue.addCrossReferences(sess, doer, true); err != nil {
+		return nil, false, err
+	}
+	return statusChangeComment, titleChanged, sess.Commit()
 }
 
 // UpdateIssueDeadline updates an issue deadline and adds comments. Setting a deadline to 0 means deleting it.

routers/api/v1/repo/issue.go

@@ -17,6 +17,7 @@ import (
 	"code.gitea.io/gitea/modules/convert"
 	issue_indexer "code.gitea.io/gitea/modules/indexer/issues"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/notification"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
@@ -544,6 +545,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
 		return
 	}
 
+	oldTitle := issue.Title
 	if len(form.Title) > 0 {
 		issue.Title = form.Title
 	}
@@ -598,20 +600,25 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
 			return
 		}
 	}
-
-	if err = models.UpdateIssueByAPI(issue); err != nil {
-		ctx.Error(http.StatusInternalServerError, "UpdateIssueByAPI", err)
-		return
-	}
 	if form.State != nil {
-		if err = issue_service.ChangeStatus(issue, ctx.User, api.StateClosed == api.StateType(*form.State)); err != nil {
+		issue.IsClosed = (api.StateClosed == api.StateType(*form.State))
+	}
+	statusChangeComment, titleChanged, err := models.UpdateIssueByAPI(issue, ctx.User)
+	if err != nil {
 		if models.IsErrDependenciesLeft(err) {
 			ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies")
 			return
 		}
-			ctx.Error(http.StatusInternalServerError, "ChangeStatus", err)
+		ctx.Error(http.StatusInternalServerError, "UpdateIssueByAPI", err)
 		return
 	}
+
+	if titleChanged {
+		notification.NotifyIssueChangeTitle(ctx.User, issue, oldTitle)
+	}
+
+	if statusChangeComment != nil {
+		notification.NotifyIssueChangeStatus(ctx.User, issue, statusChangeComment, issue.IsClosed)
 	}
 
 	// Refetch from database to assign some automatic values

routers/api/v1/repo/pull.go

@@ -16,6 +16,7 @@ import (
 	"code.gitea.io/gitea/modules/convert"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/notification"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/routers/api/v1/utils"
@@ -409,6 +410,7 @@ func EditPullRequest(ctx *context.APIContext, form api.EditPullRequestOption) {
 		return
 	}
 
+	oldTitle := issue.Title
 	if len(form.Title) > 0 {
 		issue.Title = form.Title
 	}
@@ -485,19 +487,25 @@ func EditPullRequest(ctx *context.APIContext, form api.EditPullRequestOption) {
 		}
 	}
 
-	if err = models.UpdateIssueByAPI(issue); err != nil {
-		ctx.Error(http.StatusInternalServerError, "UpdateIssueByAPI", err)
-		return
-	}
 	if form.State != nil {
-		if err = issue_service.ChangeStatus(issue, ctx.User, api.StateClosed == api.StateType(*form.State)); err != nil {
+		issue.IsClosed = (api.StateClosed == api.StateType(*form.State))
+	}
+	statusChangeComment, titleChanged, err := models.UpdateIssueByAPI(issue, ctx.User)
+	if err != nil {
 		if models.IsErrDependenciesLeft(err) {
 			ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this pull request because it still has open dependencies")
 			return
 		}
-			ctx.Error(http.StatusInternalServerError, "ChangeStatus", err)
+		ctx.Error(http.StatusInternalServerError, "UpdateIssueByAPI", err)
 		return
 	}
+
+	if titleChanged {
+		notification.NotifyIssueChangeTitle(ctx.User, issue, oldTitle)
+	}
+
+	if statusChangeComment != nil {
+		notification.NotifyIssueChangeStatus(ctx.User, issue, statusChangeComment, issue.IsClosed)
 	}
 
 	// Refetch from database