From 221a28436a080447f429fa2089d264e56f4980e2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 2 Mar 2024 23:05:07 +0800 Subject: [PATCH] Make PR form use toast to show error message (#29545) ![image](https://github.com/go-gitea/gitea/assets/2114189/b7a14ed6-db89-4f21-a590-66cd33307233) (cherry picked from commit 27deea7330f83ddb37c918afbb4159053d8847cb) --- routers/web/repo/branch.go | 2 +- routers/web/repo/editor.go | 8 ++++---- routers/web/repo/issue.go | 15 ++++++++------- routers/web/repo/pull.go | 15 +++++++-------- services/context/context_response.go | 9 +++++---- web_src/js/features/common-global.js | 11 ++++++++--- web_src/js/modules/toast.js | 5 ++--- 7 files changed, 35 insertions(+), 30 deletions(-) diff --git a/routers/web/repo/branch.go b/routers/web/repo/branch.go index 05f06a3ceb..53bd599b0d 100644 --- a/routers/web/repo/branch.go +++ b/routers/web/repo/branch.go @@ -231,7 +231,7 @@ func CreateBranch(ctx *context.Context) { if len(e.Message) == 0 { ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected_no_message")) } else { - flashError, err := ctx.RenderToString(tplAlertDetails, map[string]any{ + flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.editor.push_rejected"), "Summary": ctx.Tr("repo.editor.push_rejected_summary"), "Details": utils.SanitizeFlashErrorString(e.Message), diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index 80c212b698..4f26c0c521 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -382,7 +382,7 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b if len(errPushRej.Message) == 0 { ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplEditFile, &form) } else { - flashError, err := ctx.RenderToString(tplAlertDetails, map[string]any{ + flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.editor.push_rejected"), "Summary": ctx.Tr("repo.editor.push_rejected_summary"), "Details": utils.SanitizeFlashErrorString(errPushRej.Message), @@ -394,7 +394,7 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b ctx.RenderWithErr(flashError, tplEditFile, &form) } } else { - flashError, err := ctx.RenderToString(tplAlertDetails, map[string]any{ + flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.editor.fail_to_update_file", form.TreePath), "Summary": ctx.Tr("repo.editor.fail_to_update_file_summary"), "Details": utils.SanitizeFlashErrorString(err.Error()), @@ -590,7 +590,7 @@ func DeleteFilePost(ctx *context.Context) { if len(errPushRej.Message) == 0 { ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplDeleteFile, &form) } else { - flashError, err := ctx.RenderToString(tplAlertDetails, map[string]any{ + flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.editor.push_rejected"), "Summary": ctx.Tr("repo.editor.push_rejected_summary"), "Details": utils.SanitizeFlashErrorString(errPushRej.Message), @@ -797,7 +797,7 @@ func UploadFilePost(ctx *context.Context) { if len(errPushRej.Message) == 0 { ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplUploadFile, &form) } else { - flashError, err := ctx.RenderToString(tplAlertDetails, map[string]any{ + flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.editor.push_rejected"), "Summary": ctx.Tr("repo.editor.push_rejected_summary"), "Details": utils.SanitizeFlashErrorString(errPushRej.Message), diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index e74049810a..879cb871e7 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -9,6 +9,7 @@ import ( stdCtx "context" "errors" "fmt" + "html/template" "math/big" "net/http" "net/url" @@ -1022,7 +1023,7 @@ func NewIssue(ctx *context.Context) { ctx.HTML(http.StatusOK, tplIssueNew) } -func renderErrorOfTemplates(ctx *context.Context, errs map[string]error) string { +func renderErrorOfTemplates(ctx *context.Context, errs map[string]error) template.HTML { var files []string for k := range errs { files = append(files, k) @@ -1034,14 +1035,14 @@ func renderErrorOfTemplates(ctx *context.Context, errs map[string]error) string lines = append(lines, fmt.Sprintf("%s: %v", file, errs[file])) } - flashError, err := ctx.RenderToString(tplAlertDetails, map[string]any{ + flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.issues.choose.ignore_invalid_templates"), "Summary": ctx.Tr("repo.issues.choose.invalid_templates", len(errs)), "Details": utils.SanitizeFlashErrorString(strings.Join(lines, "\n")), }) if err != nil { log.Debug("render flash error: %v", err) - flashError = ctx.Locale.TrString("repo.issues.choose.ignore_invalid_templates") + flashError = ctx.Locale.Tr("repo.issues.choose.ignore_invalid_templates") } return flashError } @@ -3324,7 +3325,7 @@ func ChangeIssueReaction(ctx *context.Context) { return } - html, err := ctx.RenderToString(tplReactions, map[string]any{ + html, err := ctx.RenderToHTML(tplReactions, map[string]any{ "ctxData": ctx.Data, "ActionURL": fmt.Sprintf("%s/issues/%d/reactions", ctx.Repo.RepoLink, issue.Index), "Reactions": issue.Reactions.GroupByType(), @@ -3431,7 +3432,7 @@ func ChangeCommentReaction(ctx *context.Context) { return } - html, err := ctx.RenderToString(tplReactions, map[string]any{ + html, err := ctx.RenderToHTML(tplReactions, map[string]any{ "ctxData": ctx.Data, "ActionURL": fmt.Sprintf("%s/comments/%d/reactions", ctx.Repo.RepoLink, comment.ID), "Reactions": comment.Reactions.GroupByType(), @@ -3574,8 +3575,8 @@ func updateAttachments(ctx *context.Context, item any, files []string) error { return err } -func attachmentsHTML(ctx *context.Context, attachments []*repo_model.Attachment, content string) string { - attachHTML, err := ctx.RenderToString(tplAttachment, map[string]any{ +func attachmentsHTML(ctx *context.Context, attachments []*repo_model.Attachment, content string) template.HTML { + attachHTML, err := ctx.RenderToHTML(tplAttachment, map[string]any{ "ctxData": ctx.Data, "Attachments": attachments, "Content": content, diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index fdddcba2c8..c93d342e62 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1159,7 +1159,7 @@ func UpdatePullRequest(ctx *context.Context) { if err = pull_service.Update(ctx, issue.PullRequest, ctx.Doer, message, rebase); err != nil { if models.IsErrMergeConflicts(err) { conflictError := err.(models.ErrMergeConflicts) - flashError, err := ctx.RenderToString(tplAlertDetails, map[string]any{ + flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.pulls.merge_conflict"), "Summary": ctx.Tr("repo.pulls.merge_conflict_summary"), "Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "
" + utils.SanitizeFlashErrorString(conflictError.StdOut), @@ -1173,7 +1173,7 @@ func UpdatePullRequest(ctx *context.Context) { return } else if models.IsErrRebaseConflicts(err) { conflictError := err.(models.ErrRebaseConflicts) - flashError, err := ctx.RenderToString(tplAlertDetails, map[string]any{ + flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.pulls.rebase_conflict", utils.SanitizeFlashErrorString(conflictError.CommitSHA)), "Summary": ctx.Tr("repo.pulls.rebase_conflict_summary"), "Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "
" + utils.SanitizeFlashErrorString(conflictError.StdOut), @@ -1305,7 +1305,7 @@ func MergePullRequest(ctx *context.Context) { ctx.JSONError(ctx.Tr("repo.pulls.invalid_merge_option")) } else if models.IsErrMergeConflicts(err) { conflictError := err.(models.ErrMergeConflicts) - flashError, err := ctx.RenderToString(tplAlertDetails, map[string]any{ + flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.editor.merge_conflict"), "Summary": ctx.Tr("repo.editor.merge_conflict_summary"), "Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "
" + utils.SanitizeFlashErrorString(conflictError.StdOut), @@ -1318,7 +1318,7 @@ func MergePullRequest(ctx *context.Context) { ctx.JSONRedirect(issue.Link()) } else if models.IsErrRebaseConflicts(err) { conflictError := err.(models.ErrRebaseConflicts) - flashError, err := ctx.RenderToString(tplAlertDetails, map[string]any{ + flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.pulls.rebase_conflict", utils.SanitizeFlashErrorString(conflictError.CommitSHA)), "Summary": ctx.Tr("repo.pulls.rebase_conflict_summary"), "Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "
" + utils.SanitizeFlashErrorString(conflictError.StdOut), @@ -1348,7 +1348,7 @@ func MergePullRequest(ctx *context.Context) { if len(message) == 0 { ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message")) } else { - flashError, err := ctx.RenderToString(tplAlertDetails, map[string]any{ + flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.pulls.push_rejected"), "Summary": ctx.Tr("repo.pulls.push_rejected_summary"), "Details": utils.SanitizeFlashErrorString(pushrejErr.Message), @@ -1525,7 +1525,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { ctx.JSONError(ctx.Tr("repo.pulls.push_rejected_no_message")) return } - flashError, err := ctx.RenderToString(tplAlertDetails, map[string]any{ + flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.pulls.push_rejected"), "Summary": ctx.Tr("repo.pulls.push_rejected_summary"), "Details": utils.SanitizeFlashErrorString(pushrejErr.Message), @@ -1534,8 +1534,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { ctx.ServerError("CompareAndPullRequest.HTMLString", err) return } - ctx.Flash.Error(flashError) - ctx.JSONRedirect(ctx.Link + "?" + ctx.Req.URL.RawQuery) // FIXME: it's unfriendly, and will make the content lost + ctx.JSONError(flashError) return } ctx.ServerError("NewPullRequest", err) diff --git a/services/context/context_response.go b/services/context/context_response.go index 829bca1f59..372b4cb38b 100644 --- a/services/context/context_response.go +++ b/services/context/context_response.go @@ -6,6 +6,7 @@ package context import ( "errors" "fmt" + "html/template" "net" "net/http" "net/url" @@ -104,11 +105,11 @@ func (ctx *Context) JSONTemplate(tmpl base.TplName) { } } -// RenderToString renders the template content to a string -func (ctx *Context) RenderToString(name base.TplName, data map[string]any) (string, error) { +// RenderToHTML renders the template content to a HTML string +func (ctx *Context) RenderToHTML(name base.TplName, data map[string]any) (template.HTML, error) { var buf strings.Builder - err := ctx.Render.HTML(&buf, http.StatusOK, string(name), data, ctx.TemplateContext) - return buf.String(), err + err := ctx.Render.HTML(&buf, 0, string(name), data, ctx.TemplateContext) + return template.HTML(buf.String()), err } // RenderWithErr used for page has form validation but need to prompt error to users. diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index f90591aff3..c53d43cbb2 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -91,19 +91,24 @@ async function fetchActionDoRequest(actionElem, url, opt) { } else { window.location.reload(); } + return; } else if (resp.status >= 400 && resp.status < 500) { const data = await resp.json(); // the code was quite messy, sometimes the backend uses "err", sometimes it uses "error", and even "user_error" // but at the moment, as a new approach, we only use "errorMessage" here, backend can use JSONError() to respond. - showErrorToast(data.errorMessage || `server error: ${resp.status}`); + if (data.errorMessage) { + showErrorToast(data.errorMessage, {useHtmlBody: data.renderFormat === 'html'}); + } else { + showErrorToast(`server error: ${resp.status}`); + } } else { showErrorToast(`server error: ${resp.status}`); } } catch (e) { console.error('error when doRequest', e); - actionElem.classList.remove('is-loading', 'small-loading-icon'); - showErrorToast(i18n.network_error); + showErrorToast(`${i18n.network_error} ${e}`); } + actionElem.classList.remove('is-loading', 'small-loading-icon'); } async function formFetchAction(e) { diff --git a/web_src/js/modules/toast.js b/web_src/js/modules/toast.js index fa075aed48..d64359799c 100644 --- a/web_src/js/modules/toast.js +++ b/web_src/js/modules/toast.js @@ -21,13 +21,12 @@ const levels = { }; // See https://github.com/apvarun/toastify-js#api for options -function showToast(message, level, {gravity, position, duration, ...other} = {}) { +function showToast(message, level, {gravity, position, duration, useHtmlBody, ...other} = {}) { const {icon, background, duration: levelDuration} = levels[level ?? 'info']; - const toast = Toastify({ text: `
${svg(icon)}
-
${htmlEscape(message)}
+
${useHtmlBody ? message : htmlEscape(message)}
`, escapeMarkup: false,