Skip to content

Commit a745a55

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent 2158cf6 commit a745a55

File tree

4 files changed

+129
-141
lines changed

4 files changed

+129
-141
lines changed

‎routers/web/repo/issue_comment.go‎

Lines changed: 113 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,133 @@ 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+
return
7166
}
7267

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

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
78+
// ATTENTION: From now on, do not use ctx.JSONError, don't return on user error, because the comment has been created.
79+
// Always use ctx.Flash.Xxx and then redirect, then the message will be displayed
80+
// TODO: need further refactoring to the code below
8681

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-
}
82+
// Check if doer can change the status of issue (close, reopen).
83+
if (ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) || (ctx.IsSigned && issue.IsPoster(ctx.Doer.ID))) &&
84+
(form.Status == "reopen" || form.Status == "close") &&
85+
!(issue.IsPull && issue.PullRequest.HasMerged) {
86+
// Duplication and conflict check should apply to reopen pull request.
87+
88+
if form.Status == "reopen" && issue.IsPull {
89+
pull := issue.PullRequest
90+
branchOtherUnmergedPR, 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.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
9694
}
95+
}
9796

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

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-
}
119+
// get head commit of branch in the head repo
120+
if err := pull.LoadHeadRepo(ctx); err != nil {
121+
ctx.ServerError("Unable to load head repo", err)
122+
return
123+
}
124+
if exist, _ := git_model.IsBranchExist(ctx, pull.HeadRepo.ID, pull.BaseBranch); !exist {
125+
ctx.Flash.Error("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+
}
117134

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-
}
135+
err = pull.LoadIssue(ctx)
136+
if err != nil {
137+
ctx.ServerError("load the issue of pull request error", err)
138+
return
139+
}
134140

135-
err = pull.LoadIssue(ctx)
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+
})
136148
if err != nil {
137-
ctx.ServerError("load the issue of pull request error", err)
149+
ctx.ServerError("force push error", err)
138150
return
139151
}
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-
}
153152
}
154153
}
154+
}
155155

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-
}
156+
if form.Status == "close" && !issue.IsClosed {
157+
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
158+
log.Error("CloseIssue: %v", err)
159+
if issues_model.IsErrDependenciesLeft(err) {
160+
if issue.IsPull {
161+
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
170162
} 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)
163+
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.issue_close_blocked"))
180164
}
181165
}
166+
} else {
167+
if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil {
168+
ctx.ServerError("stopTimerIfAvailable", err)
169+
return
170+
}
171+
log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
172+
}
173+
} else if form.Status == "reopen" && issue.IsClosed {
174+
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
175+
log.Error("ReopenIssue: %v", err)
176+
ctx.Flash.Error("Unable to reopen.")
182177
}
183178
}
179+
} // end if: handle close or reopen
184180

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
181+
// Redirect to the comment, add hashtag if it exists
182+
redirect := fmt.Sprintf("%s/%s/%d", ctx.Repo.RepoLink, issueType, issue.Index)
183+
if comment != nil {
184+
redirect += "#" + comment.HashTag()
210185
}
211-
212-
log.Trace("Comment created: %d/%d/%d", ctx.Repo.Repository.ID, issue.ID, comment.ID)
186+
ctx.JSONRedirect(redirect)
213187
}
214188

215189
// 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: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,22 @@ 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 htmx from "htmx.org";
10+
import toggleClass = htmx.toggleClass;
11+
import {toggleElemClass} from "../utils/dom.ts";
912

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

0 commit comments

Comments
 (0)