Skip to content

Commit fc6e6dd

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent 2158cf6 commit fc6e6dd

5 files changed

Lines changed: 129 additions & 141 deletions

File tree

‎options/locale/locale_en-US.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1531,6 +1531,7 @@
15311531
"repo.issues.context.edit": "Edit",
15321532
"repo.issues.context.delete": "Delete",
15331533
"repo.issues.no_content": "No description provided.",
1534+
"repo.issues.comment_no_content": "No comment provided.",
15341535
"repo.issues.close": "Close Issue",
15351536
"repo.issues.comment_pull_merged_at": "merged commit %[1]s into %[2]s %[3]s",
15361537
"repo.issues.comment_manually_pull_merged_at": "manually merged commit %[1]s into %[2]s %[3]s",

‎routers/web/repo/issue_comment.go‎

Lines changed: 114 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
repo_module "code.gitea.io/gitea/modules/repository"
2222
"code.gitea.io/gitea/modules/setting"
2323
api "code.gitea.io/gitea/modules/structs"
24+
"code.gitea.io/gitea/modules/util"
2425
"code.gitea.io/gitea/modules/web"
2526
"code.gitea.io/gitea/services/context"
2627
"code.gitea.io/gitea/services/convert"
@@ -31,31 +32,22 @@ import (
3132

3233
// NewComment create a comment for issue
3334
func NewComment(ctx *context.Context) {
34-
form := web.GetForm(ctx).(*forms.CreateCommentForm)
3535
issue := GetActionIssue(ctx)
36-
if ctx.Written() {
36+
if issue == nil {
3737
return
3838
}
3939

40-
if !ctx.IsSigned || (ctx.Doer.ID != issue.PosterID && !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull)) {
41-
if log.IsTrace() {
42-
if ctx.IsSigned {
43-
issueType := "issues"
44-
if issue.IsPull {
45-
issueType = "pulls"
46-
}
47-
log.Trace("Permission Denied: User %-v not the Poster (ID: %d) and cannot read %s in Repo %-v.\n"+
48-
"User in Repo has Permissions: %-+v",
49-
ctx.Doer,
50-
issue.PosterID,
51-
issueType,
52-
ctx.Repo.Repository,
53-
ctx.Repo.Permission)
54-
} else {
55-
log.Trace("Permission Denied: Not logged in")
56-
}
57-
}
40+
if ctx.HasError() {
41+
ctx.JSONError(ctx.GetErrMsg())
42+
return
43+
}
44+
45+
form := web.GetForm(ctx).(*forms.CreateCommentForm)
46+
issueType := util.Iif(issue.IsPull, "pulls", "issues")
5847

48+
if !ctx.IsSigned || (ctx.Doer.ID != issue.PosterID && !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull)) {
49+
log.Trace("Permission Denied: User %-v not the Poster (ID: %d) and cannot read %s in Repo %-v.\n"+
50+
"User in Repo has Permissions: %-+v", ctx.Doer, issue.PosterID, issueType, ctx.Repo.Repository, ctx.Repo.Permission)
5951
ctx.HTTPError(http.StatusForbidden)
6052
return
6153
}
@@ -65,151 +57,134 @@ func NewComment(ctx *context.Context) {
6557
return
6658
}
6759

68-
var attachments []string
69-
if setting.Attachment.Enabled {
70-
attachments = form.Files
60+
attachments := util.Iif(setting.Attachment.Enabled, form.Files, nil)
61+
62+
// Allow empty comments, as long as we have attachments.
63+
// So, only stop if there is no content and no attachments.
64+
if len(form.Content) == 0 && len(attachments) == 0 {
65+
ctx.JSONError(ctx.Tr("repo.issues.comment_no_content"))
66+
return
7167
}
7268

73-
if ctx.HasError() {
74-
ctx.JSONError(ctx.GetErrMsg())
69+
comment, err := issue_service.CreateIssueComment(ctx, ctx.Doer, ctx.Repo.Repository, issue, form.Content, attachments)
70+
if err != nil {
71+
if errors.Is(err, user_model.ErrBlockedUser) {
72+
ctx.JSONError(ctx.Tr("repo.issues.comment.blocked_user"))
73+
} else {
74+
ctx.ServerError("CreateIssueComment", err)
75+
}
7576
return
7677
}
7778

78-
var comment *issues_model.Comment
79-
defer func() {
80-
// Check if issue admin/poster changes the status of issue.
81-
if (ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) || (ctx.IsSigned && issue.IsPoster(ctx.Doer.ID))) &&
82-
(form.Status == "reopen" || form.Status == "close") &&
83-
!(issue.IsPull && issue.PullRequest.HasMerged) {
84-
// Duplication and conflict check should apply to reopen pull request.
85-
var pr *issues_model.PullRequest
79+
// ATTENTION: From now on, do not use ctx.JSONError, don't return on user error, because the comment has been created.
80+
// Always use ctx.Flash.Xxx and then redirect, then the message will be displayed
81+
// TODO: need further refactoring to the code below
8682

87-
if form.Status == "reopen" && issue.IsPull {
88-
pull := issue.PullRequest
89-
var err error
90-
pr, err = issues_model.GetUnmergedPullRequest(ctx, pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch, pull.Flow)
91-
if err != nil {
92-
if !issues_model.IsErrPullRequestNotExist(err) {
93-
ctx.JSONError(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
94-
return
95-
}
83+
// Check if doer can change the status of issue (close, reopen).
84+
if (ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) || (ctx.IsSigned && issue.IsPoster(ctx.Doer.ID))) &&
85+
(form.Status == "reopen" || form.Status == "close") &&
86+
!(issue.IsPull && issue.PullRequest.HasMerged) {
87+
// Duplication and conflict check should apply to reopen pull request.
88+
89+
if form.Status == "reopen" && issue.IsPull {
90+
pull := issue.PullRequest
91+
branchOtherUnmergedPR, err := issues_model.GetUnmergedPullRequest(ctx, pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch, pull.Flow)
92+
if err != nil {
93+
if !issues_model.IsErrPullRequestNotExist(err) {
94+
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
9695
}
96+
}
9797

98+
if branchOtherUnmergedPR != nil {
99+
ctx.Flash.Error(ctx.Tr("repo.pulls.open_unmerged_pull_exists", branchOtherUnmergedPR.Index))
100+
} else {
98101
// Regenerate patch and test conflict.
99-
if pr == nil {
100-
issue.PullRequest.HeadCommitID = ""
101-
pull_service.StartPullRequestCheckImmediately(ctx, issue.PullRequest)
102+
issue.PullRequest.HeadCommitID = ""
103+
pull_service.StartPullRequestCheckImmediately(ctx, issue.PullRequest)
104+
}
105+
106+
// 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
107+
// get head commit of PR
108+
if branchOtherUnmergedPR != nil && pull.Flow == issues_model.PullRequestFlowGithub {
109+
prHeadRef := pull.GetGitHeadRefName()
110+
if err := pull.LoadBaseRepo(ctx); err != nil {
111+
ctx.ServerError("Unable to load base repo", err)
112+
return
113+
}
114+
prHeadCommitID, err := gitrepo.GetFullCommitID(ctx, pull.BaseRepo, prHeadRef)
115+
if err != nil {
116+
ctx.ServerError("Get head commit Id of pr fail", err)
117+
return
102118
}
103119

104-
// 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
105-
// get head commit of PR
106-
if pull.Flow == issues_model.PullRequestFlowGithub {
107-
prHeadRef := pull.GetGitHeadRefName()
108-
if err := pull.LoadBaseRepo(ctx); err != nil {
109-
ctx.ServerError("Unable to load base repo", err)
110-
return
111-
}
112-
prHeadCommitID, err := gitrepo.GetFullCommitID(ctx, pull.BaseRepo, prHeadRef)
113-
if err != nil {
114-
ctx.ServerError("Get head commit Id of pr fail", err)
115-
return
116-
}
120+
// get head commit of branch in the head repo
121+
if err := pull.LoadHeadRepo(ctx); err != nil {
122+
ctx.ServerError("Unable to load head repo", err)
123+
return
124+
}
125+
if exist, _ := git_model.IsBranchExist(ctx, pull.HeadRepo.ID, pull.BaseBranch); !exist {
126+
ctx.Flash.Error("The origin branch is delete, cannot reopen.")
127+
return
128+
}
129+
headBranchRef := git.RefNameFromBranch(pull.HeadBranch)
130+
headBranchCommitID, err := gitrepo.GetFullCommitID(ctx, pull.HeadRepo, headBranchRef.String())
131+
if err != nil {
132+
ctx.ServerError("Get head commit Id of head branch fail", err)
133+
return
134+
}
117135

118-
// get head commit of branch in the head repo
119-
if err := pull.LoadHeadRepo(ctx); err != nil {
120-
ctx.ServerError("Unable to load head repo", err)
121-
return
122-
}
123-
if exist, _ := git_model.IsBranchExist(ctx, pull.HeadRepo.ID, pull.BaseBranch); !exist {
124-
// todo localize
125-
ctx.JSONError("The origin branch is delete, cannot reopen.")
126-
return
127-
}
128-
headBranchRef := git.RefNameFromBranch(pull.HeadBranch)
129-
headBranchCommitID, err := gitrepo.GetFullCommitID(ctx, pull.HeadRepo, headBranchRef.String())
130-
if err != nil {
131-
ctx.ServerError("Get head commit Id of head branch fail", err)
132-
return
133-
}
136+
err = pull.LoadIssue(ctx)
137+
if err != nil {
138+
ctx.ServerError("load the issue of pull request error", err)
139+
return
140+
}
134141

135-
err = pull.LoadIssue(ctx)
142+
if prHeadCommitID != headBranchCommitID {
143+
// force push to base repo
144+
err := gitrepo.Push(ctx, pull.HeadRepo, pull.BaseRepo, git.PushOptions{
145+
Branch: pull.HeadBranch + ":" + prHeadRef,
146+
Force: true,
147+
Env: repo_module.InternalPushingEnvironment(pull.Issue.Poster, pull.BaseRepo),
148+
})
136149
if err != nil {
137-
ctx.ServerError("load the issue of pull request error", err)
150+
ctx.ServerError("force push error", err)
138151
return
139152
}
140-
141-
if prHeadCommitID != headBranchCommitID {
142-
// force push to base repo
143-
err := gitrepo.Push(ctx, pull.HeadRepo, pull.BaseRepo, git.PushOptions{
144-
Branch: pull.HeadBranch + ":" + prHeadRef,
145-
Force: true,
146-
Env: repo_module.InternalPushingEnvironment(pull.Issue.Poster, pull.BaseRepo),
147-
})
148-
if err != nil {
149-
ctx.ServerError("force push error", err)
150-
return
151-
}
152-
}
153153
}
154154
}
155+
}
155156

156-
if pr != nil {
157-
ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index))
158-
} else {
159-
if form.Status == "close" && !issue.IsClosed {
160-
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
161-
log.Error("CloseIssue: %v", err)
162-
if issues_model.IsErrDependenciesLeft(err) {
163-
if issue.IsPull {
164-
ctx.JSONError(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
165-
} else {
166-
ctx.JSONError(ctx.Tr("repo.issues.dependency.issue_close_blocked"))
167-
}
168-
return
169-
}
157+
if form.Status == "close" && !issue.IsClosed {
158+
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
159+
log.Error("CloseIssue: %v", err)
160+
if issues_model.IsErrDependenciesLeft(err) {
161+
if issue.IsPull {
162+
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
170163
} else {
171-
if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil {
172-
ctx.ServerError("stopTimerIfAvailable", err)
173-
return
174-
}
175-
log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
176-
}
177-
} else if form.Status == "reopen" && issue.IsClosed {
178-
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
179-
log.Error("ReopenIssue: %v", err)
164+
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.issue_close_blocked"))
180165
}
181166
}
167+
} else {
168+
if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil {
169+
ctx.ServerError("stopTimerIfAvailable", err)
170+
return
171+
}
172+
log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
173+
}
174+
} else if form.Status == "reopen" && issue.IsClosed {
175+
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
176+
log.Error("ReopenIssue: %v", err)
177+
ctx.Flash.Error("Unable to reopen.")
182178
}
183179
}
180+
} // end if: handle close or reopen
184181

185-
// Redirect to comment hashtag if there is any actual content.
186-
typeName := "issues"
187-
if issue.IsPull {
188-
typeName = "pulls"
189-
}
190-
if comment != nil {
191-
ctx.JSONRedirect(fmt.Sprintf("%s/%s/%d#%s", ctx.Repo.RepoLink, typeName, issue.Index, comment.HashTag()))
192-
} else {
193-
ctx.JSONRedirect(fmt.Sprintf("%s/%s/%d", ctx.Repo.RepoLink, typeName, issue.Index))
194-
}
195-
}()
196-
197-
// Fix #321: Allow empty comments, as long as we have attachments.
198-
if len(form.Content) == 0 && len(attachments) == 0 {
199-
return
200-
}
201-
202-
comment, err := issue_service.CreateIssueComment(ctx, ctx.Doer, ctx.Repo.Repository, issue, form.Content, attachments)
203-
if err != nil {
204-
if errors.Is(err, user_model.ErrBlockedUser) {
205-
ctx.JSONError(ctx.Tr("repo.issues.comment.blocked_user"))
206-
} else {
207-
ctx.ServerError("CreateIssueComment", err)
208-
}
209-
return
182+
// Redirect to the comment, add hashtag if it exists
183+
redirect := fmt.Sprintf("%s/%s/%d", ctx.Repo.RepoLink, issueType, issue.Index)
184+
if comment != nil {
185+
redirect += "#" + comment.HashTag()
210186
}
211-
212-
log.Trace("Comment created: %d/%d/%d", ctx.Repo.Repository.ID, issue.ID, comment.ID)
187+
ctx.JSONRedirect(redirect)
213188
}
214189

215190
// UpdateCommentContent change comment of issue's content

‎templates/repo/issue/view_content/pull_merge_box.tmpl‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@
221221
{{end}}
222222
<div class="divider"></div>
223223
<script type="module">
224+
(() => {
224225
const defaultMergeTitle = {{.DefaultMergeMessage}};
225226
const defaultSquashMergeTitle = {{.DefaultSquashMergeMessage}};
226227
const defaultMergeMessage = {{.DefaultMergeBody}};
@@ -280,7 +281,7 @@
280281
'allowed': {{$prUnit.PullRequestsConfig.AllowSquash}},
281282
'textDoMerge': {{ctx.Locale.Tr "repo.pulls.squash_merge_pull_request"}},
282283
'mergeTitleFieldText': defaultSquashMergeTitle,
283-
'mergeMessageFieldText': {{.GetCommitMessages}} + defaultSquashMergeMessage,
284+
'mergeMessageFieldText': {{.GetCommitMessages}} +defaultSquashMergeMessage,
284285
'hideAutoMerge': generalHideAutoMerge,
285286
},
286287
{
@@ -299,6 +300,7 @@
299300
}
300301
];
301302
window.config.pageData.pullRequestMergeForm = mergeForm;
303+
})();
302304
</script>
303305

304306
{{$showGeneralMergeForm = true}}

‎templates/user/dashboard/feeds.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@
110110
<a href="{{.GetCommentLink ctx}}" class="tw-inline-block tw-truncate issue title">{{(.GetIssueTitle ctx) | ctx.RenderUtils.RenderIssueSimpleTitle}}</a>
111111
{{$comment := index .GetIssueInfos 1}}
112112
{{if $comment}}
113-
<div class="render-content markup tw-text-14">{{ctx.RenderUtils.MarkdownToHtml $comment}}</div>
113+
<div class="render-content markup truncated-markup">{{ctx.RenderUtils.MarkdownToHtml $comment}}</div>
114114
{{end}}
115115
{{else if .GetOpType.InActions "merge_pull_request"}}
116116
<div class="flex-item-body tw-text-text">{{index .GetIssueInfos 1 | ctx.RenderUtils.RenderIssueSimpleTitle}}</div>

‎web_src/js/markup/content.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,20 @@ import {initMarkupTasklist} from './tasklist.ts';
66
import {registerGlobalSelectorFunc} from '../modules/observer.ts';
77
import {initMarkupRenderIframe} from './render-iframe.ts';
88
import {initMarkupRefIssue} from './refissue.ts';
9+
import {toggleElemClass} from '../utils/dom.ts';
910

1011
// code that runs for all markup content
1112
export function initMarkupContent(): void {
1213
registerGlobalSelectorFunc('.markup', (el: HTMLElement) => {
14+
if (el.matches('.truncated-markup')) {
15+
// when the rendered markup is truncated (e.g.: user's home activity feed)
16+
// we should not initialize any of the features (e.g.: code copy button), due to:
17+
// * truncated markup already means that the container doesn't want to show complex contents
18+
// * truncated markup may contain incomplete HTML/mermaid elements
19+
// so the only thing we need to do is to remove the "is-loading" class added by the backend render.
20+
toggleElemClass(el.querySelectorAll('.is-loading'), 'is-loading', false);
21+
return;
22+
}
1323
initMarkupCodeCopy(el);
1424
initMarkupTasklist(el);
1525
initMarkupCodeMermaid(el);

0 commit comments

Comments
 (0)