Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make PR form use toast to show error message #29545

Merged
merged 2 commits into from
Mar 2, 2024
Merged

Make PR form use toast to show error message #29545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8db726f
fix
wxiaoguang Mar 2, 2024
dc591cc
Merge branch 'main' into refactor-pr-msg
GiteaBot Mar 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion routers/web/repo/branch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
8 changes: 4 additions & 4 deletions routers/web/repo/editor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,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),
Expand All @@ -353,7 +353,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()),
Expand Down Expand Up @@ -542,7 +542,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),
Expand Down Expand Up @@ -742,7 +742,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),
Expand Down
15 changes: 8 additions & 7 deletions routers/web/repo/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
stdCtx "context"
"errors"
"fmt"
"html/template"
"math/big"
"net/http"
"net/url"
Expand Down Expand Up @@ -1016,7 +1017,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)
Expand All @@ -1028,14 +1029,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
}
Expand Down Expand Up @@ -3296,7 +3297,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(),
Expand Down Expand Up @@ -3403,7 +3404,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(),
Expand Down Expand Up @@ -3546,8 +3547,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,
Expand Down
15 changes: 7 additions & 8 deletions routers/web/repo/pull.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1129,7 +1129,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) + "<br>" + utils.SanitizeFlashErrorString(conflictError.StdOut),
Expand All @@ -1143,7 +1143,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) + "<br>" + utils.SanitizeFlashErrorString(conflictError.StdOut),
Expand Down Expand Up @@ -1275,7 +1275,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) + "<br>" + utils.SanitizeFlashErrorString(conflictError.StdOut),
Expand All @@ -1288,7 +1288,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) + "<br>" + utils.SanitizeFlashErrorString(conflictError.StdOut),
Expand Down Expand Up @@ -1318,7 +1318,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),
Expand Down Expand Up @@ -1491,7 +1491,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),
Expand All @@ -1500,8 +1500,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)
Expand Down
9 changes: 5 additions & 4 deletions services/context/context_response.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package context
import (
"errors"
"fmt"
"html/template"
"net"
"net/http"
"net/url"
Expand Down Expand Up @@ -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.
Expand Down
11 changes: 8 additions & 3 deletions web_src/js/features/common-global.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 2 additions & 3 deletions web_src/js/modules/toast.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
<div class='toast-icon'>${svg(icon)}</div>
<div class='toast-body'>${htmlEscape(message)}</div>
<div class='toast-body'>${useHtmlBody ? message : htmlEscape(message)}</div>
<button class='toast-close'>${svg('octicon-x')}</button>
`,
escapeMarkup: false,
Expand Down
Loading