From 414c9ee76a52a5c0152c87eec096c4e5a86457b8 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 16 May 2020 22:05:19 +0100 Subject: [PATCH] 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 * As per @6543 Signed-off-by: Andrew Thornton * update status! Signed-off-by: Andrew Thornton Co-authored-by: techknowlogick Co-authored-by: John Olheiser Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com> Co-authored-by: Lauris BH --- models/issue.go | 67 +++++++++++++++++++++++++++--------- routers/api/v1/repo/issue.go | 29 ++++++++++------ routers/api/v1/repo/pull.go | 28 +++++++++------ 3 files changed, 87 insertions(+), 37 deletions(-) diff --git a/models/issue.go b/models/issue.go index 263655c089..d9f8e929c7 100644 --- a/models/issue.go +++ b/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 isClosed { + if issue.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", - "deadline_unix", "updated_unix", "closed_unix", "is_locked"). + "name", "content", "milestone_id", "priority", + "deadline_unix", "updated_unix", "is_locked"). Update(issue); err != nil { - return err + return nil, false, err } - if err := issue.loadPoster(sess); err != nil { - return err + titleChanged = currentIssue.Title != issue.Title + 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 { - return err + if currentIssue.IsClosed != issue.IsClosed { + 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. diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 3497fe08fe..d51e80e18c 100644 --- a/routers/api/v1/repo/issue.go +++ b/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 { + if form.State != 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, "UpdateIssueByAPI", err) return } - if form.State != nil { - if err = issue_service.ChangeStatus(issue, ctx.User, api.StateClosed == api.StateType(*form.State)); 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) - 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 diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index db0b14bd70..bddf4e48f7 100644 --- a/routers/api/v1/repo/pull.go +++ b/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 { + if form.State != 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, "UpdateIssueByAPI", err) return } - if form.State != nil { - if err = issue_service.ChangeStatus(issue, ctx.User, api.StateClosed == api.StateType(*form.State)); 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) - return - } + + if titleChanged { + notification.NotifyIssueChangeTitle(ctx.User, issue, oldTitle) + } + + if statusChangeComment != nil { + notification.NotifyIssueChangeStatus(ctx.User, issue, statusChangeComment, issue.IsClosed) } // Refetch from database