diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 7cd1fa0245..e796064ce3 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -1531,6 +1531,7 @@ "repo.issues.context.edit": "Edit", "repo.issues.context.delete": "Delete", "repo.issues.no_content": "No description provided.", + "repo.issues.comment_no_content": "No comment provided.", "repo.issues.close": "Close Issue", "repo.issues.comment_pull_merged_at": "merged commit %[1]s into %[2]s %[3]s", "repo.issues.comment_manually_pull_merged_at": "manually merged commit %[1]s into %[2]s %[3]s", diff --git a/routers/web/repo/issue_comment.go b/routers/web/repo/issue_comment.go index 7f8cc23a3f..ac30b678c3 100644 --- a/routers/web/repo/issue_comment.go +++ b/routers/web/repo/issue_comment.go @@ -21,6 +21,7 @@ import ( repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/convert" @@ -31,31 +32,22 @@ import ( // NewComment create a comment for issue func NewComment(ctx *context.Context) { - form := web.GetForm(ctx).(*forms.CreateCommentForm) issue := GetActionIssue(ctx) - if ctx.Written() { + if issue == nil { return } - if !ctx.IsSigned || (ctx.Doer.ID != issue.PosterID && !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull)) { - if log.IsTrace() { - if ctx.IsSigned { - issueType := "issues" - if issue.IsPull { - issueType = "pulls" - } - log.Trace("Permission Denied: User %-v not the Poster (ID: %d) and cannot read %s in Repo %-v.\n"+ - "User in Repo has Permissions: %-+v", - ctx.Doer, - issue.PosterID, - issueType, - ctx.Repo.Repository, - ctx.Repo.Permission) - } else { - log.Trace("Permission Denied: Not logged in") - } - } + if ctx.HasError() { + ctx.JSONError(ctx.GetErrMsg()) + return + } + form := web.GetForm(ctx).(*forms.CreateCommentForm) + issueType := util.Iif(issue.IsPull, "pulls", "issues") + + if !ctx.IsSigned || (ctx.Doer.ID != issue.PosterID && !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull)) { + log.Trace("Permission Denied: User %-v not the Poster (ID: %d) and cannot read %s in Repo %-v.\n"+ + "User in Repo has Permissions: %-+v", ctx.Doer, issue.PosterID, issueType, ctx.Repo.Repository, ctx.Repo.Permission) ctx.HTTPError(http.StatusForbidden) return } @@ -65,137 +57,12 @@ func NewComment(ctx *context.Context) { return } - var attachments []string - if setting.Attachment.Enabled { - attachments = form.Files - } + attachments := util.Iif(setting.Attachment.Enabled, form.Files, nil) - if ctx.HasError() { - ctx.JSONError(ctx.GetErrMsg()) - return - } - - var comment *issues_model.Comment - defer func() { - // Check if issue admin/poster changes the status of issue. - if (ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) || (ctx.IsSigned && issue.IsPoster(ctx.Doer.ID))) && - (form.Status == "reopen" || form.Status == "close") && - !(issue.IsPull && issue.PullRequest.HasMerged) { - // Duplication and conflict check should apply to reopen pull request. - var pr *issues_model.PullRequest - - if form.Status == "reopen" && issue.IsPull { - pull := issue.PullRequest - var err error - pr, err = issues_model.GetUnmergedPullRequest(ctx, pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch, pull.Flow) - if err != nil { - if !issues_model.IsErrPullRequestNotExist(err) { - ctx.JSONError(ctx.Tr("repo.issues.dependency.pr_close_blocked")) - return - } - } - - // Regenerate patch and test conflict. - if pr == nil { - issue.PullRequest.HeadCommitID = "" - pull_service.StartPullRequestCheckImmediately(ctx, issue.PullRequest) - } - - // check whether the ref of PR in base repo is consistent with the head commit of head branch in the head repo - // get head commit of PR - if pull.Flow == issues_model.PullRequestFlowGithub { - prHeadRef := pull.GetGitHeadRefName() - if err := pull.LoadBaseRepo(ctx); err != nil { - ctx.ServerError("Unable to load base repo", err) - return - } - prHeadCommitID, err := gitrepo.GetFullCommitID(ctx, pull.BaseRepo, prHeadRef) - if err != nil { - ctx.ServerError("Get head commit Id of pr fail", err) - return - } - - // get head commit of branch in the head repo - if err := pull.LoadHeadRepo(ctx); err != nil { - ctx.ServerError("Unable to load head repo", err) - return - } - if exist, _ := git_model.IsBranchExist(ctx, pull.HeadRepo.ID, pull.BaseBranch); !exist { - // todo localize - ctx.JSONError("The origin branch is delete, cannot reopen.") - return - } - headBranchRef := git.RefNameFromBranch(pull.HeadBranch) - headBranchCommitID, err := gitrepo.GetFullCommitID(ctx, pull.HeadRepo, headBranchRef.String()) - if err != nil { - ctx.ServerError("Get head commit Id of head branch fail", err) - return - } - - err = pull.LoadIssue(ctx) - if err != nil { - ctx.ServerError("load the issue of pull request error", err) - return - } - - if prHeadCommitID != headBranchCommitID { - // force push to base repo - err := gitrepo.Push(ctx, pull.HeadRepo, pull.BaseRepo, git.PushOptions{ - Branch: pull.HeadBranch + ":" + prHeadRef, - Force: true, - Env: repo_module.InternalPushingEnvironment(pull.Issue.Poster, pull.BaseRepo), - }) - if err != nil { - ctx.ServerError("force push error", err) - return - } - } - } - } - - if pr != nil { - ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index)) - } else { - if form.Status == "close" && !issue.IsClosed { - if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil { - log.Error("CloseIssue: %v", err) - if issues_model.IsErrDependenciesLeft(err) { - if issue.IsPull { - ctx.JSONError(ctx.Tr("repo.issues.dependency.pr_close_blocked")) - } else { - ctx.JSONError(ctx.Tr("repo.issues.dependency.issue_close_blocked")) - } - return - } - } else { - if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil { - ctx.ServerError("stopTimerIfAvailable", err) - return - } - log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed) - } - } else if form.Status == "reopen" && issue.IsClosed { - if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil { - log.Error("ReopenIssue: %v", err) - } - } - } - } - - // Redirect to comment hashtag if there is any actual content. - typeName := "issues" - if issue.IsPull { - typeName = "pulls" - } - if comment != nil { - ctx.JSONRedirect(fmt.Sprintf("%s/%s/%d#%s", ctx.Repo.RepoLink, typeName, issue.Index, comment.HashTag())) - } else { - ctx.JSONRedirect(fmt.Sprintf("%s/%s/%d", ctx.Repo.RepoLink, typeName, issue.Index)) - } - }() - - // Fix #321: Allow empty comments, as long as we have attachments. - if len(form.Content) == 0 && len(attachments) == 0 { + // Can allow empty comments if there are attachments or a status change (close, reopen, approve, reject) + // So, only stop if there is no content, no attachments, and no status change. + if form.Content == "" && len(attachments) == 0 && form.Status == "" { + ctx.JSONError(ctx.Tr("repo.issues.comment_no_content")) return } @@ -209,7 +76,115 @@ func NewComment(ctx *context.Context) { return } - log.Trace("Comment created: %d/%d/%d", ctx.Repo.Repository.ID, issue.ID, comment.ID) + // ATTENTION: From now on, do not use ctx.JSONError, don't return on user error, because the comment has been created. + // Always use ctx.Flash.Xxx and then redirect, then the message will be displayed + // TODO: need further refactoring to the code below + + // Check if doer can change the status of issue (close, reopen). + if (ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) || (ctx.IsSigned && issue.IsPoster(ctx.Doer.ID))) && + (form.Status == "reopen" || form.Status == "close") && + !(issue.IsPull && issue.PullRequest.HasMerged) { + // Duplication and conflict check should apply to reopen pull request. + var branchOtherUnmergedPR *issues_model.PullRequest + if form.Status == "reopen" && issue.IsPull { + pull := issue.PullRequest + branchOtherUnmergedPR, err = issues_model.GetUnmergedPullRequest(ctx, pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch, pull.Flow) + if err != nil { + if !issues_model.IsErrPullRequestNotExist(err) { + ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked")) + } + } + + if branchOtherUnmergedPR != nil { + ctx.Flash.Error(ctx.Tr("repo.pulls.open_unmerged_pull_exists", branchOtherUnmergedPR.Index)) + } else { + // Regenerate patch and test conflict. + issue.PullRequest.HeadCommitID = "" + pull_service.StartPullRequestCheckImmediately(ctx, issue.PullRequest) + } + + // check whether the ref of PR in base repo is consistent with the head commit of head branch in the head repo + // get head commit of PR + if branchOtherUnmergedPR != nil && pull.Flow == issues_model.PullRequestFlowGithub { + prHeadRef := pull.GetGitHeadRefName() + if err := pull.LoadBaseRepo(ctx); err != nil { + ctx.ServerError("Unable to load base repo", err) + return + } + prHeadCommitID, err := gitrepo.GetFullCommitID(ctx, pull.BaseRepo, prHeadRef) + if err != nil { + ctx.ServerError("Get head commit Id of pr fail", err) + return + } + + // get head commit of branch in the head repo + if err := pull.LoadHeadRepo(ctx); err != nil { + ctx.ServerError("Unable to load head repo", err) + return + } + if exist, _ := git_model.IsBranchExist(ctx, pull.HeadRepo.ID, pull.BaseBranch); !exist { + ctx.Flash.Error("The origin branch is delete, cannot reopen.") + return + } + headBranchRef := git.RefNameFromBranch(pull.HeadBranch) + headBranchCommitID, err := gitrepo.GetFullCommitID(ctx, pull.HeadRepo, headBranchRef.String()) + if err != nil { + ctx.ServerError("Get head commit Id of head branch fail", err) + return + } + + err = pull.LoadIssue(ctx) + if err != nil { + ctx.ServerError("load the issue of pull request error", err) + return + } + + if prHeadCommitID != headBranchCommitID { + // force push to base repo + err := gitrepo.Push(ctx, pull.HeadRepo, pull.BaseRepo, git.PushOptions{ + Branch: pull.HeadBranch + ":" + prHeadRef, + Force: true, + Env: repo_module.InternalPushingEnvironment(pull.Issue.Poster, pull.BaseRepo), + }) + if err != nil { + ctx.ServerError("force push error", err) + return + } + } + } + } + + if form.Status == "close" && !issue.IsClosed { + if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil { + log.Error("CloseIssue: %v", err) + if issues_model.IsErrDependenciesLeft(err) { + if issue.IsPull { + ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked")) + } else { + ctx.Flash.Error(ctx.Tr("repo.issues.dependency.issue_close_blocked")) + } + } + } else { + if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil { + ctx.ServerError("stopTimerIfAvailable", err) + return + } + log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed) + } + } else if form.Status == "reopen" && issue.IsClosed && branchOtherUnmergedPR == nil { + if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil { + log.Error("ReopenIssue: %v", err) + ctx.Flash.Error("Unable to reopen.") + } + } + } // end if: handle close or reopen + + // Redirect to the comment, add hashtag if it exists + redirect := fmt.Sprintf("%s/%s/%d", ctx.Repo.RepoLink, issueType, issue.Index) + if comment != nil { + redirect += "#" + comment.HashTag() + } + ctx.JSONRedirect(redirect) } // UpdateCommentContent change comment of issue's content diff --git a/templates/repo/issue/view_content/pull_merge_box.tmpl b/templates/repo/issue/view_content/pull_merge_box.tmpl index 670c7f18ef..02a8db9157 100644 --- a/templates/repo/issue/view_content/pull_merge_box.tmpl +++ b/templates/repo/issue/view_content/pull_merge_box.tmpl @@ -221,6 +221,7 @@ {{end}}
{{$showGeneralMergeForm = true}} diff --git a/templates/user/dashboard/feeds.tmpl b/templates/user/dashboard/feeds.tmpl index c17fa2aa0d..de93e6a6f0 100644 --- a/templates/user/dashboard/feeds.tmpl +++ b/templates/user/dashboard/feeds.tmpl @@ -110,7 +110,7 @@ {{(.GetIssueTitle ctx) | ctx.RenderUtils.RenderIssueSimpleTitle}} {{$comment := index .GetIssueInfos 1}} {{if $comment}} -
{{ctx.RenderUtils.MarkdownToHtml $comment}}
+
{{ctx.RenderUtils.MarkdownToHtml $comment}}
{{end}} {{else if .GetOpType.InActions "merge_pull_request"}}
{{index .GetIssueInfos 1 | ctx.RenderUtils.RenderIssueSimpleTitle}}
diff --git a/web_src/js/markup/content.ts b/web_src/js/markup/content.ts index d964c88989..63510458f9 100644 --- a/web_src/js/markup/content.ts +++ b/web_src/js/markup/content.ts @@ -6,10 +6,20 @@ import {initMarkupTasklist} from './tasklist.ts'; import {registerGlobalSelectorFunc} from '../modules/observer.ts'; import {initMarkupRenderIframe} from './render-iframe.ts'; import {initMarkupRefIssue} from './refissue.ts'; +import {toggleElemClass} from '../utils/dom.ts'; // code that runs for all markup content export function initMarkupContent(): void { registerGlobalSelectorFunc('.markup', (el: HTMLElement) => { + if (el.matches('.truncated-markup')) { + // when the rendered markup is truncated (e.g.: user's home activity feed) + // we should not initialize any of the features (e.g.: code copy button), due to: + // * truncated markup already means that the container doesn't want to show complex contents + // * truncated markup may contain incomplete HTML/mermaid elements + // so the only thing we need to do is to remove the "is-loading" class added by the backend render. + toggleElemClass(el.querySelectorAll('.is-loading'), 'is-loading', false); + return; + } initMarkupCodeCopy(el); initMarkupTasklist(el); initMarkupCodeMermaid(el);