Fix various problems

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",

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,151 +57,134 @@ 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)
+
+	// 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
 	}
 
-	if ctx.HasError() {
-		ctx.JSONError(ctx.GetErrMsg())
+	comment, err := issue_service.CreateIssueComment(ctx, ctx.Doer, ctx.Repo.Repository, issue, form.Content, attachments)
+	if err != nil {
+		if errors.Is(err, user_model.ErrBlockedUser) {
+			ctx.JSONError(ctx.Tr("repo.issues.comment.blocked_user"))
+		} else {
+			ctx.ServerError("CreateIssueComment", err)
+		}
 		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
+	// 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
 
-			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
-					}
+	// 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.
-				if pr == nil {
-					issue.PullRequest.HeadCommitID = ""
-					pull_service.StartPullRequestCheckImmediately(ctx, issue.PullRequest)
+				issue.PullRequest.HeadCommitID = ""
+				pull_service.StartPullRequestCheckImmediately(ctx, issue.PullRequest)
+			}
+
+			// check whether the ref of PR <refs/pulls/pr_index/head> 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
 				}
 
-				// check whether the ref of PR <refs/pulls/pr_index/head> 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 {
+					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
+				}
 
-					// 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
+				}
 
-					err = pull.LoadIssue(ctx)
+				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("load the issue of pull request error", err)
+						ctx.ServerError("force push 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
-						}
+		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 {
-						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)
+						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 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 {
-		return
+	// 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()
 	}
-
-	comment, err := issue_service.CreateIssueComment(ctx, ctx.Doer, ctx.Repo.Repository, issue, form.Content, attachments)
-	if err != nil {
-		if errors.Is(err, user_model.ErrBlockedUser) {
-			ctx.JSONError(ctx.Tr("repo.issues.comment.blocked_user"))
-		} else {
-			ctx.ServerError("CreateIssueComment", err)
-		}
-		return
-	}
-
-	log.Trace("Comment created: %d/%d/%d", ctx.Repo.Repository.ID, issue.ID, comment.ID)
+	ctx.JSONRedirect(redirect)
 }
 
 // UpdateCommentContent change comment of issue's content

templates/repo/issue/view_content/pull_merge_box.tmpl

@@ -221,6 +221,7 @@
 						{{end}}
 						<div class="divider"></div>
 						<script type="module">
+						(() => {
 							const defaultMergeTitle = {{.DefaultMergeMessage}};
 							const defaultSquashMergeTitle = {{.DefaultSquashMergeMessage}};
 							const defaultMergeMessage = {{.DefaultMergeBody}};
@@ -299,6 +300,7 @@
 								}
 							];
 							window.config.pageData.pullRequestMergeForm = mergeForm;
+						})();
 						</script>
 
 						{{$showGeneralMergeForm = true}}

templates/user/dashboard/feeds.tmpl

@@ -110,7 +110,7 @@
 					<a href="{{.GetCommentLink ctx}}" class="tw-inline-block tw-truncate issue title">{{(.GetIssueTitle ctx) | ctx.RenderUtils.RenderIssueSimpleTitle}}</a>
 					{{$comment := index .GetIssueInfos 1}}
 					{{if $comment}}
-						<div class="render-content markup tw-text-14">{{ctx.RenderUtils.MarkdownToHtml $comment}}</div>
+						<div class="render-content markup truncated-markup">{{ctx.RenderUtils.MarkdownToHtml $comment}}</div>
 					{{end}}
 				{{else if .GetOpType.InActions "merge_pull_request"}}
 					<div class="flex-item-body tw-text-text">{{index .GetIssueInfos 1 | ctx.RenderUtils.RenderIssueSimpleTitle}}</div>

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);